From f32deb64eda449b9552be33d3789ba3f54f58c95 Mon Sep 17 00:00:00 2001 From: Daniel Lipovetsky Date: Tue, 23 Apr 2024 13:08:48 -0700 Subject: [PATCH 1/2] feat: Preserve user-managed fields when creating namespace --- pkg/handlers/generic/lifecycle/utils/utils.go | 9 +-- .../lifecycle/utils/utils_integration_test.go | 62 +++++++++++++++++++ .../lifecycle/utils/utils_suite_test.go | 17 +++++ 3 files changed, 81 insertions(+), 7 deletions(-) create mode 100644 pkg/handlers/generic/lifecycle/utils/utils_integration_test.go create mode 100644 pkg/handlers/generic/lifecycle/utils/utils_suite_test.go diff --git a/pkg/handlers/generic/lifecycle/utils/utils.go b/pkg/handlers/generic/lifecycle/utils/utils.go index 2daaff4bc..13eaea5e8 100644 --- a/pkg/handlers/generic/lifecycle/utils/utils.go +++ b/pkg/handlers/generic/lifecycle/utils/utils.go @@ -93,7 +93,7 @@ func EnsureCRSForClusterFromObjects( return nil } -// EnsureNamespace will create the namespece if it does not exist. +// EnsureNamespace will create the namespace if it does not exist. func EnsureNamespace(ctx context.Context, c ctrlclient.Client, name string) error { ns := &corev1.Namespace{ TypeMeta: metav1.TypeMeta{ @@ -105,12 +105,7 @@ func EnsureNamespace(ctx context.Context, c ctrlclient.Client, name string) erro }, } - // check if namespace exists and return early if it does - if err := c.Get(ctx, ctrlclient.ObjectKeyFromObject(ns), ns); err == nil { - return nil - } - - err := client.ServerSideApply(ctx, c, ns, client.ForceOwnership) + err := client.ServerSideApply(ctx, c, ns) if err != nil { return fmt.Errorf("failed to server side apply %w", err) } diff --git a/pkg/handlers/generic/lifecycle/utils/utils_integration_test.go b/pkg/handlers/generic/lifecycle/utils/utils_integration_test.go new file mode 100644 index 000000000..c2aed696a --- /dev/null +++ b/pkg/handlers/generic/lifecycle/utils/utils_integration_test.go @@ -0,0 +1,62 @@ +// Copyright 2023 D2iQ, Inc. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package utils + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/test/helpers" +) + +var _ = Describe("Namespace", func() { + It("creates a new namespace", func(ctx SpecContext) { + c, err := helpers.TestEnv.GetK8sClient() + Expect(err).To(BeNil()) + + namespaceName := "new" + + Expect(EnsureNamespace(ctx, c, namespaceName)).To(Succeed()) + Expect(c.Delete(ctx, &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: namespaceName, + }, + })).To((Succeed())) + }) + + It("updates a namespace, preserving user-managed fields", func(ctx SpecContext) { + c, err := helpers.TestEnv.GetK8sClient() + Expect(err).To(BeNil()) + + namespaceName := "existing" + Expect(c.Create(ctx, + &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: namespaceName, + Labels: map[string]string{ + "userkey": "uservalue", + }, + }, + })).To(Succeed()) + + Expect(EnsureNamespace(ctx, c, namespaceName)).To(Succeed()) + + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: namespaceName, + }, + } + Expect(c.Get(ctx, client.ObjectKeyFromObject(ns), ns)).To(Succeed()) + Expect(ns.GetLabels()).To(HaveKeyWithValue("userkey", "uservalue")) + + Expect(c.Delete(ctx, &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: namespaceName, + }, + })).To((Succeed())) + }) +}) diff --git a/pkg/handlers/generic/lifecycle/utils/utils_suite_test.go b/pkg/handlers/generic/lifecycle/utils/utils_suite_test.go new file mode 100644 index 000000000..45c15cdb7 --- /dev/null +++ b/pkg/handlers/generic/lifecycle/utils/utils_suite_test.go @@ -0,0 +1,17 @@ +// Copyright 2023 D2iQ, Inc. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package utils + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +// TestUtils is the entrypoint for integration (envtest) tests. +func TestUtils(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Utils") +} From 808dae79f2b018999371ad4cb061b8c77bb3fd62 Mon Sep 17 00:00:00 2001 From: Jimmi Dyson Date: Wed, 24 Apr 2024 11:54:55 +0100 Subject: [PATCH 2/2] feat: Ability to create namespace by specifying namespace object This also adds tests to ensure that SSA is adhered to properly in this case. --- .../clusterautoscaler/strategy_crs.go | 2 +- .../generic/lifecycle/utils/secrets.go | 2 +- pkg/handlers/generic/lifecycle/utils/utils.go | 15 +++- .../lifecycle/utils/utils_integration_test.go | 85 +++++++++++++------ 4 files changed, 75 insertions(+), 29 deletions(-) diff --git a/pkg/handlers/generic/lifecycle/clusterautoscaler/strategy_crs.go b/pkg/handlers/generic/lifecycle/clusterautoscaler/strategy_crs.go index 27ed3d7c2..da66724fe 100644 --- a/pkg/handlers/generic/lifecycle/clusterautoscaler/strategy_crs.go +++ b/pkg/handlers/generic/lifecycle/clusterautoscaler/strategy_crs.go @@ -105,7 +105,7 @@ func (s crsStrategy) apply( if err != nil { return fmt.Errorf("error creating remote cluster client: %w", err) } - if err = utils.EnsureNamespace(ctx, remoteClient, cluster.Namespace); err != nil { + if err = utils.EnsureNamespaceWithName(ctx, remoteClient, cluster.Namespace); err != nil { return fmt.Errorf( "failed to create Namespace in remote cluster: %w", err, diff --git a/pkg/handlers/generic/lifecycle/utils/secrets.go b/pkg/handlers/generic/lifecycle/utils/secrets.go index bd3c5a9ac..ddee32ffa 100644 --- a/pkg/handlers/generic/lifecycle/utils/secrets.go +++ b/pkg/handlers/generic/lifecycle/utils/secrets.go @@ -59,7 +59,7 @@ func CopySecretToRemoteCluster( return fmt.Errorf("error creating client for remote cluster: %w", err) } - err = EnsureNamespace(ctx, remoteClient, dstSecretKey.Namespace) + err = EnsureNamespaceWithName(ctx, remoteClient, dstSecretKey.Namespace) if err != nil { return fmt.Errorf("error creating namespace on the remote cluster: %w", err) } diff --git a/pkg/handlers/generic/lifecycle/utils/utils.go b/pkg/handlers/generic/lifecycle/utils/utils.go index 13eaea5e8..251b0d069 100644 --- a/pkg/handlers/generic/lifecycle/utils/utils.go +++ b/pkg/handlers/generic/lifecycle/utils/utils.go @@ -93,8 +93,8 @@ func EnsureCRSForClusterFromObjects( return nil } -// EnsureNamespace will create the namespace if it does not exist. -func EnsureNamespace(ctx context.Context, c ctrlclient.Client, name string) error { +// EnsureNamespaceWithName will create the namespace with the specified name if it does not exist. +func EnsureNamespaceWithName(ctx context.Context, c ctrlclient.Client, name string) error { ns := &corev1.Namespace{ TypeMeta: metav1.TypeMeta{ APIVersion: corev1.SchemeGroupVersion.String(), @@ -105,6 +105,17 @@ func EnsureNamespace(ctx context.Context, c ctrlclient.Client, name string) erro }, } + return EnsureNamespace(ctx, c, ns) +} + +// EnsureNamespace will create the namespace if it does not exist. +func EnsureNamespace(ctx context.Context, c ctrlclient.Client, ns *corev1.Namespace) error { + if ns.TypeMeta.APIVersion == "" { + ns.TypeMeta.APIVersion = corev1.SchemeGroupVersion.String() + } + if ns.TypeMeta.Kind == "" { + ns.TypeMeta.Kind = "Namespace" + } err := client.ServerSideApply(ctx, c, ns) if err != nil { return fmt.Errorf("failed to server side apply %w", err) diff --git a/pkg/handlers/generic/lifecycle/utils/utils_integration_test.go b/pkg/handlers/generic/lifecycle/utils/utils_integration_test.go index c2aed696a..e9f2c24b5 100644 --- a/pkg/handlers/generic/lifecycle/utils/utils_integration_test.go +++ b/pkg/handlers/generic/lifecycle/utils/utils_integration_test.go @@ -8,6 +8,7 @@ import ( . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apiserver/pkg/storage/names" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/test/helpers" @@ -18,9 +19,9 @@ var _ = Describe("Namespace", func() { c, err := helpers.TestEnv.GetK8sClient() Expect(err).To(BeNil()) - namespaceName := "new" + namespaceName := names.SimpleNameGenerator.GenerateName("test-") - Expect(EnsureNamespace(ctx, c, namespaceName)).To(Succeed()) + Expect(EnsureNamespaceWithName(ctx, c, namespaceName)).To(Succeed()) Expect(c.Delete(ctx, &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ Name: namespaceName, @@ -28,35 +29,69 @@ var _ = Describe("Namespace", func() { })).To((Succeed())) }) - It("updates a namespace, preserving user-managed fields", func(ctx SpecContext) { - c, err := helpers.TestEnv.GetK8sClient() - Expect(err).To(BeNil()) + It( + "updates a namespace with no changes, preserving user-managed fields", + func(ctx SpecContext) { + c, err := helpers.TestEnv.GetK8sClient() + Expect(err).To(BeNil()) + + namespaceName := names.SimpleNameGenerator.GenerateName("test-") + Expect(c.Create(ctx, + &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: namespaceName, + Labels: map[string]string{ + "userkey": "uservalue", + }, + }, + })).To(Succeed()) - namespaceName := "existing" - Expect(c.Create(ctx, - &corev1.Namespace{ + Expect(EnsureNamespaceWithName(ctx, c, namespaceName)).To(Succeed()) + + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: namespaceName, + }, + } + Expect(c.Get(ctx, client.ObjectKeyFromObject(ns), ns)).To(Succeed()) + Expect(ns.GetLabels()).To(HaveKeyWithValue("userkey", "uservalue")) + }, + ) + + It( + "updates a namespace by only sending new labels, preserving existing user-managed fields", + func(ctx SpecContext) { + c, err := helpers.TestEnv.GetK8sClient() + Expect(err).To(BeNil()) + + namespaceName := names.SimpleNameGenerator.GenerateName("test-") + Expect(c.Create(ctx, + &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: namespaceName, + Labels: map[string]string{ + "userkey": "uservalue", + }, + }, + })).To(Succeed()) + + Expect(EnsureNamespace(ctx, c, &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ Name: namespaceName, Labels: map[string]string{ - "userkey": "uservalue", + "newkey": "newvalue", }, }, })).To(Succeed()) - Expect(EnsureNamespace(ctx, c, namespaceName)).To(Succeed()) - - ns := &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: namespaceName, - }, - } - Expect(c.Get(ctx, client.ObjectKeyFromObject(ns), ns)).To(Succeed()) - Expect(ns.GetLabels()).To(HaveKeyWithValue("userkey", "uservalue")) - - Expect(c.Delete(ctx, &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: namespaceName, - }, - })).To((Succeed())) - }) + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: namespaceName, + }, + } + Expect(c.Get(ctx, client.ObjectKeyFromObject(ns), ns)).To(Succeed()) + Expect(ns.GetLabels()).To(HaveKeyWithValue("userkey", "uservalue")) + Expect(ns.GetLabels()).To(HaveKeyWithValue("newkey", "newvalue")) + }, + ) })