Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 fix clusterctl config cluster regression #4578

Merged
merged 1 commit into from
May 6, 2021
Merged

🐛 fix clusterctl config cluster regression #4578

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 12 additions & 7 deletions cmd/clusterctl/client/config.go
View file
Open in desktop
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
Open in desktop
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