From 47672afc8266e4ac6b342e67cba86a55db41a79e Mon Sep 17 00:00:00 2001 From: Daniel Feinberg Date: Thu, 5 Jul 2018 22:59:08 -0600 Subject: [PATCH 1/3] adding unit-testable client --- pkg/k8sclient/client.go | 50 +++++++++++++++++++++++++++-------------- 1 file changed, 33 insertions(+), 17 deletions(-) diff --git a/pkg/k8sclient/client.go b/pkg/k8sclient/client.go index 3175dab970..248597066c 100644 --- a/pkg/k8sclient/client.go +++ b/pkg/k8sclient/client.go @@ -33,26 +33,42 @@ import ( "k8s.io/client-go/tools/clientcmd" ) -var ( +type resourceClientFactory struct { restMapper *discovery.DeferredDiscoveryRESTMapper clientPool dynamic.ClientPool kubeClient kubernetes.Interface kubeConfig *rest.Config +} + +var ( + // this stores the singleton in a package local + singletonFactory *resourceClientFactory ) -// init initializes the restMapper and clientPool needed to create a resource client dynamically -func init() { - kubeClient, kubeConfig = mustNewKubeClientAndConfig() - cachedDiscoveryClient := cached.NewMemCacheClient(kubeClient.Discovery()) - restMapper = discovery.NewDeferredDiscoveryRESTMapper(cachedDiscoveryClient, meta.InterfacesForUnstructured) - restMapper.Reset() - kubeConfig.ContentConfig = dynamic.ContentConfig() - clientPool = dynamic.NewClientPool(kubeConfig, restMapper, dynamic.LegacyAPIPathResolverFunc) - runBackgroundCacheReset(1 * time.Minute) +// GetResourceClient returns the resource client using a singleton factory +func GetResourceClient(apiVersion, kind, namespace string) (dynamic.ResourceInterface, string, error) { + if singletonFactory == nil { + kubeClient, kubeConfig := mustNewKubeClientAndConfig() + cachedDiscoveryClient := cached.NewMemCacheClient(kubeClient.Discovery()) + restMapper := discovery.NewDeferredDiscoveryRESTMapper(cachedDiscoveryClient, meta.InterfacesForUnstructured) + restMapper.Reset() + kubeConfig.ContentConfig = dynamic.ContentConfig() + clientPool := dynamic.NewClientPool(kubeConfig, restMapper, dynamic.LegacyAPIPathResolverFunc) + + singletonFactory := &resourceClientFactory{ + kubeClient: kubeClient, + kubeConfig: kubeConfig, + restMapper: restMapper, + clientPool: clientPool, + } + singletonFactory.runBackgroundCacheReset(1 * time.Minute) + } + + return singletonFactory.GetResourceClient(apiVersion, kind, namespace) } // GetResourceClient returns the dynamic client and pluralName for the resource specified by the apiVersion and kind -func GetResourceClient(apiVersion, kind, namespace string) (dynamic.ResourceInterface, string, error) { +func (c *resourceClientFactory) GetResourceClient(apiVersion, kind, namespace string) (dynamic.ResourceInterface, string, error) { gv, err := schema.ParseGroupVersion(apiVersion) if err != nil { return nil, "", fmt.Errorf("failed to parse apiVersion: %v", err) @@ -63,11 +79,11 @@ func GetResourceClient(apiVersion, kind, namespace string) (dynamic.ResourceInte Kind: kind, } - client, err := clientPool.ClientForGroupVersionKind(gvk) + client, err := c.clientPool.ClientForGroupVersionKind(gvk) if err != nil { return nil, "", fmt.Errorf("failed to get client for GroupVersionKind(%s): %v", gvk.String(), err) } - resource, err := apiResource(gvk, restMapper) + resource, err := apiResource(gvk, c.restMapper) if err != nil { return nil, "", fmt.Errorf("failed to get resource type: %v", err) } @@ -77,8 +93,8 @@ func GetResourceClient(apiVersion, kind, namespace string) (dynamic.ResourceInte } // GetKubeClient returns the kubernetes client used to create the dynamic client -func GetKubeClient() kubernetes.Interface { - return kubeClient +func (c *resourceClientFactory) GetKubeClient() kubernetes.Interface { + return c.kubeClient } // apiResource consults the REST mapper to translate an tuple to a metav1.APIResource struct. @@ -136,11 +152,11 @@ func outOfClusterConfig() (*rest.Config, error) { // runBackgroundCacheReset - Starts the rest mapper cache reseting // at a duration given. -func runBackgroundCacheReset(duration time.Duration) { +func (c *resourceClientFactory) runBackgroundCacheReset(duration time.Duration) { ticker := time.NewTicker(duration) go func() { for range ticker.C { - restMapper.Reset() + c.restMapper.Reset() } }() } From f75b1186f7d8d8ba344174f909320375d9cd2639 Mon Sep 17 00:00:00 2001 From: Daniel Feinberg Date: Fri, 6 Jul 2018 10:08:12 -0600 Subject: [PATCH 2/3] adding thread saftey, fixing shadowed var --- Gopkg.lock | 63 +++++++++++------------------------------ pkg/k8sclient/client.go | 38 ++++++++++++++----------- 2 files changed, 37 insertions(+), 64 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index a3c37e68cb..196f8eb7ed 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -13,12 +13,6 @@ packages = ["."] revision = "de5bf2ad457846296e2031421a34e2568e304e35" -[[projects]] - branch = "master" - name = "github.com/beorn7/perks" - packages = ["quantile"] - revision = "3a771d992973f24aa725d07868b467d1ddfceafb" - [[projects]] name = "github.com/davecgh/go-spew" packages = ["spew"] @@ -172,10 +166,21 @@ revision = "32fa128f234d041f196a9f3e0fea5ac9772c08e1" [[projects]] - name = "github.com/matttproud/golang_protobuf_extensions" - packages = ["pbutil"] - revision = "c12348ce28de40eed0136aa2b644d0ee0650e56c" - version = "v1.0.1" + branch = "master" + name = "github.com/operator-framework/operator-sdk" + packages = [ + "commands/operator-sdk/cmd", + "commands/operator-sdk/cmd/cmdutil", + "commands/operator-sdk/cmd/completion", + "commands/operator-sdk/cmd/generate", + "commands/operator-sdk/cmd/up", + "commands/operator-sdk/error", + "pkg/generator", + "pkg/k8sclient", + "pkg/util/k8sutil", + "version" + ] + revision = "ec322ba6585fe5b21abafc57e7ca16abe8b924d2" [[projects]] branch = "master" @@ -189,42 +194,6 @@ revision = "5f041e8faa004a95c88a202771f4cc3e991971e6" version = "v2.0.1" -[[projects]] - name = "github.com/prometheus/client_golang" - packages = [ - "prometheus", - "prometheus/promhttp" - ] - revision = "c5b7fccd204277076155f10851dad72b76a49317" - version = "v0.8.0" - -[[projects]] - branch = "master" - name = "github.com/prometheus/client_model" - packages = ["go"] - revision = "99fa1f4be8e564e8a6b613da7fa6f46c9edafc6c" - -[[projects]] - branch = "master" - name = "github.com/prometheus/common" - packages = [ - "expfmt", - "internal/bitbucket.org/ww/goautoneg", - "model" - ] - revision = "7600349dcfe1abd18d72d3a1770870d9800a7801" - -[[projects]] - branch = "master" - name = "github.com/prometheus/procfs" - packages = [ - ".", - "internal/util", - "nfs", - "xfs" - ] - revision = "7d6f385de8bea29190f15ba9931442a0eaef9af7" - [[projects]] name = "github.com/sergi/go-diff" packages = ["diffmatchpatch"] @@ -458,6 +427,6 @@ [solve-meta] analyzer-name = "dep" analyzer-version = 1 - inputs-digest = "e39a3b50eecf50ee2f3c6ce8a36306abeea762a41fab1117f0c5e2a038b72fb4" + inputs-digest = "3c35ad715f8ab84af1963d1e25268707e5548ce00bea2dde7fe556fc3a7ce333" solver-name = "gps-cdcl" solver-version = 1 diff --git a/pkg/k8sclient/client.go b/pkg/k8sclient/client.go index 248597066c..6d2be07c7e 100644 --- a/pkg/k8sclient/client.go +++ b/pkg/k8sclient/client.go @@ -18,6 +18,7 @@ import ( "fmt" "net" "os" + "sync" "time" "github.com/operator-framework/operator-sdk/pkg/util/k8sutil" @@ -43,27 +44,30 @@ type resourceClientFactory struct { var ( // this stores the singleton in a package local singletonFactory *resourceClientFactory + once sync.Once ) -// GetResourceClient returns the resource client using a singleton factory -func GetResourceClient(apiVersion, kind, namespace string) (dynamic.ResourceInterface, string, error) { - if singletonFactory == nil { - kubeClient, kubeConfig := mustNewKubeClientAndConfig() - cachedDiscoveryClient := cached.NewMemCacheClient(kubeClient.Discovery()) - restMapper := discovery.NewDeferredDiscoveryRESTMapper(cachedDiscoveryClient, meta.InterfacesForUnstructured) - restMapper.Reset() - kubeConfig.ContentConfig = dynamic.ContentConfig() - clientPool := dynamic.NewClientPool(kubeConfig, restMapper, dynamic.LegacyAPIPathResolverFunc) - - singletonFactory := &resourceClientFactory{ - kubeClient: kubeClient, - kubeConfig: kubeConfig, - restMapper: restMapper, - clientPool: clientPool, - } - singletonFactory.runBackgroundCacheReset(1 * time.Minute) +// Private constructor for once.Do +func newSingletonFactory() { + kubeClient, kubeConfig := mustNewKubeClientAndConfig() + cachedDiscoveryClient := cached.NewMemCacheClient(kubeClient.Discovery()) + restMapper := discovery.NewDeferredDiscoveryRESTMapper(cachedDiscoveryClient, meta.InterfacesForUnstructured) + restMapper.Reset() + kubeConfig.ContentConfig = dynamic.ContentConfig() + clientPool := dynamic.NewClientPool(kubeConfig, restMapper, dynamic.LegacyAPIPathResolverFunc) + + singletonFactory = &resourceClientFactory{ + kubeClient: kubeClient, + kubeConfig: kubeConfig, + restMapper: restMapper, + clientPool: clientPool, } + singletonFactory.runBackgroundCacheReset(1 * time.Minute) +} +// GetResourceClient returns the resource client using a singleton factory +func GetResourceClient(apiVersion, kind, namespace string) (dynamic.ResourceInterface, string, error) { + once.Do(newSingletonFactory) return singletonFactory.GetResourceClient(apiVersion, kind, namespace) } From 0c6fc978f89a8ed353640286669555e05ad6e1b4 Mon Sep 17 00:00:00 2001 From: Daniel Feinberg Date: Mon, 9 Jul 2018 14:44:42 -0600 Subject: [PATCH 3/3] Code Review fixes for public methods --- Gopkg.lock | 63 ++++++++++++++++++++++++++++++----------- pkg/k8sclient/client.go | 11 +++---- 2 files changed, 53 insertions(+), 21 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index 196f8eb7ed..a3c37e68cb 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -13,6 +13,12 @@ packages = ["."] revision = "de5bf2ad457846296e2031421a34e2568e304e35" +[[projects]] + branch = "master" + name = "github.com/beorn7/perks" + packages = ["quantile"] + revision = "3a771d992973f24aa725d07868b467d1ddfceafb" + [[projects]] name = "github.com/davecgh/go-spew" packages = ["spew"] @@ -166,21 +172,10 @@ revision = "32fa128f234d041f196a9f3e0fea5ac9772c08e1" [[projects]] - branch = "master" - name = "github.com/operator-framework/operator-sdk" - packages = [ - "commands/operator-sdk/cmd", - "commands/operator-sdk/cmd/cmdutil", - "commands/operator-sdk/cmd/completion", - "commands/operator-sdk/cmd/generate", - "commands/operator-sdk/cmd/up", - "commands/operator-sdk/error", - "pkg/generator", - "pkg/k8sclient", - "pkg/util/k8sutil", - "version" - ] - revision = "ec322ba6585fe5b21abafc57e7ca16abe8b924d2" + name = "github.com/matttproud/golang_protobuf_extensions" + packages = ["pbutil"] + revision = "c12348ce28de40eed0136aa2b644d0ee0650e56c" + version = "v1.0.1" [[projects]] branch = "master" @@ -194,6 +189,42 @@ revision = "5f041e8faa004a95c88a202771f4cc3e991971e6" version = "v2.0.1" +[[projects]] + name = "github.com/prometheus/client_golang" + packages = [ + "prometheus", + "prometheus/promhttp" + ] + revision = "c5b7fccd204277076155f10851dad72b76a49317" + version = "v0.8.0" + +[[projects]] + branch = "master" + name = "github.com/prometheus/client_model" + packages = ["go"] + revision = "99fa1f4be8e564e8a6b613da7fa6f46c9edafc6c" + +[[projects]] + branch = "master" + name = "github.com/prometheus/common" + packages = [ + "expfmt", + "internal/bitbucket.org/ww/goautoneg", + "model" + ] + revision = "7600349dcfe1abd18d72d3a1770870d9800a7801" + +[[projects]] + branch = "master" + name = "github.com/prometheus/procfs" + packages = [ + ".", + "internal/util", + "nfs", + "xfs" + ] + revision = "7d6f385de8bea29190f15ba9931442a0eaef9af7" + [[projects]] name = "github.com/sergi/go-diff" packages = ["diffmatchpatch"] @@ -427,6 +458,6 @@ [solve-meta] analyzer-name = "dep" analyzer-version = 1 - inputs-digest = "3c35ad715f8ab84af1963d1e25268707e5548ce00bea2dde7fe556fc3a7ce333" + inputs-digest = "e39a3b50eecf50ee2f3c6ce8a36306abeea762a41fab1117f0c5e2a038b72fb4" solver-name = "gps-cdcl" solver-version = 1 diff --git a/pkg/k8sclient/client.go b/pkg/k8sclient/client.go index 6d2be07c7e..84cf285949 100644 --- a/pkg/k8sclient/client.go +++ b/pkg/k8sclient/client.go @@ -71,6 +71,12 @@ func GetResourceClient(apiVersion, kind, namespace string) (dynamic.ResourceInte return singletonFactory.GetResourceClient(apiVersion, kind, namespace) } +// GetKubeClient returns the kubernetes client used to create the dynamic client +func GetKubeClient() kubernetes.Interface { + once.Do(newSingletonFactory) + return singletonFactory.kubeClient +} + // GetResourceClient returns the dynamic client and pluralName for the resource specified by the apiVersion and kind func (c *resourceClientFactory) GetResourceClient(apiVersion, kind, namespace string) (dynamic.ResourceInterface, string, error) { gv, err := schema.ParseGroupVersion(apiVersion) @@ -96,11 +102,6 @@ func (c *resourceClientFactory) GetResourceClient(apiVersion, kind, namespace st return resourceClient, pluralName, nil } -// GetKubeClient returns the kubernetes client used to create the dynamic client -func (c *resourceClientFactory) GetKubeClient() kubernetes.Interface { - return c.kubeClient -} - // apiResource consults the REST mapper to translate an tuple to a metav1.APIResource struct. func apiResource(gvk schema.GroupVersionKind, restMapper *discovery.DeferredDiscoveryRESTMapper) (*metav1.APIResource, error) { mapping, err := restMapper.RESTMapping(gvk.GroupKind(), gvk.Version)