Merge pull request #35865 from madhusudancs/federation-kubefed-init-08

Automatic merge from submit-queue

[Federation][init-08] Refactor the tests by pulling the common utilities into a testing package.

Please review only the last commit here. This is based on PR #35864 which will be reviewed independently.

Design Doc: PR #34484

cc @kubernetes/sig-cluster-federation @nikhiljindal

federation/pkg/kubefed/BUILD

@@ -50,20 +50,18 @@ go_test(
     deps = [
         "//federation/apis/federation:go_default_library",
         "//federation/apis/federation/v1beta1:go_default_library",
+        "//federation/pkg/kubefed/testing:go_default_library",
         "//federation/pkg/kubefed/util:go_default_library",
         "//pkg/api:go_default_library",
         "//pkg/api/errors:go_default_library",
         "//pkg/api/testapi:go_default_library",
         "//pkg/api/unversioned:go_default_library",
         "//pkg/api/v1:go_default_library",
-        "//pkg/apimachinery/registered:go_default_library",
-        "//pkg/client/restclient:go_default_library",
         "//pkg/client/restclient/fake:go_default_library",
         "//pkg/client/typed/dynamic:go_default_library",
         "//pkg/client/unversioned/clientcmd:go_default_library",
         "//pkg/client/unversioned/clientcmd/api:go_default_library",
         "//pkg/kubectl/cmd/testing:go_default_library",
         "//pkg/kubectl/cmd/util:go_default_library",
-        "//pkg/runtime:go_default_library",
     ],
 )

federation/pkg/kubefed/init/init.go

@@ -242,7 +242,7 @@ func waitForLoadBalancerAddress(clientset *client.Clientset, svc *api.Service) (
 	ips := []string{}
 	hostnames := []string{}
 
-	err := wait.PollInfinite(lbAddrRetryInterval, func() (bool, error) {
+	err := wait.PollImmediateInfinite(lbAddrRetryInterval, func() (bool, error) {
 		pollSvc, err := clientset.Core().Services(svc.Namespace).Get(svc.Name)
 		if err != nil {
 			return false, nil

federation/pkg/kubefed/join_test.go

@@ -19,27 +19,23 @@ package kubefed
 import (
 	"bytes"
 	"fmt"
-	"io"
 	"io/ioutil"
 	"net/http"
-	"os"
 	"testing"
 
 	federationapi "k8s.io/kubernetes/federation/apis/federation/v1beta1"
+	kubefedtesting "k8s.io/kubernetes/federation/pkg/kubefed/testing"
 	"k8s.io/kubernetes/federation/pkg/kubefed/util"
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/api/testapi"
 	"k8s.io/kubernetes/pkg/api/unversioned"
 	"k8s.io/kubernetes/pkg/api/v1"
-	"k8s.io/kubernetes/pkg/apimachinery/registered"
-	"k8s.io/kubernetes/pkg/client/restclient"
 	"k8s.io/kubernetes/pkg/client/restclient/fake"
 	"k8s.io/kubernetes/pkg/client/typed/dynamic"
 	"k8s.io/kubernetes/pkg/client/unversioned/clientcmd"
 	clientcmdapi "k8s.io/kubernetes/pkg/client/unversioned/clientcmd/api"
 	cmdtesting "k8s.io/kubernetes/pkg/kubectl/cmd/testing"
 	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
-	"k8s.io/kubernetes/pkg/runtime"
 )
 
 func TestJoinFederation(t *testing.T) {
@@ -48,11 +44,11 @@ func TestJoinFederation(t *testing.T) {
 		cmdErrMsg = str
 	})
 
-	fakeKubeFiles, err := fakeKubeconfigFiles()
+	fakeKubeFiles, err := kubefedtesting.FakeKubeconfigFiles()
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
-	defer rmFakeKubeconfigFiles(fakeKubeFiles)
+	defer kubefedtesting.RemoveFakeKubeconfigFiles(fakeKubeFiles)
 
 	testCases := []struct {
 		cluster            string
@@ -111,7 +107,7 @@ func TestJoinFederation(t *testing.T) {
 			t.Fatalf("[%d] unexpected error: %v", i, err)
 		}
 
-		adminConfig, err := newFakeAdminConfig(hostFactory, tc.kubeconfigGlobal)
+		adminConfig, err := kubefedtesting.NewFakeAdminConfig(hostFactory, tc.kubeconfigGlobal)
 		if err != nil {
 			t.Fatalf("[%d] unexpected error: %v", i, err)
 		}
@@ -162,7 +158,7 @@ func testJoinFederationFactory(name, server string) cmdutil.Factory {
 				if !api.Semantic.DeepEqual(got, want) {
 					return nil, fmt.Errorf("unexpected cluster object\n\tgot: %#v\n\twant: %#v", got, want)
 				}
-				return &http.Response{StatusCode: http.StatusCreated, Header: defaultHeader(), Body: objBody(codec, &want)}, nil
+				return &http.Response{StatusCode: http.StatusCreated, Header: kubefedtesting.DefaultHeader(), Body: kubefedtesting.ObjBody(codec, &want)}, nil
 			default:
 				return nil, fmt.Errorf("unexpected request: %#v\n%#v", req.URL, req)
 			}
@@ -172,30 +168,6 @@ func testJoinFederationFactory(name, server string) cmdutil.Factory {
 	return f
 }
 
-type fakeAdminConfig struct {
-	pathOptions *clientcmd.PathOptions
-	hostFactory cmdutil.Factory
-}
-
-func newFakeAdminConfig(f cmdutil.Factory, kubeconfigGlobal string) (util.AdminConfig, error) {
-	pathOptions := clientcmd.NewDefaultPathOptions()
-	pathOptions.GlobalFile = kubeconfigGlobal
-	pathOptions.EnvVar = ""
-
-	return &fakeAdminConfig{
-		pathOptions: pathOptions,
-		hostFactory: f,
-	}, nil
-}
-
-func (f *fakeAdminConfig) PathOptions() *clientcmd.PathOptions {
-	return f.pathOptions
-}
-
-func (f *fakeAdminConfig) HostFactory(host, kubeconfigPath string) cmdutil.Factory {
-	return f.hostFactory
-}
-
 func fakeJoinHostFactory(name, server, token string) (cmdutil.Factory, error) {
 	kubeconfig := clientcmdapi.Config{
 		Clusters: map[string]*clientcmdapi.Cluster{
@@ -227,7 +199,7 @@ func fakeJoinHostFactory(name, server, token string) (cmdutil.Factory, error) {
 		},
 		ObjectMeta: v1.ObjectMeta{
 			Name:      name,
-			Namespace: "federation-system",
+			Namespace: util.DefaultFederationSystemNamespace,
 		},
 		Data: map[string][]byte{
 			"kubeconfig": configBytes,
@@ -236,7 +208,7 @@ func fakeJoinHostFactory(name, server, token string) (cmdutil.Factory, error) {
 
 	f, tf, codec, _ := cmdtesting.NewAPIFactory()
 	ns := dynamic.ContentConfig().NegotiatedSerializer
-	tf.ClientConfig = defaultClientConfig()
+	tf.ClientConfig = kubefedtesting.DefaultClientConfig()
 	tf.Client = &fake.RESTClient{
 		NegotiatedSerializer: ns,
 		Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) {
@@ -254,7 +226,7 @@ func fakeJoinHostFactory(name, server, token string) (cmdutil.Factory, error) {
 				if !api.Semantic.DeepEqual(got, secretObject) {
 					return nil, fmt.Errorf("Unexpected secret object\n\tgot: %#v\n\twant: %#v", got, secretObject)
 				}
-				return &http.Response{StatusCode: http.StatusCreated, Header: defaultHeader(), Body: objBody(codec, &secretObject)}, nil
+				return &http.Response{StatusCode: http.StatusCreated, Header: kubefedtesting.DefaultHeader(), Body: kubefedtesting.ObjBody(codec, &secretObject)}, nil
 			default:
 				return nil, fmt.Errorf("unexpected request: %#v\n%#v", req.URL, req)
 			}
@@ -281,113 +253,3 @@ func fakeCluster(name, server string) federationapi.Cluster {
 		},
 	}
 }
-
-func fakeKubeconfigFiles() ([]string, error) {
-	kubeconfigs := []clientcmdapi.Config{
-		{
-			Clusters: map[string]*clientcmdapi.Cluster{
-				"syndicate": {
-					Server: "https://10.20.30.40",
-				},
-			},
-			AuthInfos: map[string]*clientcmdapi.AuthInfo{
-				"syndicate": {
-					Token: "badge",
-				},
-			},
-			Contexts: map[string]*clientcmdapi.Context{
-				"syndicate": {
-					Cluster:  "syndicate",
-					AuthInfo: "syndicate",
-				},
-			},
-			CurrentContext: "syndicate",
-		},
-		{
-			Clusters: map[string]*clientcmdapi.Cluster{
-				"ally": {
-					Server: "ally256.example.com:80",
-				},
-			},
-			AuthInfos: map[string]*clientcmdapi.AuthInfo{
-				"ally": {
-					Token: "souvenir",
-				},
-			},
-			Contexts: map[string]*clientcmdapi.Context{
-				"ally": {
-					Cluster:  "ally",
-					AuthInfo: "ally",
-				},
-			},
-			CurrentContext: "ally",
-		},
-		{
-			Clusters: map[string]*clientcmdapi.Cluster{
-				"ally": {
-					Server: "https://ally64.example.com",
-				},
-				"confederate": {
-					Server: "10.8.8.8",
-				},
-			},
-			AuthInfos: map[string]*clientcmdapi.AuthInfo{
-				"ally": {
-					Token: "souvenir",
-				},
-				"confederate": {
-					Token: "totem",
-				},
-			},
-			Contexts: map[string]*clientcmdapi.Context{
-				"ally": {
-					Cluster:  "ally",
-					AuthInfo: "ally",
-				},
-				"confederate": {
-					Cluster:  "confederate",
-					AuthInfo: "confederate",
-				},
-			},
-			CurrentContext: "confederate",
-		},
-	}
-	kubefiles := []string{}
-	for _, cfg := range kubeconfigs {
-		fakeKubeFile, _ := ioutil.TempFile("", "")
-		err := clientcmd.WriteToFile(cfg, fakeKubeFile.Name())
-		if err != nil {
-			return nil, err
-		}
-
-		kubefiles = append(kubefiles, fakeKubeFile.Name())
-	}
-	return kubefiles, nil
-}
-
-func rmFakeKubeconfigFiles(kubefiles []string) {
-	for _, file := range kubefiles {
-		os.Remove(file)
-	}
-}
-
-func defaultHeader() http.Header {
-	header := http.Header{}
-	header.Set("Content-Type", runtime.ContentTypeJSON)
-	return header
-}
-
-func objBody(codec runtime.Codec, obj runtime.Object) io.ReadCloser {
-	return ioutil.NopCloser(bytes.NewReader([]byte(runtime.EncodeOrDie(codec, obj))))
-}
-
-func defaultClientConfig() *restclient.Config {
-	return &restclient.Config{
-		APIPath: "/api",
-		ContentConfig: restclient.ContentConfig{
-			NegotiatedSerializer: api.Codecs,
-			ContentType:          runtime.ContentTypeJSON,
-			GroupVersion:         &registered.GroupOrDie(api.GroupName).GroupVersion,
-		},
-	}
-}

federation/pkg/kubefed/testing/BUILD

@@ -0,0 +1,27 @@
+package(default_visibility = ["//visibility:public"])
+
+licenses(["notice"])
+
+load(
+    "@io_bazel_rules_go//go:def.bzl",
+    "go_binary",
+    "go_library",
+    "go_test",
+    "cgo_library",
+)
+
+go_library(
+    name = "go_default_library",
+    srcs = ["testing.go"],
+    tags = ["automanaged"],
+    deps = [
+        "//federation/pkg/kubefed/util:go_default_library",
+        "//pkg/api:go_default_library",
+        "//pkg/apimachinery/registered: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/kubectl/cmd/util:go_default_library",
+        "//pkg/runtime:go_default_library",
+    ],
+)

federation/pkg/kubefed/testing/testing.go

@@ -0,0 +1,168 @@
+/*
+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 testing
+
+import (
+	"bytes"
+	"io"
+	"io/ioutil"
+	"net/http"
+	"os"
+
+	"k8s.io/kubernetes/federation/pkg/kubefed/util"
+	"k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/apimachinery/registered"
+	"k8s.io/kubernetes/pkg/client/restclient"
+	"k8s.io/kubernetes/pkg/client/unversioned/clientcmd"
+	clientcmdapi "k8s.io/kubernetes/pkg/client/unversioned/clientcmd/api"
+	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
+	"k8s.io/kubernetes/pkg/runtime"
+)
+
+type fakeAdminConfig struct {
+	pathOptions *clientcmd.PathOptions
+	hostFactory cmdutil.Factory
+}
+
+func NewFakeAdminConfig(f cmdutil.Factory, kubeconfigGlobal string) (util.AdminConfig, error) {
+	pathOptions := clientcmd.NewDefaultPathOptions()
+	pathOptions.GlobalFile = kubeconfigGlobal
+	pathOptions.EnvVar = ""
+
+	return &fakeAdminConfig{
+		pathOptions: pathOptions,
+		hostFactory: f,
+	}, nil
+}
+
+func (f *fakeAdminConfig) PathOptions() *clientcmd.PathOptions {
+	return f.pathOptions
+}
+
+func (f *fakeAdminConfig) HostFactory(host, kubeconfigPath string) cmdutil.Factory {
+	return f.hostFactory
+}
+
+func FakeKubeconfigFiles() ([]string, error) {
+	kubeconfigs := []clientcmdapi.Config{
+		{
+			Clusters: map[string]*clientcmdapi.Cluster{
+				"syndicate": {
+					Server: "https://10.20.30.40",
+				},
+			},
+			AuthInfos: map[string]*clientcmdapi.AuthInfo{
+				"syndicate": {
+					Token: "badge",
+				},
+			},
+			Contexts: map[string]*clientcmdapi.Context{
+				"syndicate": {
+					Cluster:  "syndicate",
+					AuthInfo: "syndicate",
+				},
+			},
+			CurrentContext: "syndicate",
+		},
+		{
+			Clusters: map[string]*clientcmdapi.Cluster{
+				"ally": {
+					Server: "ally256.example.com:80",
+				},
+			},
+			AuthInfos: map[string]*clientcmdapi.AuthInfo{
+				"ally": {
+					Token: "souvenir",
+				},
+			},
+			Contexts: map[string]*clientcmdapi.Context{
+				"ally": {
+					Cluster:  "ally",
+					AuthInfo: "ally",
+				},
+			},
+			CurrentContext: "ally",
+		},
+		{
+			Clusters: map[string]*clientcmdapi.Cluster{
+				"ally": {
+					Server: "https://ally64.example.com",
+				},
+				"confederate": {
+					Server: "10.8.8.8",
+				},
+			},
+			AuthInfos: map[string]*clientcmdapi.AuthInfo{
+				"ally": {
+					Token: "souvenir",
+				},
+				"confederate": {
+					Token: "totem",
+				},
+			},
+			Contexts: map[string]*clientcmdapi.Context{
+				"ally": {
+					Cluster:  "ally",
+					AuthInfo: "ally",
+				},
+				"confederate": {
+					Cluster:  "confederate",
+					AuthInfo: "confederate",
+				},
+			},
+			CurrentContext: "confederate",
+		},
+	}
+	kubefiles := []string{}
+	for _, cfg := range kubeconfigs {
+		fakeKubeFile, _ := ioutil.TempFile("", "")
+		err := clientcmd.WriteToFile(cfg, fakeKubeFile.Name())
+		if err != nil {
+			return nil, err
+		}
+
+		kubefiles = append(kubefiles, fakeKubeFile.Name())
+	}
+	return kubefiles, nil
+}
+
+func RemoveFakeKubeconfigFiles(kubefiles []string) {
+	for _, file := range kubefiles {
+		os.Remove(file)
+	}
+}
+
+func DefaultHeader() http.Header {
+	header := http.Header{}
+	header.Set("Content-Type", runtime.ContentTypeJSON)
+	return header
+}
+
+func ObjBody(codec runtime.Codec, obj runtime.Object) io.ReadCloser {
+	return ioutil.NopCloser(bytes.NewReader([]byte(runtime.EncodeOrDie(codec, obj))))
+}
+
+func DefaultClientConfig() *restclient.Config {
+	return &restclient.Config{
+		APIPath: "/api",
+		ContentConfig: restclient.ContentConfig{
+			NegotiatedSerializer: api.Codecs,
+			ContentType:          runtime.ContentTypeJSON,
+			GroupVersion:         &registered.GroupOrDie(api.GroupName).GroupVersion,
+		},
+	}
+}

federation/pkg/kubefed/unjoin_test.go

@@ -24,6 +24,7 @@ import (
 	"testing"
 
 	federationapi "k8s.io/kubernetes/federation/apis/federation"
+	kubefedtesting "k8s.io/kubernetes/federation/pkg/kubefed/testing"
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/api/errors"
 	"k8s.io/kubernetes/pkg/api/testapi"
@@ -40,11 +41,11 @@ func TestUnjoinFederation(t *testing.T) {
 		cmdErrMsg = str
 	})
 
-	fakeKubeFiles, err := fakeKubeconfigFiles()
+	fakeKubeFiles, err := kubefedtesting.FakeKubeconfigFiles()
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
-	defer rmFakeKubeconfigFiles(fakeKubeFiles)
+	defer kubefedtesting.RemoveFakeKubeconfigFiles(fakeKubeFiles)
 
 	testCases := []struct {
 		cluster            string
@@ -125,7 +126,7 @@ func TestUnjoinFederation(t *testing.T) {
 		errBuf := bytes.NewBuffer([]byte{})
 
 		hostFactory := fakeUnjoinHostFactory(tc.cluster)
-		adminConfig, err := newFakeAdminConfig(hostFactory, tc.kubeconfigGlobal)
+		adminConfig, err := kubefedtesting.NewFakeAdminConfig(hostFactory, tc.kubeconfigGlobal)
 		if err != nil {
 			t.Fatalf("[%d] unexpected error: %v", i, err)
 		}
@@ -165,7 +166,7 @@ func testUnjoinFederationFactory(name, server, secret string) cmdutil.Factory {
 
 	f, tf, _, _ := cmdtesting.NewAPIFactory()
 	codec := testapi.Federation.Codec()
-	tf.ClientConfig = defaultClientConfig()
+	tf.ClientConfig = kubefedtesting.DefaultClientConfig()
 	ns := testapi.Federation.NegotiatedSerializer()
 	tf.Client = &fake.RESTClient{
 		NegotiatedSerializer: ns,
@@ -180,12 +181,12 @@ func testUnjoinFederationFactory(name, server, secret string) cmdutil.Factory {
 
 				switch m {
 				case http.MethodGet:
-					return &http.Response{StatusCode: http.StatusOK, Header: defaultHeader(), Body: objBody(codec, &cluster)}, nil
+					return &http.Response{StatusCode: http.StatusOK, Header: kubefedtesting.DefaultHeader(), Body: kubefedtesting.ObjBody(codec, &cluster)}, nil
 				case http.MethodDelete:
 					status := unversioned.Status{
 						Status: "Success",
 					}
-					return &http.Response{StatusCode: http.StatusOK, Header: defaultHeader(), Body: objBody(codec, &status)}, nil
+					return &http.Response{StatusCode: http.StatusOK, Header: kubefedtesting.DefaultHeader(), Body: kubefedtesting.ObjBody(codec, &status)}, nil
 				default:
 					return nil, fmt.Errorf("unexpected method: %#v\n%#v", req.URL, req)
 				}
@@ -202,7 +203,7 @@ func fakeUnjoinHostFactory(name string) cmdutil.Factory {
 	urlPrefix := "/api/v1/namespaces/federation-system/secrets/"
 	f, tf, codec, _ := cmdtesting.NewAPIFactory()
 	ns := dynamic.ContentConfig().NegotiatedSerializer
-	tf.ClientConfig = defaultClientConfig()
+	tf.ClientConfig = kubefedtesting.DefaultClientConfig()
 	tf.Client = &fake.RESTClient{
 		NegotiatedSerializer: ns,
 		Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) {
@@ -215,7 +216,7 @@ func fakeUnjoinHostFactory(name string) cmdutil.Factory {
 				status := unversioned.Status{
 					Status: "Success",
 				}
-				return &http.Response{StatusCode: http.StatusOK, Header: defaultHeader(), Body: objBody(codec, &status)}, nil
+				return &http.Response{StatusCode: http.StatusOK, Header: kubefedtesting.DefaultHeader(), Body: kubefedtesting.ObjBody(codec, &status)}, nil
 			default:
 				return nil, fmt.Errorf("unexpected request: %#v\n%#v", req.URL, req)
 			}

federation/pkg/kubefed/util/util.go

@@ -31,6 +31,10 @@ const (
 	// KubeconfigSecretDataKey is the key name used in the secret to
 	// stores a cluster's credentials.
 	KubeconfigSecretDataKey = "kubeconfig"
+
+	// DefaultFederationSystemNamespace is the namespace in which
+	// federation system components are hosted.
+	DefaultFederationSystemNamespace = "federation-system"
 )
 
 // AdminConfig provides a filesystem based kubeconfig (via
@@ -87,7 +91,7 @@ type SubcommandFlags struct {
 func AddSubcommandFlags(cmd *cobra.Command) {
 	cmd.Flags().String("kubeconfig", "", "Path to the kubeconfig file to use for CLI requests.")
 	cmd.Flags().String("host-cluster-context", "", "Host cluster context")
-	cmd.Flags().String("federation-system-namespace", "federation-system", "Namespace in the host cluster where the federation system components are installed")
+	cmd.Flags().String("federation-system-namespace", DefaultFederationSystemNamespace, "Namespace in the host cluster where the federation system components are installed")
 }
 
 // GetSubcommandFlags retrieves the command line flag values for the

pkg/util/wait/wait.go

@@ -192,6 +192,20 @@ func PollInfinite(interval time.Duration, condition ConditionFunc) error {
 	return PollUntil(interval, condition, done)
 }
 
+// PollImmediateInfinite is identical to PollInfinite, except that it
+// performs the first check immediately, not waiting interval
+// beforehand.
+func PollImmediateInfinite(interval time.Duration, condition ConditionFunc) error {
+	done, err := condition()
+	if err != nil {
+		return err
+	}
+	if done {
+		return nil
+	}
+	return PollInfinite(interval, condition)
+}
+
 // PollUntil is like Poll, but it takes a stop change instead of total duration
 func PollUntil(interval time.Duration, condition ConditionFunc, stopCh <-chan struct{}) error {
 	return WaitFor(poller(interval, 0), condition, stopCh)