Verify providers need upgrade before applying

If a user runs `upgrade plan` and the results show there are no
available upgrades, the expectation is they then do not attempt to apply
the plan. But nothing prevents them from doing so.

If a non-upgrade plan is applied, the current apply logic does not
handle the case where the "next version" is empty. To make this more
robust, and to protect against users trying to apply plans with no
available upgrades, this performs a quick validation on the plan
providers to make sure they have a version to upgrade to. If not, the
`upgrade apply` call ends up being a no-op and avoids potentially
confusing error output.

Signed-off-by: Sean McGinnis <sean.mcginnis@gmail.com>

Author: stmcginnis

cmd/clusterctl/client/cluster/upgrader.go

@@ -175,6 +175,15 @@ func (u *providerUpgrader) ApplyPlan(ctx context.Context, opts UpgradeOptions, c
 		return err
 	}
 
+	// Make sure there is something to upgrade, clear providers that do not
+	// need it
+	for i := len(upgradePlan.Providers) - 1; i >= 0; i-- {
+		if upgradePlan.Providers[i].NextVersion == "" {
+			// Remove this from our plan
+			upgradePlan.Providers = append(upgradePlan.Providers[:i], upgradePlan.Providers[i+1:]...)
+		}
+	}
+
 	// Do the upgrade
 	return u.doUpgrade(ctx, upgradePlan, opts)
 }

cmd/clusterctl/client/cluster/upgrader_test.go

@@ -1100,6 +1100,43 @@ func Test_providerUpgrader_ApplyPlan(t *testing.T) {
 			errorMsg: "detected multiple instances of the same provider",
 			opts:     UpgradeOptions{},
 		},
+		{
+			name: "does not fail when there is nothing to upgrade",
+			fields: fields{
+				// config for two providers
+				reader: test.NewFakeReader().
+					WithProvider("cluster-api", clusterctlv1.CoreProviderType, "https://somewhere.com").
+					WithProvider("infra", clusterctlv1.InfrastructureProviderType, "https://somewhere.com"),
+				// two provider repositories, with current v1alpha3 contract and new versions for v1alpha4 contract
+				repository: map[string]repository.Repository{
+					"cluster-api": repository.NewMemoryRepository().
+						WithDefaultVersion("v2.0.0").
+						WithVersions("v2.0.0").
+						WithMetadata("v2.0.0", &clusterctlv1.Metadata{
+							ReleaseSeries: []clusterctlv1.ReleaseSeries{
+								{Major: 1, Minor: 0, Contract: oldContractVersionNotSupportedAnymore},
+								{Major: 2, Minor: 0, Contract: currentContractVersion},
+							},
+						}),
+					"infrastructure-infra": repository.NewMemoryRepository().
+						WithDefaultVersion("v3.0.0").
+						WithVersions("v3.0.0").
+						WithMetadata("v3.0.0", &clusterctlv1.Metadata{
+							ReleaseSeries: []clusterctlv1.ReleaseSeries{
+								{Major: 2, Minor: 0, Contract: oldContractVersionNotSupportedAnymore},
+								{Major: 3, Minor: 0, Contract: currentContractVersion},
+							},
+						}),
+				},
+				// two providers with up to date versions
+				proxy: test.NewFakeProxy().
+					WithProviderInventory("cluster-api", clusterctlv1.CoreProviderType, "v2.0.0", "cluster-api-system").
+					WithProviderInventory("infra", clusterctlv1.InfrastructureProviderType, "v3.0.0", "infra-system-1"),
+			},
+			contract: currentContractVersion,
+			wantErr:  false,
+			opts:     UpgradeOptions{},
+		},
 	}
 
 	for _, tt := range tests {