From 7884a5975884c56248cff974b2151498f5938561 Mon Sep 17 00:00:00 2001 From: fabriziopandini Date: Thu, 6 May 2021 11:31:32 +0200 Subject: [PATCH] fix clusterctl config cluster regression --- cmd/clusterctl/client/config.go | 19 ++-- cmd/clusterctl/client/config_test.go | 145 ++++++++++++++++++++++++++- 2 files changed, 153 insertions(+), 11 deletions(-) diff --git a/cmd/clusterctl/client/config.go b/cmd/clusterctl/client/config.go index 9288825b70b9..a36397efc0b0 100644 --- a/cmd/clusterctl/client/config.go +++ b/cmd/clusterctl/client/config.go @@ -224,14 +224,14 @@ func (c *clusterctlClient) GetClusterTemplate(options GetClusterTemplateOptions) } // Gets the client for the current management cluster - cluster, err := c.clusterClientFactory(ClusterClientFactoryInput{options.Kubeconfig, options.YamlProcessor}) + clusterClient, err := c.clusterClientFactory(ClusterClientFactoryInput{options.Kubeconfig, options.YamlProcessor}) if err != nil { return nil, err } // If the option specifying the targetNamespace is empty, try to detect it. if options.TargetNamespace == "" { - currentNamespace, err := cluster.Proxy().CurrentNamespace() + currentNamespace, err := clusterClient.Proxy().CurrentNamespace() if err != nil { return nil, err } @@ -249,16 +249,21 @@ func (c *clusterctlClient) GetClusterTemplate(options GetClusterTemplateOptions) // Gets the workload cluster template from the selected source if options.ProviderRepositorySource != nil { // Ensure this command only runs against management clusters with the current Cluster API contract. - if err := cluster.ProviderInventory().CheckCAPIContract(); err != nil { - return nil, err + // NOTE: This command tolerates also not existing cluster (Kubeconfig.Path=="") or clusters not yet initialized in order to allow + // users to dry-run the command and take a look at what the cluster will look like; in both scenarios, it is required + // to pass provider:version given that auto-discovery can't work without a provider inventory installed in a cluster. + if options.Kubeconfig.Path != "" { + if err := clusterClient.ProviderInventory().CheckCAPIContract(cluster.AllowCAPINotInstalled{}); err != nil { + return nil, err + } } - return c.getTemplateFromRepository(cluster, options) + return c.getTemplateFromRepository(clusterClient, options) } if options.ConfigMapSource != nil { - return c.getTemplateFromConfigMap(cluster, *options.ConfigMapSource, options.TargetNamespace, options.ListVariablesOnly) + return c.getTemplateFromConfigMap(clusterClient, *options.ConfigMapSource, options.TargetNamespace, options.ListVariablesOnly) } if options.URLSource != nil { - return c.getTemplateFromURL(cluster, *options.URLSource, options.TargetNamespace, options.ListVariablesOnly) + return c.getTemplateFromURL(clusterClient, *options.URLSource, options.TargetNamespace, options.ListVariablesOnly) } return nil, errors.New("unable to read custom template. Please specify a template source") diff --git a/cmd/clusterctl/client/config_test.go b/cmd/clusterctl/client/config_test.go index 8f331e7aaf84..a642c4f9ff8f 100644 --- a/cmd/clusterctl/client/config_test.go +++ b/cmd/clusterctl/client/config_test.go @@ -26,14 +26,14 @@ import ( . "github.com/onsi/gomega" "github.com/pkg/errors" - "sigs.k8s.io/cluster-api/cmd/clusterctl/internal/test" - corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/pointer" clusterctlv1 "sigs.k8s.io/cluster-api/cmd/clusterctl/api/v1alpha3" "sigs.k8s.io/cluster-api/cmd/clusterctl/client/cluster" "sigs.k8s.io/cluster-api/cmd/clusterctl/client/config" + "sigs.k8s.io/cluster-api/cmd/clusterctl/client/repository" + "sigs.k8s.io/cluster-api/cmd/clusterctl/internal/test" ) func Test_clusterctlClient_GetProvidersConfig(t *testing.T) { @@ -649,8 +649,14 @@ func Test_clusterctlClient_GetClusterTemplate_onEmptyCluster(t *testing.T) { cluster1 := newFakeCluster(cluster.Kubeconfig{Path: "kubeconfig", Context: "mgmt-context"}, config1). WithObjs(configMap) + repository1 := newFakeRepository(infraProviderConfig, config1). + WithPaths("root", "components"). + WithDefaultVersion("v3.0.0"). + WithFile("v3.0.0", "cluster-template.yaml", rawTemplate) + client := newFakeClient(config1). - WithCluster(cluster1) + WithCluster(cluster1). + WithRepository(repository1) type args struct { options GetClusterTemplateOptions @@ -669,7 +675,7 @@ func Test_clusterctlClient_GetClusterTemplate_onEmptyCluster(t *testing.T) { wantErr bool }{ { - name: "repository source - fails because the cluster is not initialized/not v1alpha3", + name: "repository source - pass if the cluster is not initialized but infra provider:version are specified", args: args{ options: GetClusterTemplateOptions{ Kubeconfig: Kubeconfig{Path: "kubeconfig", Context: "mgmt-context"}, @@ -682,6 +688,26 @@ func Test_clusterctlClient_GetClusterTemplate_onEmptyCluster(t *testing.T) { ControlPlaneMachineCount: pointer.Int64Ptr(1), }, }, + want: templateValues{ + variables: []string{"CLUSTER_NAME"}, // variable detected + targetNamespace: "ns1", + yaml: templateYAML("ns1", "test"), // original template modified with target namespace and variable replacement + }, + }, + { + name: "repository source - fails if the cluster is not initialized and infra provider:version are not specified", + args: args{ + options: GetClusterTemplateOptions{ + Kubeconfig: Kubeconfig{Path: "kubeconfig", Context: "mgmt-context"}, + ProviderRepositorySource: &ProviderRepositorySourceOptions{ + InfrastructureProvider: "", + Flavor: "", + }, + ClusterName: "test", + TargetNamespace: "ns1", + ControlPlaneMachineCount: pointer.Int64Ptr(1), + }, + }, wantErr: true, }, { @@ -746,6 +772,117 @@ func Test_clusterctlClient_GetClusterTemplate_onEmptyCluster(t *testing.T) { } } +func newFakeClientWithoutCluster(configClient config.Client) *fakeClient { + fake := &fakeClient{ + configClient: configClient, + repositories: map[string]repository.Client{}, + } + + var err error + fake.internalClient, err = newClusterctlClient("fake-config", + InjectConfig(fake.configClient), + InjectRepositoryFactory(func(input RepositoryClientFactoryInput) (repository.Client, error) { + if _, ok := fake.repositories[input.Provider.ManifestLabel()]; !ok { + return nil, errors.Errorf("Repository for kubeconfig %q does not exist.", input.Provider.ManifestLabel()) + } + return fake.repositories[input.Provider.ManifestLabel()], nil + }), + ) + if err != nil { + panic(err) + } + + return fake +} + +func Test_clusterctlClient_GetClusterTemplate_withoutCluster(t *testing.T) { + rawTemplate := templateYAML("ns3", "${ CLUSTER_NAME }") + + config1 := newFakeConfig(). + WithProvider(infraProviderConfig) + + repository1 := newFakeRepository(infraProviderConfig, config1). + WithPaths("root", "components"). + WithDefaultVersion("v3.0.0"). + WithFile("v3.0.0", "cluster-template.yaml", rawTemplate) + + client := newFakeClientWithoutCluster(config1). + WithRepository(repository1) + + type args struct { + options GetClusterTemplateOptions + } + + type templateValues struct { + variables []string + targetNamespace string + yaml []byte + } + + tests := []struct { + name string + args args + want templateValues + wantErr bool + }{ + { + name: "repository source - pass without kubeconfig but infra provider:version are specified", + args: args{ + options: GetClusterTemplateOptions{ + Kubeconfig: Kubeconfig{Path: "", Context: ""}, + ProviderRepositorySource: &ProviderRepositorySourceOptions{ + InfrastructureProvider: "infra:v3.0.0", + Flavor: "", + }, + ClusterName: "test", + TargetNamespace: "ns1", + ControlPlaneMachineCount: pointer.Int64Ptr(1), + }, + }, + want: templateValues{ + variables: []string{"CLUSTER_NAME"}, // variable detected + targetNamespace: "ns1", + yaml: templateYAML("ns1", "test"), // original template modified with target namespace and variable replacement + }, + }, + { + name: "repository source - fails without kubeconfig and infra provider:version are not specified", + args: args{ + options: GetClusterTemplateOptions{ + Kubeconfig: Kubeconfig{Path: "", Context: ""}, + ProviderRepositorySource: &ProviderRepositorySourceOptions{ + InfrastructureProvider: "", + Flavor: "", + }, + ClusterName: "test", + TargetNamespace: "ns1", + ControlPlaneMachineCount: pointer.Int64Ptr(1), + }, + }, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gs := NewWithT(t) + + got, err := client.GetClusterTemplate(tt.args.options) + if tt.wantErr { + gs.Expect(err).To(HaveOccurred()) + return + } + gs.Expect(err).NotTo(HaveOccurred()) + + gs.Expect(got.Variables()).To(Equal(tt.want.variables)) + gs.Expect(got.TargetNamespace()).To(Equal(tt.want.targetNamespace)) + + gotYaml, err := got.Yaml() + gs.Expect(err).NotTo(HaveOccurred()) + gs.Expect(gotYaml).To(Equal(tt.want.yaml)) + }) + } +} + func Test_clusterctlClient_ProcessYAML(t *testing.T) { g := NewWithT(t) template := `v1: ${VAR1:=default1}