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

Design proposal for configurable kubecontext #2384

Merged
merged 11 commits into from
Aug 15, 2019

Conversation

corneliusweig
Copy link
Contributor

This is a design proposal for making the kubecontext configurable. The goal here is to lay out different options with their tradeoffs and come to an agreement what is the best way to proceed. I may have missed better alternatives, so if you find better ones, please leave a comment.
I deliberately only present different options, so that the discussion starts unbiased.

@codecov
Copy link

codecov bot commented Jul 1, 2019

Codecov Report

Merging #2384 into master will not change coverage.
The diff coverage is n/a.

@dgageot dgageot added the kokoro:run runs the kokoro jobs on a PR label Jul 2, 2019
@kokoro-team kokoro-team removed the kokoro:run runs the kokoro jobs on a PR label Jul 2, 2019
Signed-off-by: Cornelius Weig <[email protected]>
@dgageot dgageot added the kokoro:run runs the kokoro jobs on a PR label Jul 5, 2019
@kokoro-team kokoro-team removed the kokoro:run runs the kokoro jobs on a PR label Jul 5, 2019
<td>3.</td>
<td><code>skaffold.yaml</code></td>
<td>
Json-path <code>deploy.kubeContext</code>.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Json-path <code>deploy.kubeContext</code>.
Json-path <code>[profile.]deploy.kubeContext</code>.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My current feeling is that we should have the option also in the main pipeline. Although this may not be the recommended way to use this feature, I have a hunch that this will be requested nevertheless.

Copy link
Contributor

Choose a reason for hiding this comment

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

agreed, [profile.] meant to signify main pipeline or optionally in a profile...maybe there is a better way to express that :)

Include error detection for profile activation with kubecontext override.
Declare namespace-related configs as out-of-scope.
Split global config cases.

Signed-off-by: Cornelius Weig <[email protected]>
Signed-off-by: Cornelius Weig <[email protected]>
@corneliusweig
Copy link
Contributor Author

@balopat I have included the latest design decisions into this document. From my side, it's ready to be merged now.

@tejal29 tejal29 added the kokoro:run runs the kokoro jobs on a PR label Aug 15, 2019
@kokoro-team kokoro-team removed the kokoro:run runs the kokoro jobs on a PR label Aug 15, 2019
@tejal29
Copy link
Contributor

tejal29 commented Aug 15, 2019

@balopat This looks good to me now.

Copy link
Contributor

@nkubala nkubala left a comment

Choose a reason for hiding this comment

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

huge thanks @corneliusweig for all your hard work on this!

@nkubala nkubala merged commit 211d08a into GoogleContainerTools:master Aug 15, 2019
@corneliusweig corneliusweig deleted the w/context branch August 15, 2019 20:33
@tejal29 tejal29 mentioned this pull request Aug 21, 2019
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.

7 participants