Merge pull request #39 from DirectXMan12/bug/cache-list-reflection

Make sure cache Reader still works

Author: k8s-ci-robot

.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
 	}