Make sure cache Reader still works

The cache refactor broke the cache reader implementation (invalid
assumptions about names, arguments, and the way reflection works).
This fixes it, and adds back in some working tests to ensure that
it doesn't break again.

Author: DirectXMan12

.gitignore

@@ -11,4 +11,8 @@
 # Output of the go coverage tool, specifically when used with LiteIDE
 *.out
 
+# editor and IDE paraphernalia
 .idea
+*.swp
+*.swo
+*~

pkg/cache/cache.go

@@ -55,7 +55,7 @@ type Informers interface {
 	GetInformerForKind(gvk schema.GroupVersionKind) (toolscache.SharedIndexInformer, error)
 
 	// Start runs all the informers known to this cache until the given channel is closed.
-	// It does not block.
+	// It blocks.
 	Start(stopCh <-chan struct{}) error
 
 	// WaitForCacheSync waits for all the caches to sync.  Returns false if it could not sync a cache.

pkg/cache/cache_test.go

@@ -14,12 +14,83 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package cache
+package cache_test
 
 import (
+	"context"
+
 	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+	kcorev1 "k8s.io/api/core/v1"
+
+	"sigs.k8s.io/controller-runtime/pkg/cache"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
+var _ = Describe("Informer Cache", func() {
+	var stop chan struct{}
+
+	BeforeEach(func() {
+		stop = make(chan struct{})
+		Expect(cfg).NotTo(BeNil())
+	})
+	AfterEach(func() {
+		close(stop)
+	})
+
+	Describe("as a Reader", func() {
+		It("should be able to list objects that haven't been watched previously", func() {
+			By("Creating the cache")
+			reader, err := cache.New(cfg, cache.Options{})
+			Expect(err).NotTo(HaveOccurred())
+
+			By("running the cache and waiting for it to sync")
+			go func() {
+				defer GinkgoRecover()
+				Expect(reader.Start(stop)).ToNot(HaveOccurred())
+			}()
+			Expect(reader.WaitForCacheSync(stop)).NotTo(BeFalse())
+
+			By("Listing all services in the cluster")
+			listObj := &kcorev1.ServiceList{}
+			Expect(reader.List(context.Background(), nil, listObj)).NotTo(HaveOccurred())
+
+			By("Verifying that the returned list contains the Kubernetes service")
+			// NB: there has to be at least the kubernetes service in the cluster
+			Expect(listObj.Items).NotTo(BeEmpty())
+			hasKubeService := false
+			for _, svc := range listObj.Items {
+				if svc.Namespace == "default" && svc.Name == "kubernetes" {
+					hasKubeService = true
+					break
+				}
+			}
+			Expect(hasKubeService).To(BeTrue())
+		})
+
+		It("should be able to get objects that haven't been watched previously", func() {
+			By("Creating the cache")
+			reader, err := cache.New(cfg, cache.Options{})
+			Expect(err).NotTo(HaveOccurred())
+
+			By("running the cache and waiting for it to sync")
+			go func() {
+				defer GinkgoRecover()
+				Expect(reader.Start(stop)).ToNot(HaveOccurred())
+			}()
+			Expect(reader.WaitForCacheSync(stop)).NotTo(BeFalse())
+
+			By("Getting the Kubernetes service")
+			svc := &kcorev1.Service{}
+			Expect(reader.Get(context.Background(), client.ObjectKey{Namespace: "default", Name: "kubernetes"}, svc)).NotTo(HaveOccurred())
+
+			By("Verifying that the returned service looks reasonable")
+			Expect(svc.Name).To(Equal("kubernetes"))
+			Expect(svc.Namespace).To(Equal("default"))
+		})
+	})
+})
+
 var _ = Describe("Indexers", func() {
 	//three := int64(3)
 	//knownPodKey := client.ObjectKey{Name: "some-pod", Namespace: "some-ns"}

pkg/cache/informer_cache.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"fmt"
 	"reflect"
+	"strings"
 
 	apimeta "k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -30,9 +31,11 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
 )
 
-var _ Informers = &informerCache{}
-var _ client.Reader = &informerCache{}
-var _ Cache = &informerCache{}
+var (
+	_ Informers = &informerCache{}
+	_ client.Reader = &informerCache{}
+	_ Cache = &informerCache{}
+)
 
 // informerCache is a Kubernetes Object cache populated from InformersMap.  cache wraps an InformersMap.
 type informerCache struct {
@@ -60,19 +63,26 @@ func (ip *informerCache) List(ctx context.Context, opts *client.ListOptions, out
 		return nil
 	}
 
-	// http://knowyourmeme.com/memes/this-is-fine
-	outType := reflect.Indirect(reflect.ValueOf(itemsPtr)).Type().Elem()
-	cacheType, ok := outType.(runtime.Object)
-	if !ok {
-		return fmt.Errorf("cannot get cache for %T, its element is not a runtime.Object", out)
-	}
-
 	gvk, err := apiutil.GVKForObject(out, ip.Scheme)
 	if err != nil {
 		return err
 	}
 
-	cache, err := ip.InformersMap.Get(gvk, cacheType)
+	if !strings.HasSuffix(gvk.Kind, "List") {
+		return fmt.Errorf("non-list type %T (kind %q) passed as output", out, gvk)
+	}
+	// we need the non-list GVK, so chop off the "List" from the end of the kind
+	gvk.Kind = gvk.Kind[:len(gvk.Kind)-4]
+
+	// http://knowyourmeme.com/memes/this-is-fine
+	elemType := reflect.Indirect(reflect.ValueOf(itemsPtr)).Type().Elem()
+	cacheTypeValue := reflect.Zero(reflect.PtrTo(elemType))
+	cacheTypeObj, ok := cacheTypeValue.Interface().(runtime.Object)
+	if !ok {
+		return fmt.Errorf("cannot get cache for %T, its element %T is not a runtime.Object", out, cacheTypeValue.Interface())
+	}
+
+	cache, err := ip.InformersMap.Get(gvk, cacheTypeObj)
 	if err != nil {
 		return err
 	}