From 38616063a77df9f9d8389510cbf3a201166322b2 Mon Sep 17 00:00:00 2001 From: Ondra Kupka Date: Tue, 28 Apr 2026 11:39:28 +0200 Subject: [PATCH 1/3] test: add unit tests for syncStatus and deployment status helpers Cover the current behavior of syncStatus, isDeploymentStatusAvailable, isDeploymentStatusAvailableAndUpdated, and isDeploymentStatusComplete including partial availability scenarios. --- pkg/operator/status_test.go | 404 ++++++++++++++++++++++++++++++++++++ 1 file changed, 404 insertions(+) create mode 100644 pkg/operator/status_test.go diff --git a/pkg/operator/status_test.go b/pkg/operator/status_test.go new file mode 100644 index 000000000..3c106aec5 --- /dev/null +++ b/pkg/operator/status_test.go @@ -0,0 +1,404 @@ +package operator + +import ( + "testing" + + appsv1 "k8s.io/api/apps/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/utils/ptr" + + operatorv1 "github.com/openshift/api/operator/v1" + "github.com/openshift/library-go/pkg/operator/status" + "github.com/openshift/library-go/pkg/operator/v1helpers" +) + +func newTestOperator(t *testing.T) *serviceCAOperator { + t.Setenv(operatorVersionEnvName, "4.21.0") + return &serviceCAOperator{ + versionGetter: status.NewVersionGetter(), + } +} + +func makeDeployment(name string, generation, observedGeneration int64, replicas, updatedReplicas, availableReplicas int32) appsv1.Deployment { + return appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Generation: generation, + }, + Spec: appsv1.DeploymentSpec{ + Replicas: ptr.To[int32](1), + }, + Status: appsv1.DeploymentStatus{ + Replicas: replicas, + UpdatedReplicas: updatedReplicas, + AvailableReplicas: availableReplicas, + ObservedGeneration: observedGeneration, + }, + } +} + +func TestIsDeploymentStatusAvailable(t *testing.T) { + tests := []struct { + name string + deploy appsv1.Deployment + expect bool + }{ + { + name: "available replicas present", + deploy: makeDeployment("test", 1, 1, 1, 1, 1), + expect: true, + }, + { + name: "no available replicas", + deploy: makeDeployment("test", 1, 1, 1, 1, 0), + expect: false, + }, + { + name: "some but not all replicas available", + deploy: func() appsv1.Deployment { + d := makeDeployment("test", 1, 1, 3, 3, 1) + d.Spec.Replicas = ptr.To[int32](3) + return d + }(), + expect: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := isDeploymentStatusAvailable(tt.deploy) + if got != tt.expect { + t.Errorf("isDeploymentStatusAvailable() = %v, want %v", got, tt.expect) + } + }) + } +} + +func TestIsDeploymentStatusAvailableAndUpdated(t *testing.T) { + tests := []struct { + name string + deploy appsv1.Deployment + expect bool + }{ + { + name: "fully available and updated", + deploy: makeDeployment("test", 1, 1, 1, 1, 1), + expect: true, + }, + { + name: "no available replicas", + deploy: makeDeployment("test", 1, 1, 1, 1, 0), + expect: false, + }, + { + name: "generation not yet observed", + deploy: makeDeployment("test", 2, 1, 1, 1, 1), + expect: false, + }, + { + name: "updated replicas mismatch", + deploy: makeDeployment("test", 1, 1, 2, 1, 1), + expect: false, + }, + { + name: "some but not all replicas available, all updated", + deploy: func() appsv1.Deployment { + d := makeDeployment("test", 1, 1, 3, 3, 1) + d.Spec.Replicas = ptr.To[int32](3) + return d + }(), + expect: true, + }, + { + name: "some but not all replicas available, not all updated", + deploy: func() appsv1.Deployment { + d := makeDeployment("test", 1, 1, 3, 2, 1) + d.Spec.Replicas = ptr.To[int32](3) + return d + }(), + expect: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := isDeploymentStatusAvailableAndUpdated(tt.deploy) + if got != tt.expect { + t.Errorf("isDeploymentStatusAvailableAndUpdated() = %v, want %v", got, tt.expect) + } + }) + } +} + +func TestIsDeploymentStatusComplete(t *testing.T) { + tests := []struct { + name string + deploy appsv1.Deployment + expect bool + }{ + { + name: "fully complete", + deploy: makeDeployment("test", 1, 1, 1, 1, 1), + expect: true, + }, + { + name: "not all replicas available", + deploy: makeDeployment("test", 1, 1, 1, 1, 0), + expect: false, + }, + { + name: "generation not yet observed", + deploy: makeDeployment("test", 2, 1, 1, 1, 1), + expect: false, + }, + { + name: "rolling update in progress - not all pods updated", + deploy: func() appsv1.Deployment { + d := makeDeployment("test", 1, 1, 2, 1, 2) + d.Spec.Replicas = ptr.To[int32](2) + return d + }(), + expect: false, + }, + { + name: "extra replicas still terminating", + deploy: makeDeployment("test", 1, 1, 2, 1, 1), + expect: false, + }, + { + name: "nil spec replicas defaults to 1", + deploy: func() appsv1.Deployment { + d := makeDeployment("test", 1, 1, 1, 1, 1) + d.Spec.Replicas = nil + return d + }(), + expect: true, + }, + { + name: "some but not all replicas available", + deploy: func() appsv1.Deployment { + d := makeDeployment("test", 1, 1, 3, 3, 1) + d.Spec.Replicas = ptr.To[int32](3) + return d + }(), + expect: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := isDeploymentStatusComplete(tt.deploy) + if got != tt.expect { + t.Errorf("isDeploymentStatusComplete() = %v, want %v", got, tt.expect) + } + }) + } +} + +func TestSyncStatus(t *testing.T) { + defaultTargets := sets.NewString("service-ca") + + tests := []struct { + name string + targets sets.String + deployments []appsv1.Deployment + expectProgressing operatorv1.ConditionStatus + expectAvailable operatorv1.ConditionStatus + expectVersionSet bool + expectReason string + }{ + { + name: "fully complete deployment", + deployments: []appsv1.Deployment{makeDeployment("service-ca", 1, 1, 1, 1, 1)}, + expectProgressing: operatorv1.ConditionFalse, + expectAvailable: operatorv1.ConditionTrue, + expectVersionSet: true, + expectReason: "ManagedDeploymentsCompleteAndUpdated", + }, + { + name: "no available replicas", + deployments: []appsv1.Deployment{makeDeployment("service-ca", 1, 1, 1, 1, 0)}, + expectProgressing: operatorv1.ConditionTrue, + // TODO: Available should be False when no replicas are available. + // The fallthrough in syncStatus overwrites the earlier setAvailableFalse. + expectAvailable: operatorv1.ConditionTrue, + expectVersionSet: false, + expectReason: "ManagedDeploymentsAvailable", + }, + { + name: "generation mismatch - not yet observed", + deployments: []appsv1.Deployment{makeDeployment("service-ca", 2, 1, 1, 1, 1)}, + expectProgressing: operatorv1.ConditionTrue, + expectAvailable: operatorv1.ConditionTrue, + expectVersionSet: false, + expectReason: "ManagedDeploymentsAvailable", + }, + { + name: "recreate rollout - replicas scaled down", + deployments: []appsv1.Deployment{makeDeployment("service-ca", 1, 1, 0, 0, 0)}, + expectProgressing: operatorv1.ConditionTrue, + // TODO: Available should be False when all replicas are scaled down. + expectAvailable: operatorv1.ConditionTrue, + expectVersionSet: false, + expectReason: "ManagedDeploymentsAvailable", + }, + { + name: "missing deployment", + deployments: []appsv1.Deployment{}, + expectProgressing: operatorv1.ConditionTrue, + expectAvailable: operatorv1.ConditionFalse, + expectVersionSet: false, + expectReason: "ManagedDeploymentsNotFound", + }, + { + name: "deployment being deleted", + deployments: []appsv1.Deployment{ + func() appsv1.Deployment { + d := makeDeployment("service-ca", 1, 1, 1, 1, 1) + now := metav1.Now() + d.DeletionTimestamp = &now + return d + }(), + }, + expectProgressing: operatorv1.ConditionTrue, + // TODO: Available should be False when the deployment is being deleted. + expectAvailable: operatorv1.ConditionTrue, + expectVersionSet: false, + expectReason: "ManagedDeploymentsAvailable", + }, + { + name: "available and updated but not all replicas available yet", + deployments: []appsv1.Deployment{makeDeployment("service-ca", 1, 1, 2, 2, 1)}, + expectProgressing: operatorv1.ConditionTrue, + expectAvailable: operatorv1.ConditionTrue, + expectVersionSet: true, + expectReason: "ManagedDeploymentsAvailableAndUpdated", + }, + { + name: "available but old replicas still exist", + deployments: []appsv1.Deployment{makeDeployment("service-ca", 1, 1, 2, 1, 1)}, + expectProgressing: operatorv1.ConditionTrue, + expectAvailable: operatorv1.ConditionTrue, + expectVersionSet: false, + expectReason: "ManagedDeploymentsAvailable", + }, + { + name: "some replicas available, all updated, not yet complete", + deployments: []appsv1.Deployment{ + func() appsv1.Deployment { + d := makeDeployment("service-ca", 1, 1, 3, 3, 1) + d.Spec.Replicas = ptr.To[int32](3) + return d + }(), + }, + expectProgressing: operatorv1.ConditionTrue, + expectAvailable: operatorv1.ConditionTrue, + expectVersionSet: true, + expectReason: "ManagedDeploymentsAvailableAndUpdated", + }, + { + name: "some replicas available, not all updated", + deployments: []appsv1.Deployment{ + func() appsv1.Deployment { + d := makeDeployment("service-ca", 1, 1, 3, 2, 1) + d.Spec.Replicas = ptr.To[int32](3) + return d + }(), + }, + expectProgressing: operatorv1.ConditionTrue, + expectAvailable: operatorv1.ConditionTrue, + expectVersionSet: false, + expectReason: "ManagedDeploymentsAvailable", + }, + { + name: "two targets, both complete", + targets: sets.NewString("deploy-a", "deploy-b"), + deployments: []appsv1.Deployment{ + makeDeployment("deploy-a", 1, 1, 1, 1, 1), + makeDeployment("deploy-b", 1, 1, 1, 1, 1), + }, + expectProgressing: operatorv1.ConditionFalse, + expectAvailable: operatorv1.ConditionTrue, + expectVersionSet: true, + expectReason: "ManagedDeploymentsCompleteAndUpdated", + }, + { + name: "two targets, one complete and one unavailable", + targets: sets.NewString("deploy-a", "deploy-b"), + deployments: []appsv1.Deployment{ + makeDeployment("deploy-a", 1, 1, 1, 1, 1), + makeDeployment("deploy-b", 1, 1, 1, 1, 0), + }, + expectProgressing: operatorv1.ConditionTrue, + // TODO: Available should be False when a deployment has no available replicas. + expectAvailable: operatorv1.ConditionTrue, + expectVersionSet: false, + expectReason: "ManagedDeploymentsAvailable", + }, + { + name: "two targets, one complete and one updating", + targets: sets.NewString("deploy-a", "deploy-b"), + deployments: []appsv1.Deployment{ + makeDeployment("deploy-a", 1, 1, 1, 1, 1), + makeDeployment("deploy-b", 2, 1, 1, 1, 1), + }, + expectProgressing: operatorv1.ConditionTrue, + expectAvailable: operatorv1.ConditionTrue, + expectVersionSet: false, + expectReason: "ManagedDeploymentsAvailable", + }, + { + name: "two targets, one missing", + targets: sets.NewString("deploy-a", "deploy-b"), + deployments: []appsv1.Deployment{ + makeDeployment("deploy-a", 1, 1, 1, 1, 1), + }, + expectProgressing: operatorv1.ConditionTrue, + expectAvailable: operatorv1.ConditionFalse, + expectVersionSet: false, + expectReason: "ManagedDeploymentsNotFound", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + op := newTestOperator(t) + sc := &operatorv1.ServiceCA{} + depList := &appsv1.DeploymentList{Items: tt.deployments} + + targets := tt.targets + if targets == nil { + targets = defaultTargets + } + op.syncStatus(sc, depList, targets) + + progressing := v1helpers.FindOperatorCondition(sc.Status.Conditions, operatorv1.OperatorStatusTypeProgressing) + if progressing == nil { + t.Fatal("expected Progressing condition to be set") + } + if progressing.Status != tt.expectProgressing { + t.Errorf("Progressing: got %s, want %s (reason=%s, message=%s)", + progressing.Status, tt.expectProgressing, progressing.Reason, progressing.Message) + } + if progressing.Reason != tt.expectReason { + t.Errorf("Progressing reason: got %q, want %q", progressing.Reason, tt.expectReason) + } + + available := v1helpers.FindOperatorCondition(sc.Status.Conditions, operatorv1.OperatorStatusTypeAvailable) + if available == nil { + t.Fatal("expected Available condition to be set") + } + if available.Status != tt.expectAvailable { + t.Errorf("Available: got %s, want %s (reason=%s, message=%s)", + available.Status, tt.expectAvailable, available.Reason, available.Message) + } + + versions := op.versionGetter.GetVersions() + _, versionSet := versions["operator"] + if versionSet != tt.expectVersionSet { + t.Errorf("version set: got %v, want %v", versionSet, tt.expectVersionSet) + } + }) + } +} From 6a3f6bd5658bcc3600d32fd854c9cd12cfdcd916 Mon Sep 17 00:00:00 2001 From: Ondra Kupka Date: Tue, 28 Apr 2026 17:57:57 +0200 Subject: [PATCH 2/3] pkg/operator: status: Migrate to sets.Set[string] --- pkg/operator/starter.go | 2 +- pkg/operator/status.go | 4 ++-- pkg/operator/status_test.go | 12 ++++++------ 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/operator/starter.go b/pkg/operator/starter.go index bdf13e268..7af7bd09a 100644 --- a/pkg/operator/starter.go +++ b/pkg/operator/starter.go @@ -33,7 +33,7 @@ const ( operatorVersionEnvName = "OPERATOR_IMAGE_VERSION" ) -var targetDeploymentNames = sets.NewString(api.ServiceCADeploymentName) +var targetDeploymentNames = sets.New(api.ServiceCADeploymentName) func RunOperator(ctx context.Context, controllerContext *controllercmd.ControllerContext) error { diff --git a/pkg/operator/status.go b/pkg/operator/status.go index 1cbb167ec..37925b2c3 100644 --- a/pkg/operator/status.go +++ b/pkg/operator/status.go @@ -96,11 +96,11 @@ func isDeploymentStatusComplete(deploy appsv1.Deployment) bool { deploy.Status.ObservedGeneration >= deploy.Generation } -func (c *serviceCAOperator) syncStatus(operatorConfigCopy *operatorv1.ServiceCA, existingDeployments *appsv1.DeploymentList, targetDeploymentNames sets.String) { +func (c *serviceCAOperator) syncStatus(operatorConfigCopy *operatorv1.ServiceCA, existingDeployments *appsv1.DeploymentList, targetDeploymentNames sets.Set[string]) { versionUpdatable := true versionUpdatableAndDeploymentsComplete := true statusMsg := "" - existingDeploymentNames := sets.String{} + existingDeploymentNames := sets.New[string]() for _, dep := range existingDeployments.Items { existingDeploymentNames.Insert(dep.Name) // If there isn't at least one replica from each deployment, Available=False diff --git a/pkg/operator/status_test.go b/pkg/operator/status_test.go index 3c106aec5..2b307a022 100644 --- a/pkg/operator/status_test.go +++ b/pkg/operator/status_test.go @@ -197,11 +197,11 @@ func TestIsDeploymentStatusComplete(t *testing.T) { } func TestSyncStatus(t *testing.T) { - defaultTargets := sets.NewString("service-ca") + defaultTargets := sets.New("service-ca") tests := []struct { name string - targets sets.String + targets sets.Set[string] deployments []appsv1.Deployment expectProgressing operatorv1.ConditionStatus expectAvailable operatorv1.ConditionStatus @@ -313,7 +313,7 @@ func TestSyncStatus(t *testing.T) { }, { name: "two targets, both complete", - targets: sets.NewString("deploy-a", "deploy-b"), + targets: sets.New("deploy-a", "deploy-b"), deployments: []appsv1.Deployment{ makeDeployment("deploy-a", 1, 1, 1, 1, 1), makeDeployment("deploy-b", 1, 1, 1, 1, 1), @@ -325,7 +325,7 @@ func TestSyncStatus(t *testing.T) { }, { name: "two targets, one complete and one unavailable", - targets: sets.NewString("deploy-a", "deploy-b"), + targets: sets.New("deploy-a", "deploy-b"), deployments: []appsv1.Deployment{ makeDeployment("deploy-a", 1, 1, 1, 1, 1), makeDeployment("deploy-b", 1, 1, 1, 1, 0), @@ -338,7 +338,7 @@ func TestSyncStatus(t *testing.T) { }, { name: "two targets, one complete and one updating", - targets: sets.NewString("deploy-a", "deploy-b"), + targets: sets.New("deploy-a", "deploy-b"), deployments: []appsv1.Deployment{ makeDeployment("deploy-a", 1, 1, 1, 1, 1), makeDeployment("deploy-b", 2, 1, 1, 1, 1), @@ -350,7 +350,7 @@ func TestSyncStatus(t *testing.T) { }, { name: "two targets, one missing", - targets: sets.NewString("deploy-a", "deploy-b"), + targets: sets.New("deploy-a", "deploy-b"), deployments: []appsv1.Deployment{ makeDeployment("deploy-a", 1, 1, 1, 1, 1), }, From 2cef2dec3ee0c80e0f3f1dffb5905c447417b3df Mon Sep 17 00:00:00 2001 From: Ondra Kupka Date: Tue, 28 Apr 2026 17:59:47 +0200 Subject: [PATCH 3/3] fix: do not report Progressing=True during node reboots MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a deployment has zero available replicas but is fully up-to-date (all replicas running the current pod template), syncStatus was reporting Progressing=True. This happens during node reboots or pod evictions — situations where the operator is not making progress, the deployment is simply waiting for pods to be rescheduled. The root cause was that syncStatus set conditions inside the loop but post-loop code unconditionally overwrote them. Fix by collecting flags in the loop and setting conditions only after: - Up-to-date but unavailable (node reboot): Available=False, Progressing=False, version set - Mid-rollout or deleting: falls through to existing Progressing=True paths --- pkg/operator/status.go | 45 ++++++++++------ pkg/operator/status_test.go | 103 ++++++++++++++++++++++++++++-------- 2 files changed, 110 insertions(+), 38 deletions(-) diff --git a/pkg/operator/status.go b/pkg/operator/status.go index 37925b2c3..d77a17d9d 100644 --- a/pkg/operator/status.go +++ b/pkg/operator/status.go @@ -6,6 +6,7 @@ import ( appsv1 "k8s.io/api/apps/v1" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/utils/ptr" operatorv1 "github.com/openshift/api/operator/v1" "github.com/openshift/library-go/pkg/operator/v1helpers" @@ -86,38 +87,39 @@ func isDeploymentStatusAvailableAndUpdated(deploy appsv1.Deployment) bool { } func isDeploymentStatusComplete(deploy appsv1.Deployment) bool { - replicas := int32(1) - if deploy.Spec.Replicas != nil { - replicas = *(deploy.Spec.Replicas) - } - return deploy.Status.UpdatedReplicas == replicas && - deploy.Status.Replicas == replicas && - deploy.Status.AvailableReplicas == replicas && - deploy.Status.ObservedGeneration >= deploy.Generation + desiredReplicas := ptr.Deref(deploy.Spec.Replicas, 1) + return isDeploymentUpToDate(deploy) && deploy.Status.AvailableReplicas == desiredReplicas +} + +func isDeploymentUpToDate(deploy appsv1.Deployment) bool { + desiredReplicas := ptr.Deref(deploy.Spec.Replicas, 1) + return deploy.Status.ObservedGeneration >= deploy.Generation && + deploy.Status.UpdatedReplicas == desiredReplicas && + deploy.Status.Replicas == desiredReplicas } func (c *serviceCAOperator) syncStatus(operatorConfigCopy *operatorv1.ServiceCA, existingDeployments *appsv1.DeploymentList, targetDeploymentNames sets.Set[string]) { versionUpdatable := true versionUpdatableAndDeploymentsComplete := true + deploymentUnavailableButUpToDate := false statusMsg := "" existingDeploymentNames := sets.New[string]() for _, dep := range existingDeployments.Items { existingDeploymentNames.Insert(dep.Name) - // If there isn't at least one replica from each deployment, Available=False - reason := "ManagedDeploymentsNotReady" if dep.DeletionTimestamp != nil { statusMsg += fmt.Sprintf("\n%s deleting", dep.Name) - setProgressingTrue(operatorConfigCopy, reason, statusMsg) - setAvailableFalse(operatorConfigCopy, reason, statusMsg) versionUpdatable = false versionUpdatableAndDeploymentsComplete = false continue } if !isDeploymentStatusAvailable(dep) { - statusMsg += fmt.Sprintf("\n%s does not have available replicas", dep.Name) - setProgressingTrue(operatorConfigCopy, reason, statusMsg) - setAvailableFalse(operatorConfigCopy, reason, statusMsg) - versionUpdatable = false + if isDeploymentUpToDate(dep) { + statusMsg += fmt.Sprintf("\n%s: all replicas are up-to-date but not yet available", dep.Name) + deploymentUnavailableButUpToDate = true + } else { + statusMsg += fmt.Sprintf("\n%s does not have available replicas", dep.Name) + versionUpdatable = false + } versionUpdatableAndDeploymentsComplete = false continue } @@ -149,6 +151,17 @@ func (c *serviceCAOperator) syncStatus(operatorConfigCopy *operatorv1.ServiceCA, c.setVersion() return } + // Deployment is up-to-date but temporarily unavailable (e.g. node reboot, + // pod eviction). The versionUpdatable guard ensures this only fires when no + // other deployment is mid-rollout or deleting. Keep Available=True to avoid + // tripping CVO invariant monitors during transient pod rescheduling. + if deploymentUnavailableButUpToDate && versionUpdatable { + reason := "ManagedDeploymentsUpToDateButUnavailable" + setAvailableTrue(operatorConfigCopy, reason) + setProgressingFalse(operatorConfigCopy, reason, statusMsg) + c.setVersion() + return + } // No instances of previous deployments, // some replicas are missing, but each has at least 1 available; set Progressing=true if versionUpdatable { diff --git a/pkg/operator/status_test.go b/pkg/operator/status_test.go index 2b307a022..29d492187 100644 --- a/pkg/operator/status_test.go +++ b/pkg/operator/status_test.go @@ -131,6 +131,58 @@ func TestIsDeploymentStatusAvailableAndUpdated(t *testing.T) { } } +func TestIsDeploymentUpToDate(t *testing.T) { + tests := []struct { + name string + deploy appsv1.Deployment + expect bool + }{ + { + name: "fully up-to-date with available replicas", + deploy: makeDeployment("test", 1, 1, 1, 1, 1), + expect: true, + }, + { + name: "up-to-date but not available (node reboot)", + deploy: makeDeployment("test", 1, 1, 1, 1, 0), + expect: true, + }, + { + name: "generation not yet observed", + deploy: makeDeployment("test", 2, 1, 1, 1, 0), + expect: false, + }, + { + name: "updated replicas mismatch", + deploy: makeDeployment("test", 1, 1, 1, 0, 0), + expect: false, + }, + { + name: "replicas scaled down (recreate rollout mid-scale-down)", + deploy: makeDeployment("test", 1, 1, 0, 0, 0), + expect: false, + }, + { + name: "nil spec replicas defaults to 1", + deploy: func() appsv1.Deployment { + d := makeDeployment("test", 1, 1, 1, 1, 0) + d.Spec.Replicas = nil + return d + }(), + expect: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := isDeploymentUpToDate(tt.deploy) + if got != tt.expect { + t.Errorf("isDeploymentUpToDate() = %v, want %v", got, tt.expect) + } + }) + } +} + func TestIsDeploymentStatusComplete(t *testing.T) { tests := []struct { name string @@ -217,14 +269,12 @@ func TestSyncStatus(t *testing.T) { expectReason: "ManagedDeploymentsCompleteAndUpdated", }, { - name: "no available replicas", + name: "node reboot - unavailable but up-to-date", deployments: []appsv1.Deployment{makeDeployment("service-ca", 1, 1, 1, 1, 0)}, - expectProgressing: operatorv1.ConditionTrue, - // TODO: Available should be False when no replicas are available. - // The fallthrough in syncStatus overwrites the earlier setAvailableFalse. - expectAvailable: operatorv1.ConditionTrue, - expectVersionSet: false, - expectReason: "ManagedDeploymentsAvailable", + expectProgressing: operatorv1.ConditionFalse, + expectAvailable: operatorv1.ConditionTrue, + expectVersionSet: true, + expectReason: "ManagedDeploymentsUpToDateButUnavailable", }, { name: "generation mismatch - not yet observed", @@ -238,10 +288,9 @@ func TestSyncStatus(t *testing.T) { name: "recreate rollout - replicas scaled down", deployments: []appsv1.Deployment{makeDeployment("service-ca", 1, 1, 0, 0, 0)}, expectProgressing: operatorv1.ConditionTrue, - // TODO: Available should be False when all replicas are scaled down. - expectAvailable: operatorv1.ConditionTrue, - expectVersionSet: false, - expectReason: "ManagedDeploymentsAvailable", + expectAvailable: operatorv1.ConditionTrue, + expectVersionSet: false, + expectReason: "ManagedDeploymentsAvailable", }, { name: "missing deployment", @@ -262,13 +311,12 @@ func TestSyncStatus(t *testing.T) { }(), }, expectProgressing: operatorv1.ConditionTrue, - // TODO: Available should be False when the deployment is being deleted. - expectAvailable: operatorv1.ConditionTrue, - expectVersionSet: false, - expectReason: "ManagedDeploymentsAvailable", + expectAvailable: operatorv1.ConditionTrue, + expectVersionSet: false, + expectReason: "ManagedDeploymentsAvailable", }, { - name: "available and updated but not all replicas available yet", + name: "available and updated but extra replicas still terminating", deployments: []appsv1.Deployment{makeDeployment("service-ca", 1, 1, 2, 2, 1)}, expectProgressing: operatorv1.ConditionTrue, expectAvailable: operatorv1.ConditionTrue, @@ -324,17 +372,16 @@ func TestSyncStatus(t *testing.T) { expectReason: "ManagedDeploymentsCompleteAndUpdated", }, { - name: "two targets, one complete and one unavailable", + name: "two targets, one complete and one unavailable but up-to-date", targets: sets.New("deploy-a", "deploy-b"), deployments: []appsv1.Deployment{ makeDeployment("deploy-a", 1, 1, 1, 1, 1), makeDeployment("deploy-b", 1, 1, 1, 1, 0), }, - expectProgressing: operatorv1.ConditionTrue, - // TODO: Available should be False when a deployment has no available replicas. - expectAvailable: operatorv1.ConditionTrue, - expectVersionSet: false, - expectReason: "ManagedDeploymentsAvailable", + expectProgressing: operatorv1.ConditionFalse, + expectAvailable: operatorv1.ConditionTrue, + expectVersionSet: true, + expectReason: "ManagedDeploymentsUpToDateButUnavailable", }, { name: "two targets, one complete and one updating", @@ -348,6 +395,18 @@ func TestSyncStatus(t *testing.T) { expectVersionSet: false, expectReason: "ManagedDeploymentsAvailable", }, + { + name: "two targets, one up-to-date but unavailable and one mid-rollout", + targets: sets.New("deploy-a", "deploy-b"), + deployments: []appsv1.Deployment{ + makeDeployment("deploy-a", 1, 1, 1, 1, 0), + makeDeployment("deploy-b", 2, 1, 0, 0, 0), + }, + expectProgressing: operatorv1.ConditionTrue, + expectAvailable: operatorv1.ConditionTrue, + expectVersionSet: false, + expectReason: "ManagedDeploymentsAvailable", + }, { name: "two targets, one missing", targets: sets.New("deploy-a", "deploy-b"),