-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove support for Docker 1.11, 1.12 and 1.13 #8855
Conversation
dc3f49b
to
a2c2aff
Compare
This should be the simplest thing that can be cherry-picked to 1.16+. |
/cc @rifelpet |
allErrs = append(allErrs, field.Invalid(fldPath.Child("version"), config.Version, | ||
"version is no longer available: https://www.docker.com/blog/changes-dockerproject-org-apt-yum-repositories/")) | ||
} else { | ||
valid := []string{"17.03.2", "17.09.0", "18.03.1", "18.06.1", "18.06.2", "18.06.3", "18.09.3", "18.09.9", "19.03.4", "19.03.8"} |
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.
What used to happen if a user specified a version that was not in this list? like 18.09.8 for example.
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.
Any Docker version would have been accepted and NodeUp would fail if it was not listed.
It can still fail as not all versions have support for all distros.
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.
hm does that mean that if a specific version of docker was already installed in the AMI and a user specified a version not in this list, they used to have a functioning cluster with their docker version but now their update cluster
command would fail? I'm just trying to think through any concerns with adding this validation...
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.
If a specific Docker version is install on a AMI, skipInstall
would have been used and the Docker version would not matter much, I guess.
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hakman, rifelpet 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 |
…pstream-release-1.16 Automated cherry pick of #8855: Remove support for Docker 1.11, 1.12 and 1.13
…pstream-release-1.17 Automated cherry pick of #8855: Remove support for Docker 1.11, 1.12 and 1.13
The repo containing the binaries for Docker 1.11, 1.12 and 1.13 was shut down on March 31. This means support for older docker versions cannot be maintained.
https://www.docker.com/blog/changes-dockerproject-org-apt-yum-repositories/
This PR will need to be cherry-picked to 1.16+.