Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

⚠️ Functional Options Redux #536

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
111 changes: 43 additions & 68 deletions pkg/client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2000,11 +2000,17 @@ 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))
})

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

It("should produce empty metav1.CreateOptions if nil", func() {
var co *client.CreateOptions
Expect(co.AsCreateOptions()).To(Equal(&metav1.CreateOptions{}))
Expand All @@ -2016,26 +2022,33 @@ 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))
})

It("should allow setting DryRun", func() {
do := &client.DeleteOptions{}
client.DryRunAll.ApplyToDelete(do)
all := []string{metav1.DryRunAll}
Expect(do.AsDeleteOptions().DryRun).To(Equal(all))
})

It("should produce empty metav1.DeleteOptions if nil", func() {
var do *client.DeleteOptions
Expect(do.AsDeleteOptions()).To(Equal(&metav1.DeleteOptions{}))
Expand All @@ -2048,9 +2061,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 +2073,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() {
It("should be populated by 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"}
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() {
It("should be populated by 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() {
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,11 +2110,17 @@ 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))
})

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

It("should produce empty metav1.UpdateOptions if nil", func() {
var co *client.UpdateOptions
Expect(co.AsUpdateOptions()).To(Equal(&metav1.UpdateOptions{}))
Expand All @@ -2157,22 +2132,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 +2295,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
Loading