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

Control dynamic kubectl download #18

Open
carolynvs opened this issue Sep 29, 2020 · 4 comments
Open

Control dynamic kubectl download #18

carolynvs opened this issue Sep 29, 2020 · 4 comments
Labels
design 🚲🏠 Bust out your paint chips, it's time to bikeshed.

Comments

@carolynvs
Copy link
Member

carolynvs commented Sep 29, 2020

There are a few cases where dynamically downloading the kubectl client may not be what the bundle author desired and we should make it controllable by them:

  • kubectl is backwards compatible. When the bundle has a more recent version of kubectl, there isn't a reason to download an older version to match.
  • There may be bug fixes in newer patch versions of kubectl so matching down to the patch isn't always a great behavior when what we really need is API compatibility.
  • The bundle may be self-contained and not want to reach out to external resources, perhaps a thick bundle, and attempts to dynamically download the client should be strictly disabled.

Based on this (please add more in the comments below as you see them), I propose that we add the following configuration to the mixin:

mixins:
- kubernetes
     dynamicDownload:
       versionMatch: exact | minor
       useBackwardsCompatible: true
       disabled: true
  • dynamicDownload: Defines the behavior of the dynamic kubectl client download
    • versionMatch: Defines how the api server version is compared to the client version to determine whether or not a different client should be downloaded. Defaults to minor.
      A download is triggered when:
      • exact - The versions do not match exactly.
      • minor - The Major.Minor versions do not match. The patch version is allowed to be different.
    • useBackwardsCompatible - When the client is newer than the server, take advantage of the kubectl backwards compatibility guarantee and do not download an older client. Defaults to true. (I tried to make a name that worked well for defaulting to false and couldn't come up with one)
    • disabled: Specifies if the dynamic download feature is disabled. Defaults to false.

With the defaults someone who doesn't customize the config settings will only download a client when the server is a newer Major.Minor version. What do you think?

@carolynvs carolynvs added the design 🚲🏠 Bust out your paint chips, it's time to bikeshed. label Sep 29, 2020
@vdice
Copy link
Member

vdice commented Sep 29, 2020

This looks good to me.

minor - The Major.Minor versions do not match. The patch version is allowed to be different.

Should be minor - The Major.Minor versions match. The patch version is allowed to be different., right?

My only other thought is to think about if this will prove to be generically useful across mixins enough to consider placing somewhere in a/the main Porter repo such that it's inherited by any basic mixin implementation (maybe clientVersion could be included at the same time). However, it doesn't quite fit in exec's builder pkg, where we've previously placed similar interfaces... so this would probably be a separate/broader design/discussion.

@carolynvs
Copy link
Member Author

carolynvs commented Sep 29, 2020

Oh we can tweak the wording. The sentence above was "A download is triggered when: ..." so the condition for triggering a download is when the Major.Minor doesn't match. But we can flip it to talk about matching instead, whatever is less confusing.

Some of the logic we are discussing here is specific to kubectl (such as the backwards compatibility) so we would want to use interfaces again like we have in the builder package to opt into various behaviors. So yeah I think it would work well to be a common package in porter. We can find a spot for it, maybe a subpackage under pkg/mixins/FEATURE would be a way to do it going forward. But that can be worked out in an issue in the Porter repo.

@MChorfa
Copy link
Collaborator

MChorfa commented Sep 30, 2020

I really love the idea of configurable dynamic download and the adaptation of this feature to other mixins. Moreover, it could resolve this question about downloading user-defined mixins see: getporter/porter#975
I wonder when the download would happen. Is it at build-time or later at runtime? as a universal behavior

@carolynvs
Copy link
Member Author

I'm glad you are interested in this path!

The dynamic installation of the kubectl client must occur at bundle execution. This is because it relies upon credentials to connect to the user's (not bundle author's) cluster to identity the targeted cluster version. Every time the bundle runs, it can be used against a different cluster with a different version of Kubernetes.

The mixin configuration allows the bundle author to specify the default kubectl client version that should be installed at buildtime. When it is not specified, it will fall back and install the version hard coded in the mixin. We can also improve the mixin to always install the latest version of kubectl which I think would be a welcome change.

This gives us a minimum of one and possibly two downloads. There is always a download that occurs at buildtime which guarantees that there will always be a kubectl client in the invocation image. This client may then be optionally used to detect the cluster version at runtime and then download a matching version dynamically.


For the other issue you referenced, getporter/porter#975, mixin installation always occurs at buildtime. Mixins are built into bundle during porter build and never needed to be installed on the client in order to execute a bundle (runtime). If you'd like to chat about that issue further, let's do it on that issue so that we keep the discussion in one place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design 🚲🏠 Bust out your paint chips, it's time to bikeshed.
Projects
None yet
Development

No branches or pull requests

3 participants