Skip to content

Commit

Permalink
Switch to interface-based options
Browse files Browse the repository at this point in the history
These behave more-or-less like functional options, except that a given
option can be applied to more than on option type, so we can deprecate
the 3 different dryrunall variants.

Things with arguments generally become type aliases, with the exception
of MatchingField, which gets deprecated in favor of MatchFields (a type
alias to fields.Set).
  • Loading branch information
DirectXMan12 committed Jul 24, 2019
1 parent 72ab3fe commit 92743c5
Show file tree
Hide file tree
Showing 13 changed files with 230 additions and 277 deletions.
2 changes: 1 addition & 1 deletion pkg/cache/informer_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,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.ListOptionFunc) error {
func (ip *informerCache) List(ctx context.Context, out runtime.Object, opts ...client.ListOption) error {
gvk, err := apiutil.GVKForObject(out, ip.Scheme)
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion pkg/cache/informertest/fake_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,6 @@ func (c *FakeInformers) Get(ctx context.Context, key client.ObjectKey, obj runti
}

// List implements Cache
func (c *FakeInformers) List(ctx context.Context, list runtime.Object, opts ...client.ListOptionFunc) error {
func (c *FakeInformers) List(ctx context.Context, list runtime.Object, opts ...client.ListOption) error {
return nil
}
2 changes: 1 addition & 1 deletion pkg/cache/internal/cache_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,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.ListOptionFunc) error {
func (c *CacheReader) List(_ context.Context, out runtime.Object, opts ...client.ListOption) error {
var objs []interface{}
var err error

Expand Down
2 changes: 1 addition & 1 deletion pkg/cache/multi_namespace_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,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.ListOptionFunc) error {
func (c *multiNamespaceCache) List(ctx context.Context, list runtime.Object, opts ...client.ListOption) error {
listOpts := client.ListOptions{}
listOpts.ApplyOptions(opts)
if listOpts.Namespace != corev1.NamespaceAll {
Expand Down
14 changes: 7 additions & 7 deletions pkg/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ type client struct {
}

// Create implements client.Client
func (c *client) Create(ctx context.Context, obj runtime.Object, opts ...CreateOptionFunc) error {
func (c *client) Create(ctx context.Context, obj runtime.Object, opts ...CreateOption) error {
_, ok := obj.(*unstructured.Unstructured)
if ok {
return c.unstructuredClient.Create(ctx, obj, opts...)
Expand All @@ -113,7 +113,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 ...UpdateOptionFunc) error {
func (c *client) Update(ctx context.Context, obj runtime.Object, opts ...UpdateOption) error {
_, ok := obj.(*unstructured.Unstructured)
if ok {
return c.unstructuredClient.Update(ctx, obj, opts...)
Expand All @@ -122,7 +122,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 ...DeleteOptionFunc) error {
func (c *client) Delete(ctx context.Context, obj runtime.Object, opts ...DeleteOption) error {
_, ok := obj.(*unstructured.Unstructured)
if ok {
return c.unstructuredClient.Delete(ctx, obj, opts...)
Expand All @@ -131,7 +131,7 @@ func (c *client) Delete(ctx context.Context, obj runtime.Object, opts ...DeleteO
}

// Patch implements client.Client
func (c *client) Patch(ctx context.Context, obj runtime.Object, patch Patch, opts ...PatchOptionFunc) error {
func (c *client) Patch(ctx context.Context, obj runtime.Object, patch Patch, opts ...PatchOption) error {
_, ok := obj.(*unstructured.Unstructured)
if ok {
return c.unstructuredClient.Patch(ctx, obj, patch, opts...)
Expand All @@ -149,7 +149,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 ...ListOptionFunc) error {
func (c *client) List(ctx context.Context, obj runtime.Object, opts ...ListOption) error {
_, ok := obj.(*unstructured.UnstructuredList)
if ok {
return c.unstructuredClient.List(ctx, obj, opts...)
Expand All @@ -171,7 +171,7 @@ type statusWriter struct {
var _ StatusWriter = &statusWriter{}

// Update implements client.StatusWriter
func (sw *statusWriter) Update(ctx context.Context, obj runtime.Object, opts ...UpdateOptionFunc) error {
func (sw *statusWriter) Update(ctx context.Context, obj runtime.Object, opts ...UpdateOption) error {
_, ok := obj.(*unstructured.Unstructured)
if ok {
return sw.client.unstructuredClient.UpdateStatus(ctx, obj, opts...)
Expand All @@ -180,7 +180,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 ...PatchOptionFunc) error {
func (sw *statusWriter) Patch(ctx context.Context, obj runtime.Object, patch Patch, opts ...PatchOption) error {
_, ok := obj.(*unstructured.Unstructured)
if ok {
return sw.client.unstructuredClient.PatchStatus(ctx, obj, patch, opts...)
Expand Down
92 changes: 24 additions & 68 deletions pkg/client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2000,7 +2000,7 @@ var _ = Describe("Client", func() {
Describe("CreateOptions", func() {
It("should allow setting DryRun to 'all'", func() {
co := &client.CreateOptions{}
client.CreateDryRunAll(co)
client.DryRunAll.ApplyToCreate(co)
all := []string{metav1.DryRunAll}
Expect(co.AsCreateOptions().DryRun).To(Equal(all))
})
Expand All @@ -2016,22 +2016,22 @@ var _ = Describe("Client", func() {
Describe("DeleteOptions", func() {
It("should allow setting GracePeriodSeconds", func() {
do := &client.DeleteOptions{}
client.GracePeriodSeconds(1)(do)
client.GracePeriodSeconds(1).ApplyToDelete(do)
gp := int64(1)
Expect(do.AsDeleteOptions().GracePeriodSeconds).To(Equal(&gp))
})

It("should allow setting Precondition", func() {
do := &client.DeleteOptions{}
pc := metav1.NewUIDPreconditions("uid")
client.Preconditions(pc)(do)
client.Preconditions(*pc).ApplyToDelete(do)
Expect(do.AsDeleteOptions().Preconditions).To(Equal(pc))
Expect(do.Preconditions).To(Equal(pc))
})

It("should allow setting PropagationPolicy", func() {
do := &client.DeleteOptions{}
client.PropagationPolicy(metav1.DeletePropagationForeground)(do)
client.PropagationPolicy(metav1.DeletePropagationForeground).ApplyToDelete(do)
dp := metav1.DeletePropagationForeground
Expect(do.AsDeleteOptions().PropagationPolicy).To(Equal(&dp))
})
Expand All @@ -2048,9 +2048,9 @@ var _ = Describe("Client", func() {
pc := metav1.NewUIDPreconditions("uid")
dp := metav1.DeletePropagationForeground
do := &client.DeleteOptions{}
do.ApplyOptions([]client.DeleteOptionFunc{
do.ApplyOptions([]client.DeleteOption{
client.GracePeriodSeconds(gp),
client.Preconditions(pc),
client.Preconditions(*pc),
client.PropagationPolicy(dp),
})
Expect(do.GracePeriodSeconds).To(Equal(&gp))
Expand All @@ -2060,79 +2060,35 @@ var _ = Describe("Client", func() {
})

Describe("ListOptions", func() {
It("should be able to set a LabelSelector", func() {
lo := &client.ListOptions{}
err := lo.SetLabelSelector("foo=bar")
Expect(err).NotTo(HaveOccurred())
Expect(lo.LabelSelector.String()).To(Equal("foo=bar"))
})

It("should be able to set a FieldSelector", func() {
lo := &client.ListOptions{}
err := lo.SetFieldSelector("field1=bar")
Expect(err).NotTo(HaveOccurred())
Expect(lo.FieldSelector.String()).To(Equal("field1=bar"))
})

It("should be converted to metav1.ListOptions", func() {
lo := &client.ListOptions{}
labels := map[string]string{"foo": "bar"}
mlo := lo.MatchingLabels(labels).
MatchingField("field1", "bar").
InNamespace("test-namespace").
AsListOptions()
It("should be convertable to metav1.ListOptions", func() {
lo := (&client.ListOptions{}).ApplyOptions([]client.ListOption{
client.MatchingField("field1", "bar"),
client.InNamespace("test-namespace"),
client.MatchingLabels{"foo": "bar"},
})
mlo := lo.AsListOptions()
Expect(mlo).NotTo(BeNil())
Expect(mlo.LabelSelector).To(Equal("foo=bar"))
Expect(mlo.FieldSelector).To(Equal("field1=bar"))
})

It("should be able to set MatchingLabels", func() {
lo := &client.ListOptions{}
Expect(lo.LabelSelector).To(BeNil())
labels := map[string]string{"foo": "bar"}
lo = lo.MatchingLabels(labels)
Expect(lo.LabelSelector.String()).To(Equal("foo=bar"))
})

It("should be able to set MatchingField", func() {
lo := &client.ListOptions{}
Expect(lo.FieldSelector).To(BeNil())
lo = lo.MatchingField("field1", "bar")
Expect(lo.FieldSelector.String()).To(Equal("field1=bar"))
})

It("should be able to set InNamespace", func() {
lo := &client.ListOptions{}
lo = lo.InNamespace("test-namespace")
Expect(lo.Namespace).To(Equal("test-namespace"))
})

It("should be created from MatchingLabels", func() {
labels := map[string]string{"foo": "bar"}
It("should be populated by MatchingLabels", func() {
lo := &client.ListOptions{}
client.MatchingLabels(labels)(lo)
client.MatchingLabels{"foo": "bar"}.ApplyToList(lo)
Expect(lo).NotTo(BeNil())
Expect(lo.LabelSelector.String()).To(Equal("foo=bar"))
})

It("should be created from MatchingField", func() {
It("should be populated by MatchingField", func() {
lo := &client.ListOptions{}
client.MatchingField("field1", "bar")(lo)
client.MatchingField("field1", "bar").ApplyToList(lo)
Expect(lo).NotTo(BeNil())
Expect(lo.FieldSelector.String()).To(Equal("field1=bar"))
})

It("should be created from InNamespace", func() {
lo := &client.ListOptions{}
client.InNamespace("test")(lo)
Expect(lo).NotTo(BeNil())
Expect(lo.Namespace).To(Equal("test"))
})

It("should allow pre-built ListOptions", func() {
It("should be populated by InNamespace", func() {
lo := &client.ListOptions{}
newLo := &client.ListOptions{}
client.UseListOptions(newLo.InNamespace("test"))(lo)
client.InNamespace("test").ApplyToList(lo)
Expect(lo).NotTo(BeNil())
Expect(lo.Namespace).To(Equal("test"))
})
Expand All @@ -2141,7 +2097,7 @@ var _ = Describe("Client", func() {
Describe("UpdateOptions", func() {
It("should allow setting DryRun to 'all'", func() {
uo := &client.UpdateOptions{}
client.UpdateDryRunAll(uo)
client.DryRunAll.ApplyToUpdate(uo)
all := []string{metav1.DryRunAll}
Expect(uo.AsUpdateOptions().DryRun).To(Equal(all))
})
Expand All @@ -2157,22 +2113,22 @@ var _ = Describe("Client", func() {
Describe("PatchOptions", func() {
It("should allow setting DryRun to 'all'", func() {
po := &client.PatchOptions{}
client.PatchDryRunAll(po)
client.DryRunAll.ApplyToPatch(po)
all := []string{metav1.DryRunAll}
Expect(po.AsPatchOptions().DryRun).To(Equal(all))
})

It("should allow setting Force to 'true'", func() {
po := &client.PatchOptions{}
client.ForceOwnership(po)
client.ForceOwnership.ApplyToPatch(po)
mpo := po.AsPatchOptions()
Expect(mpo.Force).NotTo(BeNil())
Expect(*mpo.Force).To(BeTrue())
})

It("should allow setting the field manager", func() {
po := &client.PatchOptions{}
client.FieldOwner("some-owner")(po)
client.FieldOwner("some-owner").ApplyToPatch(po)
Expect(po.AsPatchOptions().FieldManager).To(Equal("some-owner"))
})

Expand Down Expand Up @@ -2320,7 +2276,7 @@ func (f *fakeReader) Get(ctx context.Context, key client.ObjectKey, obj runtime.
return nil
}

func (f *fakeReader) List(ctx context.Context, list runtime.Object, opts ...client.ListOptionFunc) error {
func (f *fakeReader) List(ctx context.Context, list runtime.Object, opts ...client.ListOption) error {
f.Called = f.Called + 1
return nil
}
2 changes: 1 addition & 1 deletion pkg/client/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ limitations under the License.
// Many client operations in Kubernetes support options. These options are
// represented as variadic arguments at the end of a given method call.
// For instance, to use a label selector on list, you can call
// err := someReader.List(context.Background(), &podList, client.MatchingLabels(someLabelMap))
// err := someReader.List(context.Background(), &podList, client.MatchingLabels{"somelabel": "someval"})
//
// Indexing
//
Expand Down
14 changes: 7 additions & 7 deletions pkg/client/fake/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func (c *fakeClient) Get(ctx context.Context, key client.ObjectKey, obj runtime.
return err
}

func (c *fakeClient) List(ctx context.Context, obj runtime.Object, opts ...client.ListOptionFunc) error {
func (c *fakeClient) List(ctx context.Context, obj runtime.Object, opts ...client.ListOption) error {
gvk, err := apiutil.GVKForObject(obj, c.scheme)
if err != nil {
return err
Expand Down Expand Up @@ -131,7 +131,7 @@ func (c *fakeClient) List(ctx context.Context, obj runtime.Object, opts ...clien
return nil
}

func (c *fakeClient) Create(ctx context.Context, obj runtime.Object, opts ...client.CreateOptionFunc) error {
func (c *fakeClient) Create(ctx context.Context, obj runtime.Object, opts ...client.CreateOption) error {
createOptions := &client.CreateOptions{}
createOptions.ApplyOptions(opts)

Expand All @@ -152,7 +152,7 @@ func (c *fakeClient) Create(ctx context.Context, obj runtime.Object, opts ...cli
return c.tracker.Create(gvr, obj, accessor.GetNamespace())
}

func (c *fakeClient) Delete(ctx context.Context, obj runtime.Object, opts ...client.DeleteOptionFunc) error {
func (c *fakeClient) Delete(ctx context.Context, obj runtime.Object, opts ...client.DeleteOption) error {
gvr, err := getGVRFromObject(obj, c.scheme)
if err != nil {
return err
Expand All @@ -165,7 +165,7 @@ func (c *fakeClient) Delete(ctx context.Context, obj runtime.Object, opts ...cli
return c.tracker.Delete(gvr, accessor.GetNamespace(), accessor.GetName())
}

func (c *fakeClient) Update(ctx context.Context, obj runtime.Object, opts ...client.UpdateOptionFunc) error {
func (c *fakeClient) Update(ctx context.Context, obj runtime.Object, opts ...client.UpdateOption) error {
updateOptions := &client.UpdateOptions{}
updateOptions.ApplyOptions(opts)

Expand All @@ -186,7 +186,7 @@ func (c *fakeClient) Update(ctx context.Context, obj runtime.Object, opts ...cli
return c.tracker.Update(gvr, obj, accessor.GetNamespace())
}

func (c *fakeClient) Patch(ctx context.Context, obj runtime.Object, patch client.Patch, opts ...client.PatchOptionFunc) error {
func (c *fakeClient) Patch(ctx context.Context, obj runtime.Object, patch client.Patch, opts ...client.PatchOption) error {
patchOptions := &client.PatchOptions{}
patchOptions.ApplyOptions(opts)

Expand Down Expand Up @@ -244,13 +244,13 @@ type fakeStatusWriter struct {
client *fakeClient
}

func (sw *fakeStatusWriter) Update(ctx context.Context, obj runtime.Object, opts ...client.UpdateOptionFunc) error {
func (sw *fakeStatusWriter) Update(ctx context.Context, obj runtime.Object, opts ...client.UpdateOption) error {
// TODO(droot): This results in full update of the obj (spec + status). Need
// a way to update status field only.
return sw.client.Update(ctx, obj, opts...)
}

func (sw *fakeStatusWriter) Patch(ctx context.Context, obj runtime.Object, patch client.Patch, opts ...client.PatchOptionFunc) error {
func (sw *fakeStatusWriter) Patch(ctx context.Context, obj runtime.Object, patch client.Patch, opts ...client.PatchOption) error {
// TODO(droot): This results in full update of the obj (spec + status). Need
// a way to update status field only.
return sw.client.Patch(ctx, obj, patch, opts...)
Expand Down
14 changes: 7 additions & 7 deletions pkg/client/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,24 +58,24 @@ type Reader interface {
// List retrieves list of objects for a given namespace and list options. On a
// successful call, Items field in the list will be populated with the
// result returned from the server.
List(ctx context.Context, list runtime.Object, opts ...ListOptionFunc) error
List(ctx context.Context, list runtime.Object, opts ...ListOption) error
}

// Writer knows how to create, delete, and update Kubernetes objects.
type Writer interface {
// Create saves the object obj in the Kubernetes cluster.
Create(ctx context.Context, obj runtime.Object, opts ...CreateOptionFunc) error
Create(ctx context.Context, obj runtime.Object, opts ...CreateOption) error

// Delete deletes the given obj from Kubernetes cluster.
Delete(ctx context.Context, obj runtime.Object, opts ...DeleteOptionFunc) error
Delete(ctx context.Context, obj runtime.Object, opts ...DeleteOption) error

// Update updates the given obj in the Kubernetes cluster. obj must be a
// struct pointer so that obj can be updated with the content returned by the Server.
Update(ctx context.Context, obj runtime.Object, opts ...UpdateOptionFunc) error
Update(ctx context.Context, obj runtime.Object, opts ...UpdateOption) error

// Patch patches the given obj in the Kubernetes cluster. obj must be a
// struct pointer so that obj can be updated with the content returned by the Server.
Patch(ctx context.Context, obj runtime.Object, patch Patch, opts ...PatchOptionFunc) error
Patch(ctx context.Context, obj runtime.Object, patch Patch, opts ...PatchOption) error
}

// StatusClient knows how to create a client which can update status subresource
Expand All @@ -89,12 +89,12 @@ type StatusWriter interface {
// Update updates the fields corresponding to the status subresource for the
// given obj. obj must be a struct pointer so that obj can be updated
// with the content returned by the Server.
Update(ctx context.Context, obj runtime.Object, opts ...UpdateOptionFunc) error
Update(ctx context.Context, obj runtime.Object, opts ...UpdateOption) error

// Patch patches the given object's subresource. obj must be a struct
// pointer so that obj can be updated with the content returned by the
// Server.
Patch(ctx context.Context, obj runtime.Object, patch Patch, opts ...PatchOptionFunc) error
Patch(ctx context.Context, obj runtime.Object, patch Patch, opts ...PatchOption) error
}

// Client knows how to perform CRUD operations on Kubernetes objects.
Expand Down
Loading

0 comments on commit 92743c5

Please sign in to comment.