From 9c48adf62224282f80d44daefad6c09fbab4d280 Mon Sep 17 00:00:00 2001 From: Tim Ebert Date: Thu, 20 Feb 2025 21:01:03 +0100 Subject: [PATCH 1/8] Drop `client.DeduplicateWarnings` in favor of controller-runtime `client.New` configures `logf.NewKubeAPIWarningLogger` on the client by default. --- cmd/sharder/main.go | 3 --- pkg/utils/client/warnings.go | 33 --------------------------------- 2 files changed, 36 deletions(-) delete mode 100644 pkg/utils/client/warnings.go diff --git a/cmd/sharder/main.go b/cmd/sharder/main.go index 4bdce1ff..c6d59fc5 100644 --- a/cmd/sharder/main.go +++ b/cmd/sharder/main.go @@ -23,12 +23,9 @@ import ( "sigs.k8s.io/controller-runtime/pkg/manager/signals" "github.com/timebertt/kubernetes-controller-sharding/cmd/sharder/app" - "github.com/timebertt/kubernetes-controller-sharding/pkg/utils/client" ) func main() { - client.DeduplicateWarnings() - if err := app.NewCommand().ExecuteContext(signals.SetupSignalHandler()); err != nil { fmt.Println(err) os.Exit(1) diff --git a/pkg/utils/client/warnings.go b/pkg/utils/client/warnings.go deleted file mode 100644 index 4990aa48..00000000 --- a/pkg/utils/client/warnings.go +++ /dev/null @@ -1,33 +0,0 @@ -/* -Copyright 2023 Tim Ebert. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package client - -import ( - "os" - - "k8s.io/client-go/rest" -) - -// DeduplicateWarnings configures a client-go warning handler that deduplicates API warnings in order to not spam logs. -func DeduplicateWarnings() { - rest.SetDefaultWarningHandler( - rest.NewWarningWriter(os.Stderr, rest.WarningWriterOptions{ - // only print a given warning the first time we receive it - Deduplicate: true, - }), - ) -} From e4115f7d9aeacbc812b12e96869e86fe75749d13 Mon Sep 17 00:00:00 2001 From: Tim Ebert Date: Thu, 20 Feb 2025 21:06:43 +0100 Subject: [PATCH 2/8] Add unit tests for `pkg/utils` --- pkg/utils/{utils.go => strings.go} | 0 pkg/utils/strings_test.go | 32 ++++++++++++++++++++++++++++++ pkg/utils/utils_suite_test.go | 29 +++++++++++++++++++++++++++ 3 files changed, 61 insertions(+) rename pkg/utils/{utils.go => strings.go} (100%) create mode 100644 pkg/utils/strings_test.go create mode 100644 pkg/utils/utils_suite_test.go diff --git a/pkg/utils/utils.go b/pkg/utils/strings.go similarity index 100% rename from pkg/utils/utils.go rename to pkg/utils/strings.go diff --git a/pkg/utils/strings_test.go b/pkg/utils/strings_test.go new file mode 100644 index 00000000..84347ccf --- /dev/null +++ b/pkg/utils/strings_test.go @@ -0,0 +1,32 @@ +/* +Copyright 2025 Tim Ebert. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package utils_test + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + . "github.com/timebertt/kubernetes-controller-sharding/pkg/utils" +) + +var _ = Describe("CapitalizeFirst", func() { + It("should capitalize the first letter", func() { + Expect(CapitalizeFirst("foo bar Baz")).To(Equal("Foo bar Baz")) + Expect(CapitalizeFirst("Foo BAR Baz")).To(Equal("Foo BAR Baz")) + Expect(CapitalizeFirst("FOO bar Baz")).To(Equal("FOO bar Baz")) + }) +}) diff --git a/pkg/utils/utils_suite_test.go b/pkg/utils/utils_suite_test.go new file mode 100644 index 00000000..cb165fc1 --- /dev/null +++ b/pkg/utils/utils_suite_test.go @@ -0,0 +1,29 @@ +/* +Copyright 2025 Tim Ebert. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package utils_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestUtils(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Utils Suite") +} From 5cda731c0686eee82ff0f2b272179a65b77cf7c5 Mon Sep 17 00:00:00 2001 From: Tim Ebert Date: Thu, 20 Feb 2025 21:02:12 +0100 Subject: [PATCH 3/8] Add unit tests for `pkg/utils/client` --- pkg/utils/client/client_suite_test.go | 29 +++++++++++++++++++ pkg/utils/client/options_test.go | 41 +++++++++++++++++++++++++++ 2 files changed, 70 insertions(+) create mode 100644 pkg/utils/client/client_suite_test.go create mode 100644 pkg/utils/client/options_test.go diff --git a/pkg/utils/client/client_suite_test.go b/pkg/utils/client/client_suite_test.go new file mode 100644 index 00000000..45a07a05 --- /dev/null +++ b/pkg/utils/client/client_suite_test.go @@ -0,0 +1,29 @@ +/* +Copyright 2025 Tim Ebert. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package client_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestClient(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Client Utils Suite") +} diff --git a/pkg/utils/client/options_test.go b/pkg/utils/client/options_test.go new file mode 100644 index 00000000..9a538ef0 --- /dev/null +++ b/pkg/utils/client/options_test.go @@ -0,0 +1,41 @@ +/* +Copyright 2025 Tim Ebert. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package client_test + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "sigs.k8s.io/controller-runtime/pkg/client" + + . "github.com/timebertt/kubernetes-controller-sharding/pkg/utils/client" +) + +var _ = Describe("ResourceVersion", func() { + It("should set the resourceVersion on GetOptions", func() { + opts := &client.GetOptions{} + opts.ApplyOptions([]client.GetOption{ResourceVersion("1")}) + + Expect(opts.Raw.ResourceVersion).To(Equal("1")) + }) + + It("should set the resourceVersion on ListOptions", func() { + opts := &client.ListOptions{} + opts.ApplyOptions([]client.ListOption{ResourceVersion("1")}) + + Expect(opts.Raw.ResourceVersion).To(Equal("1")) + }) +}) From 799ecaba7f3914f994a42ef6fa7f72386f516445 Mon Sep 17 00:00:00 2001 From: Tim Ebert Date: Thu, 20 Feb 2025 20:49:47 +0100 Subject: [PATCH 4/8] Add unit tests for `pkg/utils/errors` --- pkg/utils/errors/errors_suite_test.go | 29 +++++++++++++++++++ pkg/utils/errors/multi.go | 2 +- pkg/utils/errors/multi_test.go | 40 +++++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 pkg/utils/errors/errors_suite_test.go create mode 100644 pkg/utils/errors/multi_test.go diff --git a/pkg/utils/errors/errors_suite_test.go b/pkg/utils/errors/errors_suite_test.go new file mode 100644 index 00000000..7f1116ba --- /dev/null +++ b/pkg/utils/errors/errors_suite_test.go @@ -0,0 +1,29 @@ +/* +Copyright 2025 Tim Ebert. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package errors_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestErrors(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Errors Utils Suite") +} diff --git a/pkg/utils/errors/multi.go b/pkg/utils/errors/multi.go index d779a3e5..7c79c67e 100644 --- a/pkg/utils/errors/multi.go +++ b/pkg/utils/errors/multi.go @@ -22,7 +22,7 @@ import ( ) // FormatErrors is like multierror.ListFormatFunc without the noisy newlines and tabs. -// It also simplies the format for a single error. +// It also simplifies the format for a single error. func FormatErrors(es []error) string { if len(es) == 1 { return es[0].Error() diff --git a/pkg/utils/errors/multi_test.go b/pkg/utils/errors/multi_test.go new file mode 100644 index 00000000..fccc8fac --- /dev/null +++ b/pkg/utils/errors/multi_test.go @@ -0,0 +1,40 @@ +/* +Copyright 2025 Tim Ebert. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package errors_test + +import ( + "fmt" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + . "github.com/timebertt/kubernetes-controller-sharding/pkg/utils/errors" +) + +var _ = Describe("FormatErrors", func() { + It("should return the single error", func() { + Expect(FormatErrors([]error{fmt.Errorf("foo")})).To(Equal("foo")) + }) + + It("should return the error count and comma separated error list", func() { + Expect( + FormatErrors([]error{fmt.Errorf("foo"), fmt.Errorf("bar")}), + ).To( + Equal("2 errors occurred: foo, bar"), + ) + }) +}) From 741622404527443cb4c3d65a37a28778eb911d4c Mon Sep 17 00:00:00 2001 From: Tim Ebert Date: Thu, 20 Feb 2025 21:15:20 +0100 Subject: [PATCH 5/8] Add unit tests for `pkg/utils/healthz` --- pkg/utils/healthz/cache_test.go | 43 +++++++++++++++++++++++++ pkg/utils/healthz/healthz_suite_test.go | 29 +++++++++++++++++ 2 files changed, 72 insertions(+) create mode 100644 pkg/utils/healthz/cache_test.go create mode 100644 pkg/utils/healthz/healthz_suite_test.go diff --git a/pkg/utils/healthz/cache_test.go b/pkg/utils/healthz/cache_test.go new file mode 100644 index 00000000..7e755091 --- /dev/null +++ b/pkg/utils/healthz/cache_test.go @@ -0,0 +1,43 @@ +/* +Copyright 2025 Tim Ebert. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package healthz_test + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + . "github.com/timebertt/kubernetes-controller-sharding/pkg/utils/healthz" +) + +var _ = Describe("CacheSync", func() { + It("should succeed if all informers sync", func() { + checker := CacheSync(fakeSyncWaiter(true)) + Expect(checker(nil)).NotTo(HaveOccurred()) + }) + It("should fail if informers don't sync", func() { + checker := CacheSync(fakeSyncWaiter(false)) + Expect(checker(nil)).To(MatchError(ContainSubstring("not synced"))) + }) +}) + +type fakeSyncWaiter bool + +func (f fakeSyncWaiter) WaitForCacheSync(_ context.Context) bool { + return bool(f) +} diff --git a/pkg/utils/healthz/healthz_suite_test.go b/pkg/utils/healthz/healthz_suite_test.go new file mode 100644 index 00000000..d47b77fa --- /dev/null +++ b/pkg/utils/healthz/healthz_suite_test.go @@ -0,0 +1,29 @@ +/* +Copyright 2025 Tim Ebert. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package healthz_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestHealthz(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Healthz Suite") +} From 62d82f90e50446b1870f3424540f79bfec2e33c1 Mon Sep 17 00:00:00 2001 From: Tim Ebert Date: Thu, 20 Feb 2025 21:19:50 +0100 Subject: [PATCH 6/8] Drop custom `komega` package Using the upstream `komega` package should be good enough for our purposes. While using a global `context.Background` doesn't integrate nicely with ginkgo/gomega's context handling. However, we expect client calls to return fast enough in test that this doesn't really matter. --- go.mod | 2 +- pkg/utils/test/komega/default.go | 97 ---- pkg/utils/test/komega/default_test.go | 123 ---- pkg/utils/test/komega/doc.go | 25 - pkg/utils/test/komega/equalobject.go | 297 ---------- pkg/utils/test/komega/equalobject_test.go | 662 ---------------------- pkg/utils/test/komega/interfaces.go | 73 --- pkg/utils/test/komega/komega.go | 109 ---- pkg/utils/test/komega/komega_test.go | 145 ----- test/e2e/e2e_suite_test.go | 2 +- test/e2e/example_test.go | 8 +- 11 files changed, 6 insertions(+), 1537 deletions(-) delete mode 100644 pkg/utils/test/komega/default.go delete mode 100644 pkg/utils/test/komega/default_test.go delete mode 100644 pkg/utils/test/komega/doc.go delete mode 100644 pkg/utils/test/komega/equalobject.go delete mode 100644 pkg/utils/test/komega/equalobject_test.go delete mode 100644 pkg/utils/test/komega/interfaces.go delete mode 100644 pkg/utils/test/komega/komega.go delete mode 100644 pkg/utils/test/komega/komega_test.go diff --git a/go.mod b/go.mod index f3c6fb75..090b1eec 100644 --- a/go.mod +++ b/go.mod @@ -7,7 +7,6 @@ toolchain go1.24.0 require ( github.com/cespare/xxhash/v2 v2.3.0 github.com/go-logr/logr v1.4.2 - github.com/google/go-cmp v0.6.0 github.com/google/uuid v1.6.0 github.com/hashicorp/go-multierror v1.1.1 github.com/onsi/ginkgo/v2 v2.22.2 @@ -54,6 +53,7 @@ require ( github.com/google/btree v1.1.3 // indirect github.com/google/cel-go v0.22.0 // indirect github.com/google/gnostic-models v0.6.8 // indirect + github.com/google/go-cmp v0.6.0 // indirect github.com/google/gofuzz v1.2.0 // indirect github.com/google/pprof v0.0.0-20241210010833-40e02aabc2ad // indirect github.com/grpc-ecosystem/grpc-gateway/v2 v2.20.0 // indirect diff --git a/pkg/utils/test/komega/default.go b/pkg/utils/test/komega/default.go deleted file mode 100644 index 01eed5d0..00000000 --- a/pkg/utils/test/komega/default.go +++ /dev/null @@ -1,97 +0,0 @@ -package komega - -import ( - "context" - - "sigs.k8s.io/controller-runtime/pkg/client" -) - -// defaultK is the Komega used by the package global functions. -var defaultK = &komega{} - -// SetClient sets the client used by the package global functions. -func SetClient(c client.Client) { - defaultK.client = c -} - -func checkDefaultClient() { - if defaultK.client == nil { - panic("Default Komega's client is not set. Use SetClient to set it.") - } -} - -// Get returns a function that fetches a resource and returns the occurring error. -// It can be used with gomega.Eventually() like this -// -// deployment := appsv1.Deployment{ ... } -// gomega.Eventually(komega.Get(&deployment)).To(gomega.Succeed()) -// -// By calling the returned function directly it can also be used with gomega.Expect(komega.Get(...)(ctx)).To(...) -func Get(obj client.Object) func(context.Context) error { - checkDefaultClient() - return defaultK.Get(obj) -} - -// List returns a function that lists resources and returns the occurring error. -// It can be used with gomega.Eventually() like this -// -// deployments := v1.DeploymentList{ ... } -// gomega.Eventually(k.List(&deployments)).To(gomega.Succeed()) -// -// By calling the returned function directly it can also be used as gomega.Expect(k.List(...)(ctx)).To(...) -func List(list client.ObjectList, opts ...client.ListOption) func(context.Context) error { - checkDefaultClient() - return defaultK.List(list, opts...) -} - -// Update returns a function that fetches a resource, applies the provided update function and then updates the resource. -// It can be used with gomega.Eventually() like this: -// -// deployment := appsv1.Deployment{ ... } -// gomega.Eventually(k.Update(&deployment, func() { -// deployment.Spec.Replicas = 3 -// })).To(gomega.Succeed()) -// -// By calling the returned function directly it can also be used as gomega.Expect(k.Update(...)(ctx)).To(...) -func Update(obj client.Object, f func(), opts ...client.UpdateOption) func(context.Context) error { - checkDefaultClient() - return defaultK.Update(obj, f, opts...) -} - -// UpdateStatus returns a function that fetches a resource, applies the provided update function and then updates the resource's status. -// It can be used with gomega.Eventually() like this: -// -// deployment := appsv1.Deployment{ ... } -// gomega.Eventually(k.UpdateStatus(&deployment, func() { -// deployment.Status.AvailableReplicas = 1 -// })).To(gomega.Succeed()) -// -// By calling the returned function directly it can also be used as gomega.Expect(k.UpdateStatus(...)(ctx)).To(...) -func UpdateStatus(obj client.Object, f func(), opts ...client.SubResourceUpdateOption) func(context.Context) error { - checkDefaultClient() - return defaultK.UpdateStatus(obj, f, opts...) -} - -// Object returns a function that fetches a resource and returns the object. -// It can be used with gomega.Eventually() like this: -// -// deployment := appsv1.Deployment{ ... } -// gomega.Eventually(k.Object(&deployment)).To(HaveField("Spec.Replicas", gomega.Equal(ptr.To(3)))) -// -// By calling the returned function directly it can also be used as gomega.Expect(k.Object(...)(ctx)).To(...) -func Object(obj client.Object) func(context.Context) (client.Object, error) { - checkDefaultClient() - return defaultK.Object(obj) -} - -// ObjectList returns a function that fetches a resource and returns the object. -// It can be used with gomega.Eventually() like this: -// -// deployments := appsv1.DeploymentList{ ... } -// gomega.Eventually(k.ObjectList(&deployments)).To(HaveField("Items", HaveLen(1))) -// -// By calling the returned function directly it can also be used as gomega.Expect(k.ObjectList(...)(ctx)).To(...) -func ObjectList(list client.ObjectList, opts ...client.ListOption) func(context.Context) (client.ObjectList, error) { - checkDefaultClient() - return defaultK.ObjectList(list, opts...) -} diff --git a/pkg/utils/test/komega/default_test.go b/pkg/utils/test/komega/default_test.go deleted file mode 100644 index 37b4f004..00000000 --- a/pkg/utils/test/komega/default_test.go +++ /dev/null @@ -1,123 +0,0 @@ -package komega - -import ( - "context" - "testing" - - . "github.com/onsi/gomega" - appsv1 "k8s.io/api/apps/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/utils/ptr" -) - -func TestDefaultGet(t *testing.T) { - g := NewWithT(t) - ctx := context.Background() - - fc := createFakeClient() - SetClient(fc) - - fetched := appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", - Name: "test", - }, - } - g.Eventually(ctx, Get(&fetched)).Should(Succeed()) - - g.Expect(*fetched.Spec.Replicas).To(BeEquivalentTo(5)) -} - -func TestDefaultList(t *testing.T) { - g := NewWithT(t) - ctx := context.Background() - - fc := createFakeClient() - SetClient(fc) - - list := appsv1.DeploymentList{} - g.Eventually(ctx, List(&list)).Should(Succeed()) - - g.Expect(list.Items).To(HaveLen(1)) - depl := exampleDeployment() - g.Expect(list.Items[0]).To(And( - HaveField("ObjectMeta.Name", Equal(depl.ObjectMeta.Name)), - HaveField("ObjectMeta.Namespace", Equal(depl.ObjectMeta.Namespace)), - )) -} - -func TestDefaultUpdate(t *testing.T) { - g := NewWithT(t) - ctx := context.Background() - - fc := createFakeClient() - SetClient(fc) - - updateDeployment := appsv1.Deployment{ - ObjectMeta: exampleDeployment().ObjectMeta, - } - g.Eventually(ctx, Update(&updateDeployment, func() { - updateDeployment.Annotations = map[string]string{"updated": "true"} - })).Should(Succeed()) - - fetched := appsv1.Deployment{ - ObjectMeta: exampleDeployment().ObjectMeta, - } - g.Eventually(ctx, Object(&fetched)).Should(HaveField("ObjectMeta.Annotations", HaveKeyWithValue("updated", "true"))) -} - -func TestDefaultUpdateStatus(t *testing.T) { - g := NewWithT(t) - ctx := context.Background() - - fc := createFakeClient() - SetClient(fc) - - updateDeployment := appsv1.Deployment{ - ObjectMeta: exampleDeployment().ObjectMeta, - } - g.Eventually(ctx, UpdateStatus(&updateDeployment, func() { - updateDeployment.Status.AvailableReplicas = 1 - })).Should(Succeed()) - - fetched := appsv1.Deployment{ - ObjectMeta: exampleDeployment().ObjectMeta, - } - g.Eventually(ctx, Object(&fetched)).Should(HaveField("Status.AvailableReplicas", BeEquivalentTo(1))) -} - -func TestDefaultObject(t *testing.T) { - g := NewWithT(t) - ctx := context.Background() - - fc := createFakeClient() - SetClient(fc) - - fetched := appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", - Name: "test", - }, - } - g.Eventually(ctx, Object(&fetched)).Should(And( - Not(BeNil()), - HaveField("Spec.Replicas", Equal(ptr.To(int32(5)))), - )) -} - -func TestDefaultObjectList(t *testing.T) { - g := NewWithT(t) - ctx := context.Background() - - fc := createFakeClient() - SetClient(fc) - - list := appsv1.DeploymentList{} - g.Eventually(ctx, ObjectList(&list)).Should(And( - Not(BeNil()), - HaveField("Items", And( - HaveLen(1), - ContainElement(HaveField("Spec.Replicas", Equal(ptr.To(int32(5))))), - )), - )) -} diff --git a/pkg/utils/test/komega/doc.go b/pkg/utils/test/komega/doc.go deleted file mode 100644 index cb82ee62..00000000 --- a/pkg/utils/test/komega/doc.go +++ /dev/null @@ -1,25 +0,0 @@ -/* -Copyright 2024 Tim Ebert. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -// Package komega is a modified version of sigs.k8s.io/controller-runtime/pkg/envtest/komega. -// Instead of requiring users to set a global context, the returned functions accept a context to integrate nicely with -// ginkgo's/gomega's timeout/interrupt handling and polling. -// For this, users must pass a spec-specific context, e.g.: -// -// It("...", func(ctx SpecContext) { -// Eventually(ctx, Get(...)).Should(Succeed()) -// }) -package komega diff --git a/pkg/utils/test/komega/equalobject.go b/pkg/utils/test/komega/equalobject.go deleted file mode 100644 index c5a1f879..00000000 --- a/pkg/utils/test/komega/equalobject.go +++ /dev/null @@ -1,297 +0,0 @@ -/* -Copyright 2022 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - http://www.apache.org/licenses/LICENSE-2.0 -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package komega - -import ( - "fmt" - "reflect" - "strings" - - "github.com/google/go-cmp/cmp" - "github.com/onsi/gomega/format" - "github.com/onsi/gomega/types" - "k8s.io/apimachinery/pkg/runtime" -) - -// These package variables hold pre-created commonly used options that can be used to reduce the manual work involved in -// identifying the paths that need to be compared for testing equality between objects. -var ( - // IgnoreAutogeneratedMetadata contains the paths for all the metadata fields that are commonly set by the - // client and APIServer. This is used as a MatchOption for situations when only user-provided metadata is relevant. - IgnoreAutogeneratedMetadata = IgnorePaths{ - "metadata.uid", - "metadata.generation", - "metadata.creationTimestamp", - "metadata.resourceVersion", - "metadata.managedFields", - "metadata.deletionGracePeriodSeconds", - "metadata.deletionTimestamp", - "metadata.selfLink", - "metadata.generateName", - } -) - -type diffPath struct { - types []string - json []string -} - -// equalObjectMatcher is a Gomega matcher used to establish equality between two Kubernetes runtime.Objects. -type equalObjectMatcher struct { - // original holds the object that will be used to Match. - original runtime.Object - - // diffPaths contains the paths that differ between two objects. - diffPaths []diffPath - - // options holds the options that identify what should and should not be matched. - options *EqualObjectOptions -} - -// EqualObject returns a Matcher for the passed Kubernetes runtime.Object with the passed Options. This function can be -// used as a Gomega Matcher in Gomega Assertions. -func EqualObject(original runtime.Object, opts ...EqualObjectOption) types.GomegaMatcher { - matchOptions := &EqualObjectOptions{} - matchOptions = matchOptions.ApplyOptions(opts) - - return &equalObjectMatcher{ - options: matchOptions, - original: original, - } -} - -// Match compares the current object to the passed object and returns true if the objects are the same according to -// the Matcher and MatchOptions. -func (m *equalObjectMatcher) Match(actual interface{}) (success bool, err error) { - // Nil checks required first here for: - // 1) Nil equality which returns true - // 2) One object nil which returns an error - actualIsNil := reflect.ValueOf(actual).IsNil() - originalIsNil := reflect.ValueOf(m.original).IsNil() - - if actualIsNil && originalIsNil { - return true, nil - } - if actualIsNil || originalIsNil { - return false, fmt.Errorf("can not compare an object with a nil. original %v , actual %v", m.original, actual) - } - - m.diffPaths = m.calculateDiff(actual) - return len(m.diffPaths) == 0, nil -} - -// FailureMessage returns a message comparing the full objects after an unexpected failure to match has occurred. -func (m *equalObjectMatcher) FailureMessage(actual interface{}) (message string) { - return fmt.Sprintf("the following fields were expected to match but did not:\n%v\n%s", m.diffPaths, - format.Message(actual, "expected to match", m.original)) -} - -// NegatedFailureMessage returns a string stating that all fields matched, even though that was not expected. -func (m *equalObjectMatcher) NegatedFailureMessage(actual interface{}) (message string) { - return "it was expected that some fields do not match, but all of them did" -} - -func (d diffPath) String() string { - return fmt.Sprintf("(%s/%s)", strings.Join(d.types, "."), strings.Join(d.json, ".")) -} - -// diffReporter is a custom recorder for cmp.Diff which records all paths that are -// different between two objects. -type diffReporter struct { - stack []cmp.PathStep - - diffPaths []diffPath -} - -func (r *diffReporter) PushStep(s cmp.PathStep) { - r.stack = append(r.stack, s) -} - -func (r *diffReporter) Report(res cmp.Result) { - if !res.Equal() { - r.diffPaths = append(r.diffPaths, r.currentPath()) - } -} - -// currentPath converts the current stack into string representations that match -// the IgnorePaths and MatchPaths syntax. -func (r *diffReporter) currentPath() diffPath { - p := diffPath{types: []string{""}, json: []string{""}} - for si, s := range r.stack[1:] { - switch s := s.(type) { - case cmp.StructField: - p.types = append(p.types, s.String()[1:]) - // fetch the type information from the parent struct. - // Note: si has an offset of 1 compared to r.stack as we loop over r.stack[1:], so we don't need -1 - field := r.stack[si].Type().Field(s.Index()) - p.json = append(p.json, strings.Split(field.Tag.Get("json"), ",")[0]) - case cmp.SliceIndex: - key := fmt.Sprintf("[%d]", s.Key()) - p.types[len(p.types)-1] += key - p.json[len(p.json)-1] += key - case cmp.MapIndex: - key := fmt.Sprintf("%v", s.Key()) - if strings.ContainsAny(key, ".[]/\\") { - key = fmt.Sprintf("[%s]", key) - p.types[len(p.types)-1] += key - p.json[len(p.json)-1] += key - } else { - p.types = append(p.types, key) - p.json = append(p.json, key) - } - } - } - // Empty strings were added as the first element. If they're still empty, remove them again. - if len(p.json) > 0 && len(p.json[0]) == 0 { - p.json = p.json[1:] - p.types = p.types[1:] - } - return p -} - -func (r *diffReporter) PopStep() { - r.stack = r.stack[:len(r.stack)-1] -} - -// calculateDiff calculates the difference between two objects and returns the -// paths of the fields that do not match. -func (m *equalObjectMatcher) calculateDiff(actual interface{}) []diffPath { - var original interface{} = m.original - // Remove the wrapping Object from unstructured.Unstructured to make comparison behave similar to - // regular objects. - if u, isUnstructured := actual.(runtime.Unstructured); isUnstructured { - actual = u.UnstructuredContent() - } - if u, ok := m.original.(runtime.Unstructured); ok { - original = u.UnstructuredContent() - } - r := diffReporter{} - cmp.Diff(original, actual, cmp.Reporter(&r)) - return filterDiffPaths(*m.options, r.diffPaths) -} - -// filterDiffPaths filters the diff paths using the paths in EqualObjectOptions. -func filterDiffPaths(opts EqualObjectOptions, paths []diffPath) []diffPath { - result := []diffPath{} - - for _, p := range paths { - if len(opts.matchPaths) > 0 && !hasAnyPathPrefix(p, opts.matchPaths) { - continue - } - if hasAnyPathPrefix(p, opts.ignorePaths) { - continue - } - - result = append(result, p) - } - - return result -} - -// hasPathPrefix compares the segments of a path. -func hasPathPrefix(path []string, prefix []string) bool { - for i, p := range prefix { - if i >= len(path) { - return false - } - // return false if a segment doesn't match - if path[i] != p && (i < len(prefix)-1 || !segmentHasPrefix(path[i], p)) { - return false - } - } - return true -} - -func segmentHasPrefix(s, prefix string) bool { - return len(s) >= len(prefix) && s[0:len(prefix)] == prefix && - // if it is a prefix match, make sure the next character is a [ for array/map access - (len(s) == len(prefix) || s[len(prefix)] == '[') -} - -// hasAnyPathPrefix returns true if path matches any of the path prefixes. -// It respects the name boundaries within paths, so 'ObjectMeta.Name' does not -// match 'ObjectMeta.Namespace' for example. -func hasAnyPathPrefix(path diffPath, prefixes [][]string) bool { - for _, prefix := range prefixes { - if hasPathPrefix(path.types, prefix) || hasPathPrefix(path.json, prefix) { - return true - } - } - return false -} - -// EqualObjectOption describes an Option that can be applied to a Matcher. -type EqualObjectOption interface { - // ApplyToEqualObjectMatcher applies this configuration to the given MatchOption. - ApplyToEqualObjectMatcher(options *EqualObjectOptions) -} - -// EqualObjectOptions holds the available types of EqualObjectOptions that can be applied to a Matcher. -type EqualObjectOptions struct { - ignorePaths [][]string - matchPaths [][]string -} - -// ApplyOptions adds the passed MatchOptions to the MatchOptions struct. -func (o *EqualObjectOptions) ApplyOptions(opts []EqualObjectOption) *EqualObjectOptions { - for _, opt := range opts { - opt.ApplyToEqualObjectMatcher(o) - } - return o -} - -// IgnorePaths instructs the Matcher to ignore given paths when computing a diff. -// Paths are written in a syntax similar to Go with a few special cases. Both types and -// json/yaml field names are supported. -// -// Regular Paths: -// * "ObjectMeta.Name" -// * "metadata.name" -// Arrays: -// * "metadata.ownerReferences[0].name" -// Maps, if they do not contain any of .[]/\: -// * "metadata.labels.something" -// Maps, if they contain any of .[]/\: -// * "metadata.labels[kubernetes.io/something]" -type IgnorePaths []string - -// ApplyToEqualObjectMatcher applies this configuration to the given MatchOptions. -func (i IgnorePaths) ApplyToEqualObjectMatcher(opts *EqualObjectOptions) { - for _, p := range i { - opts.ignorePaths = append(opts.ignorePaths, strings.Split(p, ".")) - } -} - -// MatchPaths instructs the Matcher to restrict its diff to the given paths. If empty the Matcher will look at all paths. -// Paths are written in a syntax similar to Go with a few special cases. Both types and -// json/yaml field names are supported. -// -// Regular Paths: -// * "ObjectMeta.Name" -// * "metadata.name" -// Arrays: -// * "metadata.ownerReferences[0].name" -// Maps, if they do not contain any of .[]/\: -// * "metadata.labels.something" -// Maps, if they contain any of .[]/\: -// * "metadata.labels[kubernetes.io/something]" -type MatchPaths []string - -// ApplyToEqualObjectMatcher applies this configuration to the given MatchOptions. -func (i MatchPaths) ApplyToEqualObjectMatcher(opts *EqualObjectOptions) { - for _, p := range i { - opts.matchPaths = append(opts.matchPaths, strings.Split(p, ".")) - } -} diff --git a/pkg/utils/test/komega/equalobject_test.go b/pkg/utils/test/komega/equalobject_test.go deleted file mode 100644 index f232642c..00000000 --- a/pkg/utils/test/komega/equalobject_test.go +++ /dev/null @@ -1,662 +0,0 @@ -package komega - -import ( - "testing" - - . "github.com/onsi/gomega" - appsv1 "k8s.io/api/apps/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "sigs.k8s.io/controller-runtime/pkg/client" -) - -func TestEqualObjectMatcher(t *testing.T) { - cases := []struct { - name string - original client.Object - modified client.Object - options []EqualObjectOption - want bool - }{ - { - name: "succeed with equal objects", - original: &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - }, - modified: &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - }, - want: true, - }, - { - name: "fail with non equal objects", - original: &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - }, - modified: &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "somethingelse", - }, - }, - want: false, - }, - { - name: "succeeds if ignored fields do not match", - original: &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Labels: map[string]string{"somelabel": "somevalue"}, - OwnerReferences: []metav1.OwnerReference{{ - Name: "controller", - }}, - }, - }, - modified: &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "somethingelse", - Labels: map[string]string{"somelabel": "anothervalue"}, - OwnerReferences: []metav1.OwnerReference{{ - Name: "another", - }}, - }, - }, - want: true, - options: []EqualObjectOption{ - IgnorePaths{ - "ObjectMeta.Name", - "ObjectMeta.CreationTimestamp", - "ObjectMeta.Labels.somelabel", - "ObjectMeta.OwnerReferences[0].Name", - "Spec.Template.ObjectMeta", - }, - }, - }, - { - name: "succeeds if ignored fields in json notation do not match", - original: &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Labels: map[string]string{"somelabel": "somevalue"}, - OwnerReferences: []metav1.OwnerReference{{ - Name: "controller", - }}, - }, - }, - modified: &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "somethingelse", - Labels: map[string]string{"somelabel": "anothervalue"}, - OwnerReferences: []metav1.OwnerReference{{ - Name: "another", - }}, - }, - }, - want: true, - options: []EqualObjectOption{ - IgnorePaths{ - "metadata.name", - "metadata.creationTimestamp", - "metadata.labels.somelabel", - "metadata.ownerReferences[0].name", - "spec.template.metadata", - }, - }, - }, - { - name: "succeeds if all allowed fields match, and some others do not", - original: &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: "default", - }, - }, - modified: &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: "special", - }, - }, - want: true, - options: []EqualObjectOption{ - MatchPaths{ - "ObjectMeta.Name", - }, - }, - }, - { - name: "works with unstructured.Unstructured", - original: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "metadata": map[string]interface{}{ - "name": "something", - "namespace": "test", - }, - }, - }, - modified: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "metadata": map[string]interface{}{ - "name": "somethingelse", - "namespace": "test", - }, - }, - }, - want: true, - options: []EqualObjectOption{ - IgnorePaths{ - "metadata.name", - }, - }, - }, - - // Test when objects are equal. - { - name: "Equal field (spec) both in original and in modified", - original: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "spec": map[string]interface{}{ - "foo": "bar", - }, - }, - }, - modified: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "spec": map[string]interface{}{ - "foo": "bar", - }, - }, - }, - want: true, - }, - - { - name: "Equal nested field both in original and in modified", - original: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "spec": map[string]interface{}{ - "template": map[string]interface{}{ - "spec": map[string]interface{}{ - "A": "A", - }, - }, - }, - }, - }, - modified: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "spec": map[string]interface{}{ - "template": map[string]interface{}{ - "spec": map[string]interface{}{ - "A": "A", - }, - }, - }, - }, - }, - want: true, - }, - - // Test when there is a difference between the objects. - { - name: "Unequal field both in original and in modified", - original: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "spec": map[string]interface{}{ - "foo": "bar-changed", - }, - }, - }, - modified: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "spec": map[string]interface{}{ - "foo": "bar", - }, - }, - }, - want: false, - }, - { - name: "Unequal nested field both in original and modified", - original: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "spec": map[string]interface{}{ - "template": map[string]interface{}{ - "spec": map[string]interface{}{ - "A": "A-Changed", - }, - }, - }, - }, - }, - modified: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "spec": map[string]interface{}{ - "template": map[string]interface{}{ - "spec": map[string]interface{}{ - "A": "A", - }, - }, - }, - }, - }, - want: false, - }, - - { - name: "Value of type map with different values", - original: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "spec": map[string]interface{}{ - "map": map[string]string{ - "A": "A-changed", - "B": "B", - // C missing - }, - }, - }, - }, - modified: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "spec": map[string]interface{}{ - "map": map[string]string{ - "A": "A", - // B missing - "C": "C", - }, - }, - }, - }, - want: false, - }, - - { - name: "Value of type Array or Slice with same length but different values", - original: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "spec": map[string]interface{}{ - "slice": []string{ - "D", - "C", - "B", - }, - }, - }, - }, - modified: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "spec": map[string]interface{}{ - "slice": []string{ - "A", - "B", - "C", - }, - }, - }, - }, - want: false, - }, - - // This tests specific behavior in how Kubernetes marshals the zero value of metav1.Time{}. - { - name: "Creation timestamp set to empty value on both original and modified", - original: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "spec": map[string]interface{}{ - "A": "A", - }, - "metadata": map[string]interface{}{ - "selfLink": "foo", - "creationTimestamp": metav1.Time{}, - }, - }, - }, - modified: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "spec": map[string]interface{}{ - "A": "A", - }, - "metadata": map[string]interface{}{ - "selfLink": "foo", - "creationTimestamp": metav1.Time{}, - }, - }, - }, - want: true, - }, - - // Cases to test diff when fields exist only in modified object. - { - name: "Field only in modified", - original: &unstructured.Unstructured{ - Object: map[string]interface{}{}, - }, - modified: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "spec": map[string]interface{}{ - "foo": "bar", - }, - }, - }, - want: false, - }, - { - name: "Nested field only in modified", - original: &unstructured.Unstructured{ - Object: map[string]interface{}{}, - }, - modified: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "spec": map[string]interface{}{ - "template": map[string]interface{}{ - "spec": map[string]interface{}{ - "A": "A", - }, - }, - }, - }, - }, - want: false, - }, - { - name: "Creation timestamp exists on modified but not on original", - original: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "spec": map[string]interface{}{ - "A": "A", - }, - }, - }, - modified: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "spec": map[string]interface{}{ - "A": "A", - }, - "metadata": map[string]interface{}{ - "selfLink": "foo", - "creationTimestamp": "2021-11-03T11:05:17Z", - }, - }, - }, - want: false, - }, - - // Test when fields exists only in the original object. - { - name: "Field only in original", - original: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "spec": map[string]interface{}{ - "foo": "bar", - }, - }, - }, - modified: &unstructured.Unstructured{ - Object: map[string]interface{}{}, - }, - want: false, - }, - { - name: "Nested field only in original", - original: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "spec": map[string]interface{}{ - "template": map[string]interface{}{ - "spec": map[string]interface{}{ - "A": "A", - }, - }, - }, - }, - }, - modified: &unstructured.Unstructured{ - Object: map[string]interface{}{}, - }, - want: false, - }, - { - name: "Creation timestamp exists on original but not on modified", - original: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "spec": map[string]interface{}{ - "A": "A", - }, - "metadata": map[string]interface{}{ - "selfLink": "foo", - "creationTimestamp": "2021-11-03T11:05:17Z", - }, - }, - }, - modified: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "spec": map[string]interface{}{ - "A": "A", - }, - }, - }, - - want: false, - }, - - // Test metadata fields computed by the system or in status are compared. - { - name: "Unequal Metadata fields computed by the system or in status", - original: &unstructured.Unstructured{ - Object: map[string]interface{}{}, - }, - modified: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "metadata": map[string]interface{}{ - "selfLink": "foo", - "uid": "foo", - "resourceVersion": "foo", - "generation": "foo", - "managedFields": "foo", - }, - "status": map[string]interface{}{ - "foo": "bar", - }, - }, - }, - want: false, - }, - { - name: "Unequal labels and annotations", - original: &unstructured.Unstructured{ - Object: map[string]interface{}{}, - }, - modified: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "metadata": map[string]interface{}{ - "labels": map[string]interface{}{ - "foo": "bar", - }, - "annotations": map[string]interface{}{ - "foo": "bar", - }, - }, - }, - }, - want: false, - }, - - // Ignore fields MatchOption - { - name: "Unequal metadata fields ignored by IgnorePaths MatchOption", - original: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "metadata": map[string]interface{}{ - "name": "test", - }, - }, - }, - modified: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "metadata": map[string]interface{}{ - "name": "test", - "selfLink": "foo", - "uid": "foo", - "resourceVersion": "foo", - "generation": "foo", - "managedFields": "foo", - }, - }, - }, - options: []EqualObjectOption{IgnoreAutogeneratedMetadata}, - want: true, - }, - { - name: "Unequal labels and annotations ignored by IgnorePaths MatchOption", - original: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "metadata": map[string]interface{}{ - "name": "test", - }, - }, - }, - modified: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "metadata": map[string]interface{}{ - "name": "test", - "labels": map[string]interface{}{ - "foo": "bar", - }, - "annotations": map[string]interface{}{ - "foo": "bar", - }, - }, - }, - }, - options: []EqualObjectOption{IgnorePaths{"metadata.labels", "metadata.annotations"}}, - want: true, - }, - { - name: "Ignore fields are not compared", - original: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "spec": map[string]interface{}{}, - }, - }, - modified: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "spec": map[string]interface{}{ - "controlPlaneEndpoint": map[string]interface{}{ - "host": "", - "port": 0, - }, - }, - }, - }, - options: []EqualObjectOption{IgnorePaths{"spec.controlPlaneEndpoint"}}, - want: true, - }, - { - name: "Not-ignored fields are still compared", - original: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "metadata": map[string]interface{}{ - "annotations": map[string]interface{}{}, - }, - }, - }, - modified: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "metadata": map[string]interface{}{ - "annotations": map[string]interface{}{ - "ignored": "somevalue", - "superflous": "shouldcausefailure", - }, - }, - }, - }, - options: []EqualObjectOption{IgnorePaths{"metadata.annotations.ignored"}}, - want: false, - }, - - // MatchPaths MatchOption - { - name: "Unequal metadata fields not compared by setting MatchPaths MatchOption", - original: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "spec": map[string]interface{}{ - "A": "A", - }, - }, - }, - modified: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "spec": map[string]interface{}{ - "A": "A", - }, - "metadata": map[string]interface{}{ - "selfLink": "foo", - "uid": "foo", - }, - }, - }, - options: []EqualObjectOption{MatchPaths{"spec"}}, - want: true, - }, - - // More tests - { - name: "No changes", - original: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "spec": map[string]interface{}{ - "A": "A", - "B": "B", - "C": "C", // C only in original - }, - }, - }, - modified: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "spec": map[string]interface{}{ - "A": "A", - "B": "B", - }, - }, - }, - want: false, - }, - { - name: "Many changes", - original: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "spec": map[string]interface{}{ - "A": "A", - // B missing - "C": "C", // C only in original - }, - }, - }, - modified: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "spec": map[string]interface{}{ - "A": "A", - "B": "B", - }, - }, - }, - want: false, - }, - } - - for _, c := range cases { - t.Run(c.name, func(t *testing.T) { - g := NewWithT(t) - m := EqualObject(c.original, c.options...) - success, _ := m.Match(c.modified) - if !success { - t.Log(m.FailureMessage(c.modified)) - } - g.Expect(success).To(Equal(c.want)) - }) - } -} diff --git a/pkg/utils/test/komega/interfaces.go b/pkg/utils/test/komega/interfaces.go deleted file mode 100644 index f05894d4..00000000 --- a/pkg/utils/test/komega/interfaces.go +++ /dev/null @@ -1,73 +0,0 @@ -/* -Copyright 2021 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package komega - -import ( - "context" - - "sigs.k8s.io/controller-runtime/pkg/client" -) - -// Komega is a collection of utilites for writing tests involving a mocked -// Kubernetes API. -type Komega interface { - // Get returns a function that fetches a resource and returns the occurring error. - // It can be used with gomega.Eventually() like this - // deployment := appsv1.Deployment{ ... } - // gomega.Eventually(k.Get(&deployment)).To(gomega.Succeed()) - // By calling the returned function directly it can also be used with gomega.Expect(k.Get(...)(ctx)).To(...) - Get(client.Object) func(context.Context) error - - // List returns a function that lists resources and returns the occurring error. - // It can be used with gomega.Eventually() like this - // deployments := v1.DeploymentList{ ... } - // gomega.Eventually(k.List(&deployments)).To(gomega.Succeed()) - // By calling the returned function directly it can also be used as gomega.Expect(k.List(...)(ctx)).To(...) - List(client.ObjectList, ...client.ListOption) func(context.Context) error - - // Update returns a function that fetches a resource, applies the provided update function and then updates the resource. - // It can be used with gomega.Eventually() like this: - // deployment := appsv1.Deployment{ ... } - // gomega.Eventually(k.Update(&deployment, func() { - // deployment.Spec.Replicas = 3 - // })).To(gomega.Succeed()) - // By calling the returned function directly it can also be used as gomega.Expect(k.Update(...)(ctx)).To(...) - Update(client.Object, func(), ...client.UpdateOption) func(context.Context) error - - // UpdateStatus returns a function that fetches a resource, applies the provided update function and then updates the resource's status. - // It can be used with gomega.Eventually() like this: - // deployment := appsv1.Deployment{ ... } - // gomega.Eventually(k.Update(&deployment, func() { - // deployment.Status.AvailableReplicas = 1 - // })).To(gomega.Succeed()) - // By calling the returned function directly it can also be used as gomega.Expect(k.UpdateStatus(...)(ctx)).To(...) - UpdateStatus(client.Object, func(), ...client.SubResourceUpdateOption) func(context.Context) error - - // Object returns a function that fetches a resource and returns the object. - // It can be used with gomega.Eventually() like this: - // deployment := appsv1.Deployment{ ... } - // gomega.Eventually(k.Object(&deployment)).To(HaveField("Spec.Replicas", gomega.Equal(ptr.To(int32(3))))) - // By calling the returned function directly it can also be used as gomega.Expect(k.Object(...)(ctx)).To(...) - Object(client.Object) func(context.Context) (client.Object, error) - - // ObjectList returns a function that fetches a resource and returns the object. - // It can be used with gomega.Eventually() like this: - // deployments := appsv1.DeploymentList{ ... } - // gomega.Eventually(k.ObjectList(&deployments)).To(HaveField("Items", HaveLen(1))) - // By calling the returned function directly it can also be used as gomega.Expect(k.ObjectList(...)(ctx)).To(...) - ObjectList(client.ObjectList, ...client.ListOption) func(context.Context) (client.ObjectList, error) -} diff --git a/pkg/utils/test/komega/komega.go b/pkg/utils/test/komega/komega.go deleted file mode 100644 index eb65e5aa..00000000 --- a/pkg/utils/test/komega/komega.go +++ /dev/null @@ -1,109 +0,0 @@ -/* -Copyright 2021 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package komega - -import ( - "context" - - "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/client" -) - -// komega is a collection of utilites for writing tests involving a mocked -// Kubernetes API. -type komega struct { - client client.Client -} - -var _ Komega = &komega{} - -// New creates a new Komega instance with the given client. -func New(c client.Client) Komega { - return &komega{ - client: c, - } -} - -// Get returns a function that fetches a resource and returns the occurring error. -func (k *komega) Get(obj client.Object) func(context.Context) error { - key := types.NamespacedName{ - Name: obj.GetName(), - Namespace: obj.GetNamespace(), - } - return func(ctx context.Context) error { - return k.client.Get(ctx, key, obj) - } -} - -// List returns a function that lists resources and returns the occurring error. -func (k *komega) List(obj client.ObjectList, opts ...client.ListOption) func(context.Context) error { - return func(ctx context.Context) error { - return k.client.List(ctx, obj, opts...) - } -} - -// Update returns a function that fetches a resource, applies the provided update function and then updates the resource. -func (k *komega) Update(obj client.Object, updateFunc func(), opts ...client.UpdateOption) func(context.Context) error { - key := types.NamespacedName{ - Name: obj.GetName(), - Namespace: obj.GetNamespace(), - } - return func(ctx context.Context) error { - err := k.client.Get(ctx, key, obj) - if err != nil { - return err - } - updateFunc() - return k.client.Update(ctx, obj, opts...) - } -} - -// UpdateStatus returns a function that fetches a resource, applies the provided update function and then updates the resource's status. -func (k *komega) UpdateStatus(obj client.Object, updateFunc func(), opts ...client.SubResourceUpdateOption) func(context.Context) error { - key := types.NamespacedName{ - Name: obj.GetName(), - Namespace: obj.GetNamespace(), - } - return func(ctx context.Context) error { - err := k.client.Get(ctx, key, obj) - if err != nil { - return err - } - updateFunc() - return k.client.Status().Update(ctx, obj, opts...) - } -} - -// Object returns a function that fetches a resource and returns the object. -func (k *komega) Object(obj client.Object) func(context.Context) (client.Object, error) { - key := types.NamespacedName{ - Name: obj.GetName(), - Namespace: obj.GetNamespace(), - } - return func(ctx context.Context) (client.Object, error) { - err := k.client.Get(ctx, key, obj) - return obj, err - } -} - -// ObjectList returns a function that fetches a resource and returns the object. -func (k *komega) ObjectList(obj client.ObjectList, opts ...client.ListOption) func(context.Context) (client.ObjectList, error) { - return func(ctx context.Context) (client.ObjectList, error) { - err := k.client.List(ctx, obj, opts...) - return obj, err - } -} diff --git a/pkg/utils/test/komega/komega_test.go b/pkg/utils/test/komega/komega_test.go deleted file mode 100644 index a2b04ca2..00000000 --- a/pkg/utils/test/komega/komega_test.go +++ /dev/null @@ -1,145 +0,0 @@ -package komega - -import ( - "context" - "testing" - - _ "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - appsv1 "k8s.io/api/apps/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/utils/ptr" - - "sigs.k8s.io/controller-runtime/pkg/client" - fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake" -) - -func exampleDeployment() *appsv1.Deployment { - return &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", - Name: "test", - }, - Spec: appsv1.DeploymentSpec{ - Replicas: ptr.To(int32(5)), - }, - } -} - -func createFakeClient() client.Client { - return fakeclient.NewClientBuilder(). - WithObjects(exampleDeployment()). - Build() -} - -func TestGet(t *testing.T) { - g := NewWithT(t) - ctx := context.Background() - - fc := createFakeClient() - k := New(fc) - - fetched := appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", - Name: "test", - }, - } - g.Eventually(ctx, k.Get(&fetched)).Should(Succeed()) - - g.Expect(*fetched.Spec.Replicas).To(BeEquivalentTo(5)) -} - -func TestList(t *testing.T) { - g := NewWithT(t) - ctx := context.Background() - - fc := createFakeClient() - k := New(fc) - - list := appsv1.DeploymentList{} - g.Eventually(ctx, k.List(&list)).Should(Succeed()) - - g.Expect(list.Items).To(HaveLen(1)) - depl := exampleDeployment() - g.Expect(list.Items[0]).To(And( - HaveField("ObjectMeta.Name", Equal(depl.ObjectMeta.Name)), - HaveField("ObjectMeta.Namespace", Equal(depl.ObjectMeta.Namespace)), - )) -} - -func TestUpdate(t *testing.T) { - g := NewWithT(t) - ctx := context.Background() - - fc := createFakeClient() - k := New(fc) - - updateDeployment := appsv1.Deployment{ - ObjectMeta: exampleDeployment().ObjectMeta, - } - g.Eventually(ctx, k.Update(&updateDeployment, func() { - updateDeployment.Annotations = map[string]string{"updated": "true"} - })).Should(Succeed()) - - fetched := appsv1.Deployment{ - ObjectMeta: exampleDeployment().ObjectMeta, - } - g.Eventually(ctx, k.Object(&fetched)).Should(HaveField("ObjectMeta.Annotations", HaveKeyWithValue("updated", "true"))) -} - -func TestUpdateStatus(t *testing.T) { - g := NewWithT(t) - ctx := context.Background() - - fc := createFakeClient() - k := New(fc) - - updateDeployment := appsv1.Deployment{ - ObjectMeta: exampleDeployment().ObjectMeta, - } - g.Eventually(ctx, k.UpdateStatus(&updateDeployment, func() { - updateDeployment.Status.AvailableReplicas = 1 - })).Should(Succeed()) - - fetched := appsv1.Deployment{ - ObjectMeta: exampleDeployment().ObjectMeta, - } - g.Eventually(ctx, k.Object(&fetched)).Should(HaveField("Status.AvailableReplicas", BeEquivalentTo(1))) -} - -func TestObject(t *testing.T) { - g := NewWithT(t) - ctx := context.Background() - - fc := createFakeClient() - k := New(fc) - - fetched := appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", - Name: "test", - }, - } - g.Eventually(ctx, k.Object(&fetched)).Should(And( - Not(BeNil()), - HaveField("Spec.Replicas", Equal(ptr.To(int32(5)))), - )) -} - -func TestObjectList(t *testing.T) { - g := NewWithT(t) - ctx := context.Background() - - fc := createFakeClient() - k := New(fc) - - list := appsv1.DeploymentList{} - g.Eventually(ctx, k.ObjectList(&list)).Should(And( - Not(BeNil()), - HaveField("Items", And( - HaveLen(1), - ContainElement(HaveField("Spec.Replicas", Equal(ptr.To(int32(5))))), - )), - )) -} diff --git a/test/e2e/e2e_suite_test.go b/test/e2e/e2e_suite_test.go index c3eeb45b..6f194330 100644 --- a/test/e2e/e2e_suite_test.go +++ b/test/e2e/e2e_suite_test.go @@ -27,10 +27,10 @@ import ( clientgoscheme "k8s.io/client-go/kubernetes/scheme" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/config" + "sigs.k8s.io/controller-runtime/pkg/envtest/komega" "sigs.k8s.io/controller-runtime/pkg/log/zap" shardingv1alpha1 "github.com/timebertt/kubernetes-controller-sharding/pkg/apis/sharding/v1alpha1" - "github.com/timebertt/kubernetes-controller-sharding/pkg/utils/test/komega" ) func TestE2E(t *testing.T) { diff --git a/test/e2e/example_test.go b/test/e2e/example_test.go index 5e29e8f7..2c90ab86 100644 --- a/test/e2e/example_test.go +++ b/test/e2e/example_test.go @@ -25,10 +25,10 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" + . "sigs.k8s.io/controller-runtime/pkg/envtest/komega" shardingv1alpha1 "github.com/timebertt/kubernetes-controller-sharding/pkg/apis/sharding/v1alpha1" "github.com/timebertt/kubernetes-controller-sharding/pkg/utils/test" - . "github.com/timebertt/kubernetes-controller-sharding/pkg/utils/test/komega" . "github.com/timebertt/kubernetes-controller-sharding/pkg/utils/test/matchers" ) @@ -48,7 +48,7 @@ var _ = Describe("Example Shard", Label("example"), Ordered, func() { deployment := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "shard", Namespace: metav1.NamespaceDefault}} Eventually(ctx, func(g Gomega) { - g.Expect(Get(deployment)(ctx)).To(Succeed()) + g.Expect(Get(deployment)()).To(Succeed()) g.Expect(deployment.Spec.Replicas).To(PointTo(BeEquivalentTo(3))) g.Expect(deployment.Status.AvailableReplicas).To(BeEquivalentTo(3)) }).Should(Succeed()) @@ -60,7 +60,7 @@ var _ = Describe("Example Shard", Label("example"), Ordered, func() { Eventually(ctx, func(g Gomega) { g.Expect(List(leaseList, client.InNamespace(metav1.NamespaceDefault), client.MatchingLabels{ shardingv1alpha1.LabelControllerRing: controllerRingName, - })(ctx)).To(Succeed()) + })()).To(Succeed()) g.Expect(leaseList.Items).To(And( HaveLen(3), HaveEach(HaveLabelWithValue(shardingv1alpha1.LabelState, "ready")), @@ -70,7 +70,7 @@ var _ = Describe("Example Shard", Label("example"), Ordered, func() { It("the ControllerRing should be healthy", func(ctx SpecContext) { Eventually(ctx, func(g Gomega) { - g.Expect(Get(controllerRing)(ctx)).To(Succeed()) + g.Expect(Get(controllerRing)()).To(Succeed()) g.Expect(controllerRing.Status.Shards).To(BeEquivalentTo(3)) g.Expect(controllerRing.Status.AvailableShards).To(BeEquivalentTo(3)) g.Expect(controllerRing.Status.Conditions).To(ConsistOf( From d2ab6423982acbe8a586b84fb2efb8b98f036776 Mon Sep 17 00:00:00 2001 From: Tim Ebert Date: Fri, 21 Feb 2025 00:01:36 +0100 Subject: [PATCH 7/8] Fix `EachListItemWithAlloc` and use it --- pkg/controller/sharder/reconciler.go | 2 +- pkg/utils/pager/pager.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/controller/sharder/reconciler.go b/pkg/controller/sharder/reconciler.go index fac89394..512b9e76 100644 --- a/pkg/controller/sharder/reconciler.go +++ b/pkg/controller/sharder/reconciler.go @@ -166,7 +166,7 @@ func (r *Reconciler) resyncResource( list := &metav1.PartialObjectMetadataList{} list.SetGroupVersionKind(gvks[0]) - err = pager.New(r.Reader).EachListItem(ctx, list, + err = pager.New(r.Reader).EachListItemWithAlloc(ctx, list, func(obj client.Object) error { if !namespaces.Has(obj.GetNamespace()) { return nil diff --git a/pkg/utils/pager/pager.go b/pkg/utils/pager/pager.go index 51159db5..7736cc54 100644 --- a/pkg/utils/pager/pager.go +++ b/pkg/utils/pager/pager.go @@ -68,7 +68,7 @@ type ListPager struct { // ListPager.PageBufferSize chunks buffered concurrently in the background. // // If items passed to fn are retained for different durations, and you want to avoid -// retaining the whole slice returned by p.PageFn as long as any item is referenced, +// retaining the whole slice returned by p.Reader.List as long as any item is referenced, // use EachListItemWithAlloc instead. func (p *ListPager) EachListItem(ctx context.Context, list client.ObjectList, fn func(obj client.Object) error, opts ...client.ListOption) error { return p.eachListChunkBuffered(ctx, list, func(list client.ObjectList) error { @@ -78,13 +78,13 @@ func (p *ListPager) EachListItem(ctx context.Context, list client.ObjectList, fn }, opts...) } -// EachListItemWithAlloc works like EachListItem, but avoids retaining references to the items slice returned by p.PageFn. -// It does this by making a shallow copy of non-pointer items in the slice returned by p.PageFn. +// EachListItemWithAlloc works like EachListItem, but avoids retaining references to the items slice returned by p.Reader.List. +// It does this by making a shallow copy of non-pointer items in the slice returned by p.Reader.List. // // If the items passed to fn are not retained, or are retained for the same duration, use EachListItem instead for memory efficiency. func (p *ListPager) EachListItemWithAlloc(ctx context.Context, list client.ObjectList, fn func(obj client.Object) error, opts ...client.ListOption) error { return p.eachListChunkBuffered(ctx, list, func(list client.ObjectList) error { - return meta.EachListItem(list, func(obj runtime.Object) error { + return meta.EachListItemWithAlloc(list, func(obj runtime.Object) error { return fn(obj.(client.Object)) }) }, opts...) From aa48bf5e264d0c75ebb538e8667c1d20ff027907 Mon Sep 17 00:00:00 2001 From: Tim Ebert Date: Fri, 21 Feb 2025 00:02:23 +0100 Subject: [PATCH 8/8] Add unit tests for `pkg/utils/pager` --- pkg/utils/pager/pager.go | 9 +- pkg/utils/pager/pager_suite_test.go | 29 +++ pkg/utils/pager/pager_test.go | 287 ++++++++++++++++++++++++++++ pkg/utils/test/matchers/object.go | 5 + 4 files changed, 328 insertions(+), 2 deletions(-) create mode 100644 pkg/utils/pager/pager_suite_test.go create mode 100644 pkg/utils/pager/pager_test.go diff --git a/pkg/utils/pager/pager.go b/pkg/utils/pager/pager.go index 7736cc54..2d12f3a6 100644 --- a/pkg/utils/pager/pager.go +++ b/pkg/utils/pager/pager.go @@ -32,8 +32,13 @@ const ( defaultPageBufferSize = 10 ) +// lister is the subset of client.Reader that ListPager uses. +type lister interface { + List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error +} + // New creates a new pager from the provided reader using the default options. -func New(reader client.Reader) *ListPager { +func New(reader lister) *ListPager { return &ListPager{ Reader: reader, PageSize: defaultPageSize, @@ -48,7 +53,7 @@ func New(reader client.Reader) *ListPager { // Exception: this ListPager also fixes the `specifying resource version is not allowed when using continue` error // in EachListItem and EachListItemWithAlloc. type ListPager struct { - Reader client.Reader + Reader lister // PageSize is the maximum number of objects to retrieve in individual list calls. // If a client.Limit option is passed, the pager uses the option's value instead. diff --git a/pkg/utils/pager/pager_suite_test.go b/pkg/utils/pager/pager_suite_test.go new file mode 100644 index 00000000..4df20af3 --- /dev/null +++ b/pkg/utils/pager/pager_suite_test.go @@ -0,0 +1,29 @@ +/* +Copyright 2025 Tim Ebert. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package pager_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestPager(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Pager Suite") +} diff --git a/pkg/utils/pager/pager_test.go b/pkg/utils/pager/pager_test.go new file mode 100644 index 00000000..c6b7a8ee --- /dev/null +++ b/pkg/utils/pager/pager_test.go @@ -0,0 +1,287 @@ +/* +Copyright 2025 Tim Ebert. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package pager_test + +import ( + "context" + "fmt" + "strconv" + "sync" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + gomegatypes "github.com/onsi/gomega/types" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + . "github.com/timebertt/kubernetes-controller-sharding/pkg/utils/pager" + . "github.com/timebertt/kubernetes-controller-sharding/pkg/utils/test/matchers" +) + +var _ = Describe("ListPager", func() { + const ( + pageSize = 2 + pageCount = 5 + objectCount = 9 + ) + + var ( + ctx context.Context + + allPods []corev1.Pod + + reader *lister + pager *ListPager + ) + + BeforeEach(func() { + ctx = context.Background() + + allPods = podSlice(0, objectCount) + + reader = &lister{allPods: allPods} + + pager = New(reader) + pager.PageSize = pageSize + pager.PageBufferSize = 2 + }) + + It("should page all objects", func() { + var pods []*corev1.Pod + + Expect(pager.EachListItem(ctx, &corev1.PodList{}, func(obj client.Object) error { + pods = append(pods, obj.(*corev1.Pod)) + return nil + })).To(Succeed()) + + Expect(pods).To(havePods(1, objectCount)) + + Expect(reader.calls).To(Equal(pageCount)) + }) + + It("should page objects with the custom limit", func() { + var pods []client.Object + + i := 0 + Expect(pager.EachListItem(ctx, &corev1.PodList{}, func(obj client.Object) error { + if obj != &allPods[i] { + return fmt.Errorf("the pager should reuse the object") + } + i++ + pods = append(pods, obj) + return nil + }, client.Limit(objectCount+1))).To(Succeed()) + + Expect(pods).To(havePods(1, objectCount)) + + Expect(reader.calls).To(Equal(1)) + }) + + It("should fail due to negative page size", func() { + pager.PageSize = -1 + Expect(pager.EachListItem(ctx, &corev1.PodList{}, func(obj client.Object) error { + return nil + })).To(MatchError(ContainSubstring("PageSize must be >= 0"))) + }) + + It("should fail due to negative page buffer size", func() { + pager.PageBufferSize = -1 + Expect(pager.EachListItem(ctx, &corev1.PodList{}, func(obj client.Object) error { + return nil + })).To(MatchError(ContainSubstring("PageBufferSize must be >= 0"))) + }) + + It("should return the lister error", func() { + Expect(pager.EachListItem(ctx, &corev1.ConfigMapList{}, func(obj client.Object) error { + return nil + })).To(MatchError("expected *corev1.PodList, got *v1.ConfigMapList")) + }) + + It("should return the iterator error", func() { + Expect(pager.EachListItem(ctx, &corev1.PodList{}, func(obj client.Object) error { + if obj.GetName() == "pod-5" { + return fmt.Errorf("foo") + } + return nil + })).To(MatchError("foo")) + }) + + It("should cancel the operation when the context is canceled", func(specCtx SpecContext) { + done := make(chan struct{}) + + blockConsumer := make(chan struct{}) + defer close(blockConsumer) + + ctx, cancel := context.WithCancel(specCtx) + go func() { + defer GinkgoRecover() + + Expect(pager.EachListItem(ctx, &corev1.PodList{}, func(obj client.Object) error { + <-blockConsumer + return nil + })).To(MatchError(context.Canceled)) + + close(done) + }() + + cancel() + Eventually(specCtx, done).Should(BeClosed()) + }, SpecTimeout(time.Second)) + + It("should buffer the configured number of pages", func(specCtx SpecContext) { + done := make(chan struct{}) + + ctx, cancel := context.WithCancel(specCtx) + go func() { + defer GinkgoRecover() + + Expect(pager.EachListItem(ctx, &corev1.PodList{}, func(obj client.Object) error { + <-ctx.Done() + return nil + })).To(MatchError(context.Canceled)) + + close(done) + }() + + // consumer takes one chunk out and one chunk is produced but blocked, + // so we have made PageBufferSize + 2 calls to the lister + Eventually(specCtx, reader.getCalls).Should(BeEquivalentTo(pager.PageBufferSize + 2)) + + cancel() + Eventually(specCtx, done).Should(BeClosed()) + }, SpecTimeout(time.Second)) + + It("should correctly handle the resourceVersion fields", func() { + Expect(pager.EachListItem(ctx, &corev1.PodList{}, func(obj client.Object) error { + return nil + }, &client.ListOptions{Raw: &metav1.ListOptions{ + ResourceVersion: "0", + ResourceVersionMatch: "NotOlderThan", + }})).To(Succeed()) + }) + + Describe("#EachListItemWithAlloc", func() { + It("should page all objects", func() { + var pods []client.Object + + i := 0 + Expect(pager.EachListItemWithAlloc(ctx, &corev1.PodList{}, func(obj client.Object) error { + if obj == &allPods[i] { + return fmt.Errorf("the pager should copy the object") + } + i++ + pods = append(pods, obj) + return nil + })).To(Succeed()) + + Expect(pods).To(havePods(1, objectCount)) + + Expect(reader.calls).To(Equal(pageCount)) + }) + }) +}) + +type lister struct { + mu sync.Mutex + calls int + + allPods []corev1.Pod + previousList client.ObjectList +} + +func (l *lister) List(_ context.Context, list client.ObjectList, opts ...client.ListOption) error { + func() { + l.mu.Lock() + defer l.mu.Unlock() + l.calls++ + }() + + if list == l.previousList { + return fmt.Errorf("the pager should not reuse the list for multiple calls") + } + l.previousList = list + + podList, ok := list.(*corev1.PodList) + if !ok { + return fmt.Errorf("expected *corev1.PodList, got %T", list) + } + + listOptions := &client.ListOptions{} + listOptions.ApplyOptions(opts) + + if l.calls > 1 { + if listOptions.Raw.ResourceVersion != "" { + return fmt.Errorf("the pager should reset the resourceVersion field for consecutive calls") + } + if listOptions.Raw.ResourceVersionMatch != "" { + return fmt.Errorf("the pager should reset the resourceVersionMatch field for consecutive calls") + } + } + + limit := listOptions.Limit + if limit == 0 { + return fmt.Errorf("the pager should set limit") + } + + var offset int64 + if listOptions.Continue != "" { + var err error + offset, err = strconv.ParseInt(listOptions.Continue, 10, 64) + if err != nil { + return err + } + } + + defer func() { + if offset+limit >= int64(len(l.allPods)) { + podList.Continue = "" + } else { + podList.Continue = strconv.FormatInt(offset+int64(len(podList.Items)), 10) + } + }() + + podList.Items = l.allPods[offset:min(int64(len(l.allPods)), offset+limit)] + return nil +} + +func (l *lister) getCalls() int { + l.mu.Lock() + defer l.mu.Unlock() + return l.calls +} + +func podSlice(offset, n int64) []corev1.Pod { + pods := make([]corev1.Pod, n) + + for i := int64(0); i < n; i++ { + pods[i] = corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "pod-" + strconv.FormatInt(offset+i+1, 10)}} + } + + return pods +} + +func havePods(i, j int) gomegatypes.GomegaMatcher { + var matchers []any + + for ; i <= j; i++ { + matchers = append(matchers, HaveName("pod-"+strconv.Itoa(i))) + } + + return HaveExactElements(matchers...) +} diff --git a/pkg/utils/test/matchers/object.go b/pkg/utils/test/matchers/object.go index 3c224292..deab6452 100644 --- a/pkg/utils/test/matchers/object.go +++ b/pkg/utils/test/matchers/object.go @@ -21,6 +21,11 @@ import ( gomegatypes "github.com/onsi/gomega/types" ) +// HaveName succeeds if the actual object has a matching name. +func HaveName(name interface{}) gomegatypes.GomegaMatcher { + return HaveField("ObjectMeta.Name", name) +} + // HaveLabel succeeds if the actual object has a label with a matching key. func HaveLabel(key interface{}) gomegatypes.GomegaMatcher { return HaveField("ObjectMeta.Labels", HaveKey(key))