Move deployment collision avoidance e2e test to integration

test/e2e/apps/deployment.go

@@ -108,9 +108,6 @@ var _ = SIGDescribe("Deployment", func() {
 	It("test Deployment ReplicaSet orphaning and adoption regarding controllerRef", func() {
 		testDeploymentsControllerRef(f)
 	})
-	It("deployment can avoid hash collisions", func() {
-		testDeploymentHashCollisionAvoidance(f)
-	})
 	// TODO: add tests that cover deployment.Spec.MinReadySeconds once we solved clock-skew issues
 	// See https://github.com/kubernetes/kubernetes/issues/29229
 })
@@ -1270,47 +1267,3 @@ func orphanDeploymentReplicaSets(c clientset.Interface, d *extensions.Deployment
 	deleteOptions.Preconditions = metav1.NewUIDPreconditions(string(d.UID))
 	return c.Extensions().Deployments(d.Namespace).Delete(d.Name, deleteOptions)
 }
-
-func testDeploymentHashCollisionAvoidance(f *framework.Framework) {
-	ns := f.Namespace.Name
-	c := f.ClientSet
-
-	deploymentName := "test-hash-collision"
-	framework.Logf("Creating Deployment %q", deploymentName)
-	podLabels := map[string]string{"name": NginxImageName}
-	d := framework.NewDeployment(deploymentName, int32(0), podLabels, NginxImageName, NginxImage, extensions.RollingUpdateDeploymentStrategyType)
-	deployment, err := c.Extensions().Deployments(ns).Create(d)
-	Expect(err).NotTo(HaveOccurred())
-	err = framework.WaitForDeploymentRevisionAndImage(c, ns, deploymentName, "1", NginxImage)
-	Expect(err).NotTo(HaveOccurred())
-
-	// TODO: Switch this to do a non-cascading deletion of the Deployment, mutate the ReplicaSet
-	// once it has no owner reference, then recreate the Deployment if we ever proceed with
-	// https://github.com/kubernetes/kubernetes/issues/44237
-	framework.Logf("Mock a hash collision")
-	newRS, err := deploymentutil.GetNewReplicaSet(deployment, c.ExtensionsV1beta1())
-	Expect(err).NotTo(HaveOccurred())
-	var nilRs *extensions.ReplicaSet
-	Expect(newRS).NotTo(Equal(nilRs))
-	_, err = framework.UpdateReplicaSetWithRetries(c, ns, newRS.Name, func(update *extensions.ReplicaSet) {
-		*update.Spec.Template.Spec.TerminationGracePeriodSeconds = int64(5)
-	})
-	Expect(err).NotTo(HaveOccurred())
-
-	framework.Logf("Expect deployment collision counter to increment")
-	if err := wait.PollImmediate(time.Second, time.Minute, func() (bool, error) {
-		d, err := c.Extensions().Deployments(ns).Get(deploymentName, metav1.GetOptions{})
-		if err != nil {
-			framework.Logf("cannot get deployment %q: %v", deploymentName, err)
-			return false, nil
-		}
-		framework.Logf(spew.Sprintf("deployment status: %#v", d.Status))
-		return d.Status.CollisionCount != nil && *d.Status.CollisionCount == int32(1), nil
-	}); err != nil {
-		framework.Failf("Failed to increment collision counter for deployment %q: %v", deploymentName, err)
-	}
-
-	framework.Logf("Expect a new ReplicaSet to be created")
-	err = framework.WaitForDeploymentRevisionAndImage(c, ns, deploymentName, "2", NginxImage)
-	Expect(err).NotTo(HaveOccurred())
-}

test/e2e/framework/rs_util.go

@@ -34,27 +34,8 @@ import (
 
 type updateRsFunc func(d *extensions.ReplicaSet)
 
-func UpdateReplicaSetWithRetries(c clientset.Interface, namespace, name string, applyUpdate updateRsFunc) (*extensions.ReplicaSet, error) {
-	var rs *extensions.ReplicaSet
-	var updateErr error
-	pollErr := wait.PollImmediate(1*time.Second, 1*time.Minute, func() (bool, error) {
-		var err error
-		if rs, err = c.Extensions().ReplicaSets(namespace).Get(name, metav1.GetOptions{}); err != nil {
-			return false, err
-		}
-		// Apply the update, then attempt to push it to the apiserver.
-		applyUpdate(rs)
-		if rs, err = c.Extensions().ReplicaSets(namespace).Update(rs); err == nil {
-			Logf("Updating replica set %q", name)
-			return true, nil
-		}
-		updateErr = err
-		return false, nil
-	})
-	if pollErr == wait.ErrWaitTimeout {
-		pollErr = fmt.Errorf("couldn't apply the provided updated to replicaset %q: %v", name, updateErr)
-	}
-	return rs, pollErr
+func UpdateReplicaSetWithRetries(c clientset.Interface, namespace, name string, applyUpdate testutils.UpdateReplicaSetFunc) (*extensions.ReplicaSet, error) {
+	return testutils.UpdateReplicaSetWithRetries(c, namespace, name, applyUpdate, Logf)
 }
 
 // CheckNewRSAnnotations check if the new RS's annotation is as expected

test/integration/deployment/BUILD

@@ -20,6 +20,7 @@ go_test(
         "//vendor/k8s.io/api/core/v1:go_default_library",
         "//vendor/k8s.io/api/extensions/v1beta1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library",
     ],
 )
 

test/integration/deployment/deployment_test.go

@@ -20,10 +20,12 @@ import (
 	"reflect"
 	"strings"
 	"testing"
+	"time"
 
 	"k8s.io/api/core/v1"
 	"k8s.io/api/extensions/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/wait"
 	deploymentutil "k8s.io/kubernetes/pkg/controller/deployment/util"
 	"k8s.io/kubernetes/test/integration/framework"
 )
@@ -319,3 +321,64 @@ func TestScalePausedDeployment(t *testing.T) {
 		t.Fatal(err)
 	}
 }
+
+// Deployment rollout shouldn't be blocked on hash collisions
+func TestDeploymentHashCollision(t *testing.T) {
+	s, closeFn, rm, dc, informers, c := dcSetup(t)
+	defer closeFn()
+	name := "test-hash-collision-deployment"
+	ns := framework.CreateTestingNamespace(name, s, t)
+	defer framework.DeleteTestingNamespace(ns, s, t)
+
+	replicas := int32(1)
+	tester := &deploymentTester{t: t, c: c, deployment: newDeployment(name, ns.Name, replicas)}
+
+	var err error
+	tester.deployment, err = c.ExtensionsV1beta1().Deployments(ns.Name).Create(tester.deployment)
+	if err != nil {
+		t.Fatalf("failed to create deployment %s: %v", tester.deployment.Name, err)
+	}
+
+	// Start informer and controllers
+	stopCh := make(chan struct{})
+	defer close(stopCh)
+	informers.Start(stopCh)
+	go rm.Run(5, stopCh)
+	go dc.Run(5, stopCh)
+
+	// Wait for the Deployment to be updated to revision 1
+	if err := tester.waitForDeploymentRevisionAndImage("1", fakeImage); err != nil {
+		t.Fatal(err)
+	}
+
+	// Mock a hash collision
+	newRS, err := deploymentutil.GetNewReplicaSet(tester.deployment, c.ExtensionsV1beta1())
+	if err != nil {
+		t.Fatalf("failed getting new replicaset of deployment %s: %v", tester.deployment.Name, err)
+	}
+	if newRS == nil {
+		t.Fatalf("unable to find new replicaset of deployment %s", tester.deployment.Name)
+	}
+	_, err = tester.updateReplicaSet(newRS.Name, func(update *v1beta1.ReplicaSet) {
+		*update.Spec.Template.Spec.TerminationGracePeriodSeconds = int64(5)
+	})
+	if err != nil {
+		t.Fatalf("failed updating replicaset %s template: %v", newRS.Name, err)
+	}
+
+	// Expect deployment collision counter to increment
+	if err := wait.PollImmediate(time.Second, time.Minute, func() (bool, error) {
+		d, err := c.ExtensionsV1beta1().Deployments(ns.Name).Get(tester.deployment.Name, metav1.GetOptions{})
+		if err != nil {
+			return false, nil
+		}
+		return d.Status.CollisionCount != nil && *d.Status.CollisionCount == int32(1), nil
+	}); err != nil {
+		t.Fatalf("Failed to increment collision counter for deployment %q: %v", tester.deployment.Name, err)
+	}
+
+	// Expect a new ReplicaSet to be created
+	if err := tester.waitForDeploymentRevisionAndImage("2", fakeImage); err != nil {
+		t.Fatal(err)
+	}
+}

test/integration/deployment/util.go

@@ -249,6 +249,10 @@ func (d *deploymentTester) expectNewReplicaSet() (*v1beta1.ReplicaSet, error) {
 	return rs, nil
 }
 
+func (d *deploymentTester) updateReplicaSet(name string, applyUpdate testutil.UpdateReplicaSetFunc) (*v1beta1.ReplicaSet, error) {
+	return testutil.UpdateReplicaSetWithRetries(d.c, d.deployment.Namespace, name, applyUpdate, d.t.Logf)
+}
+
 func pauseFn() func(update *v1beta1.Deployment) {
 	return func(update *v1beta1.Deployment) {
 		update.Spec.Paused = true

test/utils/BUILD

@@ -12,6 +12,7 @@ go_library(
         "density_utils.go",
         "deployment.go",
         "pod_store.go",
+        "replicaset.go",
         "runners.go",
         "tmpdir.go",
     ],

test/utils/replicaset.go

+/*
+Copyright 2017 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package utils
+
+import (
+	"fmt"
+	"time"
+
+	extensions "k8s.io/api/extensions/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/wait"
+	clientset "k8s.io/client-go/kubernetes"
+)
+
+type UpdateReplicaSetFunc func(d *extensions.ReplicaSet)
+
+func UpdateReplicaSetWithRetries(c clientset.Interface, namespace, name string, applyUpdate UpdateReplicaSetFunc, logf LogfFn) (*extensions.ReplicaSet, error) {
+	var rs *extensions.ReplicaSet
+	var updateErr error
+	pollErr := wait.PollImmediate(1*time.Second, 1*time.Minute, func() (bool, error) {
+		var err error
+		if rs, err = c.Extensions().ReplicaSets(namespace).Get(name, metav1.GetOptions{}); err != nil {
+			return false, err
+		}
+		// Apply the update, then attempt to push it to the apiserver.
+		applyUpdate(rs)
+		if rs, err = c.Extensions().ReplicaSets(namespace).Update(rs); err == nil {
+			logf("Updating replica set %q", name)
+			return true, nil
+		}
+		updateErr = err
+		return false, nil
+	})
+	if pollErr == wait.ErrWaitTimeout {
+		pollErr = fmt.Errorf("couldn't apply the provided updated to replicaset %q: %v", name, updateErr)
+	}
+	return rs, pollErr
+}