Merge pull request #7376 from oscr/clusterctl-upgrade-syntax

✨ Improve clusterctl upgrade syntax. Don't require namespace

Author: k8s-ci-robot

cmd/clusterctl/client/client.go

@@ -127,7 +127,7 @@ type RepositoryClientFactoryInput struct {
 // RepositoryClientFactory is a factory of repository.Client from a given input.
 type RepositoryClientFactory func(RepositoryClientFactoryInput) (repository.Client, error)
 
-// ClusterClientFactoryInput reporesents the inputs required by the factory.
+// ClusterClientFactoryInput represents the inputs required by the factory.
 type ClusterClientFactoryInput struct {
 	Kubeconfig Kubeconfig
 	Processor  Processor

cmd/clusterctl/client/upgrade.go

@@ -30,6 +30,8 @@ import (
 	"sigs.k8s.io/cluster-api/cmd/clusterctl/client/cluster"
 )
 
+const upgradeItemProviderNameError = "invalid provider name %q. Provider name should be in the form namespace/provider:version or provider:version"
+
 // PlanUpgradeOptions carries the options supported by upgrade plan.
 type PlanUpgradeOptions struct {
 	// Kubeconfig defines the kubeconfig to use for accessing the management cluster. If empty, default discovery rules apply.
@@ -97,16 +99,20 @@ type ApplyUpgradeOptions struct {
 	// a more granular control on upgrade, use CoreProvider, BootstrapProviders, ControlPlaneProviders, InfrastructureProviders.
 	Contract string
 
-	// CoreProvider instance and version (e.g. capi-system/cluster-api:v1.1.5) to upgrade to. This field can be used as alternative to Contract.
+	// CoreProvider instance and version (e.g. [capi-system/]cluster-api:v1.1.5) to upgrade to. This field can be used as alternative to Contract.
+	// Specifying a namespace is now optional and in the future it will be deprecated.
 	CoreProvider string
 
-	// BootstrapProviders instance and versions (e.g. capi-kubeadm-bootstrap-system/kubeadm:v1.1.5) to upgrade to. This field can be used as alternative to Contract.
+	// BootstrapProviders instance and versions (e.g. [capi-kubeadm-bootstrap-system/]kubeadm:v1.1.5) to upgrade to. This field can be used as alternative to Contract.
+	// Specifying a namespace is now optional and in the future it will be deprecated.
 	BootstrapProviders []string
 
-	// ControlPlaneProviders instance and versions (e.g. capi-kubeadm-control-plane-system/kubeadm:v1.1.5) to upgrade to. This field can be used as alternative to Contract.
+	// ControlPlaneProviders instance and versions (e.g. [capi-kubeadm-control-plane-system/]kubeadm:v1.1.5) to upgrade to. This field can be used as alternative to Contract.
+	// Specifying a namespace is now optional and in the future it will be deprecated.
 	ControlPlaneProviders []string
 
-	// InfrastructureProviders instance and versions (e.g. capa-system/aws:v0.5.0) to upgrade to. This field can be used as alternative to Contract.
+	// InfrastructureProviders instance and versions (e.g. [capa-system/]aws:v0.5.0) to upgrade to. This field can be used as alternative to Contract.
+	// Specifying a namespace is now optional and in the future it will be deprecated.
 	InfrastructureProviders []string
 
 	// IPAMProviders instance and versions (e.g. ipam-system/infoblox:v0.0.1) to upgrade to. This field can be used as alternative to Contract.
@@ -176,28 +182,28 @@ func (c *clusterctlClient) ApplyUpgrade(options ApplyUpgradeOptions) error {
 		upgradeItems := []cluster.UpgradeItem{}
 
 		if options.CoreProvider != "" {
-			upgradeItems, err = addUpgradeItems(upgradeItems, clusterctlv1.CoreProviderType, options.CoreProvider)
+			upgradeItems, err = addUpgradeItems(clusterClient, upgradeItems, clusterctlv1.CoreProviderType, options.CoreProvider)
 			if err != nil {
 				return err
 			}
 		}
-		upgradeItems, err = addUpgradeItems(upgradeItems, clusterctlv1.BootstrapProviderType, options.BootstrapProviders...)
+		upgradeItems, err = addUpgradeItems(clusterClient, upgradeItems, clusterctlv1.BootstrapProviderType, options.BootstrapProviders...)
 		if err != nil {
 			return err
 		}
-		upgradeItems, err = addUpgradeItems(upgradeItems, clusterctlv1.ControlPlaneProviderType, options.ControlPlaneProviders...)
+		upgradeItems, err = addUpgradeItems(clusterClient, upgradeItems, clusterctlv1.ControlPlaneProviderType, options.ControlPlaneProviders...)
 		if err != nil {
 			return err
 		}
-		upgradeItems, err = addUpgradeItems(upgradeItems, clusterctlv1.InfrastructureProviderType, options.InfrastructureProviders...)
+		upgradeItems, err = addUpgradeItems(clusterClient, upgradeItems, clusterctlv1.InfrastructureProviderType, options.InfrastructureProviders...)
 		if err != nil {
 			return err
 		}
-		upgradeItems, err = addUpgradeItems(upgradeItems, clusterctlv1.IPAMProviderType, options.IPAMProviders...)
+		upgradeItems, err = addUpgradeItems(clusterClient, upgradeItems, clusterctlv1.IPAMProviderType, options.IPAMProviders...)
 		if err != nil {
 			return err
 		}
-		upgradeItems, err = addUpgradeItems(upgradeItems, clusterctlv1.RuntimeExtensionProviderType, options.RuntimeExtensionProviders...)
+		upgradeItems, err = addUpgradeItems(clusterClient, upgradeItems, clusterctlv1.RuntimeExtensionProviderType, options.RuntimeExtensionProviders...)
 		if err != nil {
 			return err
 		}
@@ -210,9 +216,9 @@ func (c *clusterctlClient) ApplyUpgrade(options ApplyUpgradeOptions) error {
 	return clusterClient.ProviderUpgrader().ApplyPlan(opts, options.Contract)
 }
 
-func addUpgradeItems(upgradeItems []cluster.UpgradeItem, providerType clusterctlv1.ProviderType, providers ...string) ([]cluster.UpgradeItem, error) {
+func addUpgradeItems(clusterClient cluster.Client, upgradeItems []cluster.UpgradeItem, providerType clusterctlv1.ProviderType, providers ...string) ([]cluster.UpgradeItem, error) {
 	for _, upgradeReference := range providers {
-		providerUpgradeItem, err := parseUpgradeItem(upgradeReference, providerType)
+		providerUpgradeItem, err := parseUpgradeItem(clusterClient, upgradeReference, providerType)
 		if err != nil {
 			return nil, err
 		}
@@ -224,20 +230,63 @@ func addUpgradeItems(upgradeItems []cluster.UpgradeItem, providerType clusterctl
 	return upgradeItems, nil
 }
 
-func parseUpgradeItem(ref string, providerType clusterctlv1.ProviderType) (*cluster.UpgradeItem, error) {
+func parseUpgradeItem(clusterClient cluster.Client, ref string, providerType clusterctlv1.ProviderType) (*cluster.UpgradeItem, error) {
+	// TODO(oscr) Remove when explicit namespaces for providers is removed
+	// ref format is old format: namespace/provider:version
+	if strings.Contains(ref, "/") {
+		return parseUpgradeItemWithNamespace(ref, providerType)
+	}
+
+	// ref format is: provider:version
+	return parseUpgradeItemWithoutNamespace(clusterClient, ref, providerType)
+}
+
+func parseUpgradeItemWithNamespace(ref string, providerType clusterctlv1.ProviderType) (*cluster.UpgradeItem, error) {
 	refSplit := strings.Split(strings.ToLower(ref), "/")
+
 	if len(refSplit) != 2 {
-		return nil, errors.Errorf("invalid provider name %q. Provider name should be in the form namespace/provider[:version]", ref)
+		return nil, errors.Errorf(upgradeItemProviderNameError, ref)
 	}
 
 	if refSplit[0] == "" {
-		return nil, errors.Errorf("invalid provider name %q. Provider name should be in the form namespace/name[:version] and namespace cannot be empty", ref)
+		return nil, errors.Errorf(upgradeItemProviderNameError, ref)
 	}
 	namespace := refSplit[0]
 
 	name, version, err := parseProviderName(refSplit[1])
 	if err != nil {
-		return nil, errors.Wrapf(err, "invalid provider name %q. Provider name should be in the form namespace/name[:version] and the namespace should be valid", ref)
+		return nil, errors.Wrapf(err, upgradeItemProviderNameError, ref)
+	}
+
+	return &cluster.UpgradeItem{
+		Provider: clusterctlv1.Provider{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: namespace,
+				Name:      clusterctlv1.ManifestLabel(name, providerType),
+			},
+			ProviderName: name,
+			Type:         string(providerType),
+			// The value for the following fields will be retrieved while
+			// creating the custom upgrade plan.
+			WatchedNamespace: "",
+		},
+		NextVersion: version,
+	}, nil
+}
+
+func parseUpgradeItemWithoutNamespace(clusterClient cluster.Client, ref string, providerType clusterctlv1.ProviderType) (*cluster.UpgradeItem, error) {
+	if !strings.Contains(ref, ":") {
+		return nil, errors.Errorf(upgradeItemProviderNameError, ref)
+	}
+
+	name, version, err := parseProviderName(ref)
+	if err != nil {
+		return nil, errors.Wrapf(err, upgradeItemProviderNameError, ref)
+	}
+
+	namespace, err := clusterClient.ProviderInventory().GetProviderNamespace(name, providerType)
+	if err != nil {
+		return nil, errors.Errorf("unable to find default namespace for provider %q", ref)
 	}
 
 	return &cluster.UpgradeItem{

cmd/clusterctl/client/upgrade_test.go

@@ -375,6 +375,11 @@ func Test_parseUpgradeItem(t *testing.T) {
 	type args struct {
 		provider string
 	}
+
+	configClient := newFakeConfig()
+	clusterClient := newFakeCluster(cluster.Kubeconfig{Path: "cluster1"}, configClient)
+	clusterClient.WithProviderInventory("best-provider", clusterctlv1.CoreProviderType, "v1.0.0", "best-provider-system")
+
 	tests := []struct {
 		name    string
 		args    args
@@ -418,9 +423,35 @@ func Test_parseUpgradeItem(t *testing.T) {
 			wantErr: false,
 		},
 		{
-			name: "namespace missing",
+			name: "provider:version",
+			args: args{
+				provider: "best-provider:v1.0.0",
+			},
+			want: &cluster.UpgradeItem{
+				Provider: clusterctlv1.Provider{
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: "best-provider-system",
+						Name:      clusterctlv1.ManifestLabel("best-provider", clusterctlv1.CoreProviderType),
+					},
+					ProviderName: "best-provider",
+					Type:         string(clusterctlv1.CoreProviderType),
+				},
+				NextVersion: "v1.0.0",
+			},
+			wantErr: false,
+		},
+		{
+			name: "provider: with no version",
+			args: args{
+				provider: "provider:",
+			},
+			want:    nil,
+			wantErr: true,
+		},
+		{
+			name: "provider with no version",
 			args: args{
-				provider: "provider:version",
+				provider: "provider",
 			},
 			want:    nil,
 			wantErr: true,
@@ -438,7 +469,7 @@ func Test_parseUpgradeItem(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			g := NewWithT(t)
 
-			got, err := parseUpgradeItem(tt.args.provider, clusterctlv1.CoreProviderType)
+			got, err := parseUpgradeItem(clusterClient, tt.args.provider, clusterctlv1.CoreProviderType)
 			if tt.wantErr {
 				g.Expect(err).To(HaveOccurred())
 				return

cmd/clusterctl/cmd/upgrade_apply.go

@@ -48,15 +48,17 @@ var upgradeApplyCmd = &cobra.Command{
 		The upgrade apply command applies new versions of Cluster API providers as defined by clusterctl upgrade plan.
 
 		New version should be applied ensuring all the providers uses the same cluster API version
-		in order to guarantee the proper functioning of the management cluster.`),
+		in order to guarantee the proper functioning of the management cluster.
+
+ 		Specifying the provider using namespace/name:version is deprecated and will be dropped in a future release.`),
 
 	Example: Examples(`
 		# Upgrades all the providers in the management cluster to the latest version available which is compliant
 		# to the v1alpha4 API Version of Cluster API (contract).
 		clusterctl upgrade apply --contract v1alpha4
 
-		# Upgrades only the capa-system/aws provider to the v0.5.0 version.
-		clusterctl upgrade apply --infrastructure capa-system/aws:v0.5.0`),
+		# Upgrades only the aws provider to the v2.0.1 version.
+		clusterctl upgrade apply --infrastructure aws:v2.0.1`),
 	Args: cobra.NoArgs,
 	RunE: func(cmd *cobra.Command, args []string) error {
 		return runUpgradeApply()
@@ -72,13 +74,13 @@ func init() {
 		"The API Version of Cluster API (contract, e.g. v1alpha4) the management cluster should upgrade to")
 
 	upgradeApplyCmd.Flags().StringVar(&ua.coreProvider, "core", "",
-		"Core provider instance version (e.g. capi-system/cluster-api:v1.1.5) to upgrade to. This flag can be used as alternative to --contract.")
+		"Core provider instance version (e.g. cluster-api:v1.1.5) to upgrade to. This flag can be used as alternative to --contract.")
 	upgradeApplyCmd.Flags().StringSliceVarP(&ua.infrastructureProviders, "infrastructure", "i", nil,
-		"Infrastructure providers instance and versions (e.g. capa-system/aws:v0.5.0) to upgrade to. This flag can be used as alternative to --contract.")
+		"Infrastructure providers instance and versions (e.g. aws:v2.0.1) to upgrade to. This flag can be used as alternative to --contract.")
 	upgradeApplyCmd.Flags().StringSliceVarP(&ua.bootstrapProviders, "bootstrap", "b", nil,
-		"Bootstrap providers instance and versions (e.g. capi-kubeadm-bootstrap-system/kubeadm:v1.1.5) to upgrade to. This flag can be used as alternative to --contract.")
+		"Bootstrap providers instance and versions (e.g. kubeadm:v1.1.5) to upgrade to. This flag can be used as alternative to --contract.")
 	upgradeApplyCmd.Flags().StringSliceVarP(&ua.controlPlaneProviders, "control-plane", "c", nil,
-		"ControlPlane providers instance and versions (e.g. capi-kubeadm-control-plane-system/kubeadm:v1.1.5) to upgrade to. This flag can be used as alternative to --contract.")
+		"ControlPlane providers instance and versions (e.g. kubeadm:v1.1.5) to upgrade to. This flag can be used as alternative to --contract.")
 	upgradeApplyCmd.Flags().StringSliceVar(&ua.ipamProviders, "ipam", nil,
 		"IPAM providers and versions (e.g. infoblox:v0.0.1) to upgrade to. This flag can be used as alternative to --contract.")
 	upgradeApplyCmd.Flags().StringSliceVar(&ua.runtimeExtensionProviders, "runtime-extension", nil,

docs/book/src/clusterctl/commands/upgrade.md

@@ -66,6 +66,13 @@ The upgrade process is composed by three steps:
 Please note that clusterctl does not upgrade Cluster API objects (Clusters, MachineDeployments, Machine etc.); upgrading
 such objects are the responsibility of the provider's controllers.
 
+It is also possible to explicitly upgrade one or more components to specific versions.
+```bash
+clusterctl upgrade apply \
+    --core cluster-api:v1.2.4 \
+    --infrastructure docker:v1.2.4
+```
+
 <aside class="note warning">
 
 <h1>Warning!</h1>
@@ -86,10 +93,10 @@ the following:
 
 ```bash
 clusterctl upgrade apply \
-    --core capi-system/cluster-api:v1.0.0 \
-    --bootstrap capi-kubeadm-bootstrap-system/kubeadm:v1.0.0 \
-    --control-plane capi-kubeadm-control-plane-system/kubeadm:v1.0.0 \
-    --infrastructure capd-system/docker:v1.0.0-rc.0
+    --core cluster-api:v1.0.0 \
+    --bootstrap kubeadm:v1.0.0 \
+    --control-plane kubeadm:v1.0.0 \
+    --infrastructure docker:v1.0.0-rc.0
 ```
 
 In this case, all the provider's versions must be explicitly stated.

docs/book/src/developer/providers/v1.3-to-v1.4.md

@@ -9,10 +9,8 @@ maintainers of providers and consumers of our Go API.
 
 ## Dependencies
 
-**Note**: Only the most relevant dependencies are listed, `k8s.io/` and `ginkgo`/`gomega` dependencies
-in Cluster API are kept in sync with the versions used by `sigs.k8s.io/controller-runtime`. 
+**Note**: Only the most relevant dependencies are listed, `k8s.io/` and `ginkgo`/`gomega` dependencies in Cluster API are kept in sync with the versions used by `sigs.k8s.io/controller-runtime`.
 
--
 
 ## Changes by Kind
 
@@ -30,4 +28,6 @@ in Cluster API are kept in sync with the versions used by `sigs.k8s.io/controlle
 
 ### Other
 
--
+- `clusterctl upgrade apply` no longer requires a namespace when updating providers. It is now optional and in a future release it will be deprecated. The new syntax is `[namespace/]provider:version`.
+
+### Suggested changes for providers