Merge pull request #32545 from nikhiljindal/fixFedSecTest

Automatic merge from submit-queue

Fixing federation secret and replicaset e2e tests

Verified that both of these pass once https://github.com/kubernetes/kubernetes/pull/32438 is merged.

cc @kubernetes/sig-cluster-federation 

Ref https://github.com/kubernetes/kubernetes/pull/31655#discussion_r78432043

federation/pkg/federation-controller/secret/secret_controller.go

@@ -18,7 +18,6 @@ package secret
 
 import (
 	"fmt"
-	"reflect"
 	"time"
 
 	federation_api "k8s.io/kubernetes/federation/apis/federation/v1beta1"
@@ -300,9 +299,7 @@ func (secretcontroller *SecretController) reconcileSecret(namespace string, secr
 			clusterSecret := clusterSecretObj.(*api_v1.Secret)
 
 			// Update existing secret, if needed.
-			if !util.ObjectMetaEquivalent(desiredSecret.ObjectMeta, clusterSecret.ObjectMeta) ||
+			if !util.SecretEquivalent(*desiredSecret, *clusterSecret) {
-				!reflect.DeepEqual(desiredSecret.Data, clusterSecret.Data) ||
-				!reflect.DeepEqual(desiredSecret.Type, clusterSecret.Type) {
 
 				secretcontroller.eventRecorder.Eventf(baseSecret, api.EventTypeNormal, "UpdateInCluster",
 					"Updating secret in cluster %s", cluster.Name)

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

@@ -0,0 +1,32 @@
+/*
+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"
+)
+
+// Checks if cluster-independent, user provided data in two given Secrets are eqaul. If in
+// the future the Secret structure is expanded then any field that is not populated.
+// by the api server should be included here.
+func SecretEquivalent(s1, s2 api_v1.Secret) bool {
+	return ObjectMetaEquivalent(s1.ObjectMeta, s2.ObjectMeta) &&
+		reflect.DeepEqual(s1.Data, s2.Data) &&
+		reflect.DeepEqual(s1.Type, s2.Type)
+}

test/e2e/federated-secret.go

@@ -18,12 +18,12 @@ package e2e
 
 import (
 	"fmt"
-	"reflect"
 	"time"
 
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	"k8s.io/kubernetes/federation/client/clientset_generated/federation_release_1_4"
+	"k8s.io/kubernetes/federation/pkg/federation-controller/util"
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/api/errors"
 	"k8s.io/kubernetes/pkg/api/v1"
@@ -34,13 +34,12 @@ import (
 
 const (
 	FederatedSecretName    = "federated-secret"
-	UpdatedFederatedSecretName = "updated-federated-secret"
 	FederatedSecretTimeout = 60 * time.Second
 	MaxRetries             = 3
 )
 
 // Create/delete secret api objects
-var _ = framework.KubeDescribe("Federation secrets [Feature:Federation]", func() {
+var _ = framework.KubeDescribe("Federation secrets [Feature:Federation12]", func() {
 	var clusters map[string]*cluster // All clusters, keyed by cluster name
 
 	f := framework.NewDefaultFederatedFramework("federated-secret")
@@ -98,19 +97,17 @@ func updateSecretOrFail(clientset *federation_release_1_4.Clientset, namespace s
 		Fail(fmt.Sprintf("Internal error: invalid parameters passed to updateSecretOrFail: clientset: %v, namespace: %v", clientset, namespace))
 	}
 
-	var err error
 	var newSecret *v1.Secret
-	secret := &v1.Secret{
-		ObjectMeta: v1.ObjectMeta{
-			Name: UpdatedFederatedSecretName,
-		},
-	}
-
 	for retryCount := 0; retryCount < MaxRetries; retryCount++ {
-		_, err = clientset.Core().Secrets(namespace).Get(FederatedSecretName)
+		secret, err := clientset.Core().Secrets(namespace).Get(FederatedSecretName)
 		if err != nil {
 			framework.Failf("failed to get secret %q: %v", FederatedSecretName, err)
 		}
+
+		// Update one of the data in the secret.
+		secret.Data = map[string][]byte{
+			"key": []byte("value"),
+		}
 		newSecret, err = clientset.Core().Secrets(namespace).Update(secret)
 		if err == nil {
 			return newSecret
@@ -149,7 +146,7 @@ func waitForSecretOrFail(clientset *release_1_3.Clientset, namespace string, sec
 	framework.ExpectNoError(err, "Failed to verify secret %q in namespace %q in cluster: Present=%v", secret.Name, namespace, present)
 
 	if present && clusterSecret != nil {
-		Expect(equivalentSecret(*clusterSecret, *secret))
+		Expect(util.SecretEquivalent(*clusterSecret, *secret))
 	}
 }
 
@@ -165,11 +162,13 @@ func waitForSecretUpdateOrFail(clientset *release_1_3.Clientset, namespace strin
 	err := wait.PollImmediate(framework.Poll, timeout, func() (bool, error) {
 		clusterSecret, err := clientset.Core().Secrets(namespace).Get(secret.Name)
 		if err == nil { // We want it present, and the Get succeeded, so we're all good.
-			if equivalentSecret(*clusterSecret, *secret) {
+			if util.SecretEquivalent(*clusterSecret, *secret) {
 				By(fmt.Sprintf("Success: shard of federated secret %q in namespace %q in cluster is updated", secret.Name, namespace))
 				return true, nil
+			} else {
+				By(fmt.Sprintf("Expected equal secrets. expected: %+v\nactual: %+v", *secret, *clusterSecret))
 			}
-			By(fmt.Sprintf("Secret %q in namespace %q in cluster, waiting for service being updated, trying again in %s (err=%v)", secret.Name, namespace, framework.Poll, err))
+			By(fmt.Sprintf("Secret %q in namespace %q in cluster, waiting for secret being updated, trying again in %s (err=%v)", secret.Name, namespace, framework.Poll, err))
 			return false, nil
 		}
 		By(fmt.Sprintf("Secret %q in namespace %q in cluster, waiting for being updated, trying again in %s (err=%v)", secret.Name, namespace, framework.Poll, err))
@@ -177,7 +176,3 @@ func waitForSecretUpdateOrFail(clientset *release_1_3.Clientset, namespace strin
 	})
 	framework.ExpectNoError(err, "Failed to verify secret %q in namespace %q in cluster", secret.Name, namespace)
 }
-
-func equivalentSecret(federatedSecret, clusterSecret v1.Secret) bool {
-	return reflect.DeepEqual(clusterSecret, federatedSecret)
-}

test/e2e/federation-replicaset.go

@@ -92,21 +92,23 @@ var _ = framework.KubeDescribe("Federation replicasets [Feature:Federation]", fu
 		})
 
 		It("should create and update matching replicasets in underling clusters", func() {
-			rs := createReplicaSetOrFail(f.FederationClientset_1_4, f.Namespace.Namespace)
+			nsName := f.FederationNamespace.Name
-			defer func() { // cleanup. deletion of replicasets is not supported for underling clusters
+			rs := createReplicaSetOrFail(f.FederationClientset_1_4, nsName)
-				By(fmt.Sprintf("zero replicas then delete replicaset %q/%q", f.Namespace.Name, rs.Name))
+			defer func() {
+				// cleanup. deletion of replicasets is not supported for underlying clusters
+				By(fmt.Sprintf("Preparing replicaset %q/%q for deletion by setting replicas to zero", nsName, rs.Name))
 				replicas := int32(0)
 				rs.Spec.Replicas = &replicas
-				f.FederationClientset_1_4.ReplicaSets(f.Namespace.Name).Update(rs)
+				f.FederationClientset_1_4.ReplicaSets(nsName).Update(rs)
-				waitForReplicaSetOrFail(f.FederationClientset_1_4, f.Namespace.Name, rs.Name, clusters)
+				waitForReplicaSetOrFail(f.FederationClientset_1_4, nsName, rs.Name, clusters)
-				f.FederationClientset_1_4.ReplicaSets(f.Namespace.Name).Delete(rs.Name, &api.DeleteOptions{})
+				f.FederationClientset_1_4.ReplicaSets(nsName).Delete(rs.Name, &api.DeleteOptions{})
 			}()
 
-			waitForReplicaSetOrFail(f.FederationClientset_1_4, f.Namespace.Name, rs.Name, clusters)
+			waitForReplicaSetOrFail(f.FederationClientset_1_4, nsName, rs.Name, clusters)
-			By(fmt.Sprintf("Successfuly created and synced replicaset %q/%q to clusters", f.Namespace.Namespace, rs.Name))
+			By(fmt.Sprintf("Successfuly created and synced replicaset %q/%q to clusters", nsName, rs.Name))
-			updateReplicaSetOrFail(f.FederationClientset_1_4, f.Namespace.Namespace)
+			updateReplicaSetOrFail(f.FederationClientset_1_4, nsName)
-			waitForReplicaSetOrFail(f.FederationClientset_1_4, f.Namespace.Name, rs.Name, clusters)
+			waitForReplicaSetOrFail(f.FederationClientset_1_4, nsName, rs.Name, clusters)
-			By(fmt.Sprintf("Successfuly updated and synced replicaset %q/%q to clusters", f.Namespace.Namespace, rs.Name))
+			By(fmt.Sprintf("Successfuly updated and synced replicaset %q/%q to clusters", nsName, rs.Name))
 		})
 	})
 })