Adding capability to pass in args parameter to kubectl #198

chrislovecnm · 2018-10-19T15:22:47Z

This PR adds a parameter to pass in arguments to kubectl. For instance
a user can pass in "-v=10 --dry-run=true" to kubectl. This work is
based off of @omadawn's work.

Once we have the design stabilized I will update the documentation.

examples/hellohttp/BUILD

chrislovecnm

Various comments

k8s/with-defaults.bzl

k8s/object.bzl

examples/hellohttp/java/BUILD

chrislovecnm

More comments.

k8s/apply.sh.tpl

examples/hellohttp/java/BUILD

chrislovecnm

more questions

k8s/with-defaults.bzl

nlopezgi

sorry for the delay getting back with comments.

chrislovecnm · 2018-10-23T19:37:06Z

I will update with the make variables call, but also put in an issue to replace them as that call is deprecated.

k8s-ci-robot · 2018-11-07T20:09:15Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: chrislovecnm
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: nlopezgi

If they are not already assigned, you can assign the PR to them by writing /assign @nlopezgi in a comment when ready.

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:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

chrislovecnm · 2018-11-07T20:51:41Z

/test pull-rules-k8s-e2e

chrislovecnm · 2018-11-07T21:09:17Z

/retest

chrislovecnm · 2018-11-09T00:25:14Z

/retest

chrislovecnm · 2018-11-09T00:51:34Z

@nlopezgi PTAL

chrislovecnm · 2018-11-09T14:41:36Z

/retest

@omadawn

Added kubectl_args to apply, delete, describe, create and replace kubectl commands. This PR adds a parameter to pass in arguments to kubectl. For intance a user can pass in "-v=10 --dry-run=true" to kubectl. This work is based off of @omadawn's work.

Allows for the passing in of arguments to kubectl via a cli run command Adding the capability of using $@ ie. args with kubectl commands.

- modified the java hellohttp example BUILD file to include --v=2 kubectl_args - added test for --v=2 in BUILD file - added test for passing in an argument via the cli

nlopezgi · 2018-11-09T18:50:34Z

README.md

@@ -394,6 +394,16 @@ Users can "describe" their environment by running:
 bazel run :dev.describe
 ```

+### kubectl Arguments Via the Command Line
+
+The apply, create, delete, and describe commands will accept kubectl arguments passed in


nit: wrap line at 80 chars

k8s/object.bzl

nlopezgi · 2018-11-09T18:58:53Z

examples/hellohttp/e2e-test.sh

+# also tests that --help can be passed in as a cli argument
+check_kubectl_args() {
+
+    # checking that --v=2 is passed in via kubect_arguments in 


s/kubect_arguments/kubectl_args

nlopezgi · 2018-11-09T18:59:53Z

examples/hellohttp/e2e-test.sh

+    # the java BUILD file
+    FILE="./bazel-bin/examples/hellohttp/java/staging.apply"
+    echo Checking kubectl args in file: "$FILE"
+    bazel build examples/hellohttp/java:staging.apply


what a painful way to test, thanks for setting it up, but we do need some infrastructure to make this easier

it works 😆

Found in the rules_docker e2e tests that we have some sort of crude infrastructure to do some basic checks, for example, that a file contains a string:
https://github.com/bazelbuild/rules_docker/blob/master/testing/e2e.sh#L45
This checks that the output of bazel run has some flags:
https://github.com/bazelbuild/rules_docker/blob/master/testing/e2e.sh#L281
should be simpler than what you have here?

what I have works ... why change it? 🤷‍♂️
I think we really need to FIX it 😆

nlopezgi · 2018-11-09T19:17:24Z

k8s/object.bzl

@@ -172,13 +172,22 @@ def _common_impl(ctx):

  kubectl_tool_info = ctx.toolchains["@io_bazel_rules_k8s//toolchains/kubectl:toolchain_type"].kubectlinfo

+  kubectl_args = " ".join(ctx.attr.kubectl_args or [])


Ok, so lets make this simple then, lets start w/o supporting make variable expansion and just avoid needing all this code (we can add it later if we need to).
Based off that assumption, all you need here is this line:
kubectl_args = " ".join(ctx.attr.kubectl_args)

(no need to have the or if default is set below in line 238)
and everything between lines 176-180 can go away.
and then just do
"%{kubectl_args}": kubectl_args,
in line 190 below

we can add it later if we need to

Is there a reason we don't add it now? I will update the code as you recommended, just want to learn more :)

nlopezgi · 2018-11-09T19:19:06Z

k8s/object.bzl

@@ -226,6 +235,7 @@ _common_attrs = {
        executable = True,
        allow_files = True,
    ),
+    "kubectl_args": attr.string_list(),


set this with default:
"kubectl_args": attr.string_list(default = []),

chrislovecnm · 2018-11-10T02:43:56Z

Closing cause we have #224

Coming in

k8s-ci-robot requested review from erain and nlopezgi October 19, 2018 15:22

k8s-ci-robot added the size/S label Oct 19, 2018

chrislovecnm added the do-not-merge/work-in-progress label Oct 19, 2018

chrislovecnm changed the title ~~Adding capability to pass in args parameter to kubectl~~ [WIP] Adding capability to pass in args parameter to kubectl Oct 19, 2018

nlopezgi reviewed Oct 19, 2018

View reviewed changes

examples/hellohttp/BUILD Outdated Show resolved Hide resolved

chrislovecnm force-pushed the kubectl-commands branch from bf05c00 to 74980ba Compare October 19, 2018 15:57

k8s-ci-robot added size/M and removed size/S labels Oct 19, 2018

chrislovecnm force-pushed the kubectl-commands branch 2 times, most recently from 075db0f to 3d96226 Compare October 19, 2018 22:02

k8s-ci-robot added size/S and removed size/M labels Oct 19, 2018

chrislovecnm commented Oct 19, 2018

View reviewed changes

k8s/with-defaults.bzl Outdated Show resolved Hide resolved

k8s/object.bzl Outdated Show resolved Hide resolved

examples/hellohttp/java/BUILD Show resolved Hide resolved

chrislovecnm commented Oct 19, 2018

View reviewed changes

k8s/apply.sh.tpl Outdated Show resolved Hide resolved

examples/hellohttp/java/BUILD Show resolved Hide resolved

chrislovecnm force-pushed the kubectl-commands branch from 3d96226 to 5042851 Compare October 19, 2018 22:35

chrislovecnm changed the title ~~[WIP] Adding capability to pass in args parameter to kubectl~~ Adding capability to pass in args parameter to kubectl Oct 19, 2018

k8s-ci-robot removed the do-not-merge/work-in-progress label Oct 19, 2018

chrislovecnm commented Oct 19, 2018

View reviewed changes

k8s/with-defaults.bzl Show resolved Hide resolved

nlopezgi reviewed Oct 23, 2018

View reviewed changes

chrislovecnm force-pushed the kubectl-commands branch from 5042851 to f39b957 Compare October 23, 2018 21:10

chrislovecnm mentioned this pull request Oct 25, 2018

Add arbitrary commands #174

Closed

chrislovecnm force-pushed the kubectl-commands branch from f39b957 to e903ac6 Compare October 25, 2018 21:55

k8s-ci-robot added size/M and removed size/S labels Oct 25, 2018

Globegitter mentioned this pull request Oct 26, 2018

Add command specific arguments #207

Closed

nlopezgi mentioned this pull request Oct 26, 2018

How to properly pass arbitrary args to kubectl command. #209

Closed

chrislovecnm force-pushed the kubectl-commands branch from e903ac6 to 095fbdb Compare November 7, 2018 20:02

chrislovecnm force-pushed the kubectl-commands branch from a5561e2 to e07758b Compare November 7, 2018 20:10

chrislovecnm force-pushed the kubectl-commands branch from 5be5b74 to db5c38f Compare November 8, 2018 01:01

chrislovecnm mentioned this pull request Nov 8, 2018

Improve e2e framework - refactor to remove shell #220

Open

chrislovecnm force-pushed the kubectl-commands branch 7 times, most recently from 51f57e5 to 8a51da1 Compare November 9, 2018 00:17

chrislovecnm force-pushed the kubectl-commands branch 3 times, most recently from b466386 to c618df5 Compare November 9, 2018 00:50

chrislovecnm force-pushed the kubectl-commands branch 2 times, most recently from eba56a8 to d627ab0 Compare November 9, 2018 01:00

chrislovecnm added 4 commits November 9, 2018 11:00

Adding $@ to kubectl commands

7b16db1

Allows for the passing in of arguments to kubectl via a cli run command Adding the capability of using $@ ie. args with kubectl commands.

Adding e2e tests for kubectl args and arguments via cli

6f206f2

- modified the java hellohttp example BUILD file to include --v=2 kubectl_args - added test for --v=2 in BUILD file - added test for passing in an argument via the cli

Adding docs for new kubectl argument functionality

d7c093e

chrislovecnm force-pushed the kubectl-commands branch from d627ab0 to d7c093e Compare November 9, 2018 18:00

nlopezgi reviewed Nov 9, 2018

View reviewed changes

chrislovecnm closed this Nov 10, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding capability to pass in args parameter to kubectl #198

Adding capability to pass in args parameter to kubectl #198

chrislovecnm commented Oct 19, 2018 •

edited

Loading

chrislovecnm left a comment

chrislovecnm left a comment

chrislovecnm left a comment

nlopezgi left a comment

chrislovecnm commented Oct 23, 2018 •

edited

Loading

k8s-ci-robot commented Nov 7, 2018

chrislovecnm commented Nov 7, 2018

chrislovecnm commented Nov 7, 2018

chrislovecnm commented Nov 9, 2018

chrislovecnm commented Nov 9, 2018

chrislovecnm commented Nov 9, 2018

nlopezgi Nov 9, 2018

nlopezgi Nov 9, 2018

nlopezgi Nov 9, 2018

chrislovecnm Nov 9, 2018

nlopezgi Nov 9, 2018

chrislovecnm Nov 10, 2018

nlopezgi Nov 9, 2018 •

edited

Loading

chrislovecnm Nov 10, 2018

nlopezgi Nov 9, 2018

chrislovecnm commented Nov 10, 2018

		@@ -172,13 +172,22 @@ def _common_impl(ctx):

		kubectl_tool_info = ctx.toolchains["@io_bazel_rules_k8s//toolchains/kubectl:toolchain_type"].kubectlinfo

		kubectl_args = " ".join(ctx.attr.kubectl_args or [])

Adding capability to pass in args parameter to kubectl #198

Adding capability to pass in args parameter to kubectl #198

Conversation

chrislovecnm commented Oct 19, 2018 • edited Loading

chrislovecnm left a comment

Choose a reason for hiding this comment

chrislovecnm left a comment

Choose a reason for hiding this comment

chrislovecnm left a comment

Choose a reason for hiding this comment

nlopezgi left a comment

Choose a reason for hiding this comment

chrislovecnm commented Oct 23, 2018 • edited Loading

k8s-ci-robot commented Nov 7, 2018

chrislovecnm commented Nov 7, 2018

chrislovecnm commented Nov 7, 2018

chrislovecnm commented Nov 9, 2018

chrislovecnm commented Nov 9, 2018

chrislovecnm commented Nov 9, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nlopezgi Nov 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrislovecnm commented Nov 10, 2018

chrislovecnm commented Oct 19, 2018 •

edited

Loading

chrislovecnm commented Oct 23, 2018 •

edited

Loading

nlopezgi Nov 9, 2018 •

edited

Loading