refactor namespace to kind of target #173

neo-liang-sap · 2020-03-16T06:50:40Z

What this PR does / why we need it:
This PR makes namespace to be one kind of target, just like garden/seed/project/shoot

Which issue(s) this PR fixes:
Fixes # #169

Special notes for your reviewer:

gardenctl ls namespace is supported
i've tested in following scenario/combination

gardenctl target -g canary-virtual -s gcp-eu2 -p everest -t ev-1373-b -n kube-system
gardenctl target -g canary-virtual -s gcp-eu2 -p everest -n kube-system
gardenctl target -g canary-virtual -s gcp-eu2 -n kube-system
gardenctl target -g canary-virtual -n kube-system
gardenctl target -s gcp-eu2 -p everest -t ev-1373-b -n kube-system
gardenctl target -s gcp-eu2 -t ev-1373-b -n kube-system
gardenctl target -s gcp-eu2 -n kube-system
gardenctl target -p everest -t ev-1373-b -n kube-system
gardenctl target -p everest -n kube-system
gardenctl target -n kube-system

gardenctl target -g canary-virtual -p everest -t ev-1373-b -n kube-system
gardenctl target -g canary-virtual -p everest -n kube-system
gardenctl target -g canary-virtual -t ev-1373-b -n kube-system

gardenctl target -t ev-1373-b -n kube-system
gardenctl target -p everest -n kube-system
gardenctl target -s gcp-eu2 -n kube-system

gardenctl target namespace kube-system

and verified target status in
cat /Users/i352986/.garden/sessions/plantingSession/target
Release note:

Namespace is now supported as one kind of target
gardenctl drop namespace is now supported
gardenctl ls namespaces is now supported

DockToFuture · 2020-03-16T08:56:19Z

Gardenctl drop is currently removing the ns from the stack but not from the kube-config.
And when I'm targeting another shoot i get the kubeconfig printed of the new shoot but I'm still targeting the same shoot.
E.g. target stack is:

target:
- kind: garden
  name: dev
- kind: project
  name: core
- kind: shoot
  name: cli-gcp
- kind: namespace
  name: kube-system

after targeting a new shoot I still have the same shoot in the target stack

$ g target shoot ts-test
Shoot:
KUBECONFIG=/Users/xxx/.garden/cache/dev/projects/core/ts-test/kubeconfig.yaml
xxx at vvv in ~/go/src/github.com/gardener/gardenctl on pr-173
$ g get target
target:
- kind: garden
  name: dev
- kind: project
  name: core
- kind: shoot
  name: cli-gcp
- kind: namespace
  name: kube-system

neo-liang-sap · 2020-03-16T10:10:22Z

Hi @DockToFuture ,

I fixed second issue in c86ab58 , targetShoot miss logic when current target.Target size is 4 (including namespace information)

For the first issue Gardenctl drop is currently removing the ns from the stack but not from the kube-config. sorry i didn't quite get the point, currently as namespace doesn't have kubeconfig information so i put the logic in b6a0bb0#diff-d0d3bde7a059e586e30272e957484afdR633 to b6a0bb0#diff-d0d3bde7a059e586e30272e957484afdR648 and this function getKubeConfigOfCurrentTarget is called in drop() function.

Could you please raise an example for this issue? then i can get an better understandinfg on this...

Thanks a lot!
-Neo

DockToFuture · 2020-03-16T15:24:12Z

I would expect in the following example, that after I have dropped the namespace from the target stack and I execute g k get pods that I get the pods in the default ns listed. Therefore gardenctl drop has to unset the namespace. WDYT?

$ g get target
target:
- kind: garden
  name: dev
- kind: project
  name: core
- kind: shoot
  name: cli-gcp
- kind: namespace
  name: kube-system
xxx at yyy in ~/go/src/github.com/gardener/gardenctl on pr-173

$ g drop
Dropped namespace kube-system
xxx at yyy in ~/go/src/github.com/gardener/gardenctl on pr-173

$ g get target
target:
- kind: garden
  name: dev
- kind: project
  name: core
- kind: shoot
  name: cli-gcp
xxx at yyy in ~/go/src/github.com/gardener/gardenctl on pr-173

$ g k get pods
NAME                                                             READY   STATUS    RESTARTS   AGE
addons-nginx-ingress-controller-5796c65bf8-cchc7                 1/1     Running   0          2d23h
addons-nginx-ingress-nginx-ingress-k8s-backend-b7c567f7b-nq2lg   1/1     Running   0          2d23h
blackbox-exporter-58cf74dd57-xlq75                               1/1     Running   0          2d23h
calico-kube-controllers-6f864659cf-plw7f                         1/1     Running   0          2d23h
calico-node-hj4wq                                                1/1     Running   0          8h
calico-node-nhj8f                                                1/1     Running   0          8h
calico-typha-deploy-6c5dc7856f-vs2s9                             1/1     Running   0          2d23h
calico-typha-horizontal-autoscaler-6455dc88c9-7fljk              1/1     Running   0          2d23h
calico-typha-vertical-autoscaler-7b9954b9df-962xs                1/1     Running   0          2d23h
coredns-db5c59c87-76pnc                                          1/1     Running   0          2d23h
coredns-db5c59c87-v92jw                                          1/1     Running   0          2d23h
kube-proxy-hhnxm                                                 1/1     Running   0          8h
kube-proxy-zptfc                                                 1/1     Running   0          8h
metrics-server-5d5788576f-t96fw                                  1/1     Running   0          2d23h
node-exporter-8wfqs                                              1/1     Running   0          8h
node-exporter-qwhqh                                              1/1     Running   0          8h
node-problem-detector-kpt7m                                      1/1     Running   0          8h
node-problem-detector-t2p2f                                      1/1     Running   0          8h
vpn-shoot-7fbc9c65b8-t8qt7                                       1/1     Running   0          2d23h

neo-liang-sap · 2020-03-16T15:53:09Z

Hi @DockToFuture ,
Thanks for the comments!
I added some logic in drop.go -

setting namespace to default while dropping an namespace
add logic for dealing with gardenctl drop project/seed while namespace in current target
add support for gardenctl drop namespace

also add some code in ls.go and ls_test.go to support gardenctl ls namespaces

Thanks
-Neo

DockToFuture · 2020-03-19T11:12:34Z

pkg/cmd/target.go

+	ReadTarget(pathTarget, &targetReal)
+
+	if len(targetReal.Target) == 1 && targetReal.Stack()[0].Kind == "namespace" {
+		panic("the target has only namespace, this is invalid, at least one garden needs to be targeted before using namespace")


Can you replace panic function with return errors.New("error message")? User will be confused by the stack trace.

DockToFuture · 2020-03-19T11:12:44Z

pkg/cmd/target.go

+	} else if len(targetReal.Target) == 1 && targetReal.Stack()[0].Kind != "namespace" {
+		target.Target = targetReal.Target
+	} else {
+		panic("length of target.Stack is illegal")


DockToFuture · 2020-03-19T11:12:56Z

pkg/cmd/target.go

+	ReadTarget(pathTarget, &target)
+
+	if len(target.Target) > 4 {
+		panic("the length is greater than 4 and illegal")


DockToFuture · 2020-03-19T11:13:04Z

pkg/cmd/target.go

+		panic("the length is greater than 4 and illegal")
+	}
+	if len(target.Target) == 0 {
+		panic("the length is 0 and illegal. at least one garden needs to be targeted")


DockToFuture · 2020-03-19T11:13:11Z

pkg/cmd/target.go

+	}
+	if len(target.Target) == 1 {
+		if string(target.Target[0].Kind) != "garden" {
+			panic("if one element in target, this needs to be garden")


neo-liang-sap · 2020-03-19T11:41:44Z

Hi @DockToFuture ,
thanks, for three occurence of panic i used return errors.New("") to replace.
For two occurence which function doesn't return error but only kubeconfigpath , i use fmt.Println("") to print error message and os.Exit(2) to fail the process

Thanks!

DockToFuture

/lgtm, thank you for the pr!

neo-liang-sap · 2020-03-19T12:26:51Z

thanks @DockToFuture !

refactor namespace to kind of target

b6a0bb0

neo-liang-sap requested review from DockToFuture and ialidzhikov as code owners March 16, 2020 06:50

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Mar 16, 2020

gardener-robot-ci-2 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Mar 16, 2020

add logic in targetShoot when current target.Stack size=4 (contains ns)

c86ab58

gardener-robot-ci-3 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Mar 16, 2020

gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Mar 16, 2020

gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Mar 16, 2020

set namespace to default while dropping an namespace

4892f69

neo-liang-sap force-pushed the issue-169 branch from c193fd5 to 4892f69 Compare March 16, 2020 15:57

gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Mar 16, 2020

support gardenctl drop namespace

844def0

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Mar 16, 2020

gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Mar 16, 2020

add support for gardenctl ls namespaces

4170934

gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Mar 19, 2020

DockToFuture suggested changes Mar 19, 2020

View reviewed changes

remove panic and use errors

f6f7d91

gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Mar 19, 2020

gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Mar 19, 2020

DockToFuture approved these changes Mar 19, 2020

View reviewed changes

DockToFuture merged commit 5ace9d2 into gardener-attic:master Mar 19, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor namespace to kind of target #173

refactor namespace to kind of target #173

neo-liang-sap commented Mar 16, 2020 •

edited

Loading

DockToFuture commented Mar 16, 2020

neo-liang-sap commented Mar 16, 2020

DockToFuture commented Mar 16, 2020

neo-liang-sap commented Mar 16, 2020 •

edited

Loading

DockToFuture Mar 19, 2020

DockToFuture Mar 19, 2020

DockToFuture Mar 19, 2020

DockToFuture Mar 19, 2020

DockToFuture Mar 19, 2020

neo-liang-sap commented Mar 19, 2020

DockToFuture left a comment

neo-liang-sap commented Mar 19, 2020

refactor namespace to kind of target #173

refactor namespace to kind of target #173

Conversation

neo-liang-sap commented Mar 16, 2020 • edited Loading

DockToFuture commented Mar 16, 2020

neo-liang-sap commented Mar 16, 2020

DockToFuture commented Mar 16, 2020

neo-liang-sap commented Mar 16, 2020 • edited Loading

DockToFuture Mar 19, 2020

Choose a reason for hiding this comment

DockToFuture Mar 19, 2020

Choose a reason for hiding this comment

DockToFuture Mar 19, 2020

Choose a reason for hiding this comment

DockToFuture Mar 19, 2020

Choose a reason for hiding this comment

DockToFuture Mar 19, 2020

Choose a reason for hiding this comment

neo-liang-sap commented Mar 19, 2020

DockToFuture left a comment

Choose a reason for hiding this comment

neo-liang-sap commented Mar 19, 2020

neo-liang-sap commented Mar 16, 2020 •

edited

Loading

neo-liang-sap commented Mar 16, 2020 •

edited

Loading