diff --git a/deploy/chart/templates/_packageserver.deployment-spec.yaml b/deploy/chart/templates/_packageserver.deployment-spec.yaml index 9da288ee34..b0833df1c8 100644 --- a/deploy/chart/templates/_packageserver.deployment-spec.yaml +++ b/deploy/chart/templates/_packageserver.deployment-spec.yaml @@ -2,6 +2,9 @@ spec: strategy: type: RollingUpdate + rollingUpdate: + maxUnavailable: {{ .Values.package.maxUnavailable }} + maxSurge: {{ .Values.package.maxSurge }} replicas: {{ .Values.package.replicaCount }} selector: matchLabels: diff --git a/deploy/chart/values.yaml b/deploy/chart/values.yaml index 0e39194892..1245abaca3 100644 --- a/deploy/chart/values.yaml +++ b/deploy/chart/values.yaml @@ -38,6 +38,8 @@ catalog: package: replicaCount: 2 + maxUnavailable: 1 + maxSurge: 1 image: ref: quay.io/operator-framework/olm:master pullPolicy: Always diff --git a/deploy/ocp/values.yaml b/deploy/ocp/values.yaml index c190c67511..0a3c2ad1bd 100644 --- a/deploy/ocp/values.yaml +++ b/deploy/ocp/values.yaml @@ -63,6 +63,8 @@ catalog: memory: 80Mi package: replicaCount: 2 + maxUnavailable: 1 + maxSurge: 1 image: ref: quay.io/operator-framework/olm@sha256:e9de77ac5c08643202ad42a0311d15c98ffbfd8a1dc2eab4e9f2dcaeed7110ac pullPolicy: IfNotPresent diff --git a/deploy/upstream/values.yaml b/deploy/upstream/values.yaml index a746012c86..8f3b8383d6 100644 --- a/deploy/upstream/values.yaml +++ b/deploy/upstream/values.yaml @@ -22,6 +22,8 @@ catalog: internalPort: 8080 package: replicaCount: 2 + maxUnavailable: 1 + maxSurge: 1 image: ref: quay.io/operator-framework/olm@sha256:e9de77ac5c08643202ad42a0311d15c98ffbfd8a1dc2eab4e9f2dcaeed7110ac pullPolicy: Always diff --git a/pkg/controller/install/deployment_test.go b/pkg/controller/install/deployment_test.go index 8a35c0a6d9..d9ec66cc2a 100644 --- a/pkg/controller/install/deployment_test.go +++ b/pkg/controller/install/deployment_test.go @@ -353,6 +353,10 @@ func TestInstallStrategyDeploymentCheckInstallErrors(t *testing.T) { dep.Spec.Template.SetAnnotations(map[string]string{"test": "annotation"}) dep.Spec.RevisionHistoryLimit = &revisionHistoryLimit dep.SetLabels(labels.CloneAndAddLabel(dep.ObjectMeta.GetLabels(), DeploymentSpecHashLabelKey, HashDeploymentSpec(dep.Spec))) + dep.Status.Conditions = append(dep.Status.Conditions, appsv1.DeploymentCondition{ + Type: appsv1.DeploymentAvailable, + Status: corev1.ConditionTrue, + }) fakeClient.FindAnyDeploymentsMatchingLabelsReturns( []*appsv1.Deployment{ &dep, diff --git a/pkg/controller/install/status_viewer.go b/pkg/controller/install/status_viewer.go index 67a857ef83..5415562993 100644 --- a/pkg/controller/install/status_viewer.go +++ b/pkg/controller/install/status_viewer.go @@ -6,6 +6,7 @@ import ( "fmt" appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" ) const TimedOutReason = "ProgressDeadlineExceeded" @@ -26,9 +27,12 @@ func DeploymentStatus(deployment *appsv1.Deployment) (string, bool, error) { if deployment.Status.Replicas > deployment.Status.UpdatedReplicas { return fmt.Sprintf("Waiting for rollout to finish: %d old replicas are pending termination...\n", deployment.Status.Replicas-deployment.Status.UpdatedReplicas), false, nil } - // waiting for new replicas to report as available - if deployment.Status.AvailableReplicas < deployment.Status.UpdatedReplicas { - return fmt.Sprintf("Waiting for rollout to finish: %d of %d updated replicas are available...\n", deployment.Status.AvailableReplicas, deployment.Status.UpdatedReplicas), false, nil + if c := getDeploymentCondition(deployment.Status, appsv1.DeploymentAvailable); c == nil || c.Status != corev1.ConditionTrue { + msg := fmt.Sprintf("deployment %q missing condition %q", deployment.Name, appsv1.DeploymentAvailable) + if c != nil { + msg = fmt.Sprintf("deployment %q not available: %s", deployment.Name, c.Message) + } + return fmt.Sprintf("Waiting for rollout to finish: %s\n", msg), false, nil } // deployment is finished return fmt.Sprintf("deployment %q successfully rolled out\n", deployment.Name), true, nil diff --git a/pkg/controller/install/status_viewer_test.go b/pkg/controller/install/status_viewer_test.go index 8fcfee725c..67d0ee5847 100644 --- a/pkg/controller/install/status_viewer_test.go +++ b/pkg/controller/install/status_viewer_test.go @@ -1,9 +1,11 @@ package install import ( + "fmt" "testing" apps "k8s.io/api/apps/v1" + core "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -15,6 +17,26 @@ func TestDeploymentStatusViewerStatus(t *testing.T) { msg string done bool }{ + { + generation: 0, + specReplicas: 1, + status: apps.DeploymentStatus{ + ObservedGeneration: 1, + Replicas: 1, + UpdatedReplicas: 0, + AvailableReplicas: 1, + UnavailableReplicas: 0, + Conditions: []apps.DeploymentCondition{ + { + Type: apps.DeploymentProgressing, + Reason: "NotTimedOut", + }, + }, + }, + + msg: "Waiting for rollout to finish: 0 out of 1 new replicas have been updated...\n", + done: false, + }, { generation: 0, specReplicas: 1, @@ -52,9 +74,16 @@ func TestDeploymentStatusViewerStatus(t *testing.T) { UpdatedReplicas: 2, AvailableReplicas: 1, UnavailableReplicas: 1, + Conditions: []apps.DeploymentCondition{ + { + Type: apps.DeploymentAvailable, + Status: core.ConditionFalse, + Message: "Deployment does not have minimum availability.", + }, + }, }, - msg: "Waiting for rollout to finish: 1 of 2 updated replicas are available...\n", + msg: "Waiting for rollout to finish: deployment \"foo\" not available: Deployment does not have minimum availability.\n", done: false, }, { @@ -64,13 +93,38 @@ func TestDeploymentStatusViewerStatus(t *testing.T) { ObservedGeneration: 1, Replicas: 2, UpdatedReplicas: 2, - AvailableReplicas: 2, - UnavailableReplicas: 0, + AvailableReplicas: 1, + UnavailableReplicas: 1, + Conditions: []apps.DeploymentCondition{ + { + Type: apps.DeploymentAvailable, + Status: core.ConditionTrue, + }, + }, }, msg: "deployment \"foo\" successfully rolled out\n", done: true, }, + { + generation: 1, + specReplicas: 2, + status: apps.DeploymentStatus{ + ObservedGeneration: 1, + Replicas: 2, + UpdatedReplicas: 2, + AvailableReplicas: 1, + UnavailableReplicas: 1, + Conditions: []apps.DeploymentCondition{ + { + Type: "Fooing", + Status: core.ConditionTrue, + }, + }, + }, + msg: "Waiting for rollout to finish: deployment \"foo\" missing condition \"Available\"\n", + done: false, + }, { generation: 2, specReplicas: 2, @@ -87,33 +141,35 @@ func TestDeploymentStatusViewerStatus(t *testing.T) { }, } - for _, test := range tests { - d := &apps.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "bar", - Name: "foo", - UID: "8764ae47-9092-11e4-8393-42010af018ff", - Generation: test.generation, - }, - Spec: apps.DeploymentSpec{ - Replicas: &test.specReplicas, - }, - Status: test.status, - } - msg, done, err := DeploymentStatus(d) - if err != nil { - t.Fatalf("DeploymentStatusViewer.Status(): %v", err) - } - if done != test.done || msg != test.msg { - t.Errorf("DeploymentStatusViewer.Status() for deployment with generation %d, %d replicas specified, and status %+v returned %q, %t, want %q, %t", - test.generation, - test.specReplicas, - test.status, - msg, - done, - test.msg, - test.done, - ) - } + for i, test := range tests { + t.Run(fmt.Sprintf("%d", i+1), func(t *testing.T) { + d := &apps.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "bar", + Name: "foo", + UID: "8764ae47-9092-11e4-8393-42010af018ff", + Generation: test.generation, + }, + Spec: apps.DeploymentSpec{ + Replicas: &test.specReplicas, + }, + Status: test.status, + } + msg, done, err := DeploymentStatus(d) + if err != nil { + t.Fatalf("DeploymentStatusViewer.Status(): %v", err) + } + if done != test.done || msg != test.msg { + t.Errorf("DeploymentStatusViewer.Status() for deployment with generation %d, %d replicas specified, and status %+v returned %q, %t, want %q, %t", + test.generation, + test.specReplicas, + test.status, + msg, + done, + test.msg, + test.done, + ) + } + }) } } diff --git a/pkg/controller/operators/olm/operator_test.go b/pkg/controller/operators/olm/operator_test.go index edef4f4fe1..00f2a5e93b 100644 --- a/pkg/controller/operators/olm/operator_test.go +++ b/pkg/controller/operators/olm/operator_test.go @@ -372,6 +372,10 @@ func deployment(deploymentName, namespace, serviceAccountName string, templateAn Replicas: singleInstance, AvailableReplicas: singleInstance, UpdatedReplicas: singleInstance, + Conditions: []appsv1.DeploymentCondition{{ + Type: appsv1.DeploymentAvailable, + Status: corev1.ConditionTrue, + }}, }, } } @@ -3420,6 +3424,10 @@ func TestUpdates(t *testing.T) { dep.Status.Replicas = 1 dep.Status.UpdatedReplicas = 1 dep.Status.AvailableReplicas = 1 + dep.Status.Conditions = []appsv1.DeploymentCondition{{ + Type: appsv1.DeploymentAvailable, + Status: corev1.ConditionTrue, + }} _, err = client.KubernetesInterface().AppsV1().Deployments(namespace).UpdateStatus(context.TODO(), dep, metav1.UpdateOptions{}) require.NoError(t, err) } @@ -4437,6 +4445,10 @@ func TestSyncOperatorGroups(t *testing.T) { dep.Status.Replicas = 1 dep.Status.UpdatedReplicas = 1 dep.Status.AvailableReplicas = 1 + dep.Status.Conditions = []appsv1.DeploymentCondition{{ + Type: appsv1.DeploymentAvailable, + Status: corev1.ConditionTrue, + }} _, err = client.KubernetesInterface().AppsV1().Deployments(tt.initial.operatorGroup.GetNamespace()).UpdateStatus(context.TODO(), dep, metav1.UpdateOptions{}) require.NoError(t, err) } diff --git a/test/e2e/csv_e2e_test.go b/test/e2e/csv_e2e_test.go index e25ae2bab2..754869d5e1 100644 --- a/test/e2e/csv_e2e_test.go +++ b/test/e2e/csv_e2e_test.go @@ -9,6 +9,8 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + "github.com/onsi/gomega/types" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" @@ -27,6 +29,7 @@ import ( v1 "github.com/operator-framework/api/pkg/operators/v1" "github.com/operator-framework/api/pkg/operators/v1alpha1" + operatorsv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1" "github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/clientset/versioned" "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install" "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorclient" @@ -35,19 +38,139 @@ import ( ) var _ = Describe("ClusterServiceVersion", func() { + HavePhase := func(goal operatorsv1alpha1.ClusterServiceVersionPhase) types.GomegaMatcher { + return WithTransform(func(csv *operatorsv1alpha1.ClusterServiceVersion) operatorsv1alpha1.ClusterServiceVersionPhase { + return csv.Status.Phase + }, Equal(goal)) + } + var ( c operatorclient.ClientInterface crc versioned.Interface ) - BeforeEach(func() { + BeforeEach(func() { c = newKubeClient() crc = newCRClient() }) + AfterEach(func() { TearDown(testNamespace) }) + When("a csv exists specifying two replicas with one max unavailable", func() { + var ( + csv v1alpha1.ClusterServiceVersion + ) + + const ( + TestReadinessGate = "operatorframework.io/test-readiness-gate" + ) + + BeforeEach(func() { + csv = v1alpha1.ClusterServiceVersion{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "test-csv", + Namespace: testNamespace, + }, + Spec: v1alpha1.ClusterServiceVersionSpec{ + InstallStrategy: operatorsv1alpha1.NamedInstallStrategy{ + StrategyName: operatorsv1alpha1.InstallStrategyNameDeployment, + StrategySpec: operatorsv1alpha1.StrategyDetailsDeployment{ + DeploymentSpecs: []operatorsv1alpha1.StrategyDeploymentSpec{ + { + Name: "deployment", + Spec: appsv1.DeploymentSpec{ + Strategy: appsv1.DeploymentStrategy{ + Type: appsv1.RollingUpdateDeploymentStrategyType, + RollingUpdate: &appsv1.RollingUpdateDeployment{ + MaxUnavailable: &[]intstr.IntOrString{intstr.FromInt(1)}[0], + }, + }, + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "foobar"}, + }, + Replicas: &[]int32{2}[0], + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"app": "foobar"}, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "foobar", + Image: *dummyImage, + }, + }, + ReadinessGates: []corev1.PodReadinessGate{ + {ConditionType: TestReadinessGate}, + }, + }, + }, + }, + }, + }, + }, + }, + InstallModes: []v1alpha1.InstallMode{{ + Type: v1alpha1.InstallModeTypeAllNamespaces, + Supported: true, + }}, + }, + } + Expect(ctx.Ctx().Client().Create(context.Background(), &csv)).To(Succeed()) + + Eventually(func() (*v1alpha1.ClusterServiceVersion, error) { + var ps corev1.PodList + if err := ctx.Ctx().Client().List(context.Background(), &ps, client.MatchingLabels{"app": "foobar"}); err != nil { + return nil, err + } + + if len(ps.Items) != 2 { + return nil, fmt.Errorf("%d pods match deployment selector, want %d", len(ps.Items), 2) + } + + for _, pod := range ps.Items { + index := -1 + for i, c := range pod.Status.Conditions { + if c.Type == TestReadinessGate { + index = i + break + } + } + if index == -1 { + index = len(pod.Status.Conditions) + pod.Status.Conditions = append(pod.Status.Conditions, corev1.PodCondition{Type: TestReadinessGate}) + } + if pod.Status.Conditions[index].Status == corev1.ConditionTrue { + continue + } + pod.Status.Conditions[index].Status = corev1.ConditionTrue + if err := ctx.Ctx().Client().Status().Update(context.Background(), &pod); err != nil { + return nil, err + } + } + + if err := ctx.Ctx().Client().Get(context.Background(), client.ObjectKeyFromObject(&csv), &csv); err != nil { + return nil, err + } + return &csv, nil + }).Should(HavePhase(operatorsv1alpha1.CSVPhaseSucceeded)) + }) + + It("remains in phase Succeeded when only one pod is available", func() { + var ps corev1.PodList + Expect(ctx.Ctx().Client().List(context.Background(), &ps, client.MatchingLabels{"app": "foobar"})).To(Succeed()) + Expect(ps.Items).To(Not(BeEmpty())) + + Expect(ctx.Ctx().Client().Delete(context.Background(), &ps.Items[0])).To(Succeed()) + + Consistently(func() (*operatorsv1alpha1.ClusterServiceVersion, error) { + return &csv, ctx.Ctx().Client().Get(context.Background(), client.ObjectKeyFromObject(&csv), &csv) + }).Should(HavePhase(operatorsv1alpha1.CSVPhaseSucceeded)) + }) + }) + When("a copied csv exists", func() { var ( target corev1.Namespace