Wait until operator is rolled out after its deployment is changed

test/e2e/configuration_test.go

@@ -213,12 +213,6 @@ func TestOperatorProxyConfiguration(t *testing.T) {
 	defer framework.TeardownImageRegistry(te)
 	defer framework.ResetClusterProxyConfig(te)
 
-	// Wait for the registry operator to be deployed
-	deployment, err := framework.WaitForRegistryOperatorDeployment(te.Client())
-	if err != nil {
-		t.Fatal(err)
-	}
-
 	// Get the service network to set as NO_PROXY so that the
 	// operator will come up once it is re-deployed
 	network, err := te.Client().Networks().Get(
@@ -265,14 +259,13 @@ func TestOperatorProxyConfiguration(t *testing.T) {
 		); err != nil {
 			t.Fatalf("failed to patch operator env vars: %v", err)
 		}
+
+		framework.WaitUntilDeploymentIsRolledOut(te, framework.OperatorDeploymentNamespace, framework.OperatorDeploymentName)
 	}()
 
 	// Wait for the registry operator to be re-deployed
 	// after the proxy information is injected into the deployment
-	_, err = framework.WaitForNewRegistryOperatorDeployment(te.Client(), deployment.Status.ObservedGeneration)
-	if err != nil {
-		t.Fatal(err)
-	}
+	framework.WaitUntilDeploymentIsRolledOut(te, framework.OperatorDeploymentNamespace, framework.OperatorDeploymentName)
 
 	// Wait for the image registry resource to have an updated StorageExists condition
 	// showing that the operator can no longer reach the storage providers api
@@ -305,6 +298,8 @@ func TestOperatorProxyConfiguration(t *testing.T) {
 		t.Fatalf("failed to patch operator env vars: %v", err)
 	}
 
+	framework.WaitUntilDeploymentIsRolledOut(te, framework.OperatorDeploymentNamespace, framework.OperatorDeploymentName)
+
 	// Wait for the image registry resource to have an updated StorageExists condition
 	// showing that operator can now reach the storage providers api
 	framework.ConditionExistsWithStatusAndReason(te, defaults.StorageExists, operatorapiv1.ConditionTrue, "")
@@ -509,6 +504,8 @@ func TestVersionReporting(t *testing.T) {
 		t.Fatalf("failed to patch operator to new version: %v", err)
 	}
 
+	framework.WaitUntilDeploymentIsRolledOut(te, framework.OperatorDeploymentNamespace, framework.OperatorDeploymentName)
+
 	err := wait.Poll(5*time.Second, 1*time.Minute, func() (bool, error) {
 		clusterOperatorStatus, err := te.Client().ClusterOperators().Get(
 			context.Background(), defaults.ImageRegistryClusterOperatorResourceName, metav1.GetOptions{},

test/framework/cvo.go

@@ -78,10 +78,6 @@ func DisableCVOForOperator(te TestEnv) {
 		}
 	}
 
-	if err := StopDeployment(te, te.Client(), "kube-apiserver-operator", "openshift-kube-apiserver-operator"); err != nil {
-		te.Fatalf("unable to stop kube apiserver operator: %v", err)
-	}
-	if err := StopDeployment(te, te.Client(), "openshift-apiserver-operator", "openshift-apiserver-operator"); err != nil {
-		te.Fatalf("unable to stop openshift apiserver operator: %v", err)
-	}
+	StopDeployment(te, "kube-apiserver-operator", "openshift-kube-apiserver-operator")
+	StopDeployment(te, "openshift-apiserver-operator", "openshift-apiserver-operator")
 }

test/framework/deployment.go

@@ -12,50 +12,38 @@ import (
 	"github.com/openshift/cluster-image-registry-operator/pkg/defaults"
 )
 
-func WaitForRegistryDeployment(client *Clientset) (*kappsapiv1.Deployment, error) {
-	var deployment *kappsapiv1.Deployment
-	err := wait.Poll(1*time.Second, AsyncOperationTimeout, func() (stop bool, err error) {
-		deployment, err = client.Deployments(defaults.ImageRegistryOperatorNamespace).Get(
-			context.Background(), defaults.ImageRegistryName, metav1.GetOptions{},
-		)
-		if err == nil {
-			return true, nil
-		}
-		if errors.IsNotFound(err) {
-			return false, nil
-		}
-		return false, err
-	})
-
-	return deployment, err
+func isDeploymentRolledOut(deploy *kappsapiv1.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
 }
 
-func WaitForNewRegistryDeployment(client *Clientset, currentGeneration int64) (*kappsapiv1.Deployment, error) {
-	var deployment *kappsapiv1.Deployment
+func WaitUntilDeploymentIsRolledOut(te TestEnv, namespace, name string) {
 	err := wait.Poll(1*time.Second, AsyncOperationTimeout, func() (stop bool, err error) {
-		deployment, err = client.Deployments(defaults.ImageRegistryOperatorNamespace).Get(
-			context.Background(), defaults.ImageRegistryName, metav1.GetOptions{},
+		deploy, err := te.Client().Deployments(namespace).Get(
+			context.Background(), name, metav1.GetOptions{},
 		)
-		if err == nil {
-			return true, nil
-		}
-		if errors.IsNotFound(err) {
-			return false, nil
+		if err != nil {
+			return false, err
 		}
-		if deployment.Status.ObservedGeneration == currentGeneration {
-			return false, nil
-		}
-		return false, err
-	})
 
-	return deployment, err
+		return isDeploymentRolledOut(deploy), nil
+	})
+	if err != nil {
+		te.Fatalf("failed to wait until deployment %s/%s is rolled out: %v", namespace, name, err)
+	}
 }
 
-func WaitForRegistryOperatorDeployment(client *Clientset) (*kappsapiv1.Deployment, error) {
+func WaitForRegistryDeployment(client *Clientset) (*kappsapiv1.Deployment, error) {
 	var deployment *kappsapiv1.Deployment
 	err := wait.Poll(1*time.Second, AsyncOperationTimeout, func() (stop bool, err error) {
-		deployment, err = client.Deployments(OperatorDeploymentNamespace).Get(
-			context.Background(), OperatorDeploymentName, metav1.GetOptions{},
+		deployment, err = client.Deployments(defaults.ImageRegistryOperatorNamespace).Get(
+			context.Background(), defaults.ImageRegistryName, metav1.GetOptions{},
 		)
 		if err == nil {
 			return true, nil
@@ -69,11 +57,11 @@ func WaitForRegistryOperatorDeployment(client *Clientset) (*kappsapiv1.Deploymen
 	return deployment, err
 }
 
-func WaitForNewRegistryOperatorDeployment(client *Clientset, currentGeneration int64) (*kappsapiv1.Deployment, error) {
+func WaitForNewRegistryDeployment(client *Clientset, currentGeneration int64) (*kappsapiv1.Deployment, error) {
 	var deployment *kappsapiv1.Deployment
 	err := wait.Poll(1*time.Second, AsyncOperationTimeout, func() (stop bool, err error) {
-		deployment, err = client.Deployments(OperatorDeploymentNamespace).Get(
-			context.Background(), OperatorDeploymentName, metav1.GetOptions{},
+		deployment, err = client.Deployments(defaults.ImageRegistryOperatorNamespace).Get(
+			context.Background(), defaults.ImageRegistryName, metav1.GetOptions{},
 		)
 		if err == nil {
 			return true, nil

test/framework/imageregistry.go

@@ -168,9 +168,7 @@ func RemoveImageRegistry(te TestEnv) {
 	te.Logf("uninstalling the image registry...")
 	ensureImageRegistryToBeRemoved(te)
 	te.Logf("stopping the operator...")
-	if err := StopDeployment(te, te.Client(), OperatorDeploymentName, OperatorDeploymentNamespace); err != nil {
-		te.Fatalf("unable to stop the operator: %s", err)
-	}
+	StopDeployment(te, OperatorDeploymentName, OperatorDeploymentNamespace)
 	te.Logf("deleting the image registry resource...")
 	deleteImageRegistryResource(te)
 }
@@ -192,9 +190,7 @@ func DeployImageRegistry(te TestEnv, spec *imageregistryapiv1.ImageRegistrySpec)
 	}
 
 	te.Logf("starting the operator...")
-	if err := startOperator(te.Client()); err != nil {
-		te.Fatalf("unable to start the operator: %s", err)
-	}
+	startOperator(te)
 }
 
 func DumpImageRegistryResource(te TestEnv) {

test/framework/operator.go

@@ -2,24 +2,24 @@ package framework
 
 import (
 	"context"
-	"fmt"
 	"time"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/wait"
 )
 
-func startOperator(client *Clientset) error {
-	if _, err := client.Deployments(OperatorDeploymentNamespace).Patch(
+func startOperator(te TestEnv) {
+	if _, err := te.Client().Deployments(OperatorDeploymentNamespace).Patch(
 		context.Background(),
 		OperatorDeploymentName,
 		types.MergePatchType, []byte(`{"spec": {"replicas": 1}}`),
 		metav1.PatchOptions{},
 	); err != nil {
-		return err
+		te.Fatalf("unable to start the operator: %s", err)
 	}
-	return nil
+
+	WaitUntilDeploymentIsRolledOut(te, OperatorDeploymentNamespace, OperatorDeploymentName)
 }
 
 func DumpOperatorDeployment(te TestEnv) {
@@ -32,35 +32,25 @@ func DumpOperatorDeployment(te TestEnv) {
 	DumpYAML(te, "the operator deployment", deployment)
 }
 
-func StopDeployment(logger Logger, client *Clientset, operatorDeploymentName, operatorDeploymentNamespace string) error {
-	var err error
-	var realErr error
-	err = wait.Poll(1*time.Second, 30*time.Second, func() (bool, error) {
-		if _, realErr = client.Deployments(operatorDeploymentNamespace).Patch(
+func StopDeployment(te TestEnv, namespace, name string) {
+	err := wait.Poll(1*time.Second, 30*time.Second, func() (bool, error) {
+		if _, realErr := te.Client().Deployments(namespace).Patch(
 			context.Background(),
-			operatorDeploymentName,
+			name,
 			types.MergePatchType,
 			[]byte(`{"spec": {"replicas": 0}}`),
 			metav1.PatchOptions{},
 		); realErr != nil {
-			logger.Logf("failed to patch operator to zero replicas: %v", realErr)
+			te.Logf("failed to patch delpoyment %s/%s to zero replicas: %v", namespace, name, realErr)
 			return false, nil
 		}
 		return true, nil
 	})
 	if err != nil {
-		return fmt.Errorf("unable to patch operator to zero replicas: %v", err)
+		te.Fatal("unable to patch deployment %s/%s to zero replicas: %v", namespace, name, err)
 	}
 
-	return wait.Poll(1*time.Second, AsyncOperationTimeout, func() (stop bool, err error) {
-		deploy, err := client.Deployments(operatorDeploymentNamespace).Get(
-			context.Background(), operatorDeploymentName, metav1.GetOptions{},
-		)
-		if err != nil {
-			return false, err
-		}
-		return deploy.Status.Replicas == 0, nil
-	})
+	WaitUntilDeploymentIsRolledOut(te, namespace, name)
 }
 
 func GetOperatorLogs(client *Clientset) (PodSetLogs, error) {