-
Notifications
You must be signed in to change notification settings - Fork 560
Reduce the Kubernetes version support matrix #3750
Reduce the Kubernetes version support matrix #3750
Conversation
pkg/api/common/versions.go
Outdated
"1.7.12": false, | ||
"1.7.13": false, | ||
"1.7.14": false, | ||
"1.7.15": false, | ||
"1.7.16": true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we keep 1.7.16?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we know if any acs-engine users still provision 1.7 clusters? Otherwise, I'd say get rid of it except that 1.7.x is still supported in AKS currently. (I assume we can't remove Kubernetes versions that AKS still expects.)
Should we keep the last two patch versions for each minor? That's the slightly more conservative policy that I've heard mentioned as a goal for AKS, for example. |
@@ -862,16 +862,6 @@ func (w *WindowsProfile) Validate(orchestratorType string) error { | |||
func (k *KubernetesConfig) Validate(k8sVersion string, hasWindows bool) error { | |||
// number of minimum retries allowed for kubelet to post node status | |||
const minKubeletRetries = 4 | |||
// k8s versions that have cloudprovider backoff enabled | |||
var backoffEnabledVersions = common.AllKubernetesSupportedVersions | |||
// at present all supported versions allow for cloudprovider backoff |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at present all supported versions allow for cloudprovider backoff
--> this validation is a no-op, remove it for now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:)
Codecov Report
@@ Coverage Diff @@
## master #3750 +/- ##
==========================================
+ Coverage 55.48% 55.49% +0.01%
==========================================
Files 108 108
Lines 16140 16138 -2
==========================================
+ Hits 8955 8956 +1
- Misses 6420 6421 +1
+ Partials 765 761 -4 |
Back-compat tests:
|
Error message when trying to deploy a cluster with a deprecated version:
|
Error message when trying to upgrade to deprecated version:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. However, it would require some changes in RP as we moved apiloader
into RP code base
Orchestrators command output:
|
pkg/api/apiloader.go
Outdated
@@ -251,14 +251,14 @@ func (a *Apiloader) LoadContainerServiceForAgentPoolOnlyCluster( | |||
|
|||
// use defaultKubernetesVersion arg if no version was supplied in the request contents | |||
if managedCluster.Properties.KubernetesVersion == "" && defaultKubernetesVersion != "" { | |||
if !common.AllKubernetesSupportedVersions[defaultKubernetesVersion] { | |||
if !common.IsSupportedKubernetesVersion(defaultKubernetesVersion, isUpdate, false) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: instead of passing in the literal false
create a hasWindows
var with the value false
and pass in hasWindows
for readability.
pkg/api/apiloader.go
Outdated
return nil, IsSSHAutoGenerated, a.Translator.Errorf("The selected orchestrator version '%s' is not supported", defaultKubernetesVersion) | ||
} | ||
managedCluster.Properties.KubernetesVersion = defaultKubernetesVersion | ||
} | ||
|
||
// verify orchestrator version | ||
if len(managedCluster.Properties.KubernetesVersion) > 0 && !common.AllKubernetesSupportedVersions[managedCluster.Properties.KubernetesVersion] { | ||
if len(managedCluster.Properties.KubernetesVersion) > 0 && !common.IsSupportedKubernetesVersion(managedCluster.Properties.KubernetesVersion, isUpdate, false) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
pkg/api/apiloader.go
Outdated
@@ -289,7 +289,7 @@ func (a *Apiloader) LoadContainerServiceForAgentPoolOnlyCluster( | |||
|
|||
// use defaultKubernetesVersion arg if no version was supplied in the request contents | |||
if managedCluster.Properties.KubernetesVersion == "" && defaultKubernetesVersion != "" { | |||
if !common.AllKubernetesSupportedVersions[defaultKubernetesVersion] { | |||
if !common.IsSupportedKubernetesVersion(defaultKubernetesVersion, isUpdate, false) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
pkg/api/apiloader.go
Outdated
@@ -300,7 +300,7 @@ func (a *Apiloader) LoadContainerServiceForAgentPoolOnlyCluster( | |||
} | |||
|
|||
// verify orchestrator version | |||
if len(managedCluster.Properties.KubernetesVersion) > 0 && !common.AllKubernetesSupportedVersions[managedCluster.Properties.KubernetesVersion] { | |||
if len(managedCluster.Properties.KubernetesVersion) > 0 && !common.IsSupportedKubernetesVersion(managedCluster.Properties.KubernetesVersion, isUpdate, false) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
/lgtm pending consideration of nits |
[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:
Approvers can indicate their approval by writing |
What this PR does / why we need it: Deprecates Kubernetes versions except for the two latest patches of each minor release while maintaining support for scale and upgrade from these versions. Exception is version 1.6.9 which we need to keep support for because of API v20160930.
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close that issue when PR gets merged): fixes #Special notes for your reviewer:
If applicable:
Release note: