Address some go vet issues

Author: pwittrock

pkg/client/cache.go

@@ -20,98 +20,115 @@ import (
 
 var log = logf.KBLog.WithName("object-cache")
 
-// ObjectCache is a ReadInterface
-var _ ReadInterface = &ObjectCache{}
+// objectCache is a ReadInterface
+var _ ReadInterface = &objectCache{}
 
-type ObjectCache struct {
-	cachesByType map[reflect.Type]*SingleObjectCache
+// objectCache is a Kubernetes Object cache populated from Informers
+type objectCache struct {
+	cachesByType map[reflect.Type]*singleObjectCache
 	scheme       *runtime.Scheme
 }
 
-func ObjectCacheFromInformers(informers map[schema.GroupVersionKind]cache.SharedIndexInformer, scheme *runtime.Scheme) *ObjectCache {
-	res := NewObjectCache(scheme)
+var _ Cache = &objectCache{}
+
+// Cache implements ReadInterface by reading objects from a cache populated by Informers
+type Cache interface {
+	ReadInterface
+	informer.Callback
+}
+
+// NewObjectCache returns a new objectCache populated from informers
+func NewObjectCache(
+	informers map[schema.GroupVersionKind]cache.SharedIndexInformer,
+	scheme *runtime.Scheme) Cache {
+	res := &objectCache{
+		cachesByType: make(map[reflect.Type]*singleObjectCache),
+		scheme:       scheme,
+	}
 	res.AddInformers(informers)
 	return res
 }
 
-func (o *ObjectCache) AddInformers(informers map[schema.GroupVersionKind]cache.SharedIndexInformer) {
+// AddInformers adds new informers to the objectCache
+func (o *objectCache) AddInformers(informers map[schema.GroupVersionKind]cache.SharedIndexInformer) {
+	if informers == nil {
+		return
+	}
 	for gvk, informer := range informers {
 		o.AddInformer(gvk, informer)
 	}
 }
 
-func (o *ObjectCache) Call(gvk schema.GroupVersionKind, c cache.SharedIndexInformer) {
+// Call implements the informer callback
+func (o *objectCache) Call(gvk schema.GroupVersionKind, c cache.SharedIndexInformer) {
 	o.AddInformer(gvk, c)
 }
 
-func (o *ObjectCache) AddInformer(gvk schema.GroupVersionKind, c cache.SharedIndexInformer) {
+// AddInformer adds an informer to the objectCache
+func (o *objectCache) AddInformer(gvk schema.GroupVersionKind, c cache.SharedIndexInformer) {
 	obj, err := o.scheme.New(gvk)
 	if err != nil {
-		log.Error(err, "could not register informer in ObjectCache for GVK", "GroupVersionKind", gvk)
+		log.Error(err, "could not register informer in objectCache for GVK", "GroupVersionKind", gvk)
 		return
 	}
-	if _, found := o.CacheFor(obj); found {
+	if _, found := o.cacheFor(obj); found {
 		return
 	}
-	o.RegisterCache(obj, gvk, c.GetIndexer())
-}
-
-func NewObjectCache(scheme *runtime.Scheme) *ObjectCache {
-	return &ObjectCache{
-		cachesByType: make(map[reflect.Type]*SingleObjectCache),
-		scheme:       scheme,
-	}
+	o.registerCache(obj, gvk, c.GetIndexer())
 }
 
-func (c *ObjectCache) RegisterCache(obj runtime.Object, gvk schema.GroupVersionKind, store cache.Indexer) {
+func (o *objectCache) registerCache(obj runtime.Object, gvk schema.GroupVersionKind, store cache.Indexer) {
 	objType := reflect.TypeOf(obj)
-	c.cachesByType[objType] = &SingleObjectCache{
+	o.cachesByType[objType] = &singleObjectCache{
 		Indexer:          store,
 		GroupVersionKind: gvk,
 	}
 }
 
-func (c *ObjectCache) CacheFor(obj runtime.Object) (*SingleObjectCache, bool) {
+func (o *objectCache) cacheFor(obj runtime.Object) (*singleObjectCache, bool) {
 	objType := reflect.TypeOf(obj)
-	cache, isKnown := c.cachesByType[objType]
+	cache, isKnown := o.cachesByType[objType]
 	return cache, isKnown
 }
 
-func (c *ObjectCache) Get(ctx context.Context, key ObjectKey, out runtime.Object) error {
-	cache, isKnown := c.CacheFor(out)
+// Get implements client.Interface
+func (o *objectCache) Get(ctx context.Context, key ObjectKey, out runtime.Object) error {
+	cache, isKnown := o.cacheFor(out)
 	if !isKnown {
 		return fmt.Errorf("no cache for objects of type %T, must have asked for an watch/informer first", out)
 	}
 	return cache.Get(ctx, key, out)
 }
 
-func (c *ObjectCache) List(ctx context.Context, opts *ListOptions, out runtime.Object) error {
+// List implements client.Interface
+func (o *objectCache) List(ctx context.Context, opts *ListOptions, out runtime.Object) error {
 	itemsPtr, err := apimeta.GetItemsPtr(out)
 	if err != nil {
 		return nil
 	}
 	// http://knowyourmeme.com/memes/this-is-fine
 	outType := reflect.Indirect(reflect.ValueOf(itemsPtr)).Type().Elem()
-	cache, isKnown := c.cachesByType[outType]
+	cache, isKnown := o.cachesByType[outType]
 	if !isKnown {
 		return fmt.Errorf("no cache for objects of type %T", out)
 	}
 	return cache.List(ctx, opts, out)
 }
 
-// SingleObjectCache is a ReadInterface
-var _ ReadInterface = &SingleObjectCache{}
+// singleObjectCache is a ReadInterface
+var _ ReadInterface = &singleObjectCache{}
 
-// SingleObjectCache is a ReadInterface that retrieves objects
+// singleObjectCache is a ReadInterface that retrieves objects
 // from a single local cache populated by a watch.
-type SingleObjectCache struct {
+type singleObjectCache struct {
 	// Indexer is the underlying indexer wrapped by this cache.
 	Indexer cache.Indexer
 	// GroupVersionKind is the group-version-kind of the resource.
 	GroupVersionKind schema.GroupVersionKind
 }
 
-func (c *SingleObjectCache) Get(_ context.Context, key ObjectKey, out runtime.Object) error {
+// Get implements client.Interface
+func (c *singleObjectCache) Get(_ context.Context, key ObjectKey, out runtime.Object) error {
 	storeKey := objectKeyToStoreKey(key)
 	obj, exists, err := c.Indexer.GetByKey(storeKey)
 	if err != nil {
@@ -143,7 +160,8 @@ func (c *SingleObjectCache) Get(_ context.Context, key ObjectKey, out runtime.Ob
 	return nil
 }
 
-func (c *SingleObjectCache) List(ctx context.Context, opts *ListOptions, out runtime.Object) error {
+// List implements client.Interface
+func (c *singleObjectCache) List(ctx context.Context, opts *ListOptions, out runtime.Object) error {
 	var objs []interface{}
 	var err error
 
@@ -196,29 +214,30 @@ func (c *SingleObjectCache) List(ctx context.Context, opts *ListOptions, out run
 }
 
 // TODO: Make an interface with this function that has an Informers as an object on the struct
-// that automatically calls InformerFor and passes in the Indexer into IndexByField
+// that automatically calls InformerFor and passes in the Indexer into indexByField
 
 // noNamespaceNamespace is used as the "namespace" when we want to list across all namespaces
 const allNamespacesNamespace = "__all_namespaces"
 
+// InformerFieldIndexer provides an in-memory index of Object fields
 type InformerFieldIndexer struct {
 	Informers informer.Informers
 }
 
+// IndexField adds an indexer to the underlying cache, using extraction function to get
+// value(s) from the given field.  This index can then be used by passing a field selector
+// to List. For one-to-one compatibility with "normal" field selectors, only return one value.
+// The values may be anything.  They will automatically be prefixed with the namespace of the
+// given object, if present.  The objects passed are guaranteed to be objects of the correct type.
 func (i *InformerFieldIndexer) IndexField(obj runtime.Object, field string, extractValue IndexerFunc) error {
 	informer, err := i.Informers.InformerFor(obj)
 	if err != nil {
 		return err
 	}
-	return IndexByField(informer.GetIndexer(), field, extractValue)
+	return indexByField(informer.GetIndexer(), field, extractValue)
 }
 
-// IndexByField adds an indexer to the underlying cache, using extraction function to get
-// value(s) from the given field.  This index can then be used by passing a field selector
-// to List. For one-to-one compatibility with "normal" field selectors, only return one value.
-// The values may be anything.  They will automatically be prefixed with the namespace of the
-// given object, if present.  The objects passed are guaranteed to be objects of the correct type.
-func IndexByField(indexer cache.Indexer, field string, extractor IndexerFunc) error {
+func indexByField(indexer cache.Indexer, field string, extractor IndexerFunc) error {
 	indexFunc := func(objRaw interface{}) ([]string, error) {
 		// TODO(directxman12): check if this is the correct type?
 		obj, isObj := objRaw.(runtime.Object)

pkg/client/cache_test.go

@@ -1,4 +1,4 @@
-package client_test
+package client
 
 import (
 	"context"
@@ -11,8 +11,8 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/tools/cache"
 
-	. "github.com/kubernetes-sigs/controller-runtime/pkg/client"
 	"k8s.io/client-go/kubernetes/scheme"
+	"reflect"
 )
 
 var _ = Describe("Indexers", func() {
@@ -60,33 +60,36 @@ var _ = Describe("Indexers", func() {
 			Namespace: knownVolumeKey.Namespace,
 		},
 	}
-	var multiCache *ObjectCache
+	var multiCache *objectCache
 
 	BeforeEach(func() {
-		multiCache = NewObjectCache(scheme.Scheme)
+		multiCache = &objectCache{
+			cachesByType: make(map[reflect.Type]*singleObjectCache),
+			scheme:       scheme.Scheme,
+		}
 		podIndexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{
 			cache.NamespaceIndex: cache.MetaNamespaceIndexFunc,
 		})
 		volumeIndexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{
 			cache.NamespaceIndex: cache.MetaNamespaceIndexFunc,
 		})
-		IndexByField(podIndexer, "spec.restartPolicy", func(obj runtime.Object) []string {
+		indexByField(podIndexer, "spec.restartPolicy", func(obj runtime.Object) []string {
 			return []string{string(obj.(*kapi.Pod).Spec.RestartPolicy)}
 		})
 		Expect(podIndexer.Add(knownPod)).NotTo(HaveOccurred())
 		Expect(podIndexer.Add(knownPod2)).NotTo(HaveOccurred())
 		Expect(podIndexer.Add(knownPod3)).NotTo(HaveOccurred())
 		Expect(volumeIndexer.Add(knownVolume)).NotTo(HaveOccurred())
-		multiCache.RegisterCache(&kapi.Pod{}, kapi.SchemeGroupVersion.WithKind("Pod"), podIndexer)
-		multiCache.RegisterCache(&kapi.PersistentVolume{}, kapi.SchemeGroupVersion.WithKind("PersistentVolume"), volumeIndexer)
+		multiCache.registerCache(&kapi.Pod{}, kapi.SchemeGroupVersion.WithKind("Pod"), podIndexer)
+		multiCache.registerCache(&kapi.PersistentVolume{}, kapi.SchemeGroupVersion.WithKind("PersistentVolume"), volumeIndexer)
 	})
 
 	Describe("client interface wrapper around an indexer", func() {
 		var singleCache ReadInterface
 
 		BeforeEach(func() {
 			var ok bool
-			singleCache, ok = multiCache.CacheFor(&kapi.Pod{})
+			singleCache, ok = multiCache.cacheFor(&kapi.Pod{})
 			Expect(ok).To(BeTrue())
 		})
 
@@ -162,11 +165,11 @@ var _ = Describe("Indexers", func() {
 		})
 
 		It("should be able to fetch single caches for known types", func() {
-			indexer, ok := multiCache.CacheFor(&kapi.Pod{})
+			indexer, ok := multiCache.cacheFor(&kapi.Pod{})
 			Expect(ok).To(BeTrue())
 			Expect(indexer).NotTo(BeNil())
 
-			_, ok2 := multiCache.CacheFor(&kapi.PersistentVolumeClaim{})
+			_, ok2 := multiCache.cacheFor(&kapi.PersistentVolumeClaim{})
 			Expect(ok2).To(BeFalse())
 		})
 	})

pkg/client/config/config.go

@@ -75,7 +75,7 @@ func GetConfig() (*rest.Config, error) {
 	return nil, fmt.Errorf("could not locate a kubeconfig")
 }
 
-// GetConfig creates a *rest.Config for talking to a Kubernetes apiserver.
+// GetConfigOrDie creates a *rest.Config for talking to a Kubernetes apiserver.
 // If --kubeconfig is set, will use the kubeconfig file at that location.  Otherwise will assume running
 // in cluster and use the cluster provided kubeconfig.
 func GetConfigOrDie() *rest.Config {
@@ -125,7 +125,7 @@ func GetKubernetesInformers() (informers.SharedInformerFactory, error) {
 	return informers.NewSharedInformerFactory(i, time.Minute*5), nil
 }
 
-// GetKubernetesInformers creates a informers.SharedInformerFactory for talking to a Kubernetes apiserver.
+// GetKubernetesInformersOrDie creates a informers.SharedInformerFactory for talking to a Kubernetes apiserver.
 // If --kubeconfig is set, will use the kubeconfig file at that location.  Otherwise will assume running
 // in cluster and use the cluster provided kubeconfig.
 //

pkg/client/config/doc.go

@@ -14,5 +14,5 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-// The config package contains libraries for initializing rest configs for talking to the Kubernetes API
+// Package config contains libraries for initializing rest configs for talking to the Kubernetes API
 package config

pkg/controller/controller_manager.go

@@ -228,7 +228,7 @@ func NewControllerManager(args ControllerManagerArgs) (ControllerManager, error)
 
 	// Inject a Read / Write client into all controllers
 	// TODO(directxman12): Figure out how to allow users to request a client without requesting a watch
-	objCache := client.ObjectCacheFromInformers(spi.KnownInformersByType(), cm.scheme)
+	objCache := client.NewObjectCache(spi.KnownInformersByType(), cm.scheme)
 	spi.Callbacks = append(spi.Callbacks, objCache)
 
 	mapper, err := apiutil.NewDiscoveryRESTMapper(cm.config)

pkg/doc.go

@@ -38,7 +38,7 @@ type Informers interface {
 
 var _ Informers = &SelfPopulatingInformers{}
 
-type InformerCallback interface {
+type Callback interface {
 	Call(gvk schema.GroupVersionKind, c cache.SharedIndexInformer)
 }
 
@@ -48,7 +48,7 @@ type SelfPopulatingInformers struct {
 	Config    *rest.Config
 	Scheme    *runtime.Scheme
 	Mapper    meta.RESTMapper
-	Callbacks []InformerCallback
+	Callbacks []Callback
 
 	once           sync.Once
 	mu             sync.Mutex