rules_k8s: Support passing in a kubeconfig file #287
Conversation
|
Hi @rohansingh. Thanks for your PR. I'm waiting for a bazelbuild member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
This doesn't include any updates to relevant documentation. If this approach is acceptable, I'll make those updates as well. |
|
/ok-to-test |
There was a problem hiding this comment.
Hi Rohan,
Thanks for your PR. Could you please add a test for this as well in addition to the docs?
For the test, recommend extending one of the k8s_object rules here https://github.com/bazelbuild/rules_k8s/blob/master/examples/hellogrpc/BUILD#L24. Either make the k8s_service or the k8s_deploy target use a kubeconfig and move some of the config to the kubeconfig.
Thanks!
| @@ -26,4 +26,5 @@ function exe() { echo "\$ ${@/eval/}" ; "$@" ; } | |||
| RUNFILES="${PYTHON_RUNFILES:-$(guess_runfiles)}" | |||
|
|
|||
| PYTHON_RUNFILES=${RUNFILES} %{resolve_script} | \ | |||
| exe %{kubectl_tool} --cluster="%{cluster}" --context="%{context}" --user="%{user}" %{namespace_arg} apply $@ -f - | |||
| exe %{kubectl_tool} --kubeconfig="%{kubeconfig}" --cluster="%{cluster}" \ | |||
There was a problem hiding this comment.
What does kubectl do when --kubeconfig is given a blank string? Would it be preferable to only specify --kubeconfig when there's something to specify? The if condition would move to the k8s_object rule.
There was a problem hiding this comment.
When --kubeconfig='', the behavior is the same as not specifying --kubeconfig at all. It's the same as --cluster='' and --context='' in that regard.
I agree that it would be cleaner and preferable to not output flags if the value is empty, since we're just getting lucky that the default value of all the current flags is an empty string. However, I think that should be a separate PR that would do this for all of the flags, rather than having different behavior for just this one flag.
| @@ -28,4 +28,5 @@ RUNFILES="${PYTHON_RUNFILES:-$(guess_runfiles)}" | |||
| # TODO(mattmoor): Should we create namespaces that do not exist? | |||
|
|
|||
| PYTHON_RUNFILES=${RUNFILES} %{resolve_script} | \ | |||
| exe %{kubectl_tool} --cluster="%{cluster}" --context="%{context}" --user="%{user}" %{namespace_arg} create $@ -f - | |||
| exe %{kubectl_tool} --kubeconfig="%{kubeconfig}" --cluster="%{cluster}" \ | |||
There was a problem hiding this comment.
Same comment here as apply
|
@smukherj1 Thanks for the prompt review! I'll add tests and docs (and fix the buildifier failure). |
|
Updated documentation and tests, and rebased. Note that I don't have an environment setup for running the e2e tests locally, so kind of shooting in the dark here. |
Add support for passing in a generated kubeconfig file to `rules_k8s` objects. This can be used to pass in a configuration that's generated by Bazel.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rohansingh, smukherj1 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 |
Add support for passing in a generated kubeconfig file to
rules_k8sobjects. This can be used to pass in a configuration that's generated by
Bazel.