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 23, 2025
1 parent c0cf7c6 commit ef9f745
Show file tree
Hide file tree
Showing 13 changed files with 335 additions and 60 deletions.
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
31 changes: 27 additions & 4 deletions test/e2e/cluster_upgrade_runtimesdk.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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)
})
Expand All @@ -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")
Expand All @@ -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,
Expand Down Expand Up @@ -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()
})
Expand Down
34 changes: 34 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,37 @@ 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 {
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",
ExtensionConfigName: "k8s-upgrade-with-runtimesdk-cross-ns",
}
})
})
Loading

0 comments on commit ef9f745

Please sign in to comment.