Skip to content

Commit

Permalink
Merge pull request #2314 from fabriziopandini/clusterctl-drop-incompl…
Browse files Browse the repository at this point in the history
…ete-plans

🐛clusterctl: drop incomplete plans if contract change
  • Loading branch information
k8s-ci-robot authored Feb 28, 2020
2 parents 3d6efa3 + 56a063b commit 349f460
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 6 deletions.
22 changes: 20 additions & 2 deletions cmd/clusterctl/pkg/client/cluster/upgrader.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,16 @@ func (u *UpgradePlan) UpgradeRef() string {
return u.CoreProvider.InstanceName()
}

// isPartialUpgrade returns true if at least one upgradeItem in the plan does not have a target version.
func (u *UpgradePlan) isPartialUpgrade() bool {
for _, i := range u.Providers {
if i.NextVersion == "" {
return true
}
}
return false
}

// UpgradeItem defines a possible upgrade target for a provider in the management group.
type UpgradeItem struct {
clusterctlv1.Provider
Expand Down Expand Up @@ -104,11 +114,19 @@ func (u *providerUpgrader) Plan() ([]UpgradePlan, error) {
// an UpgradeItem for each provider defining the next available version with the target contract, if available.
// e.g. v1alpha3, cluster-api --> v0.3.2, kubeadm bootstrap --> v0.3.2, aws --> v0.5.4
// e.g. v1alpha4, cluster-api --> v0.4.1, kubeadm bootstrap --> v0.4.1, aws --> v0.6.2
for _, apiVersion := range contractsForUpgrade {
upgradePlan, err := u.getUpgradePlan(managementGroup, apiVersion)
for _, contract := range contractsForUpgrade {
upgradePlan, err := u.getUpgradePlan(managementGroup, contract)
if err != nil {
return nil, err
}

// If the upgrade plan is partial (at least one upgradeItem in the plan does not have a target version) and
// the upgrade plan requires a change of the contract for this management group, then drop it
// (all the provider in a management group are required to change contract at the same time).
if upgradePlan.isPartialUpgrade() && coreUpgradeInfo.currentContract != contract {
continue
}

ret = append(ret, *upgradePlan)
}
}
Expand Down
17 changes: 14 additions & 3 deletions cmd/clusterctl/pkg/client/cluster/upgrader_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ type upgradeInfo struct {
// currentVersion of the provider
currentVersion *version.Version

// currentContract of the provider
currentContract string

// nextVersions return the list of versions available for upgrades, defined as the list of version available in the provider repository
// greater than the currentVersion.
nextVersions []version.Version
Expand Down Expand Up @@ -123,10 +126,18 @@ func newUpgradeInfo(metadata *clusterctlv1.Metadata, currentVersion *version.Ver
return nextVersions[i].LessThan(&nextVersions[j])
})

// Gets the current contract for the provider
// Please note this should never be empty, because getUpgradeInfo ensures the releaseSeries defined in metadata includes the current version.
currentContract := ""
if currentReleaseSeries := metadata.GetReleaseSeriesForVersion(currentVersion); currentReleaseSeries != nil {
currentContract = currentReleaseSeries.Contract
}

return &upgradeInfo{
metadata: metadata,
currentVersion: currentVersion,
nextVersions: nextVersions,
metadata: metadata,
currentVersion: currentVersion,
currentContract: currentContract,
nextVersions: nextVersions,
}
}

Expand Down
3 changes: 2 additions & 1 deletion cmd/clusterctl/pkg/client/cluster/upgrader_info_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ func Test_providerUpgrader_getUpgradeInfo(t *testing.T) {
{Major: 1, Minor: 1, Contract: "v1alpha3"},
},
},
currentVersion: version.MustParseSemantic("v1.0.1"),
currentVersion: version.MustParseSemantic("v1.0.1"),
currentContract: "v1alpha3",
nextVersions: []version.Version{
// v1.0.1 (the current version) and older are ignored
*version.MustParseSemantic("v1.0.2"),
Expand Down
49 changes: 49 additions & 0 deletions cmd/clusterctl/pkg/client/cluster/upgrader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,55 @@ func Test_providerUpgrader_Plan(t *testing.T) {
},
wantErr: false,
},
{
name: "Single Management group, no multi-tenancy, upgrade for two contracts, but the upgrade for the second one is partially available",
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, the first with a new version for current v1alpha3 contract and a new version for the v1alpha4 contract, the second without new releases
repository: map[string]repository.Repository{
"cluster-api": test.NewFakeRepository().
WithVersions("v1.0.0", "v1.0.1", "v2.0.0").
WithMetadata("v2.0.0", &clusterctlv1.Metadata{
ReleaseSeries: []clusterctlv1.ReleaseSeries{
{Major: 1, Minor: 0, Contract: "v1alpha3"},
{Major: 2, Minor: 0, Contract: "v1alpha4"},
},
}),
"infrastructure-infra": test.NewFakeRepository().
WithVersions("v2.0.0"). // no v1alpha3 or v1alpha3 new releases yet available for the infra provider (only the current release exists)
WithMetadata("v2.0.0", &clusterctlv1.Metadata{
ReleaseSeries: []clusterctlv1.ReleaseSeries{
{Major: 2, Minor: 0, Contract: "v1alpha3"},
},
}),
},
// two providers existing in the cluster
proxy: test.NewFakeProxy().
WithProviderInventory("cluster-api", clusterctlv1.CoreProviderType, "v1.0.0", "cluster-api-system", "").
WithProviderInventory("infra", clusterctlv1.InfrastructureProviderType, "v2.0.0", "infra-system", ""),
},
want: []UpgradePlan{
{ // one upgrade plan with the latest releases in the v1alpha3 contract
Contract: "v1alpha3",
CoreProvider: fakeProvider("cluster-api", clusterctlv1.CoreProviderType, "v1.0.0", "cluster-api-system", ""),
Providers: []UpgradeItem{
{
Provider: fakeProvider("cluster-api", clusterctlv1.CoreProviderType, "v1.0.0", "cluster-api-system", ""),
NextVersion: "v1.0.1",
},
{
Provider: fakeProvider("infra", clusterctlv1.InfrastructureProviderType, "v2.0.0", "infra-system", ""),
NextVersion: "", // we are already to the latest version for the infra provider in the v1alpha3 contract, but this is acceptable for the current contract
},
},
},
// the upgrade plan with the latest releases in the v1alpha4 contract should be dropped because all the provider are required to change the contract at the same time
},
wantErr: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down

0 comments on commit 349f460

Please sign in to comment.