Allow deprecating k8s versions #3493

CecileRobertMichon · 2018-07-17T19:04:03Z

What this PR does / why we need it: This PR allows removing support for k8s versions (by setting the "supported" value to false in the version map) and keep support for scaling/upgrading those clusters

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #3399

Special notes for your reviewer:

If applicable:

documentation
unit tests
tested backward compatibility (ie. deploy with previous version, upgrade with this branch)

Release note:

CecileRobertMichon · 2018-07-17T19:05:35Z

pkg/api/common/versions.go

+		allSupportedVersions = AllKubernetesWindowsSupportedVersions
+	}
+	for ver, supported := range allSupportedVersions {
+		if (isCreate && supported) || !isCreate {


this is the key change in this PR

CecileRobertMichon · 2018-07-17T21:19:27Z

pkg/api/common/versions.go

@@ -235,28 +239,11 @@ func getAllKubernetesWindowsSupportedVersionsMap() map[string]bool {
 	return ret
 }

-// GetAllSupportedKubernetesVersionsWindows returns a slice of all supported Kubernetes versions on Windows
-func GetAllSupportedKubernetesVersionsWindows() []string {


combined GetAllSupportedKubernetesVersionsWindows() with GetAllSupportedKubernetesVersions()

CecileRobertMichon · 2018-07-18T17:21:40Z

pkg/api/common/versions.go

+		allSupportedVersions = AllKubernetesWindowsSupportedVersions
+	}
+	for ver, supported := range allSupportedVersions {
+		if (!isUpdate && supported) || isUpdate {


this is the key change in this PR

codecov · 2018-07-18T20:38:19Z

Codecov Report

Merging #3493 into master will decrease coverage by 0.01%.
The diff coverage is 85.5%.

@@            Coverage Diff             @@
##           master    #3493      +/-   ##
==========================================
- Coverage   55.41%   55.39%   -0.02%     
==========================================
  Files         108      108              
  Lines       16098    16102       +4     
==========================================
  Hits         8920     8920              
- Misses       6412     6415       +3     
- Partials      766      767       +1

CecileRobertMichon · 2018-07-18T20:52:00Z

Tests running on CecileRobertMichon/acs-engine@k8s-versions-distinct...CecileRobertMichon:test-deprecate-versions (disables all versions expect [1.8.15 1.9.9 1.10.5 1.11.0])

Deploy a deprecated version (expected to fail): https://jenkins.azure-containers.io/view/acs-engine%20ad-hoc/job/k8s-deployment/206/console
Upgrade a cluster deployed with a now deprecated version: https://jenkins.azure-containers.io/view/acs-engine%20ad-hoc/job/k8s-upgrade/128/console
Scale a cluster deployed with a now deprecated version: https://jenkins.azure-containers.io/view/acs-engine%20ad-hoc/job/k8s-scale/41/console

CecileRobertMichon · 2018-07-18T22:59:14Z

TODO:

Only validate version that we are upgrading to in upgrade (not the version we are upgrading from)
Only allow upgrade to versions currently supported (allow list for create and upgrade is identical)
Fix the orchestrators command to reflect the above
Fix logic for upgrading a cluster with Windows --> only versions supported for Windows should be allowed

CecileRobertMichon · 2018-07-19T00:24:11Z

After thinking about this some more, I don't think we should do

Only validate version that we are upgrading to in upgrade (not the version we are upgrading from)

The validate happens at https://github.com/Azure/acs-engine/blob/master/cmd/upgrade.go#L141 and actually makes sure that the apimodel passed in by the user is valid. If we do not validate it, there is no garantee that the apimodel passed in with the acs-engine upgrade command has an OrchestratorProfile or is a valid acs-engine apimodel. However, we can validate the apimodel with isUpdate true such that the version is compared to the list of all existing versions, not just supported create versions. Then, the upgrade version is validated against the subset of valid Create/Upgrade versions.

CecileRobertMichon · 2018-07-19T22:55:48Z

pkg/acsengine/defaults.go

@@ -365,7 +365,7 @@ var (
 func setPropertiesDefaults(cs *api.ContainerService, isUpgrade, isScale bool) (bool, error) {
 	properties := cs.Properties

-	setOrchestratorDefaults(cs)
+	setOrchestratorDefaults(cs, isUpgrade || isScale)


isUpgrade || isScale <=> isUpdate

CecileRobertMichon · 2018-07-19T22:58:39Z

pkg/api/orchestrators.go

 	ret := []*OrchestratorProfile{}

 	currentVer, err := semver.Make(csOrch.OrchestratorVersion)
 	if err != nil {
 		return nil, err
 	}
-	nextNextMinorString := strconv.FormatUint(currentVer.Major, 10) + "." + strconv.FormatUint(currentVer.Minor+2, 10) + ".0"
-	upgradeableVersions := common.GetVersionsBetween(common.GetAllSupportedKubernetesVersions(), csOrch.OrchestratorVersion, nextNextMinorString, false, true)
+	nextNextMinorString := strconv.FormatUint(currentVer.Major, 10) + "." + strconv.FormatUint(currentVer.Minor+2, 10) + ".0-alpha.0"


fix bug where pre-releases were include in previous previous minor version upgrades

wip merge windows and non-windows k8s versions add isupdate logic add missing args add isCreate to defaults set orchestratorDefaults args fmt change isCreate to isUpdate merge getDefaultVersion fix !isUpdate arg remove windows versions

CecileRobertMichon · 2018-08-16T22:31:56Z

upgrade: https://jenkins.azure-containers.io/view/acs-engine%20ad-hoc/job/k8s-upgrade/145/console
scale: https://jenkins.azure-containers.io/view/acs-engine%20ad-hoc/job/k8s-scale/46/console

jackfrancis · 2018-08-17T17:18:19Z

/lgtm

acs-bot · 2018-08-17T17:18:23Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CecileRobertMichon, jackfrancis

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [CecileRobertMichon,jackfrancis]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ghost assigned CecileRobertMichon Jul 17, 2018

ghost added the in progress label Jul 17, 2018

CecileRobertMichon changed the title ~~Allow depricating k8s versions~~ Allow deprecating k8s versions Jul 17, 2018

CecileRobertMichon commented Jul 17, 2018

View reviewed changes

CecileRobertMichon changed the title ~~Allow deprecating k8s versions~~ [WIP] Allow deprecating k8s versions Jul 17, 2018

CecileRobertMichon force-pushed the k8s-versions-distinct branch from 708abd7 to 6aceddb Compare July 17, 2018 19:19

CecileRobertMichon commented Jul 17, 2018

View reviewed changes

CecileRobertMichon commented Jul 18, 2018

View reviewed changes

CecileRobertMichon force-pushed the k8s-versions-distinct branch from 241a61d to f0bc09d Compare July 18, 2018 18:55

CecileRobertMichon changed the title ~~[WIP] Allow deprecating k8s versions~~ Allow deprecating k8s versions Jul 18, 2018

CecileRobertMichon commented Jul 19, 2018

View reviewed changes

CecileRobertMichon added the do-not-merge/hold label Jul 20, 2018

jackfrancis force-pushed the k8s-versions-distinct branch from ecc169c to fa4a3e8 Compare July 26, 2018 00:16

ghost assigned jackfrancis Jul 26, 2018

acs-bot added approved size/L labels Jul 26, 2018

CecileRobertMichon added 7 commits August 16, 2018 10:32

allow to upgrade and scale deprecated versions

5f79587

wip merge windows and non-windows k8s versions add isupdate logic add missing args add isCreate to defaults set orchestratorDefaults args fmt change isCreate to isUpdate merge getDefaultVersion fix !isUpdate arg remove windows versions

upgrade only to supported versions

e0ad8f6

fix typo

f376125

Minor version starts at 1.x.0.alpha-0

e55e8b2

allow to specify a deprecated version

95885e3

fix upgrade for windows

ee47679

lint

12a276d

CecileRobertMichon force-pushed the k8s-versions-distinct branch from fa4a3e8 to 12a276d Compare August 16, 2018 17:32

acs-bot added the lgtm label Aug 17, 2018

jackfrancis merged commit 5645440 into Azure:master Aug 17, 2018

ghost removed the in progress label Aug 17, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow deprecating k8s versions #3493

Allow deprecating k8s versions #3493

CecileRobertMichon commented Jul 17, 2018 •

edited

Loading

CecileRobertMichon Jul 17, 2018

CecileRobertMichon Jul 17, 2018

CecileRobertMichon Jul 18, 2018

codecov bot commented Jul 18, 2018 •

edited

Loading

CecileRobertMichon commented Jul 18, 2018 •

edited

Loading

CecileRobertMichon commented Jul 18, 2018 •

edited

Loading

CecileRobertMichon commented Jul 19, 2018 •

edited

Loading

CecileRobertMichon Jul 19, 2018

CecileRobertMichon Jul 19, 2018

CecileRobertMichon commented Aug 16, 2018

jackfrancis commented Aug 17, 2018

acs-bot commented Aug 17, 2018

Allow deprecating k8s versions #3493

Allow deprecating k8s versions #3493

Conversation

CecileRobertMichon commented Jul 17, 2018 • edited Loading

CecileRobertMichon Jul 17, 2018

Choose a reason for hiding this comment

CecileRobertMichon Jul 17, 2018

Choose a reason for hiding this comment

CecileRobertMichon Jul 18, 2018

Choose a reason for hiding this comment

codecov bot commented Jul 18, 2018 • edited Loading

Codecov Report

CecileRobertMichon commented Jul 18, 2018 • edited Loading

CecileRobertMichon commented Jul 18, 2018 • edited Loading

CecileRobertMichon commented Jul 19, 2018 • edited Loading

CecileRobertMichon Jul 19, 2018

Choose a reason for hiding this comment

CecileRobertMichon Jul 19, 2018

Choose a reason for hiding this comment

CecileRobertMichon commented Aug 16, 2018

jackfrancis commented Aug 17, 2018

acs-bot commented Aug 17, 2018

CecileRobertMichon commented Jul 17, 2018 •

edited

Loading

codecov bot commented Jul 18, 2018 •

edited

Loading

CecileRobertMichon commented Jul 18, 2018 •

edited

Loading

CecileRobertMichon commented Jul 18, 2018 •

edited

Loading

CecileRobertMichon commented Jul 19, 2018 •

edited

Loading