diff --git a/cmd/gardenlet/app/app.go b/cmd/gardenlet/app/app.go index 30993f16d22..f40dbe289f1 100644 --- a/cmd/gardenlet/app/app.go +++ b/cmd/gardenlet/app/app.go @@ -358,7 +358,7 @@ func (g *garden) Start(ctx context.Context) error { // and can ensure the required policies are created. // TODO(timuthy, rfranzke): To be removed in a future release. log.Info("Migrating all relevant shoot control plane services to create required network policies") - if err := g.migrateAllShootServicesForNetworkPolicies(ctx, log); err != nil { + if err := g.migrateAllShootServicesForNetworkPolicies(ctx); err != nil { return err } @@ -453,7 +453,7 @@ func (g *garden) registerSeed(ctx context.Context, gardenClient client.Client) e }) } -func (g *garden) migrateAllShootServicesForNetworkPolicies(ctx context.Context, log logr.Logger) error { +func (g *garden) migrateAllShootServicesForNetworkPolicies(ctx context.Context) error { var taskFns []flow.TaskFn // kube-apiserver services diff --git a/extensions/pkg/controller/controlplane/genericactuator/actuator_test.go b/extensions/pkg/controller/controlplane/genericactuator/actuator_test.go index 3c80766175d..2e52a68981e 100644 --- a/extensions/pkg/controller/controlplane/genericactuator/actuator_test.go +++ b/extensions/pkg/controller/controlplane/genericactuator/actuator_test.go @@ -413,7 +413,11 @@ webhooks: vp.EXPECT().GetStorageClassesChartValues(ctx, cp, cluster).Return(storageClassesChartValues, nil) // Handle shoot access secrets and legacy secret cleanup - c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, shootAccessSecretsFunc(namespace)[0].Secret.Name), gomock.AssignableToTypeOf(&corev1.Secret{})) + c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, shootAccessSecretsFunc(namespace)[0].Secret.Name), gomock.AssignableToTypeOf(&corev1.Secret{})). + Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, _ ...client.GetOption) { + obj.SetResourceVersion("0") + }) + c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Secret{}), gomock.Any()). Do(func(ctx context.Context, obj client.Object, _ client.Patch, _ ...client.PatchOption) { Expect(obj).To(DeepEqual(&corev1.Secret{ @@ -427,6 +431,7 @@ webhooks: Labels: map[string]string{ "resources.gardener.cloud/purpose": "token-requestor", }, + ResourceVersion: "0", }, Type: corev1.SecretTypeOpaque, })) @@ -539,7 +544,10 @@ webhooks: vp.EXPECT().GetControlPlaneExposureChartValues(ctx, cpExposure, cluster, gomock.Any(), exposureChecksums).Return(controlPlaneExposureChartValues, nil) // Handle shoot access secrets and legacy secret cleanup - c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, exposureShootAccessSecretsFunc(namespace)[0].Secret.Name), gomock.AssignableToTypeOf(&corev1.Secret{})) + c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, exposureShootAccessSecretsFunc(namespace)[0].Secret.Name), gomock.AssignableToTypeOf(&corev1.Secret{})). + Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, _ ...client.GetOption) { + obj.SetResourceVersion("0") + }) c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Secret{}), gomock.Any()). Do(func(ctx context.Context, obj client.Object, _ client.Patch, _ ...client.PatchOption) { Expect(obj).To(DeepEqual(&corev1.Secret{ @@ -553,6 +561,7 @@ webhooks: Labels: map[string]string{ "resources.gardener.cloud/purpose": "token-requestor", }, + ResourceVersion: "0", }, Type: corev1.SecretTypeOpaque, })) diff --git a/pkg/component/clusterautoscaler/cluster_autoscaler_test.go b/pkg/component/clusterautoscaler/cluster_autoscaler_test.go index 3d96c82d7d5..d7c657a9ef0 100644 --- a/pkg/component/clusterautoscaler/cluster_autoscaler_test.go +++ b/pkg/component/clusterautoscaler/cluster_autoscaler_test.go @@ -227,6 +227,7 @@ var _ = Describe("ClusterAutoscaler", func() { Labels: map[string]string{ "resources.gardener.cloud/purpose": "token-requestor", }, + ResourceVersion: "0", }, Type: corev1.SecretTypeOpaque, } @@ -620,7 +621,10 @@ subjects: c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&rbacv1.ClusterRoleBinding{}), gomock.Any()), c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, serviceName), gomock.AssignableToTypeOf(&corev1.Service{})), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Service{}), gomock.Any()), - c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secret.Name), gomock.AssignableToTypeOf(&corev1.Secret{})), + c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secretName), gomock.AssignableToTypeOf(&corev1.Secret{})). + Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, _ ...client.GetOption) { + obj.SetResourceVersion("0") + }), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Secret{}), gomock.Any()).Return(fakeErr), ) @@ -635,7 +639,10 @@ subjects: c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&rbacv1.ClusterRoleBinding{}), gomock.Any()), c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, serviceName), gomock.AssignableToTypeOf(&corev1.Service{})), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Service{}), gomock.Any()), - c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secret.Name), gomock.AssignableToTypeOf(&corev1.Secret{})), + c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secretName), gomock.AssignableToTypeOf(&corev1.Secret{})). + Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, _ ...client.GetOption) { + obj.SetResourceVersion("0") + }), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Secret{}), gomock.Any()), c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, deploymentName), gomock.AssignableToTypeOf(&appsv1.Deployment{})), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&appsv1.Deployment{}), gomock.Any()).Return(fakeErr), @@ -652,7 +659,10 @@ subjects: c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&rbacv1.ClusterRoleBinding{}), gomock.Any()), c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, serviceName), gomock.AssignableToTypeOf(&corev1.Service{})), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Service{}), gomock.Any()), - c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secret.Name), gomock.AssignableToTypeOf(&corev1.Secret{})), + c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secretName), gomock.AssignableToTypeOf(&corev1.Secret{})). + Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, _ ...client.GetOption) { + obj.SetResourceVersion("0") + }), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Secret{}), gomock.Any()), c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, deploymentName), gomock.AssignableToTypeOf(&appsv1.Deployment{})), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&appsv1.Deployment{}), gomock.Any()), @@ -671,7 +681,10 @@ subjects: c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&rbacv1.ClusterRoleBinding{}), gomock.Any()), c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, serviceName), gomock.AssignableToTypeOf(&corev1.Service{})), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Service{}), gomock.Any()), - c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secret.Name), gomock.AssignableToTypeOf(&corev1.Secret{})), + c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secretName), gomock.AssignableToTypeOf(&corev1.Secret{})). + Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, _ ...client.GetOption) { + obj.SetResourceVersion("0") + }), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Secret{}), gomock.Any()), c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, deploymentName), gomock.AssignableToTypeOf(&appsv1.Deployment{})), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&appsv1.Deployment{}), gomock.Any()), @@ -692,7 +705,10 @@ subjects: c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&rbacv1.ClusterRoleBinding{}), gomock.Any()), c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, serviceName), gomock.AssignableToTypeOf(&corev1.Service{})), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Service{}), gomock.Any()), - c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secret.Name), gomock.AssignableToTypeOf(&corev1.Secret{})), + c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secretName), gomock.AssignableToTypeOf(&corev1.Secret{})). + Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, _ ...client.GetOption) { + obj.SetResourceVersion("0") + }), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Secret{}), gomock.Any()), c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, deploymentName), gomock.AssignableToTypeOf(&appsv1.Deployment{})), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&appsv1.Deployment{}), gomock.Any()), @@ -715,7 +731,10 @@ subjects: c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&rbacv1.ClusterRoleBinding{}), gomock.Any()), c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, serviceName), gomock.AssignableToTypeOf(&corev1.Service{})), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Service{}), gomock.Any()), - c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secret.Name), gomock.AssignableToTypeOf(&corev1.Secret{})), + c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secretName), gomock.AssignableToTypeOf(&corev1.Secret{})). + Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, _ ...client.GetOption) { + obj.SetResourceVersion("0") + }), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Secret{}), gomock.Any()), c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, deploymentName), gomock.AssignableToTypeOf(&appsv1.Deployment{})), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&appsv1.Deployment{}), gomock.Any()), @@ -759,7 +778,10 @@ subjects: Do(func(ctx context.Context, obj client.Object, _ client.Patch, _ ...client.PatchOption) { Expect(obj).To(DeepEqual(service)) }), - c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secretName), gomock.AssignableToTypeOf(&corev1.Secret{})), + c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secretName), gomock.AssignableToTypeOf(&corev1.Secret{})). + Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, _ ...client.GetOption) { + obj.SetResourceVersion("0") + }), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Secret{}), gomock.Any()). Do(func(ctx context.Context, obj client.Object, _ client.Patch, _ ...client.PatchOption) { Expect(obj).To(DeepEqual(secret)) diff --git a/pkg/component/kubecontrollermanager/kube_controller_manager_test.go b/pkg/component/kubecontrollermanager/kube_controller_manager_test.go index 6209ba4d54d..c652a0a723a 100644 --- a/pkg/component/kubecontrollermanager/kube_controller_manager_test.go +++ b/pkg/component/kubecontrollermanager/kube_controller_manager_test.go @@ -193,7 +193,10 @@ var _ = Describe("KubeControllerManager", func() { gomock.InOrder( c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, serviceName), gomock.AssignableToTypeOf(&corev1.Service{})), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Service{}), gomock.Any()), - c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secretName), gomock.AssignableToTypeOf(&corev1.Secret{})), + c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secretName), gomock.AssignableToTypeOf(&corev1.Secret{})). + Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, _ ...client.GetOption) { + obj.SetResourceVersion("0") + }), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Secret{}), gomock.Any()).Return(fakeErr), ) @@ -204,7 +207,10 @@ var _ = Describe("KubeControllerManager", func() { gomock.InOrder( c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, serviceName), gomock.AssignableToTypeOf(&corev1.Service{})), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Service{}), gomock.Any()), - c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secretName), gomock.AssignableToTypeOf(&corev1.Secret{})), + c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secretName), gomock.AssignableToTypeOf(&corev1.Secret{})). + Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, _ ...client.GetOption) { + obj.SetResourceVersion("0") + }), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Secret{}), gomock.Any()), c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, v1beta1constants.DeploymentNameKubeControllerManager), gomock.AssignableToTypeOf(&appsv1.Deployment{})), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&appsv1.Deployment{}), gomock.Any()).Return(fakeErr), @@ -217,7 +223,10 @@ var _ = Describe("KubeControllerManager", func() { gomock.InOrder( c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, serviceName), gomock.AssignableToTypeOf(&corev1.Service{})), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Service{}), gomock.Any()), - c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secretName), gomock.AssignableToTypeOf(&corev1.Secret{})), + c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secretName), gomock.AssignableToTypeOf(&corev1.Secret{})). + Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, _ ...client.GetOption) { + obj.SetResourceVersion("0") + }), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Secret{}), gomock.Any()), c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, v1beta1constants.DeploymentNameKubeControllerManager), gomock.AssignableToTypeOf(&appsv1.Deployment{})), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&appsv1.Deployment{}), gomock.Any()), @@ -232,7 +241,10 @@ var _ = Describe("KubeControllerManager", func() { gomock.InOrder( c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, serviceName), gomock.AssignableToTypeOf(&corev1.Service{})), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Service{}), gomock.Any()), - c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secretName), gomock.AssignableToTypeOf(&corev1.Secret{})), + c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secretName), gomock.AssignableToTypeOf(&corev1.Secret{})). + Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, _ ...client.GetOption) { + obj.SetResourceVersion("0") + }), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Secret{}), gomock.Any()), c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, v1beta1constants.DeploymentNameKubeControllerManager), gomock.AssignableToTypeOf(&appsv1.Deployment{})), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&appsv1.Deployment{}), gomock.Any()), @@ -248,7 +260,10 @@ var _ = Describe("KubeControllerManager", func() { gomock.InOrder( c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, serviceName), gomock.AssignableToTypeOf(&corev1.Service{})), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Service{}), gomock.Any()), - c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secretName), gomock.AssignableToTypeOf(&corev1.Secret{})), + c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secretName), gomock.AssignableToTypeOf(&corev1.Secret{})). + Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, _ ...client.GetOption) { + obj.SetResourceVersion("0") + }), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Secret{}), gomock.Any()), c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, v1beta1constants.DeploymentNameKubeControllerManager), gomock.AssignableToTypeOf(&appsv1.Deployment{})), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&appsv1.Deployment{}), gomock.Any()), @@ -266,7 +281,10 @@ var _ = Describe("KubeControllerManager", func() { gomock.InOrder( c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, serviceName), gomock.AssignableToTypeOf(&corev1.Service{})), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Service{}), gomock.Any()), - c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secretName), gomock.AssignableToTypeOf(&corev1.Secret{})), + c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secretName), gomock.AssignableToTypeOf(&corev1.Secret{})). + Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, _ ...client.GetOption) { + obj.SetResourceVersion("0") + }), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Secret{}), gomock.Any()), c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, v1beta1constants.DeploymentNameKubeControllerManager), gomock.AssignableToTypeOf(&appsv1.Deployment{})), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&appsv1.Deployment{}), gomock.Any()), @@ -286,7 +304,10 @@ var _ = Describe("KubeControllerManager", func() { gomock.InOrder( c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, serviceName), gomock.AssignableToTypeOf(&corev1.Service{})), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Service{}), gomock.Any()), - c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secretName), gomock.AssignableToTypeOf(&corev1.Secret{})), + c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secretName), gomock.AssignableToTypeOf(&corev1.Secret{})). + Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, _ ...client.GetOption) { + obj.SetResourceVersion("0") + }), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Secret{}), gomock.Any()), c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, v1beta1constants.DeploymentNameKubeControllerManager), gomock.AssignableToTypeOf(&appsv1.Deployment{})), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&appsv1.Deployment{}), gomock.Any()), @@ -347,6 +368,7 @@ var _ = Describe("KubeControllerManager", func() { Labels: map[string]string{ "resources.gardener.cloud/purpose": "token-requestor", }, + ResourceVersion: "0", }, Type: corev1.SecretTypeOpaque, } @@ -735,7 +757,10 @@ subjects: Do(func(ctx context.Context, obj client.Object, _ client.Patch, _ ...client.PatchOption) { Expect(obj).To(DeepEqual(serviceFor(version))) }), - c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secretName), gomock.AssignableToTypeOf(&corev1.Secret{})), + c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secretName), gomock.AssignableToTypeOf(&corev1.Secret{})). + Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, _ ...client.GetOption) { + obj.SetResourceVersion("0") + }), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Secret{}), gomock.Any()). Do(func(ctx context.Context, obj client.Object, _ client.Patch, _ ...client.PatchOption) { Expect(obj).To(DeepEqual(secret)) @@ -849,7 +874,10 @@ subjects: Do(func(ctx context.Context, obj client.Object, _ client.Patch, _ ...client.PatchOption) { Expect(obj).To(DeepEqual(serviceFor(version))) }), - c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secretName), gomock.AssignableToTypeOf(&corev1.Secret{})), + c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secretName), gomock.AssignableToTypeOf(&corev1.Secret{})). + Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, _ ...client.GetOption) { + obj.SetResourceVersion("0") + }), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Secret{}), gomock.Any()). Do(func(ctx context.Context, obj client.Object, _ client.Patch, _ ...client.PatchOption) { Expect(obj).To(DeepEqual(secret)) diff --git a/pkg/component/kubescheduler/kube_scheduler_test.go b/pkg/component/kubescheduler/kube_scheduler_test.go index 5ef227df1a5..77c5504d6c0 100644 --- a/pkg/component/kubescheduler/kube_scheduler_test.go +++ b/pkg/component/kubescheduler/kube_scheduler_test.go @@ -111,6 +111,7 @@ var _ = Describe("KubeScheduler", func() { Labels: map[string]string{ "resources.gardener.cloud/purpose": "token-requestor", }, + ResourceVersion: "0", }, Type: corev1.SecretTypeOpaque, } @@ -435,7 +436,10 @@ subjects: c.EXPECT().Create(ctx, configMapFor("testdata/component-config-1.23.yaml")), c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, serviceName), gomock.AssignableToTypeOf(&corev1.Service{})), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Service{}), gomock.Any()), - c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secret.Name), gomock.AssignableToTypeOf(&corev1.Secret{})), + c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secret.Name), gomock.AssignableToTypeOf(&corev1.Secret{})). + Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, _ ...client.GetOption) { + obj.SetResourceVersion("0") + }), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Secret{}), gomock.Any()).Return(fakeErr), ) @@ -447,7 +451,10 @@ subjects: c.EXPECT().Create(ctx, configMapFor("testdata/component-config-1.23.yaml")), c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, serviceName), gomock.AssignableToTypeOf(&corev1.Service{})), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Service{}), gomock.Any()), - c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secret.Name), gomock.AssignableToTypeOf(&corev1.Secret{})), + c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secretName), gomock.AssignableToTypeOf(&corev1.Secret{})). + Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, _ ...client.GetOption) { + obj.SetResourceVersion("0") + }), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Secret{}), gomock.Any()), c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, deploymentName), gomock.AssignableToTypeOf(&appsv1.Deployment{})), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&appsv1.Deployment{}), gomock.Any()).Return(fakeErr), @@ -461,7 +468,10 @@ subjects: c.EXPECT().Create(ctx, configMapFor("testdata/component-config-1.23.yaml")), c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, serviceName), gomock.AssignableToTypeOf(&corev1.Service{})), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Service{}), gomock.Any()), - c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secret.Name), gomock.AssignableToTypeOf(&corev1.Secret{})), + c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secretName), gomock.AssignableToTypeOf(&corev1.Secret{})). + Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, _ ...client.GetOption) { + obj.SetResourceVersion("0") + }), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Secret{}), gomock.Any()), c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, deploymentName), gomock.AssignableToTypeOf(&appsv1.Deployment{})), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&appsv1.Deployment{}), gomock.Any()), @@ -477,7 +487,10 @@ subjects: c.EXPECT().Create(ctx, configMapFor("testdata/component-config-1.23.yaml")), c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, serviceName), gomock.AssignableToTypeOf(&corev1.Service{})), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Service{}), gomock.Any()), - c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secret.Name), gomock.AssignableToTypeOf(&corev1.Secret{})), + c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secretName), gomock.AssignableToTypeOf(&corev1.Secret{})). + Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, _ ...client.GetOption) { + obj.SetResourceVersion("0") + }), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Secret{}), gomock.Any()), c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, deploymentName), gomock.AssignableToTypeOf(&appsv1.Deployment{})), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&appsv1.Deployment{}), gomock.Any()), @@ -495,7 +508,10 @@ subjects: c.EXPECT().Create(ctx, configMapFor("testdata/component-config-1.23.yaml")), c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, serviceName), gomock.AssignableToTypeOf(&corev1.Service{})), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Service{}), gomock.Any()), - c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secret.Name), gomock.AssignableToTypeOf(&corev1.Secret{})), + c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secret.Name), gomock.AssignableToTypeOf(&corev1.Secret{})). + Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, _ ...client.GetOption) { + obj.SetResourceVersion("0") + }), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Secret{}), gomock.Any()), c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, deploymentName), gomock.AssignableToTypeOf(&appsv1.Deployment{})), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&appsv1.Deployment{}), gomock.Any()), @@ -515,7 +531,10 @@ subjects: c.EXPECT().Create(ctx, configMapFor("testdata/component-config-1.23.yaml")), c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, serviceName), gomock.AssignableToTypeOf(&corev1.Service{})), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Service{}), gomock.Any()), - c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secret.Name), gomock.AssignableToTypeOf(&corev1.Secret{})), + c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secret.Name), gomock.AssignableToTypeOf(&corev1.Secret{})). + Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, _ ...client.GetOption) { + obj.SetResourceVersion("0") + }), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Secret{}), gomock.Any()), c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, deploymentName), gomock.AssignableToTypeOf(&appsv1.Deployment{})), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&appsv1.Deployment{}), gomock.Any()), @@ -549,7 +568,10 @@ subjects: Do(func(ctx context.Context, obj client.Object, _ client.Patch, _ ...client.PatchOption) { Expect(obj).To(DeepEqual(serviceFor(version))) }), - c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secretName), gomock.AssignableToTypeOf(&corev1.Secret{})), + c.EXPECT().Get(ctx, kubernetesutils.Key(namespace, secret.Name), gomock.AssignableToTypeOf(&corev1.Secret{})). + Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, _ ...client.GetOption) { + obj.SetResourceVersion("0") + }), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Secret{}), gomock.Any()). Do(func(ctx context.Context, obj client.Object, _ client.Patch, _ ...client.PatchOption) { Expect(obj).To(DeepEqual(secret)) diff --git a/pkg/component/resourcemanager/resource_manager_test.go b/pkg/component/resourcemanager/resource_manager_test.go index 53522d2d272..28a602e68cf 100644 --- a/pkg/component/resourcemanager/resource_manager_test.go +++ b/pkg/component/resourcemanager/resource_manager_test.go @@ -272,6 +272,7 @@ var _ = Describe("ResourceManager", func() { Labels: map[string]string{ "resources.gardener.cloud/purpose": "token-requestor", }, + ResourceVersion: "0", }, Type: corev1.SecretTypeOpaque, } @@ -1886,7 +1887,10 @@ subjects: Context("should successfully deploy all resources (w/ shoot access secret)", func() { JustBeforeEach(func() { gomock.InOrder( - c.EXPECT().Get(ctx, kubernetesutils.Key(deployNamespace, secret.Name), gomock.AssignableToTypeOf(&corev1.Secret{})), + c.EXPECT().Get(ctx, kubernetesutils.Key(deployNamespace, secret.Name), gomock.AssignableToTypeOf(&corev1.Secret{})). + Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, _ ...client.GetOption) { + obj.SetResourceVersion("0") + }), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Secret{}), gomock.Any()). Do(func(ctx context.Context, obj client.Object, _ client.Patch, _ ...client.PatchOption) { Expect(obj).To(DeepEqual(secret)) @@ -1980,7 +1984,10 @@ subjects: deployment = deploymentFor(configMap.Name, cfg.RuntimeKubernetesVersion, &watchedNamespace, pointer.String(gardenerutils.PathGenericKubeconfig), true, &secretNameBootstrapKubeconfig) gomock.InOrder( - c.EXPECT().Get(ctx, kubernetesutils.Key(deployNamespace, secret.Name), gomock.AssignableToTypeOf(&corev1.Secret{})), + c.EXPECT().Get(ctx, kubernetesutils.Key(deployNamespace, secret.Name), gomock.AssignableToTypeOf(&corev1.Secret{})). + Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, _ ...client.GetOption) { + obj.SetResourceVersion("0") + }), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Secret{}), gomock.Any()). Do(func(ctx context.Context, obj client.Object, _ client.Patch, _ ...client.PatchOption) { Expect(obj).To(DeepEqual(secret)) @@ -2078,7 +2085,10 @@ subjects: It("should deploy a ClusterRole allowing access to mr related resources", func() { gomock.InOrder( - c.EXPECT().Get(ctx, kubernetesutils.Key(deployNamespace, secret.Name), gomock.AssignableToTypeOf(&corev1.Secret{})), + c.EXPECT().Get(ctx, kubernetesutils.Key(deployNamespace, secret.Name), gomock.AssignableToTypeOf(&corev1.Secret{})). + Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, _ ...client.GetOption) { + obj.SetResourceVersion("0") + }), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Secret{}), gomock.Any()). Do(func(ctx context.Context, obj client.Object, _ client.Patch, _ ...client.PatchOption) { Expect(obj).To(DeepEqual(secret)) @@ -2136,7 +2146,10 @@ subjects: It("should fail because the ClusterRole can not be created", func() { gomock.InOrder( - c.EXPECT().Get(ctx, kubernetesutils.Key(deployNamespace, secret.Name), gomock.AssignableToTypeOf(&corev1.Secret{})), + c.EXPECT().Get(ctx, kubernetesutils.Key(deployNamespace, secret.Name), gomock.AssignableToTypeOf(&corev1.Secret{})). + Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, _ ...client.GetOption) { + obj.SetResourceVersion("0") + }), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Secret{}), gomock.Any()), c.EXPECT().Get(ctx, kubernetesutils.Key(deployNamespace, "gardener-resource-manager"), gomock.AssignableToTypeOf(&corev1.ServiceAccount{})), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.ServiceAccount{}), gomock.Any()), @@ -2150,7 +2163,10 @@ subjects: It("should fail because the ClusterRoleBinding can not be created", func() { gomock.InOrder( - c.EXPECT().Get(ctx, kubernetesutils.Key(deployNamespace, secret.Name), gomock.AssignableToTypeOf(&corev1.Secret{})), + c.EXPECT().Get(ctx, kubernetesutils.Key(deployNamespace, secret.Name), gomock.AssignableToTypeOf(&corev1.Secret{})). + Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, _ ...client.GetOption) { + obj.SetResourceVersion("0") + }), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Secret{}), gomock.Any()), c.EXPECT().Get(ctx, kubernetesutils.Key(deployNamespace, "gardener-resource-manager"), gomock.AssignableToTypeOf(&corev1.ServiceAccount{})), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.ServiceAccount{}), gomock.Any()), @@ -2180,7 +2196,10 @@ subjects: It("should disable controllers and webhooks properly in resource manager configuration", func() { gomock.InOrder( - c.EXPECT().Get(ctx, kubernetesutils.Key(deployNamespace, secret.Name), gomock.AssignableToTypeOf(&corev1.Secret{})), + c.EXPECT().Get(ctx, kubernetesutils.Key(deployNamespace, secret.Name), gomock.AssignableToTypeOf(&corev1.Secret{})). + Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, _ ...client.GetOption) { + obj.SetResourceVersion("0") + }), c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.Secret{}), gomock.Any()). Do(func(ctx context.Context, obj client.Object, _ client.Patch, _ ...client.PatchOption) { Expect(obj).To(DeepEqual(secret)) diff --git a/pkg/controllerutils/patch.go b/pkg/controllerutils/patch.go index 28013501414..8e89626bd2f 100644 --- a/pkg/controllerutils/patch.go +++ b/pkg/controllerutils/patch.go @@ -16,10 +16,12 @@ package controllerutils import ( "context" + "fmt" apierrors "k8s.io/apimachinery/pkg/api/errors" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + logf "sigs.k8s.io/controller-runtime/pkg/log" ) // patchFn returns a client.Patch with the given client.Object as the base object. @@ -37,6 +39,40 @@ func strategicMergeFrom(obj client.Object, opts ...client.MergeFromOption) clien return client.StrategicMergeFrom(obj, opts...) } +// PatchOptions contains several options used for calculating and sending patch requests. +type PatchOptions struct { + mergeFromOptions []client.MergeFromOption + optimisticLock bool + skipEmptyPatch bool +} + +// PatchOption can be used to define options used for calculating and sending patch requests. +type PatchOption interface { + // ApplyToPatchOptions applies this configuration to the given patch options. + ApplyToPatchOptions(*PatchOptions) +} + +// SkipEmptyPatch is a patch option that causes empty patches not being sent. +type SkipEmptyPatch struct{} + +// ApplyToPatchOptions applies the skipEmptyPatch option to the given PatchOption. +func (SkipEmptyPatch) ApplyToPatchOptions(in *PatchOptions) { + in.skipEmptyPatch = true +} + +// MergeFromOption is a patch option that allows to use a `client.MergeFromOption`. +type MergeFromOption struct { + client.MergeFromOption +} + +// ApplyToPatchOptions applies the `MergeFromOption`s to the given PatchOption. +func (m MergeFromOption) ApplyToPatchOptions(in *PatchOptions) { + if _, ok := m.MergeFromOption.(client.MergeFromWithOptimisticLock); ok { + in.optimisticLock = true + } + in.mergeFromOptions = append(in.mergeFromOptions, m) +} + // GetAndCreateOrMergePatch is similar to controllerutil.CreateOrPatch, but does not care about the object's status section. // It reads the object from the client, reconciles the desired state with the existing state using the given MutateFn // and creates or patches the object (using a merge patch) accordingly. @@ -44,7 +80,7 @@ func strategicMergeFrom(obj client.Object, opts ...client.MergeFromOption) clien // The MutateFn is called regardless of creating or updating an object. // // It returns the executed operation and an error. -func GetAndCreateOrMergePatch(ctx context.Context, c client.Client, obj client.Object, f controllerutil.MutateFn, opts ...client.MergeFromOption) (controllerutil.OperationResult, error) { +func GetAndCreateOrMergePatch(ctx context.Context, c client.Client, obj client.Object, f controllerutil.MutateFn, opts ...PatchOption) (controllerutil.OperationResult, error) { return getAndCreateOrPatch(ctx, c, obj, mergeFrom, f, opts...) } @@ -55,11 +91,26 @@ func GetAndCreateOrMergePatch(ctx context.Context, c client.Client, obj client.O // The MutateFn is called regardless of creating or updating an object. // // It returns the executed operation and an error. -func GetAndCreateOrStrategicMergePatch(ctx context.Context, c client.Client, obj client.Object, f controllerutil.MutateFn, opts ...client.MergeFromOption) (controllerutil.OperationResult, error) { +func GetAndCreateOrStrategicMergePatch(ctx context.Context, c client.Client, obj client.Object, f controllerutil.MutateFn, opts ...PatchOption) (controllerutil.OperationResult, error) { return getAndCreateOrPatch(ctx, c, obj, strategicMergeFrom, f, opts...) } -func getAndCreateOrPatch(ctx context.Context, c client.Client, obj client.Object, patchFunc patchFn, f controllerutil.MutateFn, opts ...client.MergeFromOption) (controllerutil.OperationResult, error) { +func isEmptyPatch(data []byte, optimisticLocking bool, resourceVersion string) bool { + if optimisticLocking { + // Resource version is always set when optimistic locking is used + // see https://github.com/kubernetes-sigs/controller-runtime/blob/e54088c8c7da82111b4508bdaf189c45d1344f00/pkg/client/patch.go#L104 + return string(data) == fmt.Sprintf(`{"metadata":{"resourceVersion":"%s"}}`, resourceVersion) + } + + return string(data) == "{}" +} + +func getAndCreateOrPatch(ctx context.Context, c client.Client, obj client.Object, patchFunc patchFn, f controllerutil.MutateFn, opts ...PatchOption) (controllerutil.OperationResult, error) { + patchOpts := &PatchOptions{} + for _, opt := range opts { + opt.ApplyToPatchOptions(patchOpts) + } + key := client.ObjectKeyFromObject(obj) if err := c.Get(ctx, key, obj); err != nil { if !apierrors.IsNotFound(err) { @@ -74,12 +125,22 @@ func getAndCreateOrPatch(ctx context.Context, c client.Client, obj client.Object return controllerutil.OperationResultCreated, nil } - patch := patchFunc(obj.DeepCopyObject().(client.Object), opts...) + patch := patchFunc(obj.DeepCopyObject().(client.Object), patchOpts.mergeFromOptions...) if err := f(); err != nil { return controllerutil.OperationResultNone, err } - if err := c.Patch(ctx, obj, patch); err != nil { + patchData, err := patch.Data(obj) + if err != nil { + return controllerutil.OperationResultNone, err + } + + if patchOpts.skipEmptyPatch && isEmptyPatch(patchData, patchOpts.optimisticLock, obj.GetResourceVersion()) { + logf.Log.V(1).Info("Skip sending empty patch", "objectKey", client.ObjectKeyFromObject(obj)) + return controllerutil.OperationResultNone, nil + } + + if err := c.Patch(ctx, obj, client.RawPatch(patch.Type(), patchData)); err != nil { return controllerutil.OperationResultNone, err } return controllerutil.OperationResultUpdated, nil @@ -90,7 +151,7 @@ func getAndCreateOrPatch(ctx context.Context, c client.Client, obj client.Object // The MutateFn is called regardless of creating or patching an object. // // It returns the executed operation and an error. -func CreateOrGetAndMergePatch(ctx context.Context, c client.Client, obj client.Object, f controllerutil.MutateFn, opts ...client.MergeFromOption) (controllerutil.OperationResult, error) { +func CreateOrGetAndMergePatch(ctx context.Context, c client.Client, obj client.Object, f controllerutil.MutateFn, opts ...PatchOption) (controllerutil.OperationResult, error) { return createOrGetAndPatch(ctx, c, obj, mergeFrom, f, opts...) } @@ -99,11 +160,16 @@ func CreateOrGetAndMergePatch(ctx context.Context, c client.Client, obj client.O // The MutateFn is called regardless of creating or patching an object. // // It returns the executed operation and an error. -func CreateOrGetAndStrategicMergePatch(ctx context.Context, c client.Client, obj client.Object, f controllerutil.MutateFn, opts ...client.MergeFromOption) (controllerutil.OperationResult, error) { +func CreateOrGetAndStrategicMergePatch(ctx context.Context, c client.Client, obj client.Object, f controllerutil.MutateFn, opts ...PatchOption) (controllerutil.OperationResult, error) { return createOrGetAndPatch(ctx, c, obj, strategicMergeFrom, f, opts...) } -func createOrGetAndPatch(ctx context.Context, c client.Client, obj client.Object, patchFunc patchFn, f controllerutil.MutateFn, opts ...client.MergeFromOption) (controllerutil.OperationResult, error) { +func createOrGetAndPatch(ctx context.Context, c client.Client, obj client.Object, patchFunc patchFn, f controllerutil.MutateFn, opts ...PatchOption) (controllerutil.OperationResult, error) { + patchOpts := &PatchOptions{} + for _, opt := range opts { + opt.ApplyToPatchOptions(patchOpts) + } + if err := f(); err != nil { return controllerutil.OperationResultNone, err } @@ -117,15 +183,24 @@ func createOrGetAndPatch(ctx context.Context, c client.Client, obj client.Object return controllerutil.OperationResultNone, err2 } - patch := patchFunc(obj.DeepCopyObject().(client.Object), opts...) + patch := patchFunc(obj.DeepCopyObject().(client.Object), patchOpts.mergeFromOptions...) if err2 := f(); err2 != nil { return controllerutil.OperationResultNone, err2 } + patchData, err := patch.Data(obj) + if err != nil { + return controllerutil.OperationResultNone, err + } + + if patchOpts.skipEmptyPatch && isEmptyPatch(patchData, patchOpts.optimisticLock, obj.GetResourceVersion()) { + logf.Log.V(1).Info("Skip sending empty patch", "objectKey", client.ObjectKeyFromObject(obj)) + return controllerutil.OperationResultNone, nil + } + if err2 := c.Patch(ctx, obj, patch); err2 != nil { return controllerutil.OperationResultNone, err2 } - return controllerutil.OperationResultUpdated, nil } diff --git a/pkg/controllerutils/patch_test.go b/pkg/controllerutils/patch_test.go index 632ff00408c..75196b3a31a 100644 --- a/pkg/controllerutils/patch_test.go +++ b/pkg/controllerutils/patch_test.go @@ -66,7 +66,7 @@ var _ = Describe("Patch", func() { }) Describe("GetAndCreateOr*Patch", func() { - testSuite := func(f func(ctx context.Context, c client.Client, obj client.Object, f controllerutil.MutateFn, opts ...client.MergeFromOption) (controllerutil.OperationResult, error), patchType types.PatchType) { + testSuite := func(f func(ctx context.Context, c client.Client, obj client.Object, f controllerutil.MutateFn, opts ...PatchOption) (controllerutil.OperationResult, error), patchType types.PatchType) { It("should return an error because reading the object fails", func() { c.EXPECT().Get(ctx, client.ObjectKeyFromObject(obj), obj).Return(fakeErr) @@ -159,10 +159,46 @@ var _ = Describe("Patch", func() { test.EXPECTPatchWithOptimisticLock(ctx, c, objCopy, obj, patchType), ) - result, err := f(ctx, c, obj, mutateFn(obj), client.MergeFromWithOptimisticLock{}) + result, err := f(ctx, c, obj, mutateFn(obj), MergeFromOption{client.MergeFromWithOptimisticLock{}}) Expect(result).To(Equal(controllerutil.OperationResultUpdated)) Expect(err).NotTo(HaveOccurred()) }) + + It("should skip sending an empty patch", func() { + objCopy := obj.DeepCopy() + mutateFn := func(o *corev1.ServiceAccount) func() error { + return func() error { + return nil + } + } + _ = mutateFn(objCopy)() + + gomock.InOrder( + c.EXPECT().Get(ctx, client.ObjectKeyFromObject(obj), obj), + ) + + result, err := f(ctx, c, obj, mutateFn(obj), SkipEmptyPatch{}) + Expect(result).To(Equal(controllerutil.OperationResultNone)) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should skip sending an empty patch with optimistic locking", func() { + objCopy := obj.DeepCopy() + mutateFn := func(o *corev1.ServiceAccount) func() error { + return func() error { + return nil + } + } + _ = mutateFn(objCopy)() + + gomock.InOrder( + c.EXPECT().Get(ctx, client.ObjectKeyFromObject(obj), obj), + ) + + result, err := f(ctx, c, obj, mutateFn(obj), MergeFromOption{client.MergeFromWithOptimisticLock{}}, SkipEmptyPatch{}) + Expect(result).To(Equal(controllerutil.OperationResultNone)) + Expect(err).NotTo(HaveOccurred()) + }) } Describe("#GetAndCreateOrMergePatch", func() { testSuite(GetAndCreateOrMergePatch, types.MergePatchType) }) @@ -170,7 +206,7 @@ var _ = Describe("Patch", func() { }) Describe("CreateOrGetAnd*Patch", func() { - testSuite := func(f func(ctx context.Context, c client.Client, obj client.Object, f controllerutil.MutateFn, opts ...client.MergeFromOption) (controllerutil.OperationResult, error), patchType types.PatchType) { + testSuite := func(f func(ctx context.Context, c client.Client, obj client.Object, f controllerutil.MutateFn, opts ...PatchOption) (controllerutil.OperationResult, error), patchType types.PatchType) { It("should return an error because the mutate function returned an error", func() { result, err := f(ctx, c, obj, func() error { return fakeErr }) Expect(result).To(Equal(controllerutil.OperationResultNone)) @@ -262,10 +298,50 @@ var _ = Describe("Patch", func() { test.EXPECTPatchWithOptimisticLock(ctx, c, objCopy, obj, patchType), ) - result, err := f(ctx, c, obj, mutateFn(obj), client.MergeFromWithOptimisticLock{}) + result, err := f(ctx, c, obj, mutateFn(obj), MergeFromOption{MergeFromOption: client.MergeFromWithOptimisticLock{}}) Expect(result).To(Equal(controllerutil.OperationResultUpdated)) Expect(err).NotTo(HaveOccurred()) }) + + It("should skip sending an empty patch", func() { + objCopy := obj.DeepCopy() + mutateFn := func(o *corev1.ServiceAccount) func() error { + return func() error { return nil } + } + _ = mutateFn(objCopy)() + + gomock.InOrder( + c.EXPECT().Create(ctx, obj).Return(apierrors.NewAlreadyExists(schema.GroupResource{}, "")), + c.EXPECT().Get(ctx, client.ObjectKeyFromObject(obj), obj).DoAndReturn(func(_ context.Context, _ client.ObjectKey, objToReturn *corev1.ServiceAccount, _ ...client.GetOption) error { + obj.DeepCopyInto(objToReturn) + return nil + }), + ) + + result, err := f(ctx, c, obj, mutateFn(obj), SkipEmptyPatch{}) + Expect(result).To(Equal(controllerutil.OperationResultNone)) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should skip sending an empty patch with optimistic locking", func() { + objCopy := obj.DeepCopy() + mutateFn := func(o *corev1.ServiceAccount) func() error { + return func() error { return nil } + } + _ = mutateFn(objCopy)() + + gomock.InOrder( + c.EXPECT().Create(ctx, obj).Return(apierrors.NewAlreadyExists(schema.GroupResource{}, "")), + c.EXPECT().Get(ctx, client.ObjectKeyFromObject(obj), obj).DoAndReturn(func(_ context.Context, _ client.ObjectKey, objToReturn *corev1.ServiceAccount, _ ...client.GetOption) error { + obj.DeepCopyInto(objToReturn) + return nil + }), + ) + + result, err := f(ctx, c, obj, mutateFn(obj), MergeFromOption{MergeFromOption: client.MergeFromWithOptimisticLock{}}, SkipEmptyPatch{}) + Expect(result).To(Equal(controllerutil.OperationResultNone)) + Expect(err).NotTo(HaveOccurred()) + }) } Describe("#CreateOrGetAndMergePatch", func() { testSuite(CreateOrGetAndMergePatch, types.MergePatchType) }) diff --git a/pkg/resourcemanager/controller/networkpolicy/reconciler.go b/pkg/resourcemanager/controller/networkpolicy/reconciler.go index 49013b6229d..a56a1d386a2 100644 --- a/pkg/resourcemanager/controller/networkpolicy/reconciler.go +++ b/pkg/resourcemanager/controller/networkpolicy/reconciler.go @@ -317,7 +317,7 @@ func (r *Reconciler) reconcileIngressPolicy( networkPolicy.Spec.PolicyTypes = []networkingv1.PolicyType{networkingv1.PolicyTypeIngress} return nil - }) + }, controllerutils.SkipEmptyPatch{}) return err } @@ -352,7 +352,7 @@ func (r *Reconciler) reconcileEgressPolicy( networkPolicy.Spec.PolicyTypes = []networkingv1.PolicyType{networkingv1.PolicyTypeEgress} return nil - }) + }, controllerutils.SkipEmptyPatch{}) return err } @@ -378,7 +378,7 @@ func (r *Reconciler) reconcileIngressFromWorldPolicy(ctx context.Context, servic networkPolicy.Spec.PolicyTypes = []networkingv1.PolicyType{networkingv1.PolicyTypeIngress} return nil - }) + }, controllerutils.SkipEmptyPatch{}) return err } diff --git a/pkg/utils/gardener/shoot.go b/pkg/utils/gardener/shoot.go index e4fc1baacf0..8e64b3450db 100644 --- a/pkg/utils/gardener/shoot.go +++ b/pkg/utils/gardener/shoot.go @@ -379,7 +379,7 @@ func (s *ShootAccessSecret) Reconcile(ctx context.Context, c client.Client) erro // The token-requestor might concurrently update the kubeconfig secret key to populate the token. // Hence, we need to use optimistic locking here to ensure we don't accidentally overwrite the concurrent update. // ref https://github.com/gardener/gardener/issues/6092#issuecomment-1156244514 - client.MergeFromWithOptimisticLock{}) + controllerutils.MergeFromOption{MergeFromOption: client.MergeFromWithOptimisticLock{}}) return err }