Use kubectl describe foo name #196

fejta · 2018-10-18T18:49:42Z

If we do not include the -o name then the command output includes a (dry run) suffix.

/assign @smukherj1 @nlopezgi

This seems to be causing issues in recent kubectl versions (#193 (comment)).

Formatting fixes while I'm in these files
Run delete || true before running create, in case zombie resources exist from a prior run.
Logging the steps before they run to assist debugging

erain · 2018-10-18T19:37:47Z

I thought kubectl get foo name is more commonly used than kubectl get foo/name? Example: kubectl Cheat Sheet

fejta · 2018-10-18T20:11:14Z

Maybe kubectl get foo name and kubectl get foo/name are both fine, but it doesn't like kubectl get foo foo/name

fejta · 2018-10-18T20:19:09Z

Looks like the problem is in the actual bazel rules... Let me see if I can figure this out...

fejta · 2018-10-18T20:24:18Z

Think this is the problematic bit:

rules_k8s/k8s/describe.sh.tpl

Lines 26 to 29 in 590f3b4

    
           RESOURCE_NAME=$(kubectl create --dry-run -f "%{unresolved}" | cut -d'"' -f 2) 
        
           kubectl --cluster="%{cluster}" --context="%{context}" --user="%{user}" %{namespace_arg} describe %{kind} \ 
        
             "${RESOURCE_NAME}"

fejta · 2018-10-18T20:42:41Z

/retest

fejta · 2018-10-18T20:53:00Z

These e2e tests are the WORST 😅

fejta · 2018-10-18T21:03:29Z

Okay I think I have it!

Looks like the problem here is that kubectl create --dry-run -f "%{unresolved}" now outputs (dry run):
deployment.apps/hello-grpc-staging created (dry run)
But we want the name without the (dry run) suffix:
deployment.apps/hello-grpc-staging created

We can fix this by adding an -o name flag.

fejta · 2018-10-18T21:11:37Z

/retest

fejta · 2018-10-18T21:25:14Z

These e2e tests are the WORST 😅

Dear e2e tests, please stop editing .go, .java, .py, .js files when I run you, which I don't notice and wind up including in this PR. kthxbye!

fejta · 2018-10-18T21:38:04Z

/retest

fejta · 2018-10-18T22:00:21Z

PASS! 🎆 🍾 🎊

chrislovecnm

Just bash nit picks ... look great otherwise :)

chrislovecnm · 2018-10-19T00:44:17Z

examples/hellohttp/e2e-test.sh

+    echo "Success, substitution failed."
+    return 0
+  else
+    echo "Bad substitution should fail!"


If I am a grammar critic it could read a tad better, but a nit pick

chrislovecnm · 2018-10-19T00:45:13Z

examples/hellohttp/e2e-test.sh

@@ -40,33 +41,35 @@ check_msg() {
 }

 edit() {
-   ./examples/hellohttp/${LANGUAGE}/edit.sh "$1"
+  echo Setting $LANGUAGE to "$1"


Please quote language var. We might want to lint with shellcheck

chrislovecnm · 2018-10-19T00:45:30Z

examples/hellogrpc/e2e-test.sh

@@ -24,44 +24,49 @@ if [[ -z "${1:-}" ]]; then
 fi

 get_lb_ip() {
-    kubectl --namespace=${USER} get service hello-grpc-staging \
-	-o jsonpath='{.status.loadBalancer.ingress[0].ip}'
+  kubectl --namespace=${USER} get service hello-grpc-staging \


Please put quotes around all variables

chrislovecnm · 2018-10-19T00:46:30Z

examples/todocontroller/e2e-test.sh

-    kubectl --namespace=${USER} get service hello-grpc-staging \
-	-o jsonpath='{.status.loadBalancer.ingress[0].ip}'
+  kubectl --namespace=${USER} get service hello-grpc-staging \
+    -o jsonpath='{.status.loadBalancer.ingress[0].ip}'


Again missing quotes around a bash var

chrislovecnm · 2018-10-19T00:47:40Z

/assign

chrislovecnm · 2018-10-19T00:47:59Z

Well that did not work 😂😂

erain

LGTM. Maybe quote the vars as Chris required?

chrislovecnm · 2018-10-19T13:40:20Z

I can do a follow-up PR as well. @fejta thoughts?

k8s-ci-robot · 2018-10-19T13:47:50Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chrislovecnm, erain

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~~ [erain]

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

chrislovecnm · 2018-10-19T13:48:34Z

@fejta I removed my blocking review, so up to you if you want to fix my recommendations or not. I can do a follow-up PR as I mentioned.

chrislovecnm · 2018-10-19T19:27:51Z

This is blocking testing big time. @erain / @fejta / @nlopezgi can someone merge?

k8s-ci-robot assigned nlopezgi and smukherj1 Oct 18, 2018

k8s-ci-robot requested review from erain and nlopezgi October 18, 2018 18:49

k8s-ci-robot added approved size/S labels Oct 18, 2018

fejta force-pushed the fix branch from 8ae6c46 to 7a594f5 Compare October 18, 2018 18:58

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

fejta force-pushed the fix branch from 7a594f5 to 6b2375f Compare October 18, 2018 20:18

k8s-ci-robot added size/L and removed size/M labels Oct 18, 2018

fejta force-pushed the fix branch from 6b2375f to e860638 Compare October 18, 2018 20:29

fejta changed the title ~~Use kubectl get foo/name not get foo name~~ Use kubectl describe foo name Oct 18, 2018

fejta force-pushed the fix branch from e860638 to ac641fc Compare October 18, 2018 20:33

fejta force-pushed the fix branch from ac641fc to f1431e5 Compare October 18, 2018 21:03

Use kubectl get foo/name not get foo name

e5b1c35

fejta force-pushed the fix branch from f1431e5 to e5b1c35 Compare October 18, 2018 21:16

k8s-ci-robot assigned chrislovecnm Oct 19, 2018

chrislovecnm suggested changes Oct 19, 2018

View reviewed changes

k8s-ci-robot removed the approved label Oct 19, 2018

erain approved these changes Oct 19, 2018

View reviewed changes

k8s-ci-robot assigned erain Oct 19, 2018

k8s-ci-robot added lgtm approved labels Oct 19, 2018

chrislovecnm approved these changes Oct 19, 2018

View reviewed changes

smukherj1 merged commit b9bc187 into bazelbuild:master Oct 19, 2018

fejta deleted the fix branch October 19, 2018 20:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use kubectl describe foo name #196

Use kubectl describe foo name #196

fejta commented Oct 18, 2018 •

edited

Loading

erain commented Oct 18, 2018

fejta commented Oct 18, 2018

fejta commented Oct 18, 2018

fejta commented Oct 18, 2018 •

edited

Loading

fejta commented Oct 18, 2018

fejta commented Oct 18, 2018

fejta commented Oct 18, 2018

fejta commented Oct 18, 2018

fejta commented Oct 18, 2018

fejta commented Oct 18, 2018

fejta commented Oct 18, 2018

chrislovecnm left a comment

chrislovecnm Oct 19, 2018

chrislovecnm Oct 19, 2018

chrislovecnm Oct 19, 2018

chrislovecnm Oct 19, 2018

chrislovecnm commented Oct 19, 2018

chrislovecnm commented Oct 19, 2018

erain left a comment

chrislovecnm commented Oct 19, 2018 •

edited

Loading

k8s-ci-robot commented Oct 19, 2018

chrislovecnm commented Oct 19, 2018

chrislovecnm commented Oct 19, 2018

Use kubectl describe foo name #196

Use kubectl describe foo name #196

Conversation

fejta commented Oct 18, 2018 • edited Loading

erain commented Oct 18, 2018

fejta commented Oct 18, 2018

fejta commented Oct 18, 2018

fejta commented Oct 18, 2018 • edited Loading

fejta commented Oct 18, 2018

fejta commented Oct 18, 2018

fejta commented Oct 18, 2018

fejta commented Oct 18, 2018

fejta commented Oct 18, 2018

fejta commented Oct 18, 2018

fejta commented Oct 18, 2018

chrislovecnm left a comment

Choose a reason for hiding this comment

chrislovecnm Oct 19, 2018

Choose a reason for hiding this comment

chrislovecnm Oct 19, 2018

Choose a reason for hiding this comment

chrislovecnm Oct 19, 2018

Choose a reason for hiding this comment

chrislovecnm Oct 19, 2018

Choose a reason for hiding this comment

chrislovecnm commented Oct 19, 2018

chrislovecnm commented Oct 19, 2018

erain left a comment

Choose a reason for hiding this comment

chrislovecnm commented Oct 19, 2018 • edited Loading

k8s-ci-robot commented Oct 19, 2018

chrislovecnm commented Oct 19, 2018

chrislovecnm commented Oct 19, 2018

fejta commented Oct 18, 2018 •

edited

Loading

fejta commented Oct 18, 2018 •

edited

Loading

chrislovecnm commented Oct 19, 2018 •

edited

Loading