Add e2e test to cover crossNamespace provisionging

- Prevent duplicate CC creation in template processing - Test runtimextension integration - Add changes to CC rebase e2e test - Add a note CLUSTER_CLASS_NAMESPACE to the clusterctl contract Co-authored-by: Christian Schlotter <chrischdi@users.noreply.github.com> Co-authored-by: Stefan Büringer <4662360+sbueringer@users.noreply.github.com> Signed-off-by: Danil-Grigorev <danil.grigorev@suse.com>

Author: 3 people

cmd/clusterctl/client/clusterclass.go

@@ -23,6 +23,8 @@ import (
 "github.com/pkg/errors"
 apierrors "k8s.io/apimachinery/pkg/api/errors"
 "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+"k8s.io/apimachinery/pkg/types"
+"k8s.io/klog/v2"
 "sigs.k8s.io/controller-runtime/pkg/client"
 
 clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
@@ -34,7 +36,7 @@ import (
 // addClusterClassIfMissing returns a Template that includes the base template and adds any cluster class definitions that
 // are references in the template. If the cluster class referenced already exists in the cluster it is not added to the
 // template.
-func addClusterClassIfMissing(ctx context.Context, template Template, clusterClassClient repository.ClusterClassClient, clusterClient cluster.Client, targetNamespace string, listVariablesOnly bool) (Template, error) {
+func addClusterClassIfMissing(ctx context.Context, template Template, clusterClassClient repository.ClusterClassClient, clusterClient cluster.Client, listVariablesOnly bool) (Template, error) {
 classes, err := clusterClassNamesFromTemplate(template)
 if err != nil {
 return nil, err
@@ -44,7 +46,7 @@ func addClusterClassIfMissing(ctx context.Context, template Template, clusterCla
 return template, nil
 }
 
-clusterClassesTemplate, err := fetchMissingClusterClassTemplates(ctx, clusterClassClient, clusterClient, classes, targetNamespace, listVariablesOnly)
+clusterClassesTemplate, err := fetchMissingClusterClassTemplates(ctx, clusterClassClient, clusterClient, classes, listVariablesOnly)
 if err != nil {
 return nil, err
 }
@@ -62,8 +64,8 @@ func addClusterClassIfMissing(ctx context.Context, template Template, clusterCla
 // clusterClassNamesFromTemplate returns the list of ClusterClasses referenced
 // by clusters defined in the template. If not clusters are defined in the template
 // or if no cluster uses a cluster class it returns an empty list.
-func clusterClassNamesFromTemplate(template Template) ([]string, error) {
-classes := []string{}
+func clusterClassNamesFromTemplate(template Template) ([]types.NamespacedName, error) {
+classes := []types.NamespacedName{}
 
 // loop through all the objects and if the object is a cluster
 // check and see if cluster.spec.topology.class is defined.
@@ -80,14 +82,14 @@ func clusterClassNamesFromTemplate(template Template) ([]string, error) {
 if cluster.Spec.Topology == nil {
 continue
 }
-classes = append(classes, cluster.GetClassKey().Name)
+classes = append(classes, cluster.GetClassKey())
 }
 return classes, nil
 }
 
 // fetchMissingClusterClassTemplates returns a list of templates for ClusterClasses that do not yet exist
 // in the cluster. If the cluster is not initialized, all the ClusterClasses are added.
-func fetchMissingClusterClassTemplates(ctx context.Context, clusterClassClient repository.ClusterClassClient, clusterClient cluster.Client, classes []string, targetNamespace string, listVariablesOnly bool) (Template, error) {
+func fetchMissingClusterClassTemplates(ctx context.Context, clusterClassClient repository.ClusterClassClient, clusterClient cluster.Client, classes []types.NamespacedName, listVariablesOnly bool) (Template, error) {
 // first check if the cluster is initialized.
 // If it is initialized:
 // For every ClusterClass check if it already exists in the cluster.
@@ -118,7 +120,7 @@ func fetchMissingClusterClassTemplates(ctx context.Context, clusterClassClient r
 templates := []repository.Template{}
 for _, class := range classes {
 if clusterInitialized {
-exists, err := clusterClassExists(ctx, c, class, targetNamespace)
+exists, err := clusterClassExists(ctx, c, class.Name, class.Namespace)
 if err != nil {
 return nil, err
 }
@@ -128,7 +130,7 @@ func fetchMissingClusterClassTemplates(ctx context.Context, clusterClassClient r
 }
 // The cluster is either not initialized or the ClusterClass does not yet exist in the cluster.
 // Fetch the cluster class to install.
-clusterClassTemplate, err := clusterClassClient.Get(ctx, class, targetNamespace, listVariablesOnly)
+clusterClassTemplate, err := clusterClassClient.Get(ctx, class.Name, class.Namespace, listVariablesOnly)
 if err != nil {
 return nil, errors.Wrapf(err, "failed to get the cluster class template for %q", class)
 }
@@ -142,7 +144,7 @@ func fetchMissingClusterClassTemplates(ctx context.Context, clusterClassClient r
 if exists, err := objExists(ctx, c, obj); err != nil {
 return nil, err
 } else if exists {
-return nil, fmt.Errorf("%s(%s) already exists in the cluster", obj.GetName(), obj.GetObjectKind().GroupVersionKind())
+return nil, fmt.Errorf("%s(%s) already exists in the cluster", klog.KObj(&obj), obj.GetObjectKind().GroupVersionKind())
 }
 }
 }

cmd/clusterctl/client/clusterclass_test.go

@@ -100,6 +100,7 @@ func TestAddClusterClassIfMissing(t *testing.T) {
 objs []client.Object
 clusterClassTemplateContent []byte
 targetNamespace string
+clusterClassNamespace string
 listVariablesOnly bool
 wantClusterClassInTemplate bool
 wantError bool
@@ -114,6 +115,28 @@ func TestAddClusterClassIfMissing(t *testing.T) {
 wantClusterClassInTemplate: true,
 wantError: false,
 },
+{
+name: "should add the cluster class from a different namespace to the template if cluster is not initialized",
+clusterInitialized: false,
+objs: []client.Object{},
+targetNamespace: "ns1",
+clusterClassNamespace: "ns2",
+clusterClassTemplateContent: clusterClassYAML("ns2", "dev"),
+listVariablesOnly: false,
+wantClusterClassInTemplate: true,
+wantError: false,
+},
+{
+name: "should add the cluster class form the same explicitly specified namespace to the template if cluster is not initialized",
+clusterInitialized: false,
+objs: []client.Object{},
+targetNamespace: "ns1",
+clusterClassNamespace: "ns1",
+clusterClassTemplateContent: clusterClassYAML("ns1", "dev"),
+listVariablesOnly: false,
+wantClusterClassInTemplate: true,
+wantError: false,
+},
 {
 name: "should add the cluster class to the template if cluster is initialized and cluster class is not installed",
 clusterInitialized: true,
@@ -189,17 +212,21 @@ func TestAddClusterClassIfMissing(t *testing.T) {
 
 clusterClassClient := repository1.ClusterClasses("v1.0.0")
 
-clusterWithTopology := []byte(fmt.Sprintf("apiVersion: %s\n", clusterv1.GroupVersion.String()) +
+clusterWithTopology := fmt.Sprintf("apiVersion: %s\n", clusterv1.GroupVersion.String()) +
 "kind: Cluster\n" +
 "metadata:\n" +
 " name: cluster-dev\n" +
 fmt.Sprintf(" namespace: %s\n", tt.targetNamespace) +
 "spec:\n" +
 " topology:\n" +
-" class: dev")
+" class: dev"
+
+if tt.clusterClassNamespace != "" {
+clusterWithTopology = fmt.Sprintf("%s\n classNamespace: %s", clusterWithTopology, tt.clusterClassNamespace)
+}
 
 baseTemplate, err := repository.NewTemplate(repository.TemplateInput{
-RawArtifact: clusterWithTopology,
+RawArtifact: []byte(clusterWithTopology),
 ConfigVariablesClient: test.NewFakeVariableClient(),
 Processor: yaml.NewSimpleProcessor(),
 TargetNamespace: tt.targetNamespace,
@@ -210,13 +237,22 @@ func TestAddClusterClassIfMissing(t *testing.T) {
 }
 
 g := NewWithT(t)
-template, err := addClusterClassIfMissing(ctx, baseTemplate, clusterClassClient, cluster, tt.targetNamespace, tt.listVariablesOnly)
+template, err := addClusterClassIfMissing(ctx, baseTemplate, clusterClassClient, cluster, tt.listVariablesOnly)
 if tt.wantError {
 g.Expect(err).To(HaveOccurred())
 } else {
 if tt.wantClusterClassInTemplate {
-g.Expect(template.Objs()).To(ContainElement(MatchClusterClass("dev", tt.targetNamespace)))
+if tt.clusterClassNamespace == tt.targetNamespace {
+g.Expect(template.Objs()).To(ContainElement(MatchClusterClass("dev", tt.targetNamespace)))
+} else if tt.clusterClassNamespace != "" {
+g.Expect(template.Objs()).To(ContainElement(MatchClusterClass("dev", tt.clusterClassNamespace)))
+g.Expect(template.Objs()).ToNot(ContainElement(MatchClusterClass("dev", tt.targetNamespace)))
+} else {
+g.Expect(template.Objs()).To(ContainElement(MatchClusterClass("dev", tt.targetNamespace)))
+g.Expect(template.Objs()).ToNot(ContainElement(MatchClusterClass("dev", tt.clusterClassNamespace)))
+}
 } else {
+g.Expect(template.Objs()).NotTo(ContainElement(MatchClusterClass("dev", tt.clusterClassNamespace)))
 g.Expect(template.Objs()).NotTo(ContainElement(MatchClusterClass("dev", tt.targetNamespace)))
 }
 }

cmd/clusterctl/client/config.go

@@ -344,7 +344,7 @@ func (c *clusterctlClient) getTemplateFromRepository(ctx context.Context, cluste
 
 clusterClassClient := repo.ClusterClasses(version)
 
-template, err = addClusterClassIfMissing(ctx, template, clusterClassClient, cluster, targetNamespace, listVariablesOnly)
+template, err = addClusterClassIfMissing(ctx, template, clusterClassClient, cluster, listVariablesOnly)
 if err != nil {
 return nil, err
 }

cmd/clusterctl/client/repository/template.go

@@ -17,8 +17,6 @@ limitations under the License.
 package repository
 
 import (
-"fmt"
-
 "github.com/pkg/errors"
 "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 "k8s.io/apimachinery/pkg/util/sets"
@@ -144,7 +142,7 @@ func NewTemplate(input TemplateInput) (Template, error) {
 
 // MergeTemplates merges the provided Templates into one Template.
 // Notes on the merge operation:
-// - The merge operation returns an error if all the templates do not have the same TargetNamespace.
+// - The merge operation sets targetNamespace empty if all the templates do not share the same TargetNamespace.
 // - The Variables of the resulting template is a union of all Variables in the templates.
 // - The default value is picked from the first template that defines it.
 // The defaults of the same variable in the subsequent templates will be ignored.
@@ -173,7 +171,7 @@ func MergeTemplates(templates ...Template) (Template, error) {
 }
 
 if merged.targetNamespace != tmpl.TargetNamespace() {
-return nil, fmt.Errorf("cannot merge templates with different targetNamespaces")
+merged.targetNamespace = ""
 }
 
 merged.objs = append(merged.objs, tmpl.Objs()...)

docs/book/src/developer/providers/contracts/clusterctl.md

@@ -416,6 +416,12 @@ ClusterClass definitions MUST be stored in the same location as the component YA
 in the Cluster template; Cluster template files using a ClusterClass are usually simpler because they are no longer
 required to have all the templates.
 
+Additionally, namespace of the ClusterClass can differ from the Cluster. This requires specifying
+Cluster.spec.topology.classNamespace field in the Cluster template;
+Cluster template may define classNamespace as `classNamespace: ${CLUSTER_CLASS_NAMESPACE:=""}`, which would allow to
+optionally specify namespace of the referred ClusterClass via env. Empty or missing value is uses Cluster namespace
+by default.
+
 Each provider should create user facing documentation with the list of available ClusterClass definitions.
 
 #### Target namespace

test/e2e/cluster_upgrade_runtimesdk.go

@@ -102,6 +102,9 @@ type ClusterUpgradeWithRuntimeSDKSpecInput struct {
 
 // ExtensionServiceName is the name of the service to configure in the test-namespace scoped ExtensionConfig.
 ExtensionServiceName string
+
+// DeployClusterClassInSeparateNamespace defines if the ClusterClass should be deployed in a separate namespace.
+DeployClusterClassInSeparateNamespace bool
 }
 
 // ClusterUpgradeWithRuntimeSDKSpec implements a spec that upgrades a cluster and runs the Kubernetes conformance suite.
@@ -116,9 +119,9 @@ func ClusterUpgradeWithRuntimeSDKSpec(ctx context.Context, inputGetter func() Cl
 )
 
 var (
-input ClusterUpgradeWithRuntimeSDKSpecInput
-namespace  *corev1.Namespace
-cancelWatches context.CancelFunc
+input  ClusterUpgradeWithRuntimeSDKSpecInput
+namespace, clusterClassNamespace *corev1.Namespace
+cancelWatches  context.CancelFunc
 
 controlPlaneMachineCount int64
 workerMachineCount int64
@@ -158,6 +161,10 @@ func ClusterUpgradeWithRuntimeSDKSpec(ctx context.Context, inputGetter func() Cl
 
 // Set up a Namespace where to host objects for this spec and create a watcher for the Namespace events.
 namespace, cancelWatches = framework.SetupSpecNamespace(ctx, specName, input.BootstrapClusterProxy, input.ArtifactFolder, input.PostNamespaceCreated)
+if input.DeployClusterClassInSeparateNamespace {
+clusterClassNamespace = framework.CreateNamespace(ctx, framework.CreateNamespaceInput{Creator: input.BootstrapClusterProxy.GetClient(), Name: fmt.Sprintf("%s-clusterclass", namespace.Name)}, "40s", "10s")
+Expect(clusterClassNamespace).ToNot(BeNil(), "Failed to create namespace")
+}
 clusterName = fmt.Sprintf("%s-%s", specName, util.RandomString(6))
 clusterResources = new(clusterctl.ApplyClusterTemplateAndWaitResult)
 })
@@ -171,11 +178,16 @@ func ClusterUpgradeWithRuntimeSDKSpec(ctx context.Context, inputGetter func() Cl
 
 By("Deploy Test Extension ExtensionConfig")
 
+namespaces := []string{namespace.Name}
+if input.DeployClusterClassInSeparateNamespace {
+namespaces = append(namespaces, clusterClassNamespace.Name)
+}
+
 // In this test we are defaulting all handlers to blocking because we expect the handlers to block the
 // cluster lifecycle by default. Setting defaultAllHandlersToBlocking to true enforces that the test-extension
 // automatically creates the ConfigMap with blocking preloaded responses.
 Expect(input.BootstrapClusterProxy.GetClient().Create(ctx,
-extensionConfig(input.ExtensionConfigName, input.ExtensionServiceNamespace, input.ExtensionServiceName, true, namespace.Name))).
+extensionConfig(input.ExtensionConfigName, input.ExtensionServiceNamespace, input.ExtensionServiceName, true, namespaces...))).
 To(Succeed(), "Failed to create the extension config")
 
 By("Creating a workload cluster; creation waits for BeforeClusterCreateHook to gate the operation")
@@ -194,6 +206,9 @@ func ClusterUpgradeWithRuntimeSDKSpec(ctx context.Context, inputGetter func() Cl
 // This is used to template the name of the ExtensionConfig into the ClusterClass.
 "EXTENSION_CONFIG_NAME": input.ExtensionConfigName,
 }
+if input.DeployClusterClassInSeparateNamespace {
+variables["CLUSTER_CLASS_NAMESPACE"] = clusterClassNamespace.Name
+}
 
 clusterctl.ApplyClusterTemplateAndWait(ctx, clusterctl.ApplyClusterTemplateAndWaitInput{
 ClusterProxy: input.BootstrapClusterProxy,
@@ -341,6 +356,14 @@ func ClusterUpgradeWithRuntimeSDKSpec(ctx context.Context, inputGetter func() Cl
 Deleter: input.BootstrapClusterProxy.GetClient(),
 Name: namespace.Name,
 })
+
+if input.DeployClusterClassInSeparateNamespace {
+Byf("Deleting namespace used for hosting the %q test spec ClusterClass", specName)
+framework.DeleteNamespace(ctx, framework.DeleteNamespaceInput{
+Deleter: input.BootstrapClusterProxy.GetClient(),
+Name: clusterClassNamespace.Name,
+})
+}
 }
 cancelWatches()
 })

test/e2e/cluster_upgrade_runtimesdk_test.go

@@ -60,3 +60,31 @@ var _ = Describe("When upgrading a workload cluster using ClusterClass with Runt
 }
 })
 })
+
+var _ = Describe("When upgrading a workload cluster using ClusterClass in a different NS with RuntimeSDK [ClusterClass]", Label("ClusterClass"), func() {
+ClusterUpgradeWithRuntimeSDKSpec(ctx, func() ClusterUpgradeWithRuntimeSDKSpecInput {
+return ClusterUpgradeWithRuntimeSDKSpecInput{
+E2EConfig: e2eConfig,
+ClusterctlConfigPath: clusterctlConfigPath,
+BootstrapClusterProxy: bootstrapClusterProxy,
+ArtifactFolder: artifactFolder,
+SkipCleanup: skipCleanup,
+InfrastructureProvider: ptr.To("docker"),
+PostUpgrade: func(proxy framework.ClusterProxy, namespace, clusterName string) {
+// This check ensures that the resourceVersions are stable, i.e. it verifies there are no
+// continuous reconciles when everything should be stable.
+framework.ValidateResourceVersionStable(ctx, proxy, namespace, clusterctlcluster.FilterClusterObjectsWithNameFilter(clusterName))
+},
+// "upgrades" is the same as the "topology" flavor but with an additional MachinePool.
+Flavor: ptr.To("upgrades-runtimesdk"),
+DeployClusterClassInSeparateNamespace: true,
+// The runtime extension gets deployed to the test-extension-system namespace and is exposed
+// by the test-extension-webhook-service.
+// The below values are used when creating the cluster-wide ExtensionConfig to refer
+// the actual service.
+ExtensionServiceNamespace: "test-extension-system",
+ExtensionServiceName: "test-extension-webhook-service",
+ExtensionConfigName: "k8s-upgrade-with-runtimesdk-cross-ns",
+}
+})
+})