diff --git a/federation/pkg/federation-controller/deployment/deploymentcontroller.go b/federation/pkg/federation-controller/deployment/deploymentcontroller.go index e2981c5c87f..90f04e2f9da 100644 --- a/federation/pkg/federation-controller/deployment/deploymentcontroller.go +++ b/federation/pkg/federation-controller/deployment/deploymentcontroller.go @@ -556,10 +556,7 @@ func (fdc *DeploymentController) reconcileDeployment(key string) (reconciliation } // The object can be modified. - ld := &extensionsv1.Deployment{ - ObjectMeta: fedutil.DeepCopyRelevantObjectMeta(fd.ObjectMeta), - Spec: fedutil.DeepCopyApiTypeOrPanic(fd.Spec).(extensionsv1.DeploymentSpec), - } + ld := fedutil.DeepCopyDeployment(fd) specReplicas := int32(replicas) ld.Spec.Replicas = &specReplicas @@ -579,7 +576,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) diff --git a/federation/pkg/federation-controller/util/BUILD b/federation/pkg/federation-controller/util/BUILD index 9ee677814da..9f88e9d7653 100644 --- a/federation/pkg/federation-controller/util/BUILD +++ b/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", @@ -61,10 +65,12 @@ go_test( "//federation/apis/federation/v1beta1:go_default_library", "//federation/client/clientset_generated/federation_release_1_5/fake: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", diff --git a/federation/pkg/federation-controller/util/deployment.go b/federation/pkg/federation-controller/util/deployment.go new file mode 100644 index 00000000000..e13ed23b789 --- /dev/null +++ b/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), + } +} diff --git a/federation/pkg/federation-controller/util/deployment_test.go b/federation/pkg/federation-controller/util/deployment_test.go new file mode 100644 index 00000000000..451a68d561a --- /dev/null +++ b/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, + }, + } +} diff --git a/test/e2e/federation-deployment.go b/test/e2e/federation-deployment.go index 61a4245d098..df7dc2ebfc1 100644 --- a/test/e2e/federation-deployment.go +++ b/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 { - localDeploymentSpec := localDeployment.Spec - localDeploymentSpec.Replicas = fedDeployment.Spec.Replicas - return fedutil.ObjectMetaEquivalent(fedDeployment.ObjectMeta, localDeployment.ObjectMeta) && - reflect.DeepEqual(fedDeployment.Spec, localDeploymentSpec) +func verifyDeployment(fedDeployment, localDeployment *v1beta1.Deployment) bool { + localDeployment = fedutil.DeepCopyDeployment(localDeployment) + localDeployment.Spec.Replicas = fedDeployment.Spec.Replicas + return fedutil.DeploymentEquivalent(fedDeployment, localDeployment) } func createDeploymentOrFail(clientset *fedclientset.Clientset, namespace string) *v1beta1.Deployment {