From 242c11b580c1dfb732bc9ee7d6e4800edcdec9a5 Mon Sep 17 00:00:00 2001 From: Janet Kuo Date: Tue, 2 Aug 2016 12:11:25 -0700 Subject: [PATCH] Deployment status validity should be checked in scaled rollout e2e test --- test/e2e/deployment.go | 30 ++++++++++++++++++++---------- test/e2e/framework/util.go | 22 +++++----------------- 2 files changed, 25 insertions(+), 27 deletions(-) diff --git a/test/e2e/deployment.go b/test/e2e/deployment.go index 8933f002aa..1f61d29eba 100644 --- a/test/e2e/deployment.go +++ b/test/e2e/deployment.go @@ -1042,8 +1042,14 @@ func testScaledRolloutDeployment(f *framework.Framework) { deployment, err = c.Extensions().Deployments(ns).Get(deploymentName) Expect(err).NotTo(HaveOccurred()) - By(fmt.Sprintf("Waiting for deployment status to sync (current available: %d, minimum available: %d)", deployment.Status.AvailableReplicas, deploymentutil.MinAvailable(deployment))) - err = framework.WaitForDeploymentStatusValid(c, deployment, true) + // Verify that the required pods have come up. + By("Waiting for all required pods to come up") + err = framework.VerifyPods(f.Client, ns, nginxImageName, false, deployment.Spec.Replicas) + if err != nil { + framework.Logf("error in waiting for pods to come up: %s", err) + Expect(err).NotTo(HaveOccurred()) + } + err = framework.WaitForDeploymentStatus(c, deployment) Expect(err).NotTo(HaveOccurred()) first, err := deploymentutil.GetNewReplicaSet(deployment, c) @@ -1063,9 +1069,9 @@ func testScaledRolloutDeployment(f *framework.Framework) { deployment, err = c.Extensions().Deployments(ns).Get(deploymentName) Expect(err).NotTo(HaveOccurred()) - By(fmt.Sprintf("Waiting for deployment status to sync (current available: %d, minimum available: %d)", deployment.Status.AvailableReplicas, deploymentutil.MinAvailable(deployment))) - err = framework.WaitForDeploymentStatusValid(c, deployment, false) - Expect(err).NotTo(HaveOccurred()) + if deployment.Status.AvailableReplicas < deploymentutil.MinAvailable(deployment) { + Expect(fmt.Errorf("Observed %d available replicas, less than min required %d", deployment.Status.AvailableReplicas, deploymentutil.MinAvailable(deployment))).NotTo(HaveOccurred()) + } By(fmt.Sprintf("Checking that the replica sets for %q are synced", deploymentName)) second, err := deploymentutil.GetNewReplicaSet(deployment, c) @@ -1105,7 +1111,9 @@ func testScaledRolloutDeployment(f *framework.Framework) { } By(fmt.Sprintf("Waiting for deployment status to sync (current available: %d, minimum available: %d)", deployment.Status.AvailableReplicas, deploymentutil.MinAvailable(deployment))) - err = framework.WaitForDeploymentStatusValid(c, deployment, true) + err = framework.WaitForDeploymentStatusValid(c, deployment) + Expect(err).NotTo(HaveOccurred()) + err = framework.WaitForDeploymentStatus(c, deployment) Expect(err).NotTo(HaveOccurred()) // Update the deployment with a non-existent image so that the new replica set will be blocked. @@ -1122,9 +1130,9 @@ func testScaledRolloutDeployment(f *framework.Framework) { deployment, err = c.Extensions().Deployments(ns).Get(deploymentName) Expect(err).NotTo(HaveOccurred()) - By(fmt.Sprintf("Waiting for deployment status to sync (current available: %d, minimum available: %d)", deployment.Status.AvailableReplicas, deploymentutil.MinAvailable(deployment))) - err = framework.WaitForDeploymentStatusValid(c, deployment, false) - Expect(err).NotTo(HaveOccurred()) + if deployment.Status.AvailableReplicas < deploymentutil.MinAvailable(deployment) { + Expect(fmt.Errorf("Observed %d available replicas, less than min required %d", deployment.Status.AvailableReplicas, deploymentutil.MinAvailable(deployment))).NotTo(HaveOccurred()) + } By(fmt.Sprintf("Checking that the replica sets for %q are synced", deploymentName)) oldRs, err := c.Extensions().ReplicaSets(rs.Namespace).Get(rs.Name) @@ -1164,6 +1172,8 @@ func testScaledRolloutDeployment(f *framework.Framework) { } By(fmt.Sprintf("Waiting for deployment status to sync (current available: %d, minimum available: %d)", deployment.Status.AvailableReplicas, deploymentutil.MinAvailable(deployment))) - err = framework.WaitForDeploymentStatusValid(c, deployment, true) + err = framework.WaitForDeploymentStatusValid(c, deployment) + Expect(err).NotTo(HaveOccurred()) + err = framework.WaitForDeploymentStatus(c, deployment) Expect(err).NotTo(HaveOccurred()) } diff --git a/test/e2e/framework/util.go b/test/e2e/framework/util.go index e19b163339..a9d4690bd7 100644 --- a/test/e2e/framework/util.go +++ b/test/e2e/framework/util.go @@ -3200,9 +3200,10 @@ func waitForReplicaSetPodsGone(c *client.Client, rs *extensions.ReplicaSet) erro }) } -// Waits for the deployment status to sync (i.e. max unavailable and max surge aren't violated anymore). -// If expectComplete, wait until all its replicas become up-to-date. -func WaitForDeploymentStatusValid(c clientset.Interface, d *extensions.Deployment, expectComplete bool) error { +// Waits for the deployment status to become valid (i.e. max unavailable and max surge aren't violated anymore). +// Note that the status should stay valid at all times unless shortly after a scaling event or the deployment is just created. +// To verify that the deployment status is valid and wait for the rollout to finish, use WaitForDeploymentStatus instead. +func WaitForDeploymentStatusValid(c clientset.Interface, d *extensions.Deployment) error { var ( oldRSs, allOldRSs, allRSs []*extensions.ReplicaSet newRS *extensions.ReplicaSet @@ -3252,20 +3253,7 @@ func WaitForDeploymentStatusValid(c clientset.Interface, d *extensions.Deploymen Logf(reason) return false, nil } - - if !expectComplete { - return true, nil - } - - // When the deployment status and its underlying resources reach the desired state, we're done - if deployment.Status.Replicas == deployment.Spec.Replicas && - deployment.Status.UpdatedReplicas == deployment.Spec.Replicas && - deploymentutil.GetReplicaCountForReplicaSets(oldRSs) == 0 && - deploymentutil.GetReplicaCountForReplicaSets([]*extensions.ReplicaSet{newRS}) == deployment.Spec.Replicas { - return true, nil - } - reason = fmt.Sprintf("deployment %q is yet to complete: %#v", deployment.Name, deployment.Status) - return false, nil + return true, nil }) if err == wait.ErrWaitTimeout {