Improve clusterctl upgrade syntax. Don't require namespace

cmd/clusterctl/client/client.go

@@ -125,7 +125,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/clusterclass.go

@@ -60,12 +60,12 @@ func addClusterClassIfMissing(template Template, clusterClassClient repository.C
 }
 
 // clusterClassNamesFromTemplate returns the list of ClusterClasses referenced
-// by custers defined in the template. If not clusters are defined in the template
+// by clusters defined in the template. If not clusters are defined in the template
 // or if no cluster uses a cluster class it returns an empty list.
 func clusterClassNamesFromTemplate(template Template) ([]string, error) {
 	classes := []string{}
 
-	// loop thorugh all the objects and if the object is a cluster
+	// loop through all the objects and if the object is a cluster
 	// check and see if cluster.spec.topology.class is defined.
 	// If defined, add value to the result.
 	for i := range template.Objs() {

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.
@@ -176,28 +178,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 +212,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 +226,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

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

@@ -59,13 +59,19 @@ clusterctl upgrade apply --contract v1beta1
 The upgrade process is composed by three steps:
 
 * Check the cert-manager version, and if necessary, upgrade it.
-* Delete the current version of the provider components, while preserving the namespace where the provider components
-  are hosted and the provider's CRDs.
+* Delete the current version of the provider components except the CRDs.
 * Install the new version of the provider components.
 
 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 +92,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.2-to-v1.3.md

@@ -43,6 +43,7 @@ The default value is 0, meaning that the volume can be detached without any time
 
 ### Other
 
+- `clusterctl upgrade apply` has been simplified and no longer requires namespace for components.
 - e2e tests are upgraded to use Ginkgo v2 (v2.1.4) and Gomega v1.20.0. Providers who use the test framework from this release will also need to upgrade, because Ginkgo v2 can't be imported alongside v1. Please see the [Ginkgo upgrade guide](https://onsi.github.io/ginkgo/MIGRATING_TO_V2), and note:
   * the default test timeout has been [changed to 1h](https://onsi.github.io/ginkgo/MIGRATING_TO_V2#timeout-behavior)
   * the `--junit-report` argument [replaces JUnit custom reporter](https://onsi.github.io/ginkgo/MIGRATING_TO_V2#improved-reporting-infrastructure) code