Skip to content

Commit

Permalink
Merge pull request #2221 from fabriziopandini/clusterctl-simplify-tem…
Browse files Browse the repository at this point in the history
…plate-naming

🐛clusterctl: simplify cluster template naming
  • Loading branch information
k8s-ci-robot authored Feb 3, 2020
2 parents 5cfd0f1 + d6e8d57 commit f5b4666
Show file tree
Hide file tree
Showing 10 changed files with 15 additions and 75 deletions.
7 changes: 0 additions & 7 deletions cmd/clusterctl/cmd/config_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (
type configClusterOptions struct {
kubeconfig string
flavor string
bootstrapProvider string
infrastructureProvider string

targetNamespace string
Expand All @@ -53,10 +52,6 @@ var configClusterClusterCmd = &cobra.Command{
# default infrastructure provider and default bootstrap provider installed in the cluster.
clusterctl config cluster my-cluster
# Generates a yaml file for creating a Cluster API workload cluster using
# specified infrastructure and bootstrap provider
clusterctl config cluster my-cluster --infrastructure=aws --bootstrap=kubeadm
# Generates a yaml file for creating a Cluster API workload cluster using
# specified version of the AWS infrastructure provider
clusterctl config cluster my-cluster --infrastructure=aws:v0.4.1
Expand All @@ -82,7 +77,6 @@ func init() {
configClusterClusterCmd.Flags().StringVarP(&cc.kubeconfig, "kubeconfig", "", "", "Path to the kubeconfig file to use for accessing the management cluster. If empty, default rules for kubeconfig discovery will be used")

configClusterClusterCmd.Flags().StringVarP(&cc.infrastructureProvider, "infrastructure", "i", "", "The infrastructure provider that should be used for creating the workload cluster")
configClusterClusterCmd.Flags().StringVarP(&cc.bootstrapProvider, "bootstrap", "b", "kubeadm", "The provider that should be used for bootstrapping Kubernetes nodes in the workload cluster")

configClusterClusterCmd.Flags().StringVarP(&cc.flavor, "flavor", "f", "", "The template variant to be used for creating the workload cluster")
configClusterClusterCmd.Flags().StringVarP(&cc.targetNamespace, "target-namespace", "n", "", "The namespace where the objects describing the workload cluster should be deployed. If not specified, the current namespace will be used")
Expand All @@ -103,7 +97,6 @@ func runGenerateCluster(name string) error {
Kubeconfig: cc.kubeconfig,
InfrastructureProvider: cc.infrastructureProvider,
Flavor: cc.flavor,
BootstrapProvider: cc.bootstrapProvider,
ClusterName: name,
TargetNamespace: cc.targetNamespace,
KubernetesVersion: cc.kubernetesVersion,
Expand Down
3 changes: 0 additions & 3 deletions cmd/clusterctl/pkg/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,6 @@ type GetClusterTemplateOptions struct {
// InfrastructureProvider that should be used for creating the workload cluster.
InfrastructureProvider string

// BootstrapProvider that should be used for bootstrapping Kubernetes nodes in the workload cluster.
BootstrapProvider string

// Flavor defines the template variant to be used for creating the workload cluster.
Flavor string

Expand Down
2 changes: 1 addition & 1 deletion cmd/clusterctl/pkg/client/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func (c *clusterctlClient) GetClusterTemplate(options GetClusterTemplateOptions)
return nil, err
}

template, err := repo.Templates(version).Get(options.Flavor, options.BootstrapProvider, options.TargetNamespace)
template, err := repo.Templates(version).Get(options.Flavor, options.TargetNamespace)
if err != nil {
return nil, err
}
Expand Down
16 changes: 1 addition & 15 deletions cmd/clusterctl/pkg/client/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ func Test_clusterctlClient_GetClusterTemplate(t *testing.T) {
repository1 := newFakeRepository(infraProviderConfig, config1.Variables()).
WithPaths("root", "components").
WithDefaultVersion("v3.0.0").
WithFile("v3.0.0", "config-kubeadm.yaml", templateYAML("ns3", "${ CLUSTER_NAME }"))
WithFile("v3.0.0", "cluster-template.yaml", templateYAML("ns3", "${ CLUSTER_NAME }"))

cluster1 := newFakeCluster("kubeconfig").
WithProviderInventory(infraProviderConfig.Name(), infraProviderConfig.Type(), "v3.0.0", "foo", "bar")
Expand All @@ -327,8 +327,6 @@ func Test_clusterctlClient_GetClusterTemplate(t *testing.T) {
providerType clusterctlv1.ProviderType
version string
flavor string
bootstrap string
path string
variables []string
targetNamespace string
yaml []byte
Expand All @@ -347,7 +345,6 @@ func Test_clusterctlClient_GetClusterTemplate(t *testing.T) {
Kubeconfig: "kubeconfig",
InfrastructureProvider: "infra:v3.0.0",
Flavor: "",
BootstrapProvider: "kubeadm",
ClusterName: "test",
TargetNamespace: "ns1",
ControlPlaneMachineCount: 1,
Expand All @@ -359,8 +356,6 @@ func Test_clusterctlClient_GetClusterTemplate(t *testing.T) {
providerType: clusterctlv1.InfrastructureProviderType,
version: "v3.0.0",
flavor: "",
bootstrap: "kubeadm",
path: "config-kubeadm.yaml",
variables: []string{"CLUSTER_NAME"}, // variable detected
targetNamespace: "ns1",
yaml: templateYAML("ns1", "test"), // original template modified with target namespace and variable replacement
Expand All @@ -373,7 +368,6 @@ func Test_clusterctlClient_GetClusterTemplate(t *testing.T) {
Kubeconfig: "kubeconfig",
InfrastructureProvider: "", // empty triggers auto-detection of the provider name/version
Flavor: "",
BootstrapProvider: "kubeadm",
ClusterName: "test",
TargetNamespace: "ns1",
ControlPlaneMachineCount: 1,
Expand All @@ -385,8 +379,6 @@ func Test_clusterctlClient_GetClusterTemplate(t *testing.T) {
providerType: clusterctlv1.InfrastructureProviderType,
version: "v3.0.0",
flavor: "",
bootstrap: "kubeadm",
path: "config-kubeadm.yaml",
variables: []string{"CLUSTER_NAME"}, // variable detected
targetNamespace: "ns1",
yaml: templateYAML("ns1", "test"), // original template modified with target namespace and variable replacement
Expand All @@ -399,7 +391,6 @@ func Test_clusterctlClient_GetClusterTemplate(t *testing.T) {
Kubeconfig: "kubeconfig",
InfrastructureProvider: "infra:v3.0.0",
Flavor: "",
BootstrapProvider: "kubeadm",
ClusterName: "test",
TargetNamespace: "", // empty triggers usage of the current namespace
ControlPlaneMachineCount: 1,
Expand All @@ -411,8 +402,6 @@ func Test_clusterctlClient_GetClusterTemplate(t *testing.T) {
providerType: clusterctlv1.InfrastructureProviderType,
version: "v3.0.0",
flavor: "",
bootstrap: "kubeadm",
path: "config-kubeadm.yaml",
variables: []string{"CLUSTER_NAME"}, // variable detected
targetNamespace: "default",
yaml: templateYAML("default", "test"), // original template modified with target namespace and variable replacement
Expand Down Expand Up @@ -444,9 +433,6 @@ func Test_clusterctlClient_GetClusterTemplate(t *testing.T) {
if got.Flavor() != tt.want.flavor {
t.Errorf("Flavor() got = %v, want %v", got.Flavor(), tt.want.flavor)
}
if got.Bootstrap() != tt.want.bootstrap {
t.Errorf("Bootstrap() got = %v, want %v", got.Bootstrap(), tt.want.bootstrap)
}
if !reflect.DeepEqual(got.Variables(), tt.want.variables) {
t.Errorf("Variables() got = %v, want %v", got.Variables(), tt.want.variables)
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/clusterctl/pkg/client/init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ func fakeEmptyCluster() *fakeClient {
WithDefaultVersion("v3.0.0").
WithFile("v3.0.0", "components.yaml", componentsYAML("ns4")).
WithFile("v3.1.0", "components.yaml", componentsYAML("ns4")).
WithFile("v3.0.0", "config-kubeadm.yaml", templateYAML("ns4", "test"))
WithFile("v3.0.0", "cluster-template.yaml", templateYAML("ns4", "test"))

cluster1 := newFakeCluster("kubeconfig")

Expand Down
10 changes: 0 additions & 10 deletions cmd/clusterctl/pkg/client/repository/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,6 @@ type Template interface {
// A flavor is a variant of cluster template supported by the provider, like e.g. Prod, Test.
Flavor() string

// Bootstrap provider used by the cluster template.
Bootstrap() string

// Variables required by the template.
// This value is derived by the template YAML.
Variables() []string
Expand All @@ -61,7 +58,6 @@ type template struct {
config.Provider
version string
flavor string
bootstrap string
variables []string
targetNamespace string
objs []unstructured.Unstructured
Expand All @@ -78,10 +74,6 @@ func (t *template) Flavor() string {
return t.flavor
}

func (t *template) Bootstrap() string {
return t.bootstrap
}

func (t *template) Variables() []string {
return t.variables
}
Expand All @@ -103,7 +95,6 @@ type newTemplateOptions struct {
provider config.Provider
version string
flavor string
bootstrap string
rawYaml []byte
configVariablesClient config.VariablesClient
targetNamespace string
Expand Down Expand Up @@ -134,7 +125,6 @@ func newTemplate(options newTemplateOptions) (*template, error) {
Provider: options.provider,
version: options.version,
flavor: options.flavor,
bootstrap: options.bootstrap,
variables: variables,
targetNamespace: options.targetNamespace,
objs: objs,
Expand Down
17 changes: 6 additions & 11 deletions cmd/clusterctl/pkg/client/repository/template_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ type TemplateOptions struct {
// TemplateClient has methods to work with cluster templates hosted on a provider repository.
// Templates are yaml files to be used for creating a guest cluster.
type TemplateClient interface {
Get(flavor, bootstrap, targetNamespace string) (Template, error)
Get(flavor, targetNamespace string) (Template, error)
}

// templateClient implements TemplateClient.
Expand All @@ -62,14 +62,10 @@ func newTemplateClient(provider config.Provider, version string, repository Repo
}
}

// Get return the template for the flavor/bootstrap provider specified.
// Get return the template for the flavor specified.
// In case the template does not exists, an error is returned.
// Get assumes the following naming convention for templates: config[-<flavor_name>]-<bootstrap_provider_name>.yaml
func (c *templateClient) Get(flavor, bootstrap, targetNamespace string) (Template, error) {
if bootstrap == "" {
return nil, errors.New("invalid arguments: please provide a bootstrap provider name")
}

// Get assumes the following naming convention for templates: cluster-template[-<flavor_name>].yaml
func (c *templateClient) Get(flavor, targetNamespace string) (Template, error) {
if targetNamespace == "" {
return nil, errors.New("invalid arguments: please provide a targetNamespace")
}
Expand All @@ -79,11 +75,11 @@ func (c *templateClient) Get(flavor, bootstrap, targetNamespace string) (Templat
version := c.version

// building template name according with the naming convention
name := "config"
name := "cluster-template"
if flavor != "" {
name = fmt.Sprintf("%s-%s", name, flavor)
}
name = fmt.Sprintf("%s-%s.yaml", name, bootstrap)
name = fmt.Sprintf("%s.yaml", name)

// read the component YAML, reading the local override file if it exists, otherwise read from the provider repository
rawYaml, err := getLocalOverride(c.provider, version, name)
Expand All @@ -102,7 +98,6 @@ func (c *templateClient) Get(flavor, bootstrap, targetNamespace string) (Templat
provider: c.provider,
version: version,
flavor: flavor,
bootstrap: bootstrap,
rawYaml: rawYaml,
configVariablesClient: c.configVariablesClient,
targetNamespace: targetNamespace,
Expand Down
17 changes: 3 additions & 14 deletions cmd/clusterctl/pkg/client/repository/template_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,12 @@ func Test_templates_Get(t *testing.T) {
}
type args struct {
flavor string
bootstrap string
targetNamespace string
}
type want struct {
provider config.Provider
version string
flavor string
bootstrap string
variables []string
targetNamespace string
}
Expand All @@ -64,19 +62,17 @@ func Test_templates_Get(t *testing.T) {
repository: test.NewFakeRepository().
WithPaths("root", "").
WithDefaultVersion("v1.0").
WithFile("v1.0", "config-kubeadm.yaml", templateMapYaml),
WithFile("v1.0", "cluster-template.yaml", templateMapYaml),
configVariablesClient: test.NewFakeVariableClient().WithVar(variableName, variableValue),
},
args: args{
flavor: "",
bootstrap: "kubeadm",
targetNamespace: "ns1",
},
want: want{
provider: p1,
version: "v1.0",
flavor: "",
bootstrap: "kubeadm",
variables: []string{variableName},
targetNamespace: "ns1",
},
Expand All @@ -90,19 +86,17 @@ func Test_templates_Get(t *testing.T) {
repository: test.NewFakeRepository().
WithPaths("root", "").
WithDefaultVersion("v1.0").
WithFile("v1.0", "config-prod-kubeadm.yaml", templateMapYaml),
WithFile("v1.0", "cluster-template-prod.yaml", templateMapYaml),
configVariablesClient: test.NewFakeVariableClient().WithVar(variableName, variableValue),
},
args: args{
flavor: "prod",
bootstrap: "kubeadm",
targetNamespace: "ns1",
},
want: want{
provider: p1,
version: "v1.0",
flavor: "prod",
bootstrap: "kubeadm",
variables: []string{variableName},
targetNamespace: "ns1",
},
Expand All @@ -120,7 +114,6 @@ func Test_templates_Get(t *testing.T) {
},
args: args{
flavor: "",
bootstrap: "kubeadm",
targetNamespace: "ns1",
},
wantErr: true,
Expand All @@ -129,7 +122,7 @@ func Test_templates_Get(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
f := newTemplateClient(tt.fields.provider, tt.fields.version, tt.fields.repository, tt.fields.configVariablesClient)
got, err := f.Get(tt.args.flavor, tt.args.bootstrap, tt.args.targetNamespace)
got, err := f.Get(tt.args.flavor, tt.args.targetNamespace)
if (err != nil) != tt.wantErr {
t.Fatalf("error = %v, wantErr %v", err, tt.wantErr)
}
Expand All @@ -149,10 +142,6 @@ func Test_templates_Get(t *testing.T) {
t.Errorf("got.Version() = %v, want = %v ", got.Version(), tt.want.version)
}

if got.Bootstrap() != tt.want.bootstrap {
t.Errorf("got.Bootstrap() = %v, want = %v ", got.Bootstrap(), tt.want.bootstrap)
}

if !reflect.DeepEqual(got.Variables(), tt.want.variables) {
t.Errorf("got.Variables() = %v, want = %v ", got.Variables(), tt.want.variables)
}
Expand Down
9 changes: 0 additions & 9 deletions cmd/clusterctl/pkg/client/repository/template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ func Test_newTemplate(t *testing.T) {
provider config.Provider
version string
flavor string
bootstrap string
rawYaml []byte
configVariablesClient config.VariablesClient
targetNamespace string
Expand All @@ -52,7 +51,6 @@ func Test_newTemplate(t *testing.T) {
provider config.Provider
version string
flavor string
bootstrap string
variables []string
targetNamespace string
}
Expand All @@ -68,7 +66,6 @@ func Test_newTemplate(t *testing.T) {
provider: p1,
version: "v1.2.3",
flavor: "flavor",
bootstrap: "bootstrap",
rawYaml: templateMapYaml,
configVariablesClient: test.NewFakeVariableClient().WithVar(variableName, variableValue),
targetNamespace: "ns1",
Expand All @@ -77,7 +74,6 @@ func Test_newTemplate(t *testing.T) {
provider: p1,
version: "v1.2.3",
flavor: "flavor",
bootstrap: "bootstrap",
variables: []string{variableName},
targetNamespace: "ns1",
},
Expand All @@ -90,7 +86,6 @@ func Test_newTemplate(t *testing.T) {
provider: tt.args.provider,
version: tt.args.version,
flavor: tt.args.flavor,
bootstrap: tt.args.bootstrap,
rawYaml: tt.args.rawYaml,
configVariablesClient: tt.args.configVariablesClient,
targetNamespace: tt.args.targetNamespace,
Expand All @@ -114,10 +109,6 @@ func Test_newTemplate(t *testing.T) {
t.Errorf("got.Version() = %v, want = %v ", got.Version(), tt.want.version)
}

if got.Bootstrap() != tt.want.bootstrap {
t.Errorf("got.Bootstrap() = %v, want = %v ", got.Bootstrap(), tt.want.bootstrap)
}

if !reflect.DeepEqual(got.Variables(), tt.want.variables) {
t.Errorf("got.Variables() = %v, want = %v ", got.Variables(), tt.want.variables)
}
Expand Down
7 changes: 3 additions & 4 deletions docs/book/src/clusterctl/provider-contract.md
Original file line number Diff line number Diff line change
Expand Up @@ -142,11 +142,10 @@ The following rules apply:
#### Naming conventions

Cluster templates MUST be stored in the same folder as the component YAML and follow this naming convention:
1. The default cluster template should be named `config-{bootstrap}.yaml`. e.g `config-kubeadm.yaml`
2. Additional cluster template should be named `config-{flavor}-{bootstrap}.yaml`. e.g `config-production-kubeadm.yaml`
1. The default cluster template should be named `cluster-template.yaml`.
2. Additional cluster template should be named `cluster-template-{flavor}.yaml`. e.g `cluster-template-prod.yaml`

`{bootstrap}` is the name of the bootstrap provider used in the template; `{flavor}` is the name the user can pass to the
`clusterctl config cluster --flavor` flag to identify the specific template to use.
`{flavor}` is the name the user can pass to the `clusterctl config cluster --flavor` flag to identify the specific template to use.

Each provider SHOULD create user facing documentation with the list of available cluster templates.

Expand Down

0 comments on commit f5b4666

Please sign in to comment.