From 36c34af2fae27a42e293d9f6759019bfa6dedeb1 Mon Sep 17 00:00:00 2001 From: Matt Liggett Date: Fri, 1 Jul 2016 09:50:57 -0700 Subject: [PATCH 1/6] cluster type --- test/e2e/federated-service.go | 62 +++++++++++++++++++---------------- 1 file changed, 33 insertions(+), 29 deletions(-) diff --git a/test/e2e/federated-service.go b/test/e2e/federated-service.go index 3728b3afe19..54a22677350 100644 --- a/test/e2e/federated-service.go +++ b/test/e2e/federated-service.go @@ -62,9 +62,13 @@ var FederatedServiceLabels = map[string]string{ "foo": "bar", } +type cluster struct { + *release_1_3.Clientset +} + var _ = framework.KubeDescribe("[Feature:Federation]", func() { f := framework.NewDefaultFederatedFramework("federated-service") - var clusterClientSets []*release_1_3.Clientset + var clusters []cluster var clusterNamespaceCreated []bool // Did we need to create a new namespace in each of the above clusters? If so, we should delete it. var federationName string @@ -106,7 +110,7 @@ var _ = framework.KubeDescribe("[Feature:Federation]", func() { } framework.Logf("%d clusters are Ready", len(contexts)) - clusterClientSets = make([]*release_1_3.Clientset, len(clusterList.Items)) + clusters = make([]cluster, len(clusterList.Items)) for i, cluster := range clusterList.Items { framework.Logf("Creating a clientset for the cluster %s", cluster.Name) @@ -126,19 +130,19 @@ var _ = framework.KubeDescribe("[Feature:Federation]", func() { cfg.QPS = KubeAPIQPS cfg.Burst = KubeAPIBurst clset := release_1_3.NewForConfigOrDie(restclient.AddUserAgent(cfg, UserAgentName)) - clusterClientSets[i] = clset + clusters[i].Clientset = clset } - clusterNamespaceCreated = make([]bool, len(clusterClientSets)) - for i, cs := range clusterClientSets { + clusterNamespaceCreated = make([]bool, len(clusters)) + for i, c := range clusters { // The e2e Framework created the required namespace in one of the clusters, but we need to create it in all the others, if it doesn't yet exist. - if _, err := cs.Core().Namespaces().Get(f.Namespace.Name); errors.IsNotFound(err) { + if _, err := c.Clientset.Core().Namespaces().Get(f.Namespace.Name); errors.IsNotFound(err) { ns := &v1.Namespace{ ObjectMeta: v1.ObjectMeta{ Name: f.Namespace.Name, }, } - _, err := cs.Core().Namespaces().Create(ns) + _, err := c.Clientset.Core().Namespaces().Create(ns) if err == nil { clusterNamespaceCreated[i] = true } @@ -151,10 +155,10 @@ var _ = framework.KubeDescribe("[Feature:Federation]", func() { }) AfterEach(func() { - for i, cs := range clusterClientSets { + for i, c := range clusters { if clusterNamespaceCreated[i] { - if _, err := cs.Core().Namespaces().Get(f.Namespace.Name); !errors.IsNotFound(err) { - err := cs.Core().Namespaces().Delete(f.Namespace.Name, &api.DeleteOptions{}) + if _, err := c.Clientset.Core().Namespaces().Get(f.Namespace.Name); !errors.IsNotFound(err) { + err := c.Clientset.Core().Namespaces().Delete(f.Namespace.Name, &api.DeleteOptions{}) framework.ExpectNoError(err, "Couldn't delete the namespace %s in cluster [%d]: %v", f.Namespace.Name, i, err) } framework.Logf("Namespace %s deleted in cluster [%d]", f.Namespace.Name, i) @@ -199,7 +203,7 @@ var _ = framework.KubeDescribe("[Feature:Federation]", func() { err := f.FederationClientset_1_3.Services(f.Namespace.Name).Delete(service.Name, &api.DeleteOptions{}) framework.ExpectNoError(err, "Error deleting service %q in namespace %q", service.Name, f.Namespace.Name) }() - waitForServiceShardsOrFail(f.Namespace.Name, service, clusterClientSets, nil) + waitForServiceShardsOrFail(f.Namespace.Name, service, clusters, nil) }) }) @@ -212,15 +216,15 @@ var _ = framework.KubeDescribe("[Feature:Federation]", func() { BeforeEach(func() { framework.SkipUnlessFederated(f.Client) - backendPods = createBackendPodsOrFail(clusterClientSets, f.Namespace.Name, FederatedServicePodName) + backendPods = createBackendPodsOrFail(clusters, f.Namespace.Name, FederatedServicePodName) service = createServiceOrFail(f.FederationClientset_1_3, f.Namespace.Name) - waitForServiceShardsOrFail(f.Namespace.Name, service, clusterClientSets, nil) + waitForServiceShardsOrFail(f.Namespace.Name, service, clusters, nil) }) AfterEach(func() { framework.SkipUnlessFederated(f.Client) if backendPods != nil { - deleteBackendPodsOrFail(clusterClientSets, f.Namespace.Name, backendPods) + deleteBackendPodsOrFail(clusters, f.Namespace.Name, backendPods) backendPods = nil } else { By("No backend pods to delete. BackendPods is nil.") @@ -257,7 +261,7 @@ var _ = framework.KubeDescribe("[Feature:Federation]", func() { framework.SkipUnlessFederated(f.Client) // Delete all the backend pods from the shard which is local to the discovery pod. - deleteBackendPodsOrFail([]*release_1_3.Clientset{f.Clientset_1_3}, f.Namespace.Name, []*v1.Pod{backendPods[0]}) + deleteBackendPodsOrFail([]cluster{{f.Clientset_1_3}}, f.Namespace.Name, []*v1.Pod{backendPods[0]}) }) @@ -345,19 +349,19 @@ func waitForServiceOrFail(clientset *release_1_3.Clientset, namespace string, se If presentInCluster[n] is true, then wait for service shard to exist in the cluster specifid in clientsets[n] If presentInCluster[n] is false, then wait for service shard to not exist in the cluster specifid in clientsets[n] */ -func waitForServiceShardsOrFail(namespace string, service *v1.Service, clientsets []*release_1_3.Clientset, presentInCluster []bool) { +func waitForServiceShardsOrFail(namespace string, service *v1.Service, clusters []cluster, presentInCluster []bool) { if presentInCluster != nil { - Expect(len(presentInCluster)).To(Equal(len(clientsets)), "Internal error: Number of presence flags does not equal number of clients/clusters") + Expect(len(presentInCluster)).To(Equal(len(clusters)), "Internal error: Number of presence flags does not equal number of clients/clusters") } - framework.Logf("Waiting for service %q in %d clusters", service.Name, len(clientsets)) - for i, clientset := range clientsets { + framework.Logf("Waiting for service %q in %d clusters", service.Name, len(clusters)) + for i, c := range clusters { var present bool // Should the service be present or absent in this cluster? if presentInCluster == nil { present = true } else { present = presentInCluster[i] } - waitForServiceOrFail(clientset, namespace, service, present, FederatedServiceTimeout) + waitForServiceOrFail(c.Clientset, namespace, service, present, FederatedServiceTimeout) } } @@ -484,7 +488,7 @@ func discoverService(f *framework.Framework, name string, exists bool, podName s createBackendPodsOrFail creates one pod in each cluster, and returns the created pods (in the same order as clusterClientSets). If creation of any pod fails, the test fails (possibly with a partially created set of pods). No retries are attempted. */ -func createBackendPodsOrFail(clusterClientSets []*release_1_3.Clientset, namespace string, name string) []*v1.Pod { +func createBackendPodsOrFail(clusters []cluster, namespace string, name string) []*v1.Pod { pod := &v1.Pod{ ObjectMeta: v1.ObjectMeta{ Name: name, @@ -501,10 +505,10 @@ func createBackendPodsOrFail(clusterClientSets []*release_1_3.Clientset, namespa RestartPolicy: v1.RestartPolicyAlways, }, } - pods := make([]*v1.Pod, len(clusterClientSets)) - for i, client := range clusterClientSets { + pods := make([]*v1.Pod, len(clusters)) + for i, c := range clusters { By(fmt.Sprintf("Creating pod %q in namespace %q in cluster %d", pod.Name, namespace, i)) - createdPod, err := client.Core().Pods(namespace).Create(pod) + createdPod, err := c.Clientset.Core().Pods(namespace).Create(pod) framework.ExpectNoError(err, "Creating pod %q in namespace %q in cluster %d", name, namespace, i) By(fmt.Sprintf("Successfully created pod %q in namespace %q in cluster %d: %v", pod.Name, namespace, i, *createdPod)) pods[i] = createdPod @@ -516,13 +520,13 @@ func createBackendPodsOrFail(clusterClientSets []*release_1_3.Clientset, namespa deleteBackendPodsOrFail deletes one pod from each cluster (unless pods[n] is nil for that cluster) If deletion of any pod fails, the test fails (possibly with a partially deleted set of pods). No retries are attempted. */ -func deleteBackendPodsOrFail(clusterClientSets []*release_1_3.Clientset, namespace string, pods []*v1.Pod) { - if len(clusterClientSets) != len(pods) { - Fail(fmt.Sprintf("Internal error: number of clients (%d) does not equal number of pods (%d). One pod per client please.", len(clusterClientSets), len(pods))) +func deleteBackendPodsOrFail(clusters []cluster, namespace string, pods []*v1.Pod) { + if len(clusters) != len(pods) { + Fail(fmt.Sprintf("Internal error: number of clients (%d) does not equal number of pods (%d). One pod per client please.", len(clusters), len(pods))) } - for i, client := range clusterClientSets { + for i, c := range clusters { if pods[i] != nil { - err := client.Core().Pods(namespace).Delete(pods[i].Name, api.NewDeleteOptions(0)) + err := c.Clientset.Core().Pods(namespace).Delete(pods[i].Name, api.NewDeleteOptions(0)) if errors.IsNotFound(err) { By(fmt.Sprintf("Pod %q in namespace %q in cluster %d does not exist. No need to delete it.", pods[i].Name, namespace, i)) } else { From 49a69b17f7f20d36902ea54c811aabeec84bd8a6 Mon Sep 17 00:00:00 2001 From: Matt Liggett Date: Fri, 1 Jul 2016 10:34:46 -0700 Subject: [PATCH 2/6] add clusterNamespaceCreated to it --- test/e2e/federated-service.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/test/e2e/federated-service.go b/test/e2e/federated-service.go index 54a22677350..08a8f1c01bf 100644 --- a/test/e2e/federated-service.go +++ b/test/e2e/federated-service.go @@ -62,14 +62,18 @@ var FederatedServiceLabels = map[string]string{ "foo": "bar", } +/* +type cluster keeps track of the assorted objects and state related to each +cluster in the federation +*/ type cluster struct { *release_1_3.Clientset + namespaceCreated bool // Did we need to create a new namespace in this cluster? If so, we should delete it. } var _ = framework.KubeDescribe("[Feature:Federation]", func() { f := framework.NewDefaultFederatedFramework("federated-service") var clusters []cluster - var clusterNamespaceCreated []bool // Did we need to create a new namespace in each of the above clusters? If so, we should delete it. var federationName string var _ = Describe("Federated Services", func() { @@ -133,7 +137,6 @@ var _ = framework.KubeDescribe("[Feature:Federation]", func() { clusters[i].Clientset = clset } - clusterNamespaceCreated = make([]bool, len(clusters)) for i, c := range clusters { // The e2e Framework created the required namespace in one of the clusters, but we need to create it in all the others, if it doesn't yet exist. if _, err := c.Clientset.Core().Namespaces().Get(f.Namespace.Name); errors.IsNotFound(err) { @@ -144,7 +147,7 @@ var _ = framework.KubeDescribe("[Feature:Federation]", func() { } _, err := c.Clientset.Core().Namespaces().Create(ns) if err == nil { - clusterNamespaceCreated[i] = true + c.namespaceCreated = true } framework.ExpectNoError(err, "Couldn't create the namespace %s in cluster [%d]", f.Namespace.Name, i) framework.Logf("Namespace %s created in cluster [%d]", f.Namespace.Name, i) @@ -156,7 +159,7 @@ var _ = framework.KubeDescribe("[Feature:Federation]", func() { AfterEach(func() { for i, c := range clusters { - if clusterNamespaceCreated[i] { + if c.namespaceCreated { if _, err := c.Clientset.Core().Namespaces().Get(f.Namespace.Name); !errors.IsNotFound(err) { err := c.Clientset.Core().Namespaces().Delete(f.Namespace.Name, &api.DeleteOptions{}) framework.ExpectNoError(err, "Couldn't delete the namespace %s in cluster [%d]: %v", f.Namespace.Name, i, err) @@ -261,7 +264,8 @@ var _ = framework.KubeDescribe("[Feature:Federation]", func() { framework.SkipUnlessFederated(f.Client) // Delete all the backend pods from the shard which is local to the discovery pod. - deleteBackendPodsOrFail([]cluster{{f.Clientset_1_3}}, f.Namespace.Name, []*v1.Pod{backendPods[0]}) + // FIXME(mml): Use a function that deletes backends only in one cluster. + deleteBackendPodsOrFail([]cluster{{f.Clientset_1_3, false}}, f.Namespace.Name, []*v1.Pod{backendPods[0]}) }) From 088b871729c4f5a5b1b122ee8c676a6b1af49289 Mon Sep 17 00:00:00 2001 From: Matt Liggett Date: Fri, 1 Jul 2016 14:54:29 -0700 Subject: [PATCH 3/6] Switch to a map. --- test/e2e/federated-service.go | 127 ++++++++++++++++------------------ 1 file changed, 60 insertions(+), 67 deletions(-) diff --git a/test/e2e/federated-service.go b/test/e2e/federated-service.go index 08a8f1c01bf..6a25aafeb1f 100644 --- a/test/e2e/federated-service.go +++ b/test/e2e/federated-service.go @@ -67,14 +67,17 @@ type cluster keeps track of the assorted objects and state related to each cluster in the federation */ type cluster struct { + name string *release_1_3.Clientset - namespaceCreated bool // Did we need to create a new namespace in this cluster? If so, we should delete it. + namespaceCreated bool // Did we need to create a new namespace in this cluster? If so, we should delete it. + backendPod *v1.Pod // The backend pod, if one's been created. } var _ = framework.KubeDescribe("[Feature:Federation]", func() { f := framework.NewDefaultFederatedFramework("federated-service") - var clusters []cluster + var clusters map[string]cluster var federationName string + var primaryClusterName string // The name of the "primary" cluster var _ = Describe("Federated Services", func() { BeforeEach(func() { @@ -114,9 +117,12 @@ var _ = framework.KubeDescribe("[Feature:Federation]", func() { } framework.Logf("%d clusters are Ready", len(contexts)) - clusters = make([]cluster, len(clusterList.Items)) - for i, cluster := range clusterList.Items { - framework.Logf("Creating a clientset for the cluster %s", cluster.Name) + // clusters = make([]cluster, len(clusterList.Items)) + clusters = map[string]cluster{} + primaryClusterName = clusterList.Items[0].Name + By(fmt.Sprintf("Labeling %q as the first cluster", primaryClusterName)) + for i, c := range clusterList.Items { + framework.Logf("Creating a clientset for the cluster %s", c.Name) Expect(framework.TestContext.KubeConfig).ToNot(Equal(""), "KubeConfig must be specified to load clusters' client config") kubecfg, err := clientcmd.LoadFromFile(framework.TestContext.KubeConfig) @@ -124,20 +130,20 @@ var _ = framework.KubeDescribe("[Feature:Federation]", func() { cfgOverride := &clientcmd.ConfigOverrides{ ClusterInfo: clientcmdapi.Cluster{ - Server: cluster.Spec.ServerAddressByClientCIDRs[0].ServerAddress, + Server: c.Spec.ServerAddressByClientCIDRs[0].ServerAddress, }, } - ccfg := clientcmd.NewNonInteractiveClientConfig(*kubecfg, cluster.Name, cfgOverride, clientcmd.NewDefaultClientConfigLoadingRules()) + ccfg := clientcmd.NewNonInteractiveClientConfig(*kubecfg, c.Name, cfgOverride, clientcmd.NewDefaultClientConfigLoadingRules()) cfg, err := ccfg.ClientConfig() - framework.ExpectNoError(err, "Error creating client config in cluster #%d", i) + framework.ExpectNoError(err, "Error creating client config in cluster #%d (%q)", i, c.Name) cfg.QPS = KubeAPIQPS cfg.Burst = KubeAPIBurst clset := release_1_3.NewForConfigOrDie(restclient.AddUserAgent(cfg, UserAgentName)) - clusters[i].Clientset = clset + clusters[c.Name] = cluster{c.Name, clset, false, nil} } - for i, c := range clusters { + for name, c := range clusters { // The e2e Framework created the required namespace in one of the clusters, but we need to create it in all the others, if it doesn't yet exist. if _, err := c.Clientset.Core().Namespaces().Get(f.Namespace.Name); errors.IsNotFound(err) { ns := &v1.Namespace{ @@ -149,22 +155,22 @@ var _ = framework.KubeDescribe("[Feature:Federation]", func() { if err == nil { c.namespaceCreated = true } - framework.ExpectNoError(err, "Couldn't create the namespace %s in cluster [%d]", f.Namespace.Name, i) - framework.Logf("Namespace %s created in cluster [%d]", f.Namespace.Name, i) + framework.ExpectNoError(err, "Couldn't create the namespace %s in cluster %q", f.Namespace.Name, name) + framework.Logf("Namespace %s created in cluster %q", f.Namespace.Name, name) } else if err != nil { - framework.Logf("Couldn't create the namespace %s in cluster [%d]: %v", f.Namespace.Name, i, err) + framework.Logf("Couldn't create the namespace %s in cluster %q: %v", f.Namespace.Name, name, err) } } }) AfterEach(func() { - for i, c := range clusters { + for name, c := range clusters { if c.namespaceCreated { if _, err := c.Clientset.Core().Namespaces().Get(f.Namespace.Name); !errors.IsNotFound(err) { err := c.Clientset.Core().Namespaces().Delete(f.Namespace.Name, &api.DeleteOptions{}) - framework.ExpectNoError(err, "Couldn't delete the namespace %s in cluster [%d]: %v", f.Namespace.Name, i, err) + framework.ExpectNoError(err, "Couldn't delete the namespace %s in cluster %q: %v", f.Namespace.Name, name, err) } - framework.Logf("Namespace %s deleted in cluster [%d]", f.Namespace.Name, i) + framework.Logf("Namespace %s deleted in cluster %q", f.Namespace.Name, name) } } @@ -206,32 +212,26 @@ var _ = framework.KubeDescribe("[Feature:Federation]", func() { err := f.FederationClientset_1_3.Services(f.Namespace.Name).Delete(service.Name, &api.DeleteOptions{}) framework.ExpectNoError(err, "Error deleting service %q in namespace %q", service.Name, f.Namespace.Name) }() - waitForServiceShardsOrFail(f.Namespace.Name, service, clusters, nil) + waitForServiceShardsOrFail(f.Namespace.Name, service, clusters) }) }) var _ = Describe("DNS", func() { var ( - service *v1.Service - backendPods []*v1.Pod + service *v1.Service ) BeforeEach(func() { framework.SkipUnlessFederated(f.Client) - backendPods = createBackendPodsOrFail(clusters, f.Namespace.Name, FederatedServicePodName) + createBackendPodsOrFail(clusters, f.Namespace.Name, FederatedServicePodName) service = createServiceOrFail(f.FederationClientset_1_3, f.Namespace.Name) - waitForServiceShardsOrFail(f.Namespace.Name, service, clusters, nil) + waitForServiceShardsOrFail(f.Namespace.Name, service, clusters) }) AfterEach(func() { framework.SkipUnlessFederated(f.Client) - if backendPods != nil { - deleteBackendPodsOrFail(clusters, f.Namespace.Name, backendPods) - backendPods = nil - } else { - By("No backend pods to delete. BackendPods is nil.") - } + deleteBackendPodsOrFail(clusters, f.Namespace.Name) if service != nil { deleteServiceOrFail(f.FederationClientset_1_3, f.Namespace.Name, service.Name) @@ -264,8 +264,7 @@ var _ = framework.KubeDescribe("[Feature:Federation]", func() { framework.SkipUnlessFederated(f.Client) // Delete all the backend pods from the shard which is local to the discovery pod. - // FIXME(mml): Use a function that deletes backends only in one cluster. - deleteBackendPodsOrFail([]cluster{{f.Clientset_1_3, false}}, f.Namespace.Name, []*v1.Pod{backendPods[0]}) + deleteOneBackendPodOrFail(clusters[primaryClusterName]) }) @@ -349,23 +348,12 @@ func waitForServiceOrFail(clientset *release_1_3.Clientset, namespace string, se } /* - waitForServiceShardsOrFail waits for the service to appear (or disappear) in the clientsets specifed in presentInCluster (or all if presentInCluster is nil). - If presentInCluster[n] is true, then wait for service shard to exist in the cluster specifid in clientsets[n] - If presentInCluster[n] is false, then wait for service shard to not exist in the cluster specifid in clientsets[n] + waitForServiceShardsOrFail waits for the service to appear in all clusters */ -func waitForServiceShardsOrFail(namespace string, service *v1.Service, clusters []cluster, presentInCluster []bool) { - if presentInCluster != nil { - Expect(len(presentInCluster)).To(Equal(len(clusters)), "Internal error: Number of presence flags does not equal number of clients/clusters") - } +func waitForServiceShardsOrFail(namespace string, service *v1.Service, clusters map[string]cluster) { framework.Logf("Waiting for service %q in %d clusters", service.Name, len(clusters)) - for i, c := range clusters { - var present bool // Should the service be present or absent in this cluster? - if presentInCluster == nil { - present = true - } else { - present = presentInCluster[i] - } - waitForServiceOrFail(c.Clientset, namespace, service, present, FederatedServiceTimeout) + for _, c := range clusters { + waitForServiceOrFail(c.Clientset, namespace, service, true, FederatedServiceTimeout) } } @@ -492,7 +480,7 @@ func discoverService(f *framework.Framework, name string, exists bool, podName s createBackendPodsOrFail creates one pod in each cluster, and returns the created pods (in the same order as clusterClientSets). If creation of any pod fails, the test fails (possibly with a partially created set of pods). No retries are attempted. */ -func createBackendPodsOrFail(clusters []cluster, namespace string, name string) []*v1.Pod { +func createBackendPodsOrFail(clusters map[string]cluster, namespace string, name string) { pod := &v1.Pod{ ObjectMeta: v1.ObjectMeta{ Name: name, @@ -509,36 +497,41 @@ func createBackendPodsOrFail(clusters []cluster, namespace string, name string) RestartPolicy: v1.RestartPolicyAlways, }, } - pods := make([]*v1.Pod, len(clusters)) - for i, c := range clusters { - By(fmt.Sprintf("Creating pod %q in namespace %q in cluster %d", pod.Name, namespace, i)) + for name, c := range clusters { + By(fmt.Sprintf("Creating pod %q in namespace %q in cluster %q", pod.Name, namespace, name)) createdPod, err := c.Clientset.Core().Pods(namespace).Create(pod) - framework.ExpectNoError(err, "Creating pod %q in namespace %q in cluster %d", name, namespace, i) - By(fmt.Sprintf("Successfully created pod %q in namespace %q in cluster %d: %v", pod.Name, namespace, i, *createdPod)) - pods[i] = createdPod + framework.ExpectNoError(err, "Creating pod %q in namespace %q in cluster %q", name, namespace, name) + By(fmt.Sprintf("Successfully created pod %q in namespace %q in cluster %q: %v", pod.Name, namespace, name, *createdPod)) + c.backendPod = createdPod } - return pods } /* -deleteBackendPodsOrFail deletes one pod from each cluster (unless pods[n] is nil for that cluster) -If deletion of any pod fails, the test fails (possibly with a partially deleted set of pods). No retries are attempted. +deletes exactly one backend pod which must not be nil +The test fails if there are any errors. */ -func deleteBackendPodsOrFail(clusters []cluster, namespace string, pods []*v1.Pod) { - if len(clusters) != len(pods) { - Fail(fmt.Sprintf("Internal error: number of clients (%d) does not equal number of pods (%d). One pod per client please.", len(clusters), len(pods))) +func deleteOneBackendPodOrFail(c cluster) { + pod := c.backendPod + err := c.Clientset.Core().Pods(pod.Namespace).Delete(pod.Name, api.NewDeleteOptions(0)) + if errors.IsNotFound(err) { + By(fmt.Sprintf("Pod %q in namespace %q in cluster %q does not exist. No need to delete it.", pod.Name, pod.Namespace, c.name)) + } else { + framework.ExpectNoError(err, "Deleting pod %q in namespace %q from cluster %q", pod.Name, pod.Namespace, c.name) } - for i, c := range clusters { - if pods[i] != nil { - err := c.Clientset.Core().Pods(namespace).Delete(pods[i].Name, api.NewDeleteOptions(0)) - if errors.IsNotFound(err) { - By(fmt.Sprintf("Pod %q in namespace %q in cluster %d does not exist. No need to delete it.", pods[i].Name, namespace, i)) - } else { - framework.ExpectNoError(err, "Deleting pod %q in namespace %q from cluster %d", pods[i].Name, namespace, i) - } - By(fmt.Sprintf("Backend pod %q in namespace %q in cluster %d deleted or does not exist", pods[i].Name, namespace, i)) + By(fmt.Sprintf("Backend pod %q in namespace %q in cluster %q deleted or does not exist", pod.Name, pod.Namespace, c.name)) +} + +/* +deleteBackendPodsOrFail deletes one pod from each cluster that has one. +If deletion of any pod fails, the test fails (possibly with a partially deleted set of pods). No retries are attempted. +*/ +func deleteBackendPodsOrFail(clusters map[string]cluster, namespace string) { + for name, c := range clusters { + if c.backendPod != nil { + deleteOneBackendPodOrFail(c) + c.backendPod = nil } else { - By(fmt.Sprintf("No backend pod to delete for cluster %d", i)) + By(fmt.Sprintf("No backend pod to delete for cluster %q", name)) } } } From 55e642cb9f6533aba5d6c03893be3505b3877e6d Mon Sep 17 00:00:00 2001 From: Matt Liggett Date: Fri, 1 Jul 2016 15:18:34 -0700 Subject: [PATCH 4/6] Use a pointer. Duh. --- test/e2e/federated-service.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/test/e2e/federated-service.go b/test/e2e/federated-service.go index 6a25aafeb1f..2885f813d63 100644 --- a/test/e2e/federated-service.go +++ b/test/e2e/federated-service.go @@ -75,7 +75,7 @@ type cluster struct { var _ = framework.KubeDescribe("[Feature:Federation]", func() { f := framework.NewDefaultFederatedFramework("federated-service") - var clusters map[string]cluster + var clusters map[string]*cluster var federationName string var primaryClusterName string // The name of the "primary" cluster @@ -118,7 +118,7 @@ var _ = framework.KubeDescribe("[Feature:Federation]", func() { framework.Logf("%d clusters are Ready", len(contexts)) // clusters = make([]cluster, len(clusterList.Items)) - clusters = map[string]cluster{} + clusters = map[string]*cluster{} primaryClusterName = clusterList.Items[0].Name By(fmt.Sprintf("Labeling %q as the first cluster", primaryClusterName)) for i, c := range clusterList.Items { @@ -140,7 +140,7 @@ var _ = framework.KubeDescribe("[Feature:Federation]", func() { cfg.QPS = KubeAPIQPS cfg.Burst = KubeAPIBurst clset := release_1_3.NewForConfigOrDie(restclient.AddUserAgent(cfg, UserAgentName)) - clusters[c.Name] = cluster{c.Name, clset, false, nil} + clusters[c.Name] = &cluster{c.Name, clset, false, nil} } for name, c := range clusters { @@ -350,7 +350,7 @@ func waitForServiceOrFail(clientset *release_1_3.Clientset, namespace string, se /* waitForServiceShardsOrFail waits for the service to appear in all clusters */ -func waitForServiceShardsOrFail(namespace string, service *v1.Service, clusters map[string]cluster) { +func waitForServiceShardsOrFail(namespace string, service *v1.Service, clusters map[string]*cluster) { framework.Logf("Waiting for service %q in %d clusters", service.Name, len(clusters)) for _, c := range clusters { waitForServiceOrFail(c.Clientset, namespace, service, true, FederatedServiceTimeout) @@ -480,7 +480,7 @@ func discoverService(f *framework.Framework, name string, exists bool, podName s createBackendPodsOrFail creates one pod in each cluster, and returns the created pods (in the same order as clusterClientSets). If creation of any pod fails, the test fails (possibly with a partially created set of pods). No retries are attempted. */ -func createBackendPodsOrFail(clusters map[string]cluster, namespace string, name string) { +func createBackendPodsOrFail(clusters map[string]*cluster, namespace string, name string) { pod := &v1.Pod{ ObjectMeta: v1.ObjectMeta{ Name: name, @@ -510,8 +510,9 @@ func createBackendPodsOrFail(clusters map[string]cluster, namespace string, name deletes exactly one backend pod which must not be nil The test fails if there are any errors. */ -func deleteOneBackendPodOrFail(c cluster) { +func deleteOneBackendPodOrFail(c *cluster) { pod := c.backendPod + Expect(pod).ToNot(BeNil()) err := c.Clientset.Core().Pods(pod.Namespace).Delete(pod.Name, api.NewDeleteOptions(0)) if errors.IsNotFound(err) { By(fmt.Sprintf("Pod %q in namespace %q in cluster %q does not exist. No need to delete it.", pod.Name, pod.Namespace, c.name)) @@ -525,7 +526,7 @@ func deleteOneBackendPodOrFail(c cluster) { deleteBackendPodsOrFail deletes one pod from each cluster that has one. If deletion of any pod fails, the test fails (possibly with a partially deleted set of pods). No retries are attempted. */ -func deleteBackendPodsOrFail(clusters map[string]cluster, namespace string) { +func deleteBackendPodsOrFail(clusters map[string]*cluster, namespace string) { for name, c := range clusters { if c.backendPod != nil { deleteOneBackendPodOrFail(c) From 5b8113dffd716733defbf40db8e810f09ae9bd68 Mon Sep 17 00:00:00 2001 From: Matt Liggett Date: Wed, 6 Jul 2016 11:41:35 -0700 Subject: [PATCH 5/6] code review fixes --- test/e2e/federated-service.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/test/e2e/federated-service.go b/test/e2e/federated-service.go index 2885f813d63..156c08e3090 100644 --- a/test/e2e/federated-service.go +++ b/test/e2e/federated-service.go @@ -63,8 +63,8 @@ var FederatedServiceLabels = map[string]string{ } /* -type cluster keeps track of the assorted objects and state related to each -cluster in the federation +cluster keeps track of the assorted objects and state related to each cluster +in the federation */ type cluster struct { name string @@ -75,7 +75,7 @@ type cluster struct { var _ = framework.KubeDescribe("[Feature:Federation]", func() { f := framework.NewDefaultFederatedFramework("federated-service") - var clusters map[string]*cluster + var clusters map[string]*cluster // All clusters, keyed by cluster name var federationName string var primaryClusterName string // The name of the "primary" cluster @@ -117,7 +117,6 @@ var _ = framework.KubeDescribe("[Feature:Federation]", func() { } framework.Logf("%d clusters are Ready", len(contexts)) - // clusters = make([]cluster, len(clusterList.Items)) clusters = map[string]*cluster{} primaryClusterName = clusterList.Items[0].Name By(fmt.Sprintf("Labeling %q as the first cluster", primaryClusterName)) @@ -507,7 +506,7 @@ func createBackendPodsOrFail(clusters map[string]*cluster, namespace string, nam } /* -deletes exactly one backend pod which must not be nil +deleteOneBackendPodOrFail deletes exactly one backend pod which must not be nil The test fails if there are any errors. */ func deleteOneBackendPodOrFail(c *cluster) { From d618e5329bfb86c266ab324add9bcf10cdb77bbc Mon Sep 17 00:00:00 2001 From: "Madhusudan.C.S" Date: Wed, 6 Jul 2016 10:40:11 -0700 Subject: [PATCH 6/6] Copy FEDERATIONS_DOMAIN_MAP to a local variable since the helper script doesn't allow overwriting the existing variable. --- cluster/gce/configure-vm.sh | 10 +++++----- cluster/gce/gci/configure-helper.sh | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/cluster/gce/configure-vm.sh b/cluster/gce/configure-vm.sh index 7cef0f9d197..0c41162ce7b 100755 --- a/cluster/gce/configure-vm.sh +++ b/cluster/gce/configure-vm.sh @@ -552,13 +552,13 @@ autoscaler_mig_config: '$(echo "${AUTOSCALER_MIG_CONFIG}" | sed -e "s/'/''/g")' EOF fi if [[ "${FEDERATION:-}" == "true" ]]; then - FEDERATIONS_DOMAIN_MAP="${FEDERATIONS_DOMAIN_MAP:-}" - if [[ -z "${FEDERATIONS_DOMAIN_MAP}" && -n "${FEDERATION_NAME:-}" && -n "${DNS_ZONE_NAME:-}" ]]; then - FEDERATIONS_DOMAIN_MAP="${FEDERATION_NAME}=${DNS_ZONE_NAME}" + local federations_domain_map="${FEDERATIONS_DOMAIN_MAP:-}" + if [[ -z "${federations_domain_map}" && -n "${FEDERATION_NAME:-}" && -n "${DNS_ZONE_NAME:-}" ]]; then + federations_domain_map="${FEDERATION_NAME}=${DNS_ZONE_NAME}" fi - if [[ -n "${FEDERATIONS_DOMAIN_MAP}" ]]; then + if [[ -n "${federations_domain_map}" ]]; then cat <>/srv/salt-overlay/pillar/cluster-params.sls -federations_domain_map: '$(echo "- --federations=${FEDERATIONS_DOMAIN_MAP}" | sed -e "s/'/''/g")' +federations_domain_map: '$(echo "- --federations=${federations_domain_map}" | sed -e "s/'/''/g")' EOF else cat <>/srv/salt-overlay/pillar/cluster-params.sls diff --git a/cluster/gce/gci/configure-helper.sh b/cluster/gce/gci/configure-helper.sh index 57caeaec2a2..12226574fbd 100644 --- a/cluster/gce/gci/configure-helper.sh +++ b/cluster/gce/gci/configure-helper.sh @@ -839,12 +839,12 @@ function start-kube-addons { sed -i -e "s@{{ *pillar\['dns_server'\] *}}@${DNS_SERVER_IP}@g" "${dns_svc_file}" if [[ "${FEDERATION:-}" == "true" ]]; then - FEDERATIONS_DOMAIN_MAP="${FEDERATIONS_DOMAIN_MAP:-}" - if [[ -z "${FEDERATIONS_DOMAIN_MAP}" && -n "${FEDERATION_NAME:-}" && -n "${DNS_ZONE_NAME:-}" ]]; then - FEDERATIONS_DOMAIN_MAP="${FEDERATION_NAME}=${DNS_ZONE_NAME}" + local federations_domain_map="${FEDERATIONS_DOMAIN_MAP:-}" + if [[ -z "${federations_domain_map}" && -n "${FEDERATION_NAME:-}" && -n "${DNS_ZONE_NAME:-}" ]]; then + federations_domain_map="${FEDERATION_NAME}=${DNS_ZONE_NAME}" fi - if [[ -n "${FEDERATIONS_DOMAIN_MAP}" ]]; then - sed -i -e "s@{{ *pillar\['federations_domain_map'\] *}}@- --federations=${FEDERATIONS_DOMAIN_MAP}@g" "${dns_rc_file}" + if [[ -n "${federations_domain_map}" ]]; then + sed -i -e "s@{{ *pillar\['federations_domain_map'\] *}}@- --federations=${federations_domain_map}@g" "${dns_rc_file}" else sed -i -e "/{{ *pillar\['federations_domain_map'\] *}}/d" "${dns_rc_file}" fi