Revision handling in federated deployment controller

federation/pkg/federation-controller/deployment/deploymentcontroller.go

@@ -562,10 +562,7 @@ func (fdc *DeploymentController) reconcileDeployment(key string) (reconciliation
 		}
 
 		// The object can be modified.
-		ld := &extensionsv1.Deployment{
+		ld := fedutil.DeepCopyDeployment(fd)
-			ObjectMeta: fedutil.DeepCopyRelevantObjectMeta(fd.ObjectMeta),
-			Spec:       fedutil.DeepCopyApiTypeOrPanic(fd.Spec).(extensionsv1.DeploymentSpec),
-		}
 		specReplicas := int32(replicas)
 		ld.Spec.Replicas = &specReplicas
 
@@ -585,7 +582,7 @@ func (fdc *DeploymentController) reconcileDeployment(key string) (reconciliation
 
 			currentLd := ldObj.(*extensionsv1.Deployment)
 			// Update existing replica set, if needed.
-			if !fedutil.ObjectMetaAndSpecEquivalent(ld, currentLd) {
+			if !fedutil.DeploymentEquivalent(ld, currentLd) {
 				fdc.eventRecorder.Eventf(fd, api.EventTypeNormal, "UpdateInCluster",
 					"Updating deployment in cluster %s", clusterName)
 

federation/pkg/federation-controller/util/BUILD

@@ -17,6 +17,7 @@ go_library(
         "cluster_util.go",
         "configmap.go",
         "delaying_deliverer.go",
+        "deployment.go",
         "federated_informer.go",
         "federated_updater.go",
         "handlers.go",
@@ -30,12 +31,14 @@ go_library(
         "//federation/client/clientset_generated/federation_release_1_5:go_default_library",
         "//pkg/api:go_default_library",
         "//pkg/api/v1:go_default_library",
+        "//pkg/apis/extensions/v1beta1:go_default_library",
         "//pkg/client/cache:go_default_library",
         "//pkg/client/clientset_generated/internalclientset:go_default_library",
         "//pkg/client/clientset_generated/release_1_5:go_default_library",
         "//pkg/client/restclient:go_default_library",
         "//pkg/client/unversioned/clientcmd:go_default_library",
         "//pkg/client/unversioned/clientcmd/api:go_default_library",
+        "//pkg/controller/deployment/util:go_default_library",
         "//pkg/conversion:go_default_library",
         "//pkg/runtime:go_default_library",
         "//pkg/util/flowcontrol:go_default_library",
@@ -50,6 +53,7 @@ go_test(
     name = "go_default_test",
     srcs = [
         "delaying_deliverer_test.go",
+        "deployment_test.go",
         "federated_informer_test.go",
         "federated_updater_test.go",
         "handlers_test.go",
@@ -62,10 +66,12 @@ go_test(
         "//federation/client/clientset_generated/federation_release_1_5/fake:go_default_library",
         "//pkg/api:go_default_library",
         "//pkg/api/v1:go_default_library",
+        "//pkg/apis/extensions/v1beta1:go_default_library",
         "//pkg/client/cache:go_default_library",
         "//pkg/client/clientset_generated/release_1_5:go_default_library",
         "//pkg/client/clientset_generated/release_1_5/fake:go_default_library",
         "//pkg/client/testing/core:go_default_library",
+        "//pkg/controller/deployment/util:go_default_library",
         "//pkg/runtime:go_default_library",
         "//pkg/watch:go_default_library",
         "//vendor:github.com/stretchr/testify/assert",

federation/pkg/federation-controller/util/deployment.go

@@ -0,0 +1,75 @@
+/*
+Copyright 2016 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 util
+
+import (
+	"reflect"
+
+	api_v1 "k8s.io/kubernetes/pkg/api/v1"
+	extensions_v1 "k8s.io/kubernetes/pkg/apis/extensions/v1beta1"
+	deputils "k8s.io/kubernetes/pkg/controller/deployment/util"
+)
+
+// Checks if cluster-independent, user provided data in two given Deployment are eqaul.
+// This function assumes that revisions are not kept in sync across the clusters.
+func DeploymentEquivalent(a, b *extensions_v1.Deployment) bool {
+	if a.Name != b.Name {
+		return false
+	}
+	if a.Namespace != b.Namespace {
+		return false
+	}
+	if !reflect.DeepEqual(a.Labels, b.Labels) && (len(a.Labels) != 0 || len(b.Labels) != 0) {
+		return false
+	}
+	hasKeysAndVals := func(x, y map[string]string) bool {
+		if x == nil {
+			x = map[string]string{}
+		}
+		if y == nil {
+			y = map[string]string{}
+		}
+		for k, v := range x {
+			if k == deputils.RevisionAnnotation {
+				continue
+			}
+			v2, found := y[k]
+			if !found || v != v2 {
+				return false
+			}
+		}
+		return true
+	}
+	return hasKeysAndVals(a.Annotations, b.Annotations) &&
+		hasKeysAndVals(b.Annotations, a.Annotations) &&
+		reflect.DeepEqual(a.Spec, b.Spec)
+}
+
+// Copies object meta for Deployment, skipping revision information.
+func DeepCopyDeploymentObjectMeta(meta api_v1.ObjectMeta) api_v1.ObjectMeta {
+	meta = DeepCopyRelevantObjectMeta(meta)
+	delete(meta.Annotations, deputils.RevisionAnnotation)
+	return meta
+}
+
+// Copies object meta for Deployment, skipping revision information.
+func DeepCopyDeployment(a *extensions_v1.Deployment) *extensions_v1.Deployment {
+	return &extensions_v1.Deployment{
+		ObjectMeta: DeepCopyDeploymentObjectMeta(a.ObjectMeta),
+		Spec:       DeepCopyApiTypeOrPanic(a.Spec).(extensions_v1.DeploymentSpec),
+	}
+}

federation/pkg/federation-controller/util/deployment_test.go

@@ -0,0 +1,70 @@
+/*
+Copyright 2016 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 util
+
+import (
+	"testing"
+
+	apiv1 "k8s.io/kubernetes/pkg/api/v1"
+	extensionsv1 "k8s.io/kubernetes/pkg/apis/extensions/v1beta1"
+	deputils "k8s.io/kubernetes/pkg/controller/deployment/util"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestDeploymentEquivalent(t *testing.T) {
+	d1 := newDeployment()
+	d2 := newDeployment()
+	d2.Annotations = make(map[string]string)
+
+	d3 := newDeployment()
+	d3.Annotations = map[string]string{"a": "b"}
+
+	d4 := newDeployment()
+	d4.Annotations = map[string]string{deputils.RevisionAnnotation: "9"}
+
+	assert.True(t, DeploymentEquivalent(d1, d2))
+	assert.True(t, DeploymentEquivalent(d1, d2))
+	assert.True(t, DeploymentEquivalent(d1, d4))
+	assert.True(t, DeploymentEquivalent(d4, d1))
+	assert.False(t, DeploymentEquivalent(d3, d4))
+	assert.False(t, DeploymentEquivalent(d3, d1))
+	assert.True(t, DeploymentEquivalent(d3, d3))
+}
+
+func TestDeploymentCopy(t *testing.T) {
+	d1 := newDeployment()
+	d1.Annotations = map[string]string{deputils.RevisionAnnotation: "9", "a": "b"}
+	d2 := DeepCopyDeployment(d1)
+	assert.True(t, DeploymentEquivalent(d1, d2))
+	assert.Contains(t, d2.Annotations, "a")
+	assert.NotContains(t, d2.Annotations, deputils.RevisionAnnotation)
+}
+
+func newDeployment() *extensionsv1.Deployment {
+	replicas := int32(5)
+	return &extensionsv1.Deployment{
+		ObjectMeta: apiv1.ObjectMeta{
+			Name:      "wrr",
+			Namespace: apiv1.NamespaceDefault,
+			SelfLink:  "/api/v1/namespaces/default/deployments/name123",
+		},
+		Spec: extensionsv1.DeploymentSpec{
+			Replicas: &replicas,
+		},
+	}
+}

test/e2e/federation-deployment.go

@@ -30,8 +30,6 @@ import (
 	"k8s.io/kubernetes/pkg/util/wait"
 	"k8s.io/kubernetes/test/e2e/framework"
 
-	"reflect"
-
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	"k8s.io/kubernetes/pkg/api/errors"
@@ -207,7 +205,7 @@ func waitForDeployment(c *fedclientset.Clientset, namespace string, deploymentNa
 				return false, err
 			}
 			if err == nil {
-				if !equivalentDeployment(fdep, dep) {
+				if !verifyDeployment(fdep, dep) {
 					By(fmt.Sprintf("Deployment meta or spec not match for cluster %q:\n    federation: %v\n    cluster: %v", cluster.name, fdep, dep))
 					return false, nil
 				}
@@ -225,11 +223,10 @@ func waitForDeployment(c *fedclientset.Clientset, namespace string, deploymentNa
 	return err
 }
 
-func equivalentDeployment(fedDeployment, localDeployment *v1beta1.Deployment) bool {
+func verifyDeployment(fedDeployment, localDeployment *v1beta1.Deployment) bool {
-	localDeploymentSpec := localDeployment.Spec
+	localDeployment = fedutil.DeepCopyDeployment(localDeployment)
-	localDeploymentSpec.Replicas = fedDeployment.Spec.Replicas
+	localDeployment.Spec.Replicas = fedDeployment.Spec.Replicas
-	return fedutil.ObjectMetaEquivalent(fedDeployment.ObjectMeta, localDeployment.ObjectMeta) &&
+	return fedutil.DeploymentEquivalent(fedDeployment, localDeployment)
-		reflect.DeepEqual(fedDeployment.Spec, localDeploymentSpec)
 }
 
 func createDeploymentOrFail(clientset *fedclientset.Clientset, namespace string) *v1beta1.Deployment {