From 107ead78ce0545a858738f07f188a380d63e603d Mon Sep 17 00:00:00 2001 From: Phillip Wittrock Date: Sun, 13 Aug 2017 10:22:03 -0700 Subject: [PATCH 1/5] Implement kind visitor library for kubectl - Will be used to replace "Kind" switch statements - Allow for removal of go library dependency on kubernetes/kubernetes unversioned api packages - Better support for ensuring all types are accounted for --- pkg/kubectl/BUILD | 1 + pkg/kubectl/apps/BUILD | 42 ++++++ pkg/kubectl/apps/apps_suite_test.go | 29 ++++ pkg/kubectl/apps/kind_visitor.go | 95 +++++++++++++ pkg/kubectl/apps/kind_visitor_test.go | 190 ++++++++++++++++++++++++++ 5 files changed, 357 insertions(+) create mode 100644 pkg/kubectl/apps/BUILD create mode 100644 pkg/kubectl/apps/apps_suite_test.go create mode 100644 pkg/kubectl/apps/kind_visitor.go create mode 100644 pkg/kubectl/apps/kind_visitor_test.go diff --git a/pkg/kubectl/BUILD b/pkg/kubectl/BUILD index a408dad1dea..5e2a4a91140 100644 --- a/pkg/kubectl/BUILD +++ b/pkg/kubectl/BUILD @@ -184,6 +184,7 @@ filegroup( name = "all-srcs", srcs = [ ":package-srcs", + "//pkg/kubectl/apps:all-srcs", "//pkg/kubectl/cmd:all-srcs", "//pkg/kubectl/metricsutil:all-srcs", "//pkg/kubectl/plugins:all-srcs", diff --git a/pkg/kubectl/apps/BUILD b/pkg/kubectl/apps/BUILD new file mode 100644 index 00000000000..0d9d988760f --- /dev/null +++ b/pkg/kubectl/apps/BUILD @@ -0,0 +1,42 @@ +package(default_visibility = ["//visibility:public"]) + +licenses(["notice"]) + +load( + "@io_bazel_rules_go//go:def.bzl", + "go_library", + "go_test", +) + +go_library( + name = "go_default_library", + srcs = ["kind_visitor.go"], + tags = ["automanaged"], + deps = ["//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library"], +) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [":package-srcs"], + tags = ["automanaged"], +) + +go_test( + name = "go_default_xtest", + srcs = [ + "apps_suite_test.go", + "kind_visitor_test.go", + ], + deps = [ + ":go_default_library", + "//vendor/github.com/onsi/ginkgo:go_default_library", + "//vendor/github.com/onsi/gomega:go_default_library", + ], +) diff --git a/pkg/kubectl/apps/apps_suite_test.go b/pkg/kubectl/apps/apps_suite_test.go new file mode 100644 index 00000000000..a2300f1e445 --- /dev/null +++ b/pkg/kubectl/apps/apps_suite_test.go @@ -0,0 +1,29 @@ +/* +Copyright 2017 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 apps_test + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + "testing" +) + +func TestApps(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Apps Suite") +} diff --git a/pkg/kubectl/apps/kind_visitor.go b/pkg/kubectl/apps/kind_visitor.go new file mode 100644 index 00000000000..98443c60a51 --- /dev/null +++ b/pkg/kubectl/apps/kind_visitor.go @@ -0,0 +1,95 @@ +/* +Copyright 2017 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 apps + +import ( + "fmt" + + "k8s.io/apimachinery/pkg/runtime/schema" +) + +// KindVisitor is used with GroupKindElement to call a particular function depending on the +// Kind of a schema.GroupKind +type KindVisitor interface { + VisitDaemonSet(kind GroupKindElement) + VisitDeployment(kind GroupKindElement) + VisitJob(kind GroupKindElement) + VisitPod(kind GroupKindElement) + VisitReplicaSet(kind GroupKindElement) + VisitReplicationController(kind GroupKindElement) + VisitStatefulSet(kind GroupKindElement) +} + +// GroupKindElement defines a Kubernetes API group elem +type GroupKindElement schema.GroupKind + +// Accept calls the Visit method on visitor that corresponds to elem's Kind +func (elem GroupKindElement) Accept(visitor KindVisitor) error { + if elem.GroupMatch("apps", "extensions") && elem.Kind == "DaemonSet" { + visitor.VisitDaemonSet(elem) + return nil + } + if elem.GroupMatch("apps", "extensions") && elem.Kind == "Deployment" { + visitor.VisitDeployment(elem) + return nil + } + if elem.GroupMatch("apps", "extensions") && elem.Kind == "Job" { + visitor.VisitDeployment(elem) + return nil + } + if elem.GroupMatch("", "core") && elem.Kind == "Pod" { + visitor.VisitPod(elem) + return nil + } + if elem.GroupMatch("apps", "extensions") && elem.Kind == "ReplicaSet" { + visitor.VisitReplicationController(elem) + return nil + } + if elem.GroupMatch("", "core") && elem.Kind == "ReplicationController" { + visitor.VisitReplicationController(elem) + return nil + } + if elem.GroupMatch("apps") && elem.Kind == "StatefulSet" { + visitor.VisitStatefulSet(elem) + return nil + } + return fmt.Errorf("no visitor method exists for %v", elem) +} + +// GroupMatch returns true if and only if elem's group matches one +// of the group arguments +func (elem GroupKindElement) GroupMatch(groups ...string) bool { + for _, g := range groups { + if elem.Group == g { + return true + } + } + return false +} + +// DefaultKindVisitor implements KindVisitor with no-op functions. +type DefaultKindVisitor struct{} + +var _ KindVisitor = &DefaultKindVisitor{} + +func (*DefaultKindVisitor) VisitDaemonSet(kind GroupKindElement) {} +func (*DefaultKindVisitor) VisitDeployment(kind GroupKindElement) {} +func (*DefaultKindVisitor) VisitJob(kind GroupKindElement) {} +func (*DefaultKindVisitor) VisitPod(kind GroupKindElement) {} +func (*DefaultKindVisitor) VisitReplicaSet(kind GroupKindElement) {} +func (*DefaultKindVisitor) VisitReplicationController(kind GroupKindElement) {} +func (*DefaultKindVisitor) VisitStatefulSet(kind GroupKindElement) {} diff --git a/pkg/kubectl/apps/kind_visitor_test.go b/pkg/kubectl/apps/kind_visitor_test.go new file mode 100644 index 00000000000..7d392b04910 --- /dev/null +++ b/pkg/kubectl/apps/kind_visitor_test.go @@ -0,0 +1,190 @@ +/* +Copyright 2017 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 apps_test + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + "k8s.io/kubernetes/pkg/kubectl/apps" +) + +var _ = Describe("When KindVisitor accepts a GroupKind", func() { + + var visitor *TestKindVisitor + + BeforeEach(func() { + visitor = &TestKindVisitor{map[string]int{}} + }) + + It("should Visit DaemonSet iff the Kind is a DaemonSet", func() { + kind := apps.GroupKindElement{ + Kind: "DaemonSet", + Group: "apps", + } + Expect(kind.Accept(visitor)).ShouldNot(HaveOccurred()) + Expect(visitor.visits).To(Equal(map[string]int{ + "DaemonSet": 1, + })) + + kind = apps.GroupKindElement{ + Kind: "DaemonSet", + Group: "extensions", + } + Expect(kind.Accept(visitor)).ShouldNot(HaveOccurred()) + Expect(visitor.visits).To(Equal(map[string]int{ + "DaemonSet": 2, + })) + }) + + It("should Visit Deployment iff the Kind is a Deployment", func() { + kind := apps.GroupKindElement{ + Kind: "Deployment", + Group: "apps", + } + Expect(kind.Accept(visitor)).ShouldNot(HaveOccurred()) + Expect(visitor.visits).To(Equal(map[string]int{ + "Deployment": 1, + })) + + kind = apps.GroupKindElement{ + Kind: "Deployment", + Group: "extensions", + } + Expect(kind.Accept(visitor)).ShouldNot(HaveOccurred()) + Expect(visitor.visits).To(Equal(map[string]int{ + "Deployment": 2, + })) + }) + + It("should Visit Job iff the Kind is a Job", func() { + kind := apps.GroupKindElement{ + Kind: "Job", + Group: "apps", + } + Expect(kind.Accept(visitor)).ShouldNot(HaveOccurred()) + Expect(visitor.visits).To(Equal(map[string]int{ + "Job": 1, + })) + + kind = apps.GroupKindElement{ + Kind: "Job", + Group: "extensions", + } + Expect(kind.Accept(visitor)).ShouldNot(HaveOccurred()) + Expect(visitor.visits).To(Equal(map[string]int{ + "Job": 2, + })) + }) + + It("should Visit Pod iff the Kind is a Pod", func() { + kind := apps.GroupKindElement{ + Kind: "Pod", + Group: "", + } + Expect(kind.Accept(visitor)).ShouldNot(HaveOccurred()) + Expect(visitor.visits).To(Equal(map[string]int{ + "Pod": 1, + })) + + kind = apps.GroupKindElement{ + Kind: "Pod", + Group: "core", + } + Expect(kind.Accept(visitor)).ShouldNot(HaveOccurred()) + Expect(visitor.visits).To(Equal(map[string]int{ + "Pod": 2, + })) + }) + + It("should Visit ReplicationController iff the Kind is a ReplicationController", func() { + kind := apps.GroupKindElement{ + Kind: "ReplicationController", + Group: "", + } + Expect(kind.Accept(visitor)).ShouldNot(HaveOccurred()) + Expect(visitor.visits).To(Equal(map[string]int{ + "ReplicationController": 1, + })) + + kind = apps.GroupKindElement{ + Kind: "ReplicationController", + Group: "core", + } + Expect(kind.Accept(visitor)).ShouldNot(HaveOccurred()) + Expect(visitor.visits).To(Equal(map[string]int{ + "ReplicationController": 2, + })) + }) + + It("should Visit ReplicaSet iff the Kind is a ReplicaSet", func() { + kind := apps.GroupKindElement{ + Kind: "ReplicaSet", + Group: "apps", + } + Expect(kind.Accept(visitor)).ShouldNot(HaveOccurred()) + Expect(visitor.visits).To(Equal(map[string]int{ + "ReplicaSet": 1, + })) + + kind = apps.GroupKindElement{ + Kind: "ReplicaSet", + Group: "extensions", + } + Expect(kind.Accept(visitor)).ShouldNot(HaveOccurred()) + Expect(visitor.visits).To(Equal(map[string]int{ + "ReplicaSet": 2, + })) + }) + + It("should Visit StatefulSet iff the Kind is a StatefulSet", func() { + kind := apps.GroupKindElement{ + Kind: "StatefulSet", + Group: "apps", + } + Expect(kind.Accept(visitor)).ShouldNot(HaveOccurred()) + Expect(visitor.visits).To(Equal(map[string]int{ + "StatefulSet": 1, + })) + }) + + It("should give an error if the Kind is unknown", func() { + kind := apps.GroupKindElement{ + Kind: "Unknown", + Group: "apps", + } + Expect(kind.Accept(visitor)).Should(HaveOccurred()) + Expect(visitor.visits).To(Equal(map[string]int{})) + }) +}) + +// TestKindVisitor increments a value each time a Visit method was called +type TestKindVisitor struct { + visits map[string]int +} + +var _ apps.KindVisitor = &TestKindVisitor{} + +func (t *TestKindVisitor) Visit(kind apps.GroupKindElement) { t.visits[kind.Kind] += 1 } + +func (t *TestKindVisitor) VisitDaemonSet(kind apps.GroupKindElement) { t.Visit(kind) } +func (t *TestKindVisitor) VisitDeployment(kind apps.GroupKindElement) { t.Visit(kind) } +func (t *TestKindVisitor) VisitJob(kind apps.GroupKindElement) { t.Visit(kind) } +func (t *TestKindVisitor) VisitPod(kind apps.GroupKindElement) { t.Visit(kind) } +func (t *TestKindVisitor) VisitReplicaSet(kind apps.GroupKindElement) { t.Visit(kind) } +func (t *TestKindVisitor) VisitReplicationController(kind apps.GroupKindElement) { t.Visit(kind) } +func (t *TestKindVisitor) VisitStatefulSet(kind apps.GroupKindElement) { t.Visit(kind) } From e97a18b4358a9ea622624b1addac568d7bd91105 Mon Sep 17 00:00:00 2001 From: Phillip Wittrock Date: Sun, 13 Aug 2017 10:53:55 -0700 Subject: [PATCH 2/5] Support for using a client-go client from kubectl --- pkg/kubectl/cmd/testing/BUILD | 1 + pkg/kubectl/cmd/testing/fake.go | 32 +++++++++++++++++++ pkg/kubectl/cmd/util/BUILD | 1 + pkg/kubectl/cmd/util/clientcache.go | 29 +++++++++++++++++ pkg/kubectl/cmd/util/factory.go | 5 +++ pkg/kubectl/cmd/util/factory_client_access.go | 5 +++ 6 files changed, 73 insertions(+) diff --git a/pkg/kubectl/cmd/testing/BUILD b/pkg/kubectl/cmd/testing/BUILD index 62f5708a2d0..e6b0ab3045c 100644 --- a/pkg/kubectl/cmd/testing/BUILD +++ b/pkg/kubectl/cmd/testing/BUILD @@ -35,6 +35,7 @@ go_library( "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/serializer:go_default_library", "//vendor/k8s.io/client-go/discovery:go_default_library", + "//vendor/k8s.io/client-go/kubernetes:go_default_library", "//vendor/k8s.io/client-go/rest:go_default_library", "//vendor/k8s.io/client-go/rest/fake:go_default_library", ], diff --git a/pkg/kubectl/cmd/testing/fake.go b/pkg/kubectl/cmd/testing/fake.go index 23c27a45653..8413823798b 100644 --- a/pkg/kubectl/cmd/testing/fake.go +++ b/pkg/kubectl/cmd/testing/fake.go @@ -34,6 +34,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/runtime/serializer" "k8s.io/client-go/discovery" + "k8s.io/client-go/kubernetes" restclient "k8s.io/client-go/rest" "k8s.io/client-go/rest/fake" fedclientset "k8s.io/kubernetes/federation/client/clientset_generated/federation_clientset" @@ -307,6 +308,10 @@ func (f *FakeFactory) RESTClient() (*restclient.RESTClient, error) { return nil, nil } +func (f *FakeFactory) KubernetesClientSet() (*kubernetes.Clientset, error) { + return nil, nil +} + func (f *FakeFactory) ClientSet() (internalclientset.Interface, error) { return nil, nil } @@ -582,6 +587,33 @@ func (f *fakeAPIFactory) JSONEncoder() runtime.Encoder { return testapi.Default.Codec() } +func (f *fakeAPIFactory) KubernetesClientSet() (*kubernetes.Clientset, error) { + fakeClient := f.tf.Client.(*fake.RESTClient) + clientset := kubernetes.NewForConfigOrDie(f.tf.ClientConfig) + + clientset.CoreV1().RESTClient().(*restclient.RESTClient).Client = fakeClient.Client + clientset.AuthorizationV1().RESTClient().(*restclient.RESTClient).Client = fakeClient.Client + clientset.AuthorizationV1beta1().RESTClient().(*restclient.RESTClient).Client = fakeClient.Client + clientset.AuthorizationV1().RESTClient().(*restclient.RESTClient).Client = fakeClient.Client + clientset.AuthorizationV1beta1().RESTClient().(*restclient.RESTClient).Client = fakeClient.Client + clientset.AutoscalingV1().RESTClient().(*restclient.RESTClient).Client = fakeClient.Client + clientset.AutoscalingV2alpha1().RESTClient().(*restclient.RESTClient).Client = fakeClient.Client + clientset.BatchV1().RESTClient().(*restclient.RESTClient).Client = fakeClient.Client + clientset.BatchV2alpha1().RESTClient().(*restclient.RESTClient).Client = fakeClient.Client + clientset.CertificatesV1beta1().RESTClient().(*restclient.RESTClient).Client = fakeClient.Client + clientset.ExtensionsV1beta1().RESTClient().(*restclient.RESTClient).Client = fakeClient.Client + clientset.RbacV1alpha1().RESTClient().(*restclient.RESTClient).Client = fakeClient.Client + clientset.RbacV1beta1().RESTClient().(*restclient.RESTClient).Client = fakeClient.Client + clientset.StorageV1().RESTClient().(*restclient.RESTClient).Client = fakeClient.Client + clientset.StorageV1beta1().RESTClient().(*restclient.RESTClient).Client = fakeClient.Client + clientset.AppsV1beta1().RESTClient().(*restclient.RESTClient).Client = fakeClient.Client + clientset.AppsV1beta2().RESTClient().(*restclient.RESTClient).Client = fakeClient.Client + clientset.PolicyV1beta1().RESTClient().(*restclient.RESTClient).Client = fakeClient.Client + clientset.DiscoveryClient.RESTClient().(*restclient.RESTClient).Client = fakeClient.Client + + return clientset, f.tf.Err +} + func (f *fakeAPIFactory) ClientSet() (internalclientset.Interface, error) { // Swap the HTTP client out of the REST client with the fake // version. diff --git a/pkg/kubectl/cmd/util/BUILD b/pkg/kubectl/cmd/util/BUILD index 99473b51dc0..68eee0a23bc 100644 --- a/pkg/kubectl/cmd/util/BUILD +++ b/pkg/kubectl/cmd/util/BUILD @@ -66,6 +66,7 @@ go_library( "//vendor/k8s.io/apiserver/pkg/util/flag:go_default_library", "//vendor/k8s.io/client-go/discovery:go_default_library", "//vendor/k8s.io/client-go/dynamic:go_default_library", + "//vendor/k8s.io/client-go/kubernetes:go_default_library", "//vendor/k8s.io/client-go/rest:go_default_library", "//vendor/k8s.io/client-go/tools/clientcmd:go_default_library", "//vendor/k8s.io/client-go/util/homedir:go_default_library", diff --git a/pkg/kubectl/cmd/util/clientcache.go b/pkg/kubectl/cmd/util/clientcache.go index da7c45cc6e7..9de532ecc98 100644 --- a/pkg/kubectl/cmd/util/clientcache.go +++ b/pkg/kubectl/cmd/util/clientcache.go @@ -21,6 +21,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/discovery" + "k8s.io/client-go/kubernetes" restclient "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" fedclientset "k8s.io/kubernetes/federation/client/clientset_generated/federation_clientset" @@ -58,6 +59,34 @@ type ClientCache struct { // argument evaluation discoveryClientFactory DiscoveryClientFactory discoveryClient discovery.DiscoveryInterface + + kubernetesClientLoader KubernetesClientCache +} + +// kubernetesClientLoader provides a new kubernetes.Clientset +type KubernetesClientCache struct { + // once makes sure the client is only initialized once + once sync.Once + // client is the cached client value + client *kubernetes.Clientset + // err is the cached error value + err error +} + +// KubernetesClientSet returns a new kubernetes.Clientset. It will cache the value +// the first time it is called and return the cached value on subsequent calls. +// If an error is encountered the first time KubernetesClientSet is called, +// the error will be cached. +func (c *ClientCache) KubernetesClientSet(requiredVersion *schema.GroupVersion) (*kubernetes.Clientset, error) { + c.kubernetesClientLoader.once.Do(func() { + config, err := c.ClientConfigForVersion(requiredVersion) + if err != nil { + c.kubernetesClientLoader.err = err + return + } + c.kubernetesClientLoader.client, c.kubernetesClientLoader.err = kubernetes.NewForConfig(config) + }) + return c.kubernetesClientLoader.client, c.kubernetesClientLoader.err } // also looks up the discovery client. We can't do this during init because the flags won't have been set diff --git a/pkg/kubectl/cmd/util/factory.go b/pkg/kubectl/cmd/util/factory.go index 13419716ce5..40a879bd884 100644 --- a/pkg/kubectl/cmd/util/factory.go +++ b/pkg/kubectl/cmd/util/factory.go @@ -43,6 +43,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/serializer/json" "k8s.io/apimachinery/pkg/watch" "k8s.io/client-go/discovery" + "k8s.io/client-go/kubernetes" restclient "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" fedclientset "k8s.io/kubernetes/federation/client/clientset_generated/federation_clientset" @@ -91,6 +92,10 @@ type ClientAccessFactory interface { // ClientSet gives you back an internal, generated clientset ClientSet() (internalclientset.Interface, error) + + // KubernetesClientSet gives you back an external clientset + KubernetesClientSet() (*kubernetes.Clientset, error) + // Returns a RESTClient for accessing Kubernetes resources or an error. RESTClient() (*restclient.RESTClient, error) // Returns a client.Config for accessing the Kubernetes server. diff --git a/pkg/kubectl/cmd/util/factory_client_access.go b/pkg/kubectl/cmd/util/factory_client_access.go index 9aaa4680c81..2cf9c7a39f2 100644 --- a/pkg/kubectl/cmd/util/factory_client_access.go +++ b/pkg/kubectl/cmd/util/factory_client_access.go @@ -37,6 +37,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" utilflag "k8s.io/apiserver/pkg/util/flag" "k8s.io/client-go/discovery" + "k8s.io/client-go/kubernetes" restclient "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" "k8s.io/client-go/util/homedir" @@ -167,6 +168,10 @@ func (f *ring0Factory) DiscoveryClient() (discovery.CachedDiscoveryInterface, er return f.discoveryFactory.DiscoveryClient() } +func (f *ring0Factory) KubernetesClientSet() (*kubernetes.Clientset, error) { + return f.clientCache.KubernetesClientSet(nil) +} + func (f *ring0Factory) ClientSet() (internalclientset.Interface, error) { return f.clientCache.ClientSetForVersion(nil) } From aca62a174148eb929f0fbed707fbf991dc397e43 Mon Sep 17 00:00:00 2001 From: Phillip Wittrock Date: Wed, 16 Aug 2017 14:22:32 -0700 Subject: [PATCH 3/5] Address PR comments --- pkg/kubectl/apps/kind_visitor.go | 28 +++++++++---------- pkg/kubectl/cmd/util/clientcache.go | 19 +++++++------ pkg/kubectl/cmd/util/factory.go | 2 +- pkg/kubectl/cmd/util/factory_client_access.go | 2 +- 4 files changed, 26 insertions(+), 25 deletions(-) diff --git a/pkg/kubectl/apps/kind_visitor.go b/pkg/kubectl/apps/kind_visitor.go index 98443c60a51..0170883e1e5 100644 --- a/pkg/kubectl/apps/kind_visitor.go +++ b/pkg/kubectl/apps/kind_visitor.go @@ -47,16 +47,16 @@ func (elem GroupKindElement) Accept(visitor KindVisitor) error { visitor.VisitDeployment(elem) return nil } - if elem.GroupMatch("apps", "extensions") && elem.Kind == "Job" { - visitor.VisitDeployment(elem) + if elem.GroupMatch("batch") && elem.Kind == "Job" { + visitor.VisitJob(elem) return nil } if elem.GroupMatch("", "core") && elem.Kind == "Pod" { visitor.VisitPod(elem) return nil } - if elem.GroupMatch("apps", "extensions") && elem.Kind == "ReplicaSet" { - visitor.VisitReplicationController(elem) + if elem.GroupMatch("extensions") && elem.Kind == "ReplicaSet" { + visitor.VisitReplicaSet(elem) return nil } if elem.GroupMatch("", "core") && elem.Kind == "ReplicationController" { @@ -81,15 +81,15 @@ func (elem GroupKindElement) GroupMatch(groups ...string) bool { return false } -// DefaultKindVisitor implements KindVisitor with no-op functions. -type DefaultKindVisitor struct{} +// NoOpKindVisitor implements KindVisitor with no-op functions. +type NoOpKindVisitor struct{} -var _ KindVisitor = &DefaultKindVisitor{} +var _ KindVisitor = &NoOpKindVisitor{} -func (*DefaultKindVisitor) VisitDaemonSet(kind GroupKindElement) {} -func (*DefaultKindVisitor) VisitDeployment(kind GroupKindElement) {} -func (*DefaultKindVisitor) VisitJob(kind GroupKindElement) {} -func (*DefaultKindVisitor) VisitPod(kind GroupKindElement) {} -func (*DefaultKindVisitor) VisitReplicaSet(kind GroupKindElement) {} -func (*DefaultKindVisitor) VisitReplicationController(kind GroupKindElement) {} -func (*DefaultKindVisitor) VisitStatefulSet(kind GroupKindElement) {} +func (*NoOpKindVisitor) VisitDaemonSet(kind GroupKindElement) {} +func (*NoOpKindVisitor) VisitDeployment(kind GroupKindElement) {} +func (*NoOpKindVisitor) VisitJob(kind GroupKindElement) {} +func (*NoOpKindVisitor) VisitPod(kind GroupKindElement) {} +func (*NoOpKindVisitor) VisitReplicaSet(kind GroupKindElement) {} +func (*NoOpKindVisitor) VisitReplicationController(kind GroupKindElement) {} +func (*NoOpKindVisitor) VisitStatefulSet(kind GroupKindElement) {} diff --git a/pkg/kubectl/cmd/util/clientcache.go b/pkg/kubectl/cmd/util/clientcache.go index 9de532ecc98..5d7440308b1 100644 --- a/pkg/kubectl/cmd/util/clientcache.go +++ b/pkg/kubectl/cmd/util/clientcache.go @@ -60,10 +60,11 @@ type ClientCache struct { discoveryClientFactory DiscoveryClientFactory discoveryClient discovery.DiscoveryInterface - kubernetesClientLoader KubernetesClientCache + kubernetesClientCache KubernetesClientCache } -// kubernetesClientLoader provides a new kubernetes.Clientset +// KubernetesClientCache creates a new kubernetes.Clientset one time +// and then returns the result for all future requests type KubernetesClientCache struct { // once makes sure the client is only initialized once once sync.Once @@ -73,20 +74,20 @@ type KubernetesClientCache struct { err error } -// KubernetesClientSet returns a new kubernetes.Clientset. It will cache the value +// KubernetesClientSetForVersion returns a new kubernetes.Clientset. It will cache the value // the first time it is called and return the cached value on subsequent calls. -// If an error is encountered the first time KubernetesClientSet is called, +// If an error is encountered the first time KubernetesClientSetForVersion is called, // the error will be cached. -func (c *ClientCache) KubernetesClientSet(requiredVersion *schema.GroupVersion) (*kubernetes.Clientset, error) { - c.kubernetesClientLoader.once.Do(func() { +func (c *ClientCache) KubernetesClientSetForVersion(requiredVersion *schema.GroupVersion) (*kubernetes.Clientset, error) { + c.kubernetesClientCache.once.Do(func() { config, err := c.ClientConfigForVersion(requiredVersion) if err != nil { - c.kubernetesClientLoader.err = err + c.kubernetesClientCache.err = err return } - c.kubernetesClientLoader.client, c.kubernetesClientLoader.err = kubernetes.NewForConfig(config) + c.kubernetesClientCache.client, c.kubernetesClientCache.err = kubernetes.NewForConfig(config) }) - return c.kubernetesClientLoader.client, c.kubernetesClientLoader.err + return c.kubernetesClientCache.client, c.kubernetesClientCache.err } // also looks up the discovery client. We can't do this during init because the flags won't have been set diff --git a/pkg/kubectl/cmd/util/factory.go b/pkg/kubectl/cmd/util/factory.go index 40a879bd884..6ce8f6d7098 100644 --- a/pkg/kubectl/cmd/util/factory.go +++ b/pkg/kubectl/cmd/util/factory.go @@ -93,7 +93,7 @@ type ClientAccessFactory interface { // ClientSet gives you back an internal, generated clientset ClientSet() (internalclientset.Interface, error) - // KubernetesClientSet gives you back an external clientset + // KubernetesClientSetForVersion gives you back an external clientset KubernetesClientSet() (*kubernetes.Clientset, error) // Returns a RESTClient for accessing Kubernetes resources or an error. diff --git a/pkg/kubectl/cmd/util/factory_client_access.go b/pkg/kubectl/cmd/util/factory_client_access.go index 2cf9c7a39f2..e71a5a2571b 100644 --- a/pkg/kubectl/cmd/util/factory_client_access.go +++ b/pkg/kubectl/cmd/util/factory_client_access.go @@ -169,7 +169,7 @@ func (f *ring0Factory) DiscoveryClient() (discovery.CachedDiscoveryInterface, er } func (f *ring0Factory) KubernetesClientSet() (*kubernetes.Clientset, error) { - return f.clientCache.KubernetesClientSet(nil) + return f.clientCache.KubernetesClientSetForVersion(nil) } func (f *ring0Factory) ClientSet() (internalclientset.Interface, error) { From 32c5d938eb8cdaf1d404210441d6e3ba9cc89904 Mon Sep 17 00:00:00 2001 From: Phillip Wittrock Date: Thu, 17 Aug 2017 11:48:18 -0700 Subject: [PATCH 4/5] Update with PR comments --- pkg/kubectl/apps/apps_suite_test.go | 4 ++-- pkg/kubectl/apps/kind_visitor_test.go | 21 ++------------------- pkg/kubectl/cmd/util/factory.go | 2 +- 3 files changed, 5 insertions(+), 22 deletions(-) diff --git a/pkg/kubectl/apps/apps_suite_test.go b/pkg/kubectl/apps/apps_suite_test.go index a2300f1e445..b1bc1310ea0 100644 --- a/pkg/kubectl/apps/apps_suite_test.go +++ b/pkg/kubectl/apps/apps_suite_test.go @@ -17,10 +17,10 @@ limitations under the License. package apps_test import ( + "testing" + . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" - - "testing" ) func TestApps(t *testing.T) { diff --git a/pkg/kubectl/apps/kind_visitor_test.go b/pkg/kubectl/apps/kind_visitor_test.go index 7d392b04910..c5c2d1a631c 100644 --- a/pkg/kubectl/apps/kind_visitor_test.go +++ b/pkg/kubectl/apps/kind_visitor_test.go @@ -74,21 +74,13 @@ var _ = Describe("When KindVisitor accepts a GroupKind", func() { It("should Visit Job iff the Kind is a Job", func() { kind := apps.GroupKindElement{ Kind: "Job", - Group: "apps", + Group: "batch", } Expect(kind.Accept(visitor)).ShouldNot(HaveOccurred()) Expect(visitor.visits).To(Equal(map[string]int{ "Job": 1, })) - kind = apps.GroupKindElement{ - Kind: "Job", - Group: "extensions", - } - Expect(kind.Accept(visitor)).ShouldNot(HaveOccurred()) - Expect(visitor.visits).To(Equal(map[string]int{ - "Job": 2, - })) }) It("should Visit Pod iff the Kind is a Pod", func() { @@ -133,21 +125,12 @@ var _ = Describe("When KindVisitor accepts a GroupKind", func() { It("should Visit ReplicaSet iff the Kind is a ReplicaSet", func() { kind := apps.GroupKindElement{ - Kind: "ReplicaSet", - Group: "apps", - } - Expect(kind.Accept(visitor)).ShouldNot(HaveOccurred()) - Expect(visitor.visits).To(Equal(map[string]int{ - "ReplicaSet": 1, - })) - - kind = apps.GroupKindElement{ Kind: "ReplicaSet", Group: "extensions", } Expect(kind.Accept(visitor)).ShouldNot(HaveOccurred()) Expect(visitor.visits).To(Equal(map[string]int{ - "ReplicaSet": 2, + "ReplicaSet": 1, })) }) diff --git a/pkg/kubectl/cmd/util/factory.go b/pkg/kubectl/cmd/util/factory.go index 6ce8f6d7098..40a879bd884 100644 --- a/pkg/kubectl/cmd/util/factory.go +++ b/pkg/kubectl/cmd/util/factory.go @@ -93,7 +93,7 @@ type ClientAccessFactory interface { // ClientSet gives you back an internal, generated clientset ClientSet() (internalclientset.Interface, error) - // KubernetesClientSetForVersion gives you back an external clientset + // KubernetesClientSet gives you back an external clientset KubernetesClientSet() (*kubernetes.Clientset, error) // Returns a RESTClient for accessing Kubernetes resources or an error. From bc94cd5807c980b53986b036c7ef86d85b91895c Mon Sep 17 00:00:00 2001 From: Phillip Wittrock Date: Mon, 21 Aug 2017 16:06:48 -0700 Subject: [PATCH 5/5] More PR comments --- pkg/kubectl/cmd/util/clientcache.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/kubectl/cmd/util/clientcache.go b/pkg/kubectl/cmd/util/clientcache.go index 5d7440308b1..5312cb3095c 100644 --- a/pkg/kubectl/cmd/util/clientcache.go +++ b/pkg/kubectl/cmd/util/clientcache.go @@ -60,12 +60,12 @@ type ClientCache struct { discoveryClientFactory DiscoveryClientFactory discoveryClient discovery.DiscoveryInterface - kubernetesClientCache KubernetesClientCache + kubernetesClientCache kubernetesClientCache } -// KubernetesClientCache creates a new kubernetes.Clientset one time +// kubernetesClientCache creates a new kubernetes.Clientset one time // and then returns the result for all future requests -type KubernetesClientCache struct { +type kubernetesClientCache struct { // once makes sure the client is only initialized once once sync.Once // client is the cached client value