Skip to content

Commit

Permalink
wip
Browse files Browse the repository at this point in the history
  • Loading branch information
@Starefossen
Starefossen committed Dec 11, 2023
1 parent 586435d commit 6d55a76
Show file tree
Hide file tree
Showing 5 changed files with 176 additions and 23 deletions.
4 changes: 3 additions & 1 deletion controllers/remoteunleash_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,9 @@ var _ = Describe("RemoteUnleash controller", func() {
Expect(promGaugeVecVal(remoteUnleashStatus, RemoteUnleashNamespace, RemoteUnleashName, unleashv1.UnleashStatusConditionTypeConnected)).To(Equal(1.0))

Expect(httpmock.GetCallCountInfo()).To(HaveLen(1))
Expect(httpmock.GetCallCountInfo()[fmt.Sprintf("GET %s", unleashclient.InstanceAdminStatsEndpoint)]).To(Equal(1))
// This should ideally be 1, but due "object has been modified; please apply your changes to the latest version and try again" error
// it can be 2 or more as the reconciler retries.
Expect(httpmock.GetCallCountInfo()[fmt.Sprintf("GET %s", unleashclient.InstanceAdminStatsEndpoint)]).ToNot(Equal(0))
})
})

Expand Down
76 changes: 54 additions & 22 deletions controllers/unleash_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/client-go/tools/record"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand All @@ -30,13 +31,17 @@ import (
"github.com/nais/unleasherator/pkg/federation"
"github.com/nais/unleasherator/pkg/resources"
"github.com/nais/unleasherator/pkg/unleashclient"
"github.com/nais/unleasherator/pkg/utils"
)

const (
unleashFinalizer = "unleash.nais.io/finalizer"
unleashPublishMetricStatusSending = "sending"
unleashPublishMetricStatusSuccess = "success"
unleashPublishMetricStatusFailed = "failed"

deploymentTimeout = 10 * time.Minute
requeueAfter = 1 * time.Hour
)

var (
Expand Down Expand Up @@ -205,38 +210,38 @@ func (r *UnleashReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct

res, err = r.reconcileSecrets(ctx, unleash)
if err != nil {
if err := r.updateStatusReconcileFailed(ctx, unleash, err, "Failed to reconcile Secrets"); err != nil {
return ctrl.Result{}, err
if statsusErr := r.updateStatusReconcileFailed(ctx, unleash, err, "Failed to reconcile Secrets"); statsusErr != nil {
return ctrl.Result{}, statsusErr
}
return ctrl.Result{}, err
} else if res.Requeue {
return res, nil
}

res, err = r.reconcileDeployment(ctx, unleash)
res, err = r.reconcileNetworkPolicy(ctx, unleash)
if err != nil {
if err := r.updateStatusReconcileFailed(ctx, unleash, err, "Failed to reconcile Deployment"); err != nil {
return ctrl.Result{}, err
if statsusErr := r.updateStatusReconcileFailed(ctx, unleash, err, "Failed to reconcile NetworkPolicy"); statsusErr != nil {
return ctrl.Result{}, statsusErr
}
return ctrl.Result{}, err
} else if res.Requeue {
return res, nil
}

res, err = r.reconcileService(ctx, unleash)
res, err = r.reconcileDeployment(ctx, unleash)
if err != nil {
if err := r.updateStatusReconcileFailed(ctx, unleash, err, "Failed to reconcile Service"); err != nil {
return ctrl.Result{}, err
if statsusErr := r.updateStatusReconcileFailed(ctx, unleash, err, "Failed to reconcile Deployment"); statsusErr != nil {
return ctrl.Result{}, statsusErr
}
return ctrl.Result{}, err
} else if res.Requeue {
return res, nil
}

res, err = r.reconcileNetworkPolicy(ctx, unleash)
res, err = r.reconcileService(ctx, unleash)
if err != nil {
if err := r.updateStatusReconcileFailed(ctx, unleash, err, "Failed to reconcile NetworkPolicy"); err != nil {
return ctrl.Result{}, err
if statsusErr := r.updateStatusReconcileFailed(ctx, unleash, err, "Failed to reconcile Service"); statsusErr != nil {
return ctrl.Result{}, statsusErr
}
return ctrl.Result{}, err
} else if res.Requeue {
Expand All @@ -245,8 +250,8 @@ func (r *UnleashReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct

res, err = r.reconcileIngresses(ctx, unleash)
if err != nil {
if err := r.updateStatusReconcileFailed(ctx, unleash, err, "Failed to reconcile Ingresses"); err != nil {
return ctrl.Result{}, err
if statsusErr := r.updateStatusReconcileFailed(ctx, unleash, err, "Failed to reconcile Ingresses"); statsusErr != nil {
return ctrl.Result{}, statsusErr
}
return ctrl.Result{}, err
} else if res.Requeue {
Expand All @@ -255,8 +260,8 @@ func (r *UnleashReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct

res, err = r.reconcileServiceMonitor(ctx, unleash)
if err != nil {
if err := r.updateStatusReconcileFailed(ctx, unleash, err, "Failed to reconcile ServiceMonitor"); err != nil {
return ctrl.Result{}, err
if statsusErr := r.updateStatusReconcileFailed(ctx, unleash, err, "Failed to reconcile ServiceMonitor"); statsusErr != nil {
return ctrl.Result{}, statsusErr
}
return ctrl.Result{}, err
} else if res.Requeue {
Expand All @@ -276,26 +281,36 @@ func (r *UnleashReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
return ctrl.Result{}, err
}

// Wait for Deployment rollout to finish before testing connection This is to
// avoid testing connection to the previous instance if the Deployment is not
// ready yet
err = r.waitForDeployment(ctx, deploymentTimeout, req.NamespacedName)
if err != nil {
if statusErr := r.updateStatusReconcileFailed(ctx, unleash, err, "Deployment rollout timed out"); statusErr != nil {
return ctrl.Result{}, statusErr
}
return ctrl.Result{}, err
}

// Test connection to Unleash instance
stats, err := r.testConnection(unleash, ctx, log)
if err != nil {
if err := r.updateStatusConnectionFailed(ctx, unleash, err, "Failed to connect to Unleash instance"); err != nil {
return ctrl.Result{}, err
if statusErr := r.updateStatusConnectionFailed(ctx, unleash, err, "Failed to connect to Unleash instance"); statusErr != nil {
return ctrl.Result{}, statusErr
}

return ctrl.Result{}, err
}

// Set the connection status of the Unleash instance to available
log.Info("Successfully connected to Unleash instance", "version", stats.VersionOSS)
err = r.updateStatusConnectionSuccess(ctx, unleash, stats)
if err != nil {
return ctrl.Result{}, err
statusErr := r.updateStatusConnectionSuccess(ctx, unleash, stats)
if statusErr != nil {
return ctrl.Result{}, statusErr
}

// Publish the Unleash instance to federation if enabled
err = r.publish(ctx, unleash)
if err != nil {
if err = r.publish(ctx, unleash); err != nil {
return ctrl.Result{}, err
}

Expand Down Expand Up @@ -677,6 +692,23 @@ func (r *UnleashReconciler) reconcileService(ctx context.Context, unleash *unlea
return ctrl.Result{}, nil
}

// waitForDeployment will wait for the deployment to be available
func (r *UnleashReconciler) waitForDeployment(ctx context.Context, timeout time.Duration, key types.NamespacedName) error {
ctx, cancel := context.WithTimeout(ctx, timeout)
defer cancel()

err := wait.PollUntilContextCancel(ctx, time.Second, true, func(ctx context.Context) (bool, error) {
deployment := &appsv1.Deployment{}
if err := r.Client.Get(ctx, key, deployment); err != nil {
return false, err
}

return utils.DeploymentIsReady(deployment), nil
})

return err
}

// testConnection will test the connection to the Unleash instance
func (r *UnleashReconciler) testConnection(unleash resources.UnleashInstance, ctx context.Context, log logr.Logger) (*unleashclient.InstanceAdminStatsResult, error) {
client, err := unleash.ApiClient(ctx, r.Client, r.OperatorNamespace)
Expand Down
35 changes: 35 additions & 0 deletions controllers/unleash_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,27 @@ func getUnleash(k8sClient client.Client, ctx context.Context, unleash *unleashv1
return unsetConditionLastTransitionTime(unleash.Status.Conditions), nil
}

func getDeployment(k8sClient client.Client, ctx context.Context, namespacedName client.ObjectKey, deployment *appsv1.Deployment) error {
return k8sClient.Get(ctx, namespacedName, deployment)
}

func setDeploymentStatusAvailable(deployment *appsv1.Deployment) {
deployment.Status.Conditions = []appsv1.DeploymentCondition{
{
Type: appsv1.DeploymentProgressing,
Status: corev1.ConditionTrue,
Reason: "NewReplicaSetAvailable",
Message: `ReplicaSet "fake-abc123" has successfully progressed.`,
LastUpdateTime: metav1.Time{
Time: time.Now(),
},
LastTransitionTime: metav1.Time{
Time: time.Now(),
},
},
}
}

var _ = Describe("Unleash controller", func() {
const (
UnleashNamespace = "default"
Expand Down Expand Up @@ -89,6 +110,13 @@ var _ = Describe("Unleash controller", func() {
})
Expect(k8sClient.Create(ctx, unleash)).Should(Succeed())

By("By faking Deployment status as available")
createdDeployment := &appsv1.Deployment{}
Eventually(getDeployment, timeout, interval).WithArguments(k8sClient, ctx, unleash.NamespacedName(), createdDeployment).Should(Succeed())
setDeploymentStatusAvailable(createdDeployment)
Expect(k8sClient.Status().Update(ctx, createdDeployment)).Should(Succeed())

By("By checking that Unleash is connected")
createdUnleash := &unleashv1.Unleash{ObjectMeta: unleash.ObjectMeta}
Eventually(getUnleash, timeout, interval).WithArguments(k8sClient, ctx, createdUnleash).Should(ContainElement(metav1.Condition{
Type: unleashv1.UnleashStatusConditionTypeConnected,
Expand Down Expand Up @@ -157,6 +185,13 @@ var _ = Describe("Unleash controller", func() {
})
Expect(k8sClient.Create(ctx, unleash)).Should(Succeed())

By("By faking Deployment status as available")
createdDeployment := &appsv1.Deployment{}
Eventually(getDeployment, timeout, interval).WithArguments(k8sClient, ctx, unleash.NamespacedName(), createdDeployment).Should(Succeed())
setDeploymentStatusAvailable(createdDeployment)
Expect(k8sClient.Status().Update(ctx, createdDeployment)).Should(Succeed())

By("By checking that Unleash is connected")
createdUnleash := &unleashv1.Unleash{ObjectMeta: unleash.ObjectMeta}
Eventually(getUnleash, timeout, interval).WithArguments(k8sClient, ctx, createdUnleash).Should(ContainElement(metav1.Condition{
Type: unleashv1.UnleashStatusConditionTypeConnected,
Expand Down
11 changes: 11 additions & 0 deletions pkg/utils/k8s.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"sync"

appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
"sigs.k8s.io/controller-runtime/pkg/client"

Expand Down Expand Up @@ -33,6 +34,16 @@ func SecretEnvVar(name, secretName, secretKey string) corev1.EnvVar {
}
}

// DeploymentIsReady returns true if the rollout of the given deployment has completed successfully.
func DeploymentIsReady(deployment *appsv1.Deployment) bool {
for _, condition := range deployment.Status.Conditions {
if condition.Type == appsv1.DeploymentProgressing && condition.Status == corev1.ConditionTrue && condition.Reason == "NewReplicaSetAvailable" {
return true
}
}
return false
}

// UpsertObject upserts the given object in Kubernetes. If the object already exists, it is updated.
// If the object does not exist, it is created. The object is identified by its key, which is extracted
// from the object itself. The function returns an error if the upsert operation fails.
Expand Down
73 changes: 73 additions & 0 deletions pkg/utils/k8s_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"reflect"
"testing"

appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
)

Expand Down Expand Up @@ -104,3 +105,75 @@ func TestSecretEnvVar(t *testing.T) {
})
}
}

func TestDeploymentIsReady(t *testing.T) {
tests := []struct {
name string
deployment *appsv1.Deployment
expected bool
}{
{
name: "Deployment is ready",
deployment: &appsv1.Deployment{
Status: appsv1.DeploymentStatus{
Conditions: []appsv1.DeploymentCondition{
{
Type: appsv1.DeploymentProgressing,
Status: corev1.ConditionTrue,
Reason: "NewReplicaSetAvailable",
},
},
},
},
expected: true,
},
{
name: "Deployment is not ready",
deployment: &appsv1.Deployment{
Status: appsv1.DeploymentStatus{
Conditions: []appsv1.DeploymentCondition{
{
Type: appsv1.DeploymentProgressing,
Status: corev1.ConditionTrue,
Reason: "NewReplicaSetCreated",
},
},
},
},
expected: false,
},
{
name: "Deployment failed",
deployment: &appsv1.Deployment{
Status: appsv1.DeploymentStatus{
Conditions: []appsv1.DeploymentCondition{
{
Type: appsv1.DeploymentProgressing,
Status: corev1.ConditionFalse,
Reason: "ProgressDeadlineExceeded",
},
},
},
},
expected: false,
},
{
name: "No conditions",
deployment: &appsv1.Deployment{
Status: appsv1.DeploymentStatus{
Conditions: []appsv1.DeploymentCondition{},
},
},
expected: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := DeploymentIsReady(tt.deployment)
if got != tt.expected {
t.Errorf("DeploymentIsReady() = %v, want %v", got, tt.expected)
}
})
}
}

0 comments on commit 6d55a76

Please sign in to comment.