Skip to content
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

fix(dockerfile): use deterministic version for kubectl #4064

Merged
merged 1 commit into from
Oct 2, 2019

Conversation

ethanfrogers
Copy link
Contributor

reading from stable.txt causes builds to be non-deterministic which could lead to problems in the future. we should be explicit about the version of kubectl that we ship. v1.16.0 was the latest version as of this PR.

reading from `stable.txt` causes builds to be non-deterministic which could lead to problems in the future. we should be explicit about the version of `kubectl` that we ship. `v1.16.0` was the latest version as of this PR.
@ezimanyi
Copy link
Contributor

ezimanyi commented Oct 2, 2019

I wonder if we might want to actually stay one release behind the latest, given that kubectl officially supports versions of Kubernetes +/- 1 version from its version.

So if we ship kubectl 1.16 we technically are only supporting Kubernetes 1.15 and 1.16 (though in principle probably further back). Shipping 1.15 would give us support for Kubernetes 1.14-1.16. Given that 1.16 is still really new and not likely to be used in production, I even wonder if we should ship kubectl 1.14 to officially support 1.13-1.15 and then have things outside that range be probably supported.

@ethanfrogers
Copy link
Contributor Author

So, I went with 1.16.0 because that's what got shipped with the 1.16 and 1.15 releases yesterday (10/1/2019). I'm hesitant to go backwards after that, honestly.

How do you feel about shipping multiple versions of kubectl? Kubernetes accounts allow you to specify a path to a specific kubectl executable. We could package multiple, default a specific one, but tell users where other exists on the filesystem if they need to override it.

@ezimanyi
Copy link
Contributor

ezimanyi commented Oct 2, 2019

Given that all the calls to kubectl are orchestrated by clouddriver it seems like it would be complicated for users to try to override it. (I guess you could point the symlink to a different one---but if we're going to make people ssh to their docker containers and modify symlinks, I'm not sure that making them directly download what they want is any worse.)

It's true that we probably don't want to go backwards, but maybe going forward we can think more about the policy, and pin kubectl to 1.16 until whatever policy we decide tells us to bump it.

@ethanfrogers
Copy link
Contributor Author

The ManagedAccount class exposes kubectlExecutable so it should be as simple as saying "kubectl executables are located here". It should be as simple as adding a property to the accounts they need to override.

@ethanfrogers ethanfrogers merged commit 133a6e6 into master Oct 2, 2019
@ethanfrogers ethanfrogers deleted the ethanfrogers-patch-1 branch October 2, 2019 21:39
@ethanfrogers
Copy link
Contributor Author

@spinnakerbot cherry-pick 1.16

@ethanfrogers
Copy link
Contributor Author

@spinnakerbot cherry-pick 1.15

spinnakerbot pushed a commit that referenced this pull request Oct 2, 2019
reading from `stable.txt` causes builds to be non-deterministic which could lead to problems in the future. we should be explicit about the version of `kubectl` that we ship. `v1.16.0` was the latest version as of this PR.
@spinnakerbot
Copy link
Contributor

Cherry pick successful: #4065

spinnakerbot pushed a commit that referenced this pull request Oct 2, 2019
reading from `stable.txt` causes builds to be non-deterministic which could lead to problems in the future. we should be explicit about the version of `kubectl` that we ship. `v1.16.0` was the latest version as of this PR.
@spinnakerbot
Copy link
Contributor

Cherry pick successful: #4066

@ezimanyi
Copy link
Contributor

ezimanyi commented Oct 2, 2019

Oh, nice, didn't realize you could configure kubectlExecutable; that might be a good solution then. (I guess it still does have the downside of bringing along multiple kubectl versions in the container even for people who don't need them, though maybe that's minor.)

louisjimenez pushed a commit that referenced this pull request Oct 3, 2019
reading from `stable.txt` causes builds to be non-deterministic which could lead to problems in the future. we should be explicit about the version of `kubectl` that we ship. `v1.16.0` was the latest version as of this PR.
louisjimenez pushed a commit that referenced this pull request Oct 3, 2019
reading from `stable.txt` causes builds to be non-deterministic which could lead to problems in the future. we should be explicit about the version of `kubectl` that we ship. `v1.16.0` was the latest version as of this PR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants