Skip to content

Commit

Permalink
Add e2e test to cover crossNamespace provisionging
Browse files Browse the repository at this point in the history
- 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

Signed-off-by: Danil-Grigorev <[email protected]>
  • Loading branch information
Danil-Grigorev committed Jan 22, 2025
1 parent da9bc79 commit 809d9a4
Show file tree
Hide file tree
Showing 14 changed files with 320 additions and 60 deletions.
1 change: 0 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -615,7 +615,6 @@ generate-e2e-templates-main: $(KUSTOMIZE)
$(KUSTOMIZE) build $(DOCKER_TEMPLATES)/main/cluster-template-topology --load-restrictor LoadRestrictionsNone > $(DOCKER_TEMPLATES)/main/cluster-template-topology.yaml
$(KUSTOMIZE) build $(DOCKER_TEMPLATES)/main/cluster-template-ignition --load-restrictor LoadRestrictionsNone > $(DOCKER_TEMPLATES)/main/cluster-template-ignition.yaml
$(KUSTOMIZE) build $(DOCKER_TEMPLATES)/main/clusterclass-quick-start-kcp-only --load-restrictor LoadRestrictionsNone > $(DOCKER_TEMPLATES)/main/clusterclass-quick-start-kcp-only.yaml

$(KUSTOMIZE) build $(INMEMORY_TEMPLATES)/main/cluster-template --load-restrictor LoadRestrictionsNone > $(INMEMORY_TEMPLATES)/main/cluster-template.yaml

.PHONY: generate-metrics-config
Expand Down
20 changes: 11 additions & 9 deletions cmd/clusterctl/client/clusterclass.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand All @@ -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
}
Expand All @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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
}
Expand All @@ -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)
}
Expand All @@ -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())
}
}
}
Expand Down
46 changes: 41 additions & 5 deletions cmd/clusterctl/client/clusterclass_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ func TestAddClusterClassIfMissing(t *testing.T) {
objs []client.Object
clusterClassTemplateContent []byte
targetNamespace string
clusterClassNamespace string
listVariablesOnly bool
wantClusterClassInTemplate bool
wantError bool
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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)))
}
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/clusterctl/client/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
6 changes: 2 additions & 4 deletions cmd/clusterctl/client/repository/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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()...)
Expand Down
6 changes: 6 additions & 0 deletions docs/book/src/developer/providers/contracts/clusterctl.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
44 changes: 35 additions & 9 deletions test/e2e/cluster_upgrade_runtimesdk.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ type ClusterUpgradeWithRuntimeSDKSpecInput struct {
ExtensionServiceNamespace string
// 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.
Expand All @@ -109,9 +112,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
Expand Down Expand Up @@ -148,6 +151,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)
})
Expand All @@ -161,9 +168,14 @@ func ClusterUpgradeWithRuntimeSDKSpec(ctx context.Context, inputGetter func() Cl

By("Deploy Test Extension ExtensionConfig")

Expect(input.BootstrapClusterProxy.GetClient().Create(ctx,
extensionConfig(specName, namespace.Name, input.ExtensionServiceNamespace, input.ExtensionServiceName))).
To(Succeed(), "Failed to create the extension config")
namespaces := []string{namespace.Name}
if input.DeployClusterClassInSeparateNamespace {
namespaces = append(namespaces, clusterClassNamespace.Name)
}
Eventually(func() error {
return input.BootstrapClusterProxy.GetClient().Create(ctx,
extensionConfig(specName, input.ExtensionServiceNamespace, input.ExtensionServiceName, namespaces...))
}, 10*time.Second, 1*time.Second).Should(Succeed(), "Failed to create the extension config")

By("Creating a workload cluster; creation waits for BeforeClusterCreateHook to gate the operation")

Expand All @@ -177,6 +189,11 @@ func ClusterUpgradeWithRuntimeSDKSpec(ctx context.Context, inputGetter func() Cl
infrastructureProvider = *input.InfrastructureProvider
}

variables := map[string]string{}
if input.DeployClusterClassInSeparateNamespace {
variables["CLUSTER_CLASS_NAMESPACE"] = clusterClassNamespace.Name
}

clusterctl.ApplyClusterTemplateAndWait(ctx, clusterctl.ApplyClusterTemplateAndWaitInput{
ClusterProxy: input.BootstrapClusterProxy,
ConfigCluster: clusterctl.ConfigClusterInput{
Expand All @@ -190,6 +207,7 @@ func ClusterUpgradeWithRuntimeSDKSpec(ctx context.Context, inputGetter func() Cl
KubernetesVersion: input.E2EConfig.GetVariable(KubernetesVersionUpgradeFrom),
ControlPlaneMachineCount: ptr.To[int64](controlPlaneMachineCount),
WorkerMachineCount: ptr.To[int64](workerMachineCount),
ClusterctlVariables: variables,
},
PreWaitForCluster: func() {
beforeClusterCreateTestHandler(ctx,
Expand Down Expand Up @@ -304,7 +322,7 @@ func ClusterUpgradeWithRuntimeSDKSpec(ctx context.Context, inputGetter func() Cl
if !input.SkipCleanup {
// Delete the extensionConfig first to ensure the BeforeDeleteCluster hook doesn't block deletion.
Eventually(func() error {
return input.BootstrapClusterProxy.GetClient().Delete(ctx, extensionConfig(specName, namespace.Name, input.ExtensionServiceNamespace, input.ExtensionServiceName))
return input.BootstrapClusterProxy.GetClient().Delete(ctx, extensionConfig(specName, input.ExtensionServiceNamespace, input.ExtensionServiceName))
}, 10*time.Second, 1*time.Second).Should(Succeed(), "delete extensionConfig failed")

Byf("Deleting cluster %s", klog.KObj(clusterResources.Cluster))
Expand All @@ -322,6 +340,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()
})
Expand Down Expand Up @@ -429,7 +455,7 @@ func machineSetPreflightChecksTestHandler(ctx context.Context, c client.Client,
// We make sure this cluster-wide object does not conflict with others by using a random generated
// name and a NamespaceSelector selecting on the namespace of the current test.
// Thus, this object is "namespaced" to the current test even though it's a cluster-wide object.
func extensionConfig(name, namespace, extensionServiceNamespace, extensionServiceName string) *runtimev1.ExtensionConfig {
func extensionConfig(name, extensionServiceNamespace, extensionServiceName string, namespaces ...string) *runtimev1.ExtensionConfig {
return &runtimev1.ExtensionConfig{
ObjectMeta: metav1.ObjectMeta{
// Note: We have to use a constant name here as we have to be able to reference it in the ClusterClass
Expand All @@ -454,7 +480,7 @@ func extensionConfig(name, namespace, extensionServiceNamespace, extensionServic
{
Key: "kubernetes.io/metadata.name",
Operator: metav1.LabelSelectorOpIn,
Values: []string{namespace},
Values: namespaces,
},
},
},
Expand Down
33 changes: 33 additions & 0 deletions test/e2e/cluster_upgrade_runtimesdk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,36 @@ 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]", func() {
ClusterUpgradeWithRuntimeSDKSpec(ctx, func() ClusterUpgradeWithRuntimeSDKSpecInput {
version, err := semver.ParseTolerant(e2eConfig.GetVariable(KubernetesVersionUpgradeFrom))
Expect(err).ToNot(HaveOccurred(), "Invalid argument, KUBERNETES_VERSION_UPGRADE_FROM is not a valid version")
if version.LT(semver.MustParse("1.24.0")) {
Fail("This test only supports upgrades from Kubernetes >= v1.24.0")
}

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",
}
})
})
Loading

0 comments on commit 809d9a4

Please sign in to comment.