Skip to content

Commit

Permalink
⚠️ Introduce and use client.Object and client.ObjectList
Browse files Browse the repository at this point in the history 
This change introduces two interfaces: client.Object and
client.ObjectList.

These interfaces are now used throughout the codebase, more specifically
in the client.

Signed-off-by: Vince Prignano <vincepri@vmware.com>
  • Loading branch information
@vincepri
vincepri committed Oct 1, 2020
1 parent 933bd00 commit af35a4f
Show file tree
Hide file tree
Showing 27 changed files with 155 additions and 123 deletions.
10 changes: 5 additions & 5 deletions pkg/builder/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ import (
"strings"

"github.com/go-logr/logr"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/client-go/rest"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/handler"
Expand Down Expand Up @@ -57,7 +57,7 @@ func ControllerManagedBy(m manager.Manager) *Builder {

// ForInput represents the information set by For method.
type ForInput struct {
object runtime.Object
object client.Object
predicates []predicate.Predicate
err error
}
Expand All @@ -66,7 +66,7 @@ type ForInput struct {
// update events by *reconciling the object*.
// This is the equivalent of calling
// Watches(&source.Kind{Type: apiType}, &handler.EnqueueRequestForObject{})
func (blder *Builder) For(object runtime.Object, opts ...ForOption) *Builder {
func (blder *Builder) For(object client.Object, opts ...ForOption) *Builder {
if blder.forInput.object != nil {
blder.forInput.err = fmt.Errorf("For(...) should only be called once, could not assign multiple objects for reconciliation")
return blder
Expand All @@ -82,14 +82,14 @@ func (blder *Builder) For(object runtime.Object, opts ...ForOption) *Builder {

// OwnsInput represents the information set by Owns method.
type OwnsInput struct {
object runtime.Object
object client.Object
predicates []predicate.Predicate
}

// Owns defines types of Objects being *generated* by the ControllerManagedBy, and configures the ControllerManagedBy to respond to
// create / delete / update events by *reconciling the owner object*. This is the equivalent of calling
// Watches(&source.Kind{Type: <ForType-forInput>}, &handler.EnqueueRequestForOwner{OwnerType: apiType, IsController: true})
func (blder *Builder) Owns(object runtime.Object, opts ...OwnsOption) *Builder {
func (blder *Builder) Owns(object client.Object, opts ...OwnsOption) *Builder {
input := OwnsInput{object: object}
for _, opt := range opts {
opt.ApplyToOwns(&input)
Expand Down
5 changes: 4 additions & 1 deletion pkg/builder/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,10 @@ func doReconcileTest(nameSuffix string, stop chan struct{}, blder *Builder, mgr

var _ runtime.Object = &fakeType{}

type fakeType struct{}
type fakeType struct {
metav1.TypeMeta
metav1.ObjectMeta
}

func (*fakeType) GetObjectKind() schema.ObjectKind { return nil }
func (*fakeType) DeepCopyObject() runtime.Object { return nil }
4 changes: 4 additions & 0 deletions pkg/builder/webhook_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"sigs.k8s.io/controller-runtime/pkg/controller"
Expand Down Expand Up @@ -465,6 +466,9 @@ func (v *TestValidator) ValidateDelete() error {
var _ runtime.Object = &TestDefaultValidator{}

type TestDefaultValidator struct {
metav1.TypeMeta
metav1.ObjectMeta

Replica int `json:"replica,omitempty"`
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/cache/cache.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ type Cache interface {
type Informers interface {
// GetInformer fetches or constructs an informer for the given object that corresponds to a single
// API kind and resource.
GetInformer(ctx context.Context, obj runtime.Object) (Informer, error)
GetInformer(ctx context.Context, obj client.Object) (Informer, error)

// GetInformerForKind is similar to GetInformer, except that it takes a group-version-kind, instead
// of the underlying object.
Expand Down
17 changes: 8 additions & 9 deletions pkg/cache/cache_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
"k8s.io/apimachinery/pkg/api/errors"
kmetav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
kscheme "k8s.io/client-go/kubernetes/scheme"
"k8s.io/client-go/rest"
Expand All @@ -42,7 +41,7 @@ const testNamespaceThree = "test-namespace-3"

// TODO(community): Pull these helper functions into testenv.
// Restart policy is included to allow indexing on that field.
func createPod(name, namespace string, restartPolicy kcorev1.RestartPolicy) runtime.Object {
func createPod(name, namespace string, restartPolicy kcorev1.RestartPolicy) client.Object {
three := int64(3)
pod := &kcorev1.Pod{
ObjectMeta: kmetav1.ObjectMeta{
Expand All @@ -65,7 +64,7 @@ func createPod(name, namespace string, restartPolicy kcorev1.RestartPolicy) runt
return pod
}

func deletePod(pod runtime.Object) {
func deletePod(pod client.Object) {
cl, err := client.New(cfg, client.Options{})
Expect(err).NotTo(HaveOccurred())
err = cl.Delete(context.Background(), pod)
Expand All @@ -86,10 +85,10 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca
informerCache cache.Cache
informerCacheCtx context.Context
informerCacheCancel context.CancelFunc
knownPod1 runtime.Object
knownPod2 runtime.Object
knownPod3 runtime.Object
knownPod4 runtime.Object
knownPod1 client.Object
knownPod2 client.Object
knownPod3 client.Object
knownPod4 client.Object
)

BeforeEach(func() {
Expand Down Expand Up @@ -566,7 +565,7 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca

By("indexing the restartPolicy field of the Pod object before starting")
pod := &kcorev1.Pod{}
indexFunc := func(obj runtime.Object) []string {
indexFunc := func(obj client.Object) []string {
return []string{string(obj.(*kcorev1.Pod).Spec.RestartPolicy)}
}
Expect(informer.IndexField(context.TODO(), pod, "spec.restartPolicy", indexFunc)).To(Succeed())
Expand Down Expand Up @@ -684,7 +683,7 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca
Version: "v1",
Kind: "Pod",
})
indexFunc := func(obj runtime.Object) []string {
indexFunc := func(obj client.Object) []string {
s, ok := obj.(*unstructured.Unstructured).Object["spec"]
if !ok {
return []string{}
Expand Down
12 changes: 6 additions & 6 deletions pkg/cache/informer_cache.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ type informerCache struct {
}

// Get implements Reader
func (ip *informerCache) Get(ctx context.Context, key client.ObjectKey, out runtime.Object) error {
func (ip *informerCache) Get(ctx context.Context, key client.ObjectKey, out client.Object) error {
gvk, err := apiutil.GVKForObject(out, ip.Scheme)
if err != nil {
return err
Expand All @@ -69,7 +69,7 @@ func (ip *informerCache) Get(ctx context.Context, key client.ObjectKey, out runt
}

// List implements Reader
func (ip *informerCache) List(ctx context.Context, out runtime.Object, opts ...client.ListOption) error {
func (ip *informerCache) List(ctx context.Context, out client.ObjectList, opts ...client.ListOption) error {

gvk, cacheTypeObj, err := ip.objectTypeForListObject(out)
if err != nil {
Expand All @@ -91,7 +91,7 @@ func (ip *informerCache) List(ctx context.Context, out runtime.Object, opts ...c
// objectTypeForListObject tries to find the runtime.Object and associated GVK
// for a single object corresponding to the passed-in list type. We need them
// because they are used as cache map key.
func (ip *informerCache) objectTypeForListObject(list runtime.Object) (*schema.GroupVersionKind, runtime.Object, error) {
func (ip *informerCache) objectTypeForListObject(list client.ObjectList) (*schema.GroupVersionKind, runtime.Object, error) {
gvk, err := apiutil.GVKForObject(list, ip.Scheme)
if err != nil {
return nil, nil, err
Expand Down Expand Up @@ -146,7 +146,7 @@ func (ip *informerCache) GetInformerForKind(ctx context.Context, gvk schema.Grou
}

// GetInformer returns the informer for the obj
func (ip *informerCache) GetInformer(ctx context.Context, obj runtime.Object) (Informer, error) {
func (ip *informerCache) GetInformer(ctx context.Context, obj client.Object) (Informer, error) {
gvk, err := apiutil.GVKForObject(obj, ip.Scheme)
if err != nil {
return nil, err
Expand All @@ -170,7 +170,7 @@ func (ip *informerCache) NeedLeaderElection() bool {
// 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 (ip *informerCache) IndexField(ctx context.Context, obj runtime.Object, field string, extractValue client.IndexerFunc) error {
func (ip *informerCache) IndexField(ctx context.Context, obj client.Object, field string, extractValue client.IndexerFunc) error {
informer, err := ip.GetInformer(ctx, obj)
if err != nil {
return err
Expand All @@ -181,7 +181,7 @@ func (ip *informerCache) IndexField(ctx context.Context, obj runtime.Object, fie
func indexByField(indexer Informer, field string, extractor client.IndexerFunc) error {
indexFunc := func(objRaw interface{}) ([]string, error) {
// TODO(directxman12): check if this is the correct type?
obj, isObj := objRaw.(runtime.Object)
obj, isObj := objRaw.(client.Object)
if !isObj {
return nil, fmt.Errorf("object of type %T is not an Object", objRaw)
}
Expand Down
6 changes: 0 additions & 6 deletions pkg/cache/informer_cache_unit_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,6 @@ var _ = Describe("ip.objectTypeForListObject", func() {
InformersMap: &internal.InformersMap{Scheme: scheme.Scheme},
}

It("should error on non-list types", func() {
_, _, err := ip.objectTypeForListObject(&corev1.Pod{})
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(Equal(`non-list type *v1.Pod (kind "/v1, Kind=Pod") passed as output`))
})

It("should find the object type for unstructured lists", func() {
unstructuredList := &unstructured.UnstructuredList{}
unstructuredList.SetAPIVersion("v1")
Expand Down
8 changes: 4 additions & 4 deletions pkg/cache/informertest/fake_cache.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func (c *FakeInformers) FakeInformerForKind(ctx context.Context, gvk schema.Grou
}

// GetInformer implements Informers
func (c *FakeInformers) GetInformer(ctx context.Context, obj runtime.Object) (cache.Informer, error) {
func (c *FakeInformers) GetInformer(ctx context.Context, obj client.Object) (cache.Informer, error) {
if c.Scheme == nil {
c.Scheme = scheme.Scheme
}
Expand Down Expand Up @@ -126,16 +126,16 @@ func (c *FakeInformers) Start(ctx context.Context) error {
}

// IndexField implements Cache
func (c *FakeInformers) IndexField(ctx context.Context, obj runtime.Object, field string, extractValue client.IndexerFunc) error {
func (c *FakeInformers) IndexField(ctx context.Context, obj client.Object, field string, extractValue client.IndexerFunc) error {
return nil
}

// Get implements Cache
func (c *FakeInformers) Get(ctx context.Context, key client.ObjectKey, obj runtime.Object) error {
func (c *FakeInformers) Get(ctx context.Context, key client.ObjectKey, obj client.Object) error {
return nil
}

// List implements Cache
func (c *FakeInformers) List(ctx context.Context, list runtime.Object, opts ...client.ListOption) error {
func (c *FakeInformers) List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error {
return nil
}
4 changes: 2 additions & 2 deletions pkg/cache/internal/cache_reader.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ type CacheReader struct {
}

// Get checks the indexer for the object and writes a copy of it if found
func (c *CacheReader) Get(_ context.Context, key client.ObjectKey, out runtime.Object) error {
func (c *CacheReader) Get(_ context.Context, key client.ObjectKey, out client.Object) error {
storeKey := objectKeyToStoreKey(key)

// Lookup the object from the indexer cache
Expand Down Expand Up @@ -87,7 +87,7 @@ func (c *CacheReader) Get(_ context.Context, key client.ObjectKey, out runtime.O
}

// List lists items out of the indexer and writes them to out
func (c *CacheReader) List(_ context.Context, out runtime.Object, opts ...client.ListOption) error {
func (c *CacheReader) List(_ context.Context, out client.ObjectList, opts ...client.ListOption) error {
var objs []interface{}
var err error

Expand Down
10 changes: 5 additions & 5 deletions pkg/cache/multi_namespace_cache.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ type multiNamespaceCache struct {
var _ Cache = &multiNamespaceCache{}

// Methods for multiNamespaceCache to conform to the Informers interface
func (c *multiNamespaceCache) GetInformer(ctx context.Context, obj runtime.Object) (Informer, error) {
func (c *multiNamespaceCache) GetInformer(ctx context.Context, obj client.Object) (Informer, error) {
informers := map[string]Informer{}
for ns, cache := range c.namespaceToCache {
informer, err := cache.GetInformer(ctx, obj)
Expand Down Expand Up @@ -117,7 +117,7 @@ func (c *multiNamespaceCache) WaitForCacheSync(ctx context.Context) bool {
return synced
}

func (c *multiNamespaceCache) IndexField(ctx context.Context, obj runtime.Object, field string, extractValue client.IndexerFunc) error {
func (c *multiNamespaceCache) IndexField(ctx context.Context, obj client.Object, field string, extractValue client.IndexerFunc) error {
for _, cache := range c.namespaceToCache {
if err := cache.IndexField(ctx, obj, field, extractValue); err != nil {
return err
Expand All @@ -126,7 +126,7 @@ func (c *multiNamespaceCache) IndexField(ctx context.Context, obj runtime.Object
return nil
}

func (c *multiNamespaceCache) Get(ctx context.Context, key client.ObjectKey, obj runtime.Object) error {
func (c *multiNamespaceCache) Get(ctx context.Context, key client.ObjectKey, obj client.Object) error {
cache, ok := c.namespaceToCache[key.Namespace]
if !ok {
return fmt.Errorf("unable to get: %v because of unknown namespace for the cache", key)
Expand All @@ -135,7 +135,7 @@ func (c *multiNamespaceCache) Get(ctx context.Context, key client.ObjectKey, obj
}

// List multi namespace cache will get all the objects in the namespaces that the cache is watching if asked for all namespaces.
func (c *multiNamespaceCache) List(ctx context.Context, list runtime.Object, opts ...client.ListOption) error {
func (c *multiNamespaceCache) List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error {
listOpts := client.ListOptions{}
listOpts.ApplyOptions(opts)
if listOpts.Namespace != corev1.NamespaceAll {
Expand All @@ -157,7 +157,7 @@ func (c *multiNamespaceCache) List(ctx context.Context, list runtime.Object, opt
}
var resourceVersion string
for _, cache := range c.namespaceToCache {
listObj := list.DeepCopyObject()
listObj := list.DeepCopyObject().(client.ObjectList)
err = cache.List(ctx, listObj, opts...)
if err != nil {
return err
Expand Down
18 changes: 9 additions & 9 deletions pkg/client/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func (c *client) RESTMapper() meta.RESTMapper {
}

// Create implements client.Client
func (c *client) Create(ctx context.Context, obj runtime.Object, opts ...CreateOption) error {
func (c *client) Create(ctx context.Context, obj Object, opts ...CreateOption) error {
_, ok := obj.(*unstructured.Unstructured)
if ok {
return c.unstructuredClient.Create(ctx, obj, opts...)
Expand All @@ -133,7 +133,7 @@ func (c *client) Create(ctx context.Context, obj runtime.Object, opts ...CreateO
}

// Update implements client.Client
func (c *client) Update(ctx context.Context, obj runtime.Object, opts ...UpdateOption) error {
func (c *client) Update(ctx context.Context, obj Object, opts ...UpdateOption) error {
defer c.resetGroupVersionKind(obj, obj.GetObjectKind().GroupVersionKind())
_, ok := obj.(*unstructured.Unstructured)
if ok {
Expand All @@ -143,7 +143,7 @@ func (c *client) Update(ctx context.Context, obj runtime.Object, opts ...UpdateO
}

// Delete implements client.Client
func (c *client) Delete(ctx context.Context, obj runtime.Object, opts ...DeleteOption) error {
func (c *client) Delete(ctx context.Context, obj Object, opts ...DeleteOption) error {
_, ok := obj.(*unstructured.Unstructured)
if ok {
return c.unstructuredClient.Delete(ctx, obj, opts...)
Expand All @@ -152,7 +152,7 @@ func (c *client) Delete(ctx context.Context, obj runtime.Object, opts ...DeleteO
}

// DeleteAllOf implements client.Client
func (c *client) DeleteAllOf(ctx context.Context, obj runtime.Object, opts ...DeleteAllOfOption) error {
func (c *client) DeleteAllOf(ctx context.Context, obj Object, opts ...DeleteAllOfOption) error {
_, ok := obj.(*unstructured.Unstructured)
if ok {
return c.unstructuredClient.DeleteAllOf(ctx, obj, opts...)
Expand All @@ -161,7 +161,7 @@ func (c *client) DeleteAllOf(ctx context.Context, obj runtime.Object, opts ...De
}

// Patch implements client.Client
func (c *client) Patch(ctx context.Context, obj runtime.Object, patch Patch, opts ...PatchOption) error {
func (c *client) Patch(ctx context.Context, obj Object, patch Patch, opts ...PatchOption) error {
defer c.resetGroupVersionKind(obj, obj.GetObjectKind().GroupVersionKind())
_, ok := obj.(*unstructured.Unstructured)
if ok {
Expand All @@ -171,7 +171,7 @@ func (c *client) Patch(ctx context.Context, obj runtime.Object, patch Patch, opt
}

// Get implements client.Client
func (c *client) Get(ctx context.Context, key ObjectKey, obj runtime.Object) error {
func (c *client) Get(ctx context.Context, key ObjectKey, obj Object) error {
_, ok := obj.(*unstructured.Unstructured)
if ok {
return c.unstructuredClient.Get(ctx, key, obj)
Expand All @@ -180,7 +180,7 @@ func (c *client) Get(ctx context.Context, key ObjectKey, obj runtime.Object) err
}

// List implements client.Client
func (c *client) List(ctx context.Context, obj runtime.Object, opts ...ListOption) error {
func (c *client) List(ctx context.Context, obj ObjectList, opts ...ListOption) error {
_, ok := obj.(*unstructured.UnstructuredList)
if ok {
return c.unstructuredClient.List(ctx, obj, opts...)
Expand All @@ -202,7 +202,7 @@ type statusWriter struct {
var _ StatusWriter = &statusWriter{}

// Update implements client.StatusWriter
func (sw *statusWriter) Update(ctx context.Context, obj runtime.Object, opts ...UpdateOption) error {
func (sw *statusWriter) Update(ctx context.Context, obj Object, opts ...UpdateOption) error {
defer sw.client.resetGroupVersionKind(obj, obj.GetObjectKind().GroupVersionKind())
_, ok := obj.(*unstructured.Unstructured)
if ok {
Expand All @@ -212,7 +212,7 @@ func (sw *statusWriter) Update(ctx context.Context, obj runtime.Object, opts ...
}

// Patch implements client.Client
func (sw *statusWriter) Patch(ctx context.Context, obj runtime.Object, patch Patch, opts ...PatchOption) error {
func (sw *statusWriter) Patch(ctx context.Context, obj Object, patch Patch, opts ...PatchOption) error {
defer sw.client.resetGroupVersionKind(obj, obj.GetObjectKind().GroupVersionKind())
_, ok := obj.(*unstructured.Unstructured)
if ok {
Expand Down
4 changes: 2 additions & 2 deletions pkg/client/client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2815,12 +2815,12 @@ type fakeReader struct {
Called int
}

func (f *fakeReader) Get(ctx context.Context, key client.ObjectKey, obj runtime.Object) error {
func (f *fakeReader) Get(ctx context.Context, key client.ObjectKey, obj client.Object) error {
f.Called = f.Called + 1
return nil
}

func (f *fakeReader) List(ctx context.Context, list runtime.Object, opts ...client.ListOption) error {
func (f *fakeReader) List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error {
f.Called = f.Called + 1
return nil
}

0 comments on commit af35a4f

Please sign in to comment.