Support --all-namespaces option for service and revision list #32

nak3 · 2019-03-04T08:17:58Z

Currently kn {service, revision} list does not have the all-namespaces
options. Although it is possible to list them by kn service list -n ""(empty string), it is difficult to find such secret trick.

Hence, this patch adds --all-namespaces option.

cppforlife · 2019-03-05T18:15:59Z

@nak3 we should probably move this into KnParams or some other object (like NamespaceParams) that's injecting; otherwise, we would have to duplicate this functionality everywhere.

nak3 · 2019-03-11T13:23:57Z

@cppforlife Thank you! Updated by adding the option into KnParams and adding the flag to revision,service command instead of list only.

cppforlife · 2019-03-12T00:56:38Z

pkg/kn/commands/revision.go

@@ -24,6 +24,9 @@ func NewRevisionCommand(p *KnParams) *cobra.Command {
 		Short: "Revision command group",
 	}
 	revisionCmd.PersistentFlags().StringP("namespace", "n", "default", "Namespace to use.")
+	revisionCmd.PersistentFlags().BoolVar(&p.getAllNamespaces, "all-namespaces", false,


could we also share following 3 lines as well between commands?

Do you mean that --all-namespace should move to root level option like --config and --kubeconfig?
I put --all-namespace here to align with --namespace(-n). Should I share --namespace between commands as well?

@nak3 probably not global, but doesnt mean we cannot reuse this code so that we dont have duplicate flag definitions.

I'd love to see an AddNamespaceFlags fn and a GetNamespace(cmd) fn.

nak3 · 2019-03-19T09:43:05Z

@cppforlife @sixolet Thank you! Updated.

pkg/kn/commands/namespaced.go

cppforlife · 2019-03-21T17:41:35Z

+1 for @navidshaikh suggestions

nak3 · 2019-03-21T22:56:58Z

Thank you, updated the help messages.

navidshaikh

LGTM!

cppforlife · 2019-03-26T18:03:02Z

/ok-to-test

bbrowning · 2019-03-29T11:33:57Z

Kicking these tests since a fix for these CI failures has since been merged.

/retest

nak3 · 2019-03-30T05:20:13Z

rebased to solve confliction.

cppforlife · 2019-04-08T17:39:54Z

/lgtm
/approve

nak3 · 2019-04-08T23:47:14Z

/test pull-knative-client-unit-tests

nak3 · 2019-04-08T23:49:58Z

The test was failed at TestServiceCreateImage. It looks like I need to rebase this PR again(?) Let's see the result of secod try.

I0408 17:44:11.183] --- FAIL: TestServiceCreateImage (0.00s)
I0408 17:44:11.183] panic: runtime error: invalid memory address or nil pointer dereference [recovered]
I0408 17:44:11.183] 	panic: runtime error: invalid memory address or nil pointer dereference
I0408 17:44:11.184] [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1827351]
I0408 17:44:11.184] 
I0408 17:44:11.184] goroutine 26 [running]:
I0408 17:44:11.184] testing.tRunner.func1(0xc0000efa00)
I0408 17:44:11.185] 	/usr/local/go/src/testing/testing.go:830 +0x68a
I0408 17:44:11.185] panic(0x1944e40, 0x29115e0)
I0408 17:44:11.185] 	/usr/local/go/src/runtime/panic.go:522 +0x1b5
I0408 17:44:11.185] github.com/knative/client/pkg/kn/commands.NewServiceCreateCommand.func1(0xc000492280, 0xc000289d40, 0x1, 0x3, 0x0, 0x0)
I0408 17:44:11.185] 	/go/src/github.com/knative/client/pkg/kn/commands/service_create.go:42 +0x1e1
I0408 17:44:11.186] github.com/spf13/cobra.(*Command).execute(0xc000492280, 0xc000289cb0, 0x3, 0x3, 0xc000492280, 0xc000289cb0)
I0408 17:44:11.186] 	/go/pkg/mod/github.com/spf13/cobra@v0.0.3/command.go:762 +0x528
I0408 17:44:11.186] github.com/spf13/cobra.(*Command).ExecuteC(0xc0002df680, 0x0, 0x0, 0x0)
I0408 17:44:11.187] 	/go/pkg/mod/github.com/spf13/cobra@v0.0.3/command.go:852 +0x3ee
I0408 17:44:11.187] github.com/spf13/cobra.(*Command).Execute(...)
I0408 17:44:11.187] 	/go/pkg/mod/github.com/spf13/cobra@v0.0.3/command.go:800
I0408 17:44:11.187] github.com/knative/client/pkg/kn/commands.fakeServiceCreate(0xc0000fc9b0, 0x5, 0x5, 0xc00019d760, 0xc00009b380, 0xc000482000, 0xc00019d730, 0x455cdd, 0x1eda418, 0xf)
I0408 17:44:11.188] 	/go/src/github.com/knative/client/pkg/kn/commands/service_create_test.go:59 +0x514
I0408 17:44:11.188] github.com/knative/client/pkg/kn/commands.TestServiceCreateImage(0xc0000efa00)
I0408 17:44:11.188] 	/go/src/github.com/knative/client/pkg/kn/commands/service_create_test.go:68 +0xc1
I0408 17:44:11.188] testing.tRunner(0xc0000efa00, 0x1b157f0)
I0408 17:44:11.189] 	/usr/local/go/src/testing/testing.go:865 +0x164
I0408 17:44:11.189] created by testing.(*T).Run
I0408 17:44:11.189] 	/usr/local/go/src/testing/testing.go:916 +0x69a
I0408 17:44:11.189] FAIL	github.com/knative/client/pkg/kn/commands	1.051s

`kn {service, revision} list` does not have the all-namespaces options. Although it is possible to list them by `kn service list -n ""`(empty string), it is difficult to find such secret trick. Hence, This patch adds `--all-namespaces` option.

knative-metrics-robot · 2019-04-08T23:58:59Z

The following is the coverage report on pkg/.
Say /test pull-knative-client-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/kn/commands/namespaced.go	Do not exist	78.6%
pkg/kn/commands/revision_describe.go	81.0%	79.2%	-1.8
pkg/kn/commands/revision_list.go	21.1%	22.7%	1.7
pkg/kn/commands/service_create.go	75.0%	74.1%	-0.9
pkg/kn/commands/service_describe.go	81.0%	79.2%	-1.8
pkg/kn/commands/service_list.go	78.9%	77.3%	-1.7

nak3 · 2019-04-09T00:12:29Z

@cppforlife @cppforlife @bbrowning @navidshaikh
Thank you. Sorry LGTM label was dropped as I rebased and added AddNamespaceFlags() to service create command - 3cad3dc. Could you please take a look again?

cppforlife · 2019-04-09T16:09:50Z

/lgtm
/approve

knative-prow-robot · 2019-04-09T16:09:57Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cppforlife, nak3, navidshaikh

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

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

Add missing jobs, group similar tasks, update settings.

knative-prow-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Mar 4, 2019

knative-prow-robot requested review from cppforlife and mattmoor March 4, 2019 08:18

knative-prow-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Mar 4, 2019

knative deleted a comment from cppforlife Mar 11, 2019

cppforlife reviewed Mar 12, 2019

View reviewed changes

nak3 force-pushed the all-namespaces branch from 3906744 to 010baa1 Compare March 19, 2019 08:29

knative-prow-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 19, 2019

nak3 force-pushed the all-namespaces branch from 010baa1 to 862eeda Compare March 19, 2019 09:29

navidshaikh reviewed Mar 19, 2019

View reviewed changes

pkg/kn/commands/namespaced.go Outdated Show resolved Hide resolved

navidshaikh reviewed Mar 19, 2019

View reviewed changes

pkg/kn/commands/namespaced.go Outdated Show resolved Hide resolved

navidshaikh approved these changes Mar 22, 2019

View reviewed changes

knative-prow-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Mar 26, 2019

nak3 force-pushed the all-namespaces branch from 5c98aad to 63a2ccf Compare March 30, 2019 05:15

knative-prow-robot assigned cppforlife Apr 8, 2019

knative-prow-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Apr 8, 2019

nak3 added 5 commits April 9, 2019 08:51

Add all-namespace option to KnParams

f39cd57

Add AddNamespaceFlags function

10a32d5

Add GetNamespace func

e186c06

Update help message

c7d6bf6

Use GetNamespace util in service create cmmand

3cad3dc

nak3 force-pushed the all-namespaces branch from 63a2ccf to 3cad3dc Compare April 8, 2019 23:57

knative-prow-robot removed the lgtm Indicates that a PR is ready to be merged. label Apr 8, 2019

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 9, 2019

knative-prow-robot merged commit 6b2b7f2 into knative:master Apr 9, 2019

nak3 deleted the all-namespaces branch April 10, 2019 00:44

coryrc pushed a commit to coryrc/client that referenced this pull request May 14, 2020

Update Prow config (knative#32)

c79ab57

Add missing jobs, group similar tasks, update settings.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support --all-namespaces option for service and revision list #32

Support --all-namespaces option for service and revision list #32

nak3 commented Mar 4, 2019 •

edited

cppforlife commented Mar 5, 2019

nak3 commented Mar 11, 2019

cppforlife Mar 12, 2019

nak3 Mar 12, 2019 •

edited

cppforlife Mar 12, 2019

sixolet Mar 19, 2019

nak3 commented Mar 19, 2019

cppforlife commented Mar 21, 2019

nak3 commented Mar 21, 2019

navidshaikh left a comment

cppforlife commented Mar 26, 2019

bbrowning commented Mar 29, 2019

nak3 commented Mar 30, 2019

cppforlife commented Apr 8, 2019

nak3 commented Apr 8, 2019

nak3 commented Apr 8, 2019

knative-metrics-robot commented Apr 8, 2019

nak3 commented Apr 9, 2019 •

edited

cppforlife commented Apr 9, 2019

knative-prow-robot commented Apr 9, 2019

Support --all-namespaces option for service and revision list #32

Support --all-namespaces option for service and revision list #32

Conversation

nak3 commented Mar 4, 2019 • edited

cppforlife commented Mar 5, 2019

nak3 commented Mar 11, 2019

cppforlife Mar 12, 2019

Choose a reason for hiding this comment

nak3 Mar 12, 2019 • edited

Choose a reason for hiding this comment

cppforlife Mar 12, 2019

Choose a reason for hiding this comment

sixolet Mar 19, 2019

Choose a reason for hiding this comment

nak3 commented Mar 19, 2019

cppforlife commented Mar 21, 2019

nak3 commented Mar 21, 2019

navidshaikh left a comment

Choose a reason for hiding this comment

cppforlife commented Mar 26, 2019

bbrowning commented Mar 29, 2019

nak3 commented Mar 30, 2019

cppforlife commented Apr 8, 2019

nak3 commented Apr 8, 2019

nak3 commented Apr 8, 2019

knative-metrics-robot commented Apr 8, 2019

nak3 commented Apr 9, 2019 • edited

cppforlife commented Apr 9, 2019

knative-prow-robot commented Apr 9, 2019

nak3 commented Mar 4, 2019 •

edited

nak3 Mar 12, 2019 •

edited

nak3 commented Apr 9, 2019 •

edited