From 90d46c86707c9bfa7b689e14fdcbda4e562eb894 Mon Sep 17 00:00:00 2001 From: "Madhusudan.C.S" Date: Mon, 24 Oct 2016 21:20:21 -0700 Subject: [PATCH] [Federation][unjoin-00] Implement `kubefed unjoin` command. Also, add unit tests for `kubefed unjoin`. --- federation/pkg/kubefed/BUILD | 16 +- federation/pkg/kubefed/join.go | 8 +- federation/pkg/kubefed/unjoin.go | 150 +++++++++++++++++ federation/pkg/kubefed/unjoin_test.go | 225 ++++++++++++++++++++++++++ hack/verify-flags/known-flags.txt | 1 + 5 files changed, 394 insertions(+), 6 deletions(-) create mode 100644 federation/pkg/kubefed/unjoin.go create mode 100644 federation/pkg/kubefed/unjoin_test.go diff --git a/federation/pkg/kubefed/BUILD b/federation/pkg/kubefed/BUILD index dc5914b9005..9559f3d48a2 100644 --- a/federation/pkg/kubefed/BUILD +++ b/federation/pkg/kubefed/BUILD @@ -12,16 +12,23 @@ load( go_library( name = "go_default_library", - srcs = ["join.go"], + srcs = [ + "join.go", + "unjoin.go", + ], tags = ["automanaged"], deps = [ + "//federation/apis/federation:go_default_library", "//pkg/api:go_default_library", + "//pkg/api/errors:go_default_library", + "//pkg/api/unversioned:go_default_library", "//pkg/client/unversioned/clientcmd:go_default_library", "//pkg/client/unversioned/clientcmd/api:go_default_library", "//pkg/kubectl:go_default_library", "//pkg/kubectl/cmd:go_default_library", "//pkg/kubectl/cmd/templates:go_default_library", "//pkg/kubectl/cmd/util:go_default_library", + "//pkg/kubectl/resource:go_default_library", "//pkg/runtime:go_default_library", "//vendor:github.com/golang/glog", "//vendor:github.com/spf13/cobra", @@ -30,12 +37,17 @@ go_library( go_test( name = "go_default_test", - srcs = ["join_test.go"], + srcs = [ + "join_test.go", + "unjoin_test.go", + ], library = "go_default_library", tags = ["automanaged"], deps = [ + "//federation/apis/federation:go_default_library", "//federation/apis/federation/v1beta1: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", diff --git a/federation/pkg/kubefed/join.go b/federation/pkg/kubefed/join.go index 91bfd2c7072..83e8c7d8f9f 100644 --- a/federation/pkg/kubefed/join.go +++ b/federation/pkg/kubefed/join.go @@ -56,7 +56,7 @@ var ( # Join a cluster to a federation by specifying the # cluster context name and the context name of the # federation control plane's host cluster. - kubectl join foo --host=bar`) + kubectl join foo --host-cluster-context=bar`) ) // JoinFederationConfig provides a filesystem based kubeconfig (via @@ -106,7 +106,7 @@ func (j *joinFederationConfig) HostFactory(host, kubeconfigPath string) cmdutil. // federation. func NewCmdJoin(f cmdutil.Factory, cmdOut io.Writer, config JoinFederationConfig) *cobra.Command { cmd := &cobra.Command{ - Use: "join CLUSTER_CONTEXT --host=HOST_CONTEXT", + Use: "join CLUSTER_CONTEXT --host-cluster-context=HOST_CONTEXT", Short: "Join a cluster to a federation", Long: join_long, Example: join_example, @@ -130,7 +130,7 @@ func joinFederation(f cmdutil.Factory, cmdOut io.Writer, config JoinFederationCo if err != nil { return err } - host := cmdutil.GetFlagString(cmd, "host") + host := cmdutil.GetFlagString(cmd, "host-cluster-context") hostSystemNamespace := cmdutil.GetFlagString(cmd, "host-system-namespace") kubeconfig := cmdutil.GetFlagString(cmd, "kubeconfig") dryRun := cmdutil.GetDryRunFlag(cmd) @@ -183,7 +183,7 @@ func joinFederation(f cmdutil.Factory, cmdOut io.Writer, config JoinFederationCo func addJoinFlags(cmd *cobra.Command) { cmd.Flags().String("kubeconfig", "", "Path to the kubeconfig file to use for CLI requests.") - cmd.Flags().String("host", "", "Host cluster context") + cmd.Flags().String("host-cluster-context", "", "Host cluster context") cmd.Flags().String("host-system-namespace", "federation-system", "Namespace in the host cluster where the federation system components are installed") } diff --git a/federation/pkg/kubefed/unjoin.go b/federation/pkg/kubefed/unjoin.go new file mode 100644 index 00000000000..279cdc990a4 --- /dev/null +++ b/federation/pkg/kubefed/unjoin.go @@ -0,0 +1,150 @@ +/* +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 kubefed + +import ( + "fmt" + "io" + "net/url" + + federationapi "k8s.io/kubernetes/federation/apis/federation" + "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/api/errors" + "k8s.io/kubernetes/pkg/api/unversioned" + kubectlcmd "k8s.io/kubernetes/pkg/kubectl/cmd" + "k8s.io/kubernetes/pkg/kubectl/cmd/templates" + cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" + "k8s.io/kubernetes/pkg/kubectl/resource" + + "github.com/spf13/cobra" +) + +var ( + unjoin_long = templates.LongDesc(` + Unjoin removes a cluster from a federation. + + Current context is assumed to be a federation endpoint. + Please use the --context flag otherwise.`) + unjoin_example = templates.Examples(` + # Unjoin removes the specified cluster from a federation. + # Federation control plane's host cluster context name + # must be specified via the --host-cluster-context flag + # to properly cleanup the credentials. + kubectl unjoin foo --host-cluster-context=bar`) +) + +// NewCmdUnjoin defines the `unjoin` command that removes a cluster +// from a federation. +func NewCmdUnjoin(f cmdutil.Factory, cmdOut, cmdErr io.Writer, config JoinFederationConfig) *cobra.Command { + cmd := &cobra.Command{ + Use: "unjoin CLUSTER_NAME --host-cluster-context=HOST_CONTEXT", + Short: "Unjoins a cluster from a federation", + Long: unjoin_long, + Example: unjoin_example, + Run: func(cmd *cobra.Command, args []string) { + err := unjoinFederation(f, cmdOut, cmdErr, config, cmd, args) + cmdutil.CheckErr(err) + }, + } + + addJoinFlags(cmd) + return cmd +} + +// unjoinFederation is the implementation of the `unjoin` command. +func unjoinFederation(f cmdutil.Factory, cmdOut, cmdErr io.Writer, config JoinFederationConfig, cmd *cobra.Command, args []string) error { + name, err := kubectlcmd.NameFromCommandArgs(cmd, args) + if err != nil { + return err + } + host := cmdutil.GetFlagString(cmd, "host-cluster-context") + hostSystemNamespace := cmdutil.GetFlagString(cmd, "host-system-namespace") + kubeconfig := cmdutil.GetFlagString(cmd, "kubeconfig") + + cluster, err := popCluster(f, name) + if err != nil { + return err + } + if cluster == nil { + fmt.Fprintf(cmdErr, "WARNING: cluster %q not found in federation, so its credentials' secret couldn't be deleted", name) + return nil + } + + // We want a separate client factory to communicate with the + // federation host cluster. See join_federation.go for details. + hostFactory := config.HostFactory(host, kubeconfig) + err = deleteSecret(hostFactory, cluster.Spec.SecretRef.Name, hostSystemNamespace) + if isNotFound(err) { + fmt.Fprintf(cmdErr, "WARNING: secret %q not found in the host cluster, so it couldn't be deleted", cluster.Spec.SecretRef.Name) + } else if err != nil { + return err + } + _, err = fmt.Fprintf(cmdOut, "Successfully removed cluster %q from federation\n", name) + return err +} + +func popCluster(f cmdutil.Factory, name string) (*federationapi.Cluster, error) { + // Boilerplate to create the secret in the host cluster. + mapper, typer := f.Object() + gvks, _, err := typer.ObjectKinds(&federationapi.Cluster{}) + if err != nil { + return nil, err + } + gvk := gvks[0] + mapping, err := mapper.RESTMapping(unversioned.GroupKind{Group: gvk.Group, Kind: gvk.Kind}, gvk.Version) + if err != nil { + return nil, err + } + client, err := f.ClientForMapping(mapping) + if err != nil { + return nil, err + } + + rh := resource.NewHelper(client, mapping) + obj, err := rh.Get("", name, false) + + if isNotFound(err) { + // Cluster isn't registered, there isn't anything to be done here. + return nil, nil + } else if err != nil { + return nil, err + } + cluster, ok := obj.(*federationapi.Cluster) + if !ok { + return nil, fmt.Errorf("unexpected object type: expected \"federation/v1beta1.Cluster\", got %T: obj: %#v", obj, obj) + } + + // Remove the cluster resource in the federation API server by + // calling rh.Delete() + return cluster, rh.Delete("", name) +} + +func deleteSecret(hostFactory cmdutil.Factory, name, namespace string) error { + clientset, err := hostFactory.ClientSet() + if err != nil { + return err + } + return clientset.Core().Secrets(namespace).Delete(name, &api.DeleteOptions{}) +} + +func isNotFound(err error) bool { + statusErr := err + if urlErr, ok := err.(*url.Error); ok { + statusErr = urlErr.Err + } + return errors.IsNotFound(statusErr) +} diff --git a/federation/pkg/kubefed/unjoin_test.go b/federation/pkg/kubefed/unjoin_test.go new file mode 100644 index 00000000000..9469d341dcd --- /dev/null +++ b/federation/pkg/kubefed/unjoin_test.go @@ -0,0 +1,225 @@ +/* +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 kubefed + +import ( + "bytes" + "fmt" + "net/http" + "strings" + "testing" + + federationapi "k8s.io/kubernetes/federation/apis/federation" + "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/api/errors" + "k8s.io/kubernetes/pkg/api/testapi" + "k8s.io/kubernetes/pkg/api/unversioned" + "k8s.io/kubernetes/pkg/client/restclient/fake" + "k8s.io/kubernetes/pkg/client/typed/dynamic" + cmdtesting "k8s.io/kubernetes/pkg/kubectl/cmd/testing" + cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" +) + +func TestUnjoinFederation(t *testing.T) { + cmdErrMsg := "" + cmdutil.BehaviorOnFatal(func(str string, code int) { + cmdErrMsg = str + }) + + fakeKubeFiles, err := fakeKubeconfigFiles() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + defer rmFakeKubeconfigFiles(fakeKubeFiles) + + testCases := []struct { + cluster string + wantCluster string + wantSecret string + kubeconfigGlobal string + kubeconfigExplicit string + expectedServer string + expectedErr string + }{ + // Tests that the contexts and credentials are read from the + // global, default kubeconfig and the correct cluster resource + // is deregisterd. + { + cluster: "syndicate", + wantCluster: "syndicate", + wantSecret: "", + kubeconfigGlobal: fakeKubeFiles[0], + kubeconfigExplicit: "", + expectedServer: "https://10.20.30.40", + expectedErr: "", + }, + // Tests that the contexts and credentials are read from the + // explicit kubeconfig file specified and the correct cluster + // resource is deregisterd. kubeconfig contains a single + // cluster and context. + { + cluster: "ally", + wantCluster: "ally", + wantSecret: "", + kubeconfigGlobal: fakeKubeFiles[0], + kubeconfigExplicit: fakeKubeFiles[1], + expectedServer: "http://ally256.example.com:80", + expectedErr: "", + }, + // Tests that the contexts and credentials are read from the + // explicit kubeconfig file specified and the correct cluster + // resource is deregisterd. kubeconfig consists of multiple + // clusters and contexts. + { + cluster: "confederate", + wantCluster: "confederate", + wantSecret: "", + kubeconfigGlobal: fakeKubeFiles[1], + kubeconfigExplicit: fakeKubeFiles[2], + expectedServer: "https://10.8.8.8", + expectedErr: "", + }, + // Negative test to ensure that we get the right warning + // when the specified cluster to deregister is not found. + { + cluster: "noexist", + wantCluster: "affiliate", + wantSecret: "", + kubeconfigGlobal: fakeKubeFiles[0], + kubeconfigExplicit: "", + expectedServer: "https://10.20.30.40", + expectedErr: fmt.Sprintf("WARNING: cluster %q not found in federation, so its credentials' secret couldn't be deleted", "affiliate"), + }, + // Negative test to ensure that we get the right warning + // when the specified cluster's credentials secret is not + // found. + { + cluster: "affiliate", + wantCluster: "affiliate", + wantSecret: "noexist", + kubeconfigGlobal: fakeKubeFiles[0], + kubeconfigExplicit: "", + expectedServer: "https://10.20.30.40", + expectedErr: fmt.Sprintf("WARNING: secret %q not found in the host cluster, so it couldn't be deleted", "noexist"), + }, + } + + for i, tc := range testCases { + cmdErrMsg = "" + f := testUnjoinFederationFactory(tc.cluster, tc.expectedServer, tc.wantSecret) + buf := bytes.NewBuffer([]byte{}) + errBuf := bytes.NewBuffer([]byte{}) + + hostFactory := fakeUnjoinHostFactory(tc.cluster) + joinConfig, err := newFakeJoinFederationConfig(hostFactory, tc.kubeconfigGlobal) + if err != nil { + t.Fatalf("[%d] unexpected error: %v", i, err) + } + + cmd := NewCmdUnjoin(f, buf, errBuf, joinConfig) + + cmd.Flags().Set("kubeconfig", tc.kubeconfigExplicit) + cmd.Flags().Set("host", "substrate") + cmd.Run(cmd, []string{tc.wantCluster}) + + if tc.expectedErr == "" { + // uses the name from the cluster, not the response + // Actual data passed are tested in the fake secret and cluster + // REST clients. + if msg := buf.String(); msg != fmt.Sprintf("Successfully removed cluster %q from federation\n", tc.cluster) { + t.Errorf("[%d] unexpected output: %s", i, msg) + if cmdErrMsg != "" { + t.Errorf("[%d] unexpected error message: %s", i, cmdErrMsg) + } + } + } else { + if errMsg := errBuf.String(); errMsg != tc.expectedErr { + t.Errorf("[%d] expected warning: %s, got: %s, output: %s", i, tc.expectedErr, errMsg, buf.String()) + } + + } + } +} + +func testUnjoinFederationFactory(name, server, secret string) cmdutil.Factory { + urlPrefix := "/clusters/" + + cluster := fakeCluster(name, server) + if secret != "" { + cluster.Spec.SecretRef.Name = secret + } + + f, tf, _, _ := cmdtesting.NewAPIFactory() + codec := testapi.Federation.Codec() + tf.ClientConfig = defaultClientConfig() + ns := testapi.Federation.NegotiatedSerializer() + tf.Client = &fake.RESTClient{ + NegotiatedSerializer: ns, + GroupName: "federation", + Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { + switch p, m := req.URL.Path, req.Method; { + case strings.HasPrefix(p, urlPrefix): + got := strings.TrimPrefix(p, urlPrefix) + if got != name { + return nil, errors.NewNotFound(federationapi.Resource("clusters"), got) + } + + switch m { + case http.MethodGet: + return &http.Response{StatusCode: http.StatusOK, Header: defaultHeader(), Body: 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 + default: + return nil, fmt.Errorf("unexpected method: %#v\n%#v", req.URL, req) + } + default: + return nil, fmt.Errorf("unexpected request: %#v\n%#v", req.URL, req) + } + }), + } + tf.Namespace = "test" + return f +} + +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.Client = &fake.RESTClient{ + NegotiatedSerializer: ns, + Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { + switch p, m := req.URL.Path, req.Method; { + case strings.HasPrefix(p, urlPrefix) && m == http.MethodDelete: + got := strings.TrimPrefix(p, urlPrefix) + if got != name { + return nil, errors.NewNotFound(api.Resource("secrets"), got) + } + status := unversioned.Status{ + Status: "Success", + } + return &http.Response{StatusCode: http.StatusOK, Header: defaultHeader(), Body: objBody(codec, &status)}, nil + default: + return nil, fmt.Errorf("unexpected request: %#v\n%#v", req.URL, req) + } + }), + } + return f +} diff --git a/hack/verify-flags/known-flags.txt b/hack/verify-flags/known-flags.txt index da713be0e39..f6b6ac8871a 100644 --- a/hack/verify-flags/known-flags.txt +++ b/hack/verify-flags/known-flags.txt @@ -238,6 +238,7 @@ hard-pod-affinity-symmetric-weight healthz-bind-address healthz-port horizontal-pod-autoscaler-sync-period +host-cluster-context host-ipc-sources host-network-sources host-pid-sources