From caef02cf43e4a05b48a8a8aeaa78f86b7844975d Mon Sep 17 00:00:00 2001 From: "Madhusudan.C.S" Date: Sat, 29 Oct 2016 18:15:11 -0700 Subject: [PATCH 1/3] [Federation][init-06] Check for the availability of federation API server's service loadbalancer address before waiting. This speeds up the tests. Otherwise tests end up unnecessarily waiting for the poll interval/duration which is 5 seconds right now. --- federation/pkg/kubefed/init/init.go | 2 +- pkg/util/wait/wait.go | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/federation/pkg/kubefed/init/init.go b/federation/pkg/kubefed/init/init.go index e5c819cba3c..ad2a8475fd2 100644 --- a/federation/pkg/kubefed/init/init.go +++ b/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 diff --git a/pkg/util/wait/wait.go b/pkg/util/wait/wait.go index 65f2e0b7618..34d833b44d9 100644 --- a/pkg/util/wait/wait.go +++ b/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) From dd365b42bf8b2877eef0650d856718a556098f0b Mon Sep 17 00:00:00 2001 From: "Madhusudan.C.S" Date: Sat, 29 Oct 2016 18:16:43 -0700 Subject: [PATCH 2/3] [Federation][init-07] Pull the default federation namespace into a constant. --- federation/pkg/kubefed/util/util.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/federation/pkg/kubefed/util/util.go b/federation/pkg/kubefed/util/util.go index 67c5c04c2f0..c6dd285de49 100644 --- a/federation/pkg/kubefed/util/util.go +++ b/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 From efea27055372cb0b0abdcf710ad929c6a22cf6de Mon Sep 17 00:00:00 2001 From: "Madhusudan.C.S" Date: Sat, 29 Oct 2016 18:23:03 -0700 Subject: [PATCH 3/3] [Federation][init-08] Refactor the tests by pulling the common utilities into a testing package. This makes these utilities reusable by other packages. --- federation/pkg/kubefed/BUILD | 4 +- federation/pkg/kubefed/join_test.go | 154 ++------------------ federation/pkg/kubefed/testing/BUILD | 27 ++++ federation/pkg/kubefed/testing/testing.go | 168 ++++++++++++++++++++++ federation/pkg/kubefed/unjoin_test.go | 17 +-- 5 files changed, 213 insertions(+), 157 deletions(-) create mode 100644 federation/pkg/kubefed/testing/BUILD create mode 100644 federation/pkg/kubefed/testing/testing.go diff --git a/federation/pkg/kubefed/BUILD b/federation/pkg/kubefed/BUILD index 0548694c444..b2271accfd5 100644 --- a/federation/pkg/kubefed/BUILD +++ b/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", ], ) diff --git a/federation/pkg/kubefed/join_test.go b/federation/pkg/kubefed/join_test.go index 843de4841af..2865f6849be 100644 --- a/federation/pkg/kubefed/join_test.go +++ b/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: ®istered.GroupOrDie(api.GroupName).GroupVersion, - }, - } -} diff --git a/federation/pkg/kubefed/testing/BUILD b/federation/pkg/kubefed/testing/BUILD new file mode 100644 index 00000000000..1697957c850 --- /dev/null +++ b/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", + ], +) diff --git a/federation/pkg/kubefed/testing/testing.go b/federation/pkg/kubefed/testing/testing.go new file mode 100644 index 00000000000..befcdebfbdd --- /dev/null +++ b/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: ®istered.GroupOrDie(api.GroupName).GroupVersion, + }, + } +} diff --git a/federation/pkg/kubefed/unjoin_test.go b/federation/pkg/kubefed/unjoin_test.go index eb9cedf326e..6fc5e1405de 100644 --- a/federation/pkg/kubefed/unjoin_test.go +++ b/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) }