Skip to content

Commit

Permalink
Merge pull request #4578 from fabriziopandini/fix-clusterctl-config-c…
Browse files Browse the repository at this point in the history 
…luster-regression

🐛 fix clusterctl config cluster regression
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored May 6, 2021
2 parents 760a91e + 7884a59 commit eeac0f6
Show file tree
Hide file tree
Showing 2 changed files with 153 additions and 11 deletions.
19 changes: 12 additions & 7 deletions cmd/clusterctl/client/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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")
Expand Down
145 changes: 141 additions & 4 deletions cmd/clusterctl/client/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand All @@ -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"},
Expand All @@ -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,
},
{
Expand Down Expand Up @@ -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}
Expand Down

0 comments on commit eeac0f6

Please sign in to comment.