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

Added Client Version for Kubernetes Mixin #1221

Merged
merged 6 commits into from Aug 24, 2020
Merged

Conversation

donmstewart
Copy link
Contributor

@donmstewart donmstewart commented Aug 21, 2020

Changes to the Kubernetes Mixin in order the user can pass in the version of kubectl to install which then downloads kubectl in the build action.

What issue does it fix

Closes #1135

@ghost
Copy link

ghost commented Aug 21, 2020

CLA assistant check
All CLA requirements met.

Copy link
Member

@vdice vdice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR @donmstewart. Code looks great!

My only request would be to add unit test coverage for this feature. (A recent example of prior art can be found in https://github.com/deislabs/porter-docker/pull/25/files#diff-702f25c50cf68ba27df0514c69a8012a)

@vdice
Copy link
Member

vdice commented Aug 24, 2020

Added tests look great @donmstewart - thank you.

One quibble: I don't think https://github.com/deislabs/porter/issues/1136 should be listed as being closed by this PR, as it appears to relate to runtime version matching as opposed to build-time version fetching, which is what this PR accomplishes.

@donmstewart
Copy link
Contributor Author

I don't think thats a quibble I think thats a perfectly valid point. Can we mark this just for #1135 for now and I'll do #1136 as a new PR after this is in?

Copy link
Member

@vdice vdice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thank you @donmstewart

@vdice vdice merged commit 05bffc7 into getporter:main Aug 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Configure kubernetes mixin version
2 participants