feat(source): Add 'kn source list' #666

navidshaikh · 2020-02-14T07:48:17Z

/lint
Fixes #480

Proposed Changes

Add 'kn source list' listing the available sources COs
Use dynamic client to find available source objects
Add --type flag to filter source list based on source type
Printed a source with machine readable format:
- If source objects are of one type: apiVersion and kind is set to respective source type's object
- If source objects are of different types in list: apiVersion is set to "" and kind is to List.
- If source objects are of different types but only one --type filter is given to subset source objects, the machine readable format sets apiVersion and List to respective source's type.

Default output:

kn source list
NAME   TYPE              RESOURCE                                        SINK         READY
a1     ApiServerSource   apiserversources.sources.eventing.knative.dev   svc:eshow2   True
c1     CronJobSource     cronjobsources.sources.eventing.knative.dev     svc:eshow3   True
p1     PingSource        pingsources.sources.knative.dev                 svc:eshow1   True

Using (case insensitive) type filters:

Single source type

➜  client git:(pr/source-list) kn source list --type ApiServerSource
NAME   TYPE              RESOURCE                                        SINK         READY
a1     ApiServerSource   apiserversources.sources.eventing.knative.dev   svc:eshow2   True

Multiple source types

➜  client git:(pr/source-list) kn source list --type ApiServerSource --type pingsource
NAME   TYPE              RESOURCE                                        SINK         READY
a1     ApiServerSource   apiserversources.sources.eventing.knative.dev   svc:eshow2   True
p1     PingSource        pingsources.sources.knative.dev                 svc:eshow1   True

See `kind` and `apiVersion` of `List` for machine readable format:

For multiple types of source objects

➜  client git:(pr/source-list) kn source list -oyaml| grep kind | tail -1
kind: List

➜  client git:(pr/source-list) kn source list -oyaml| grep apiVersion | head -1
apiVersion: ""

For single source type objects (or sub-set a single type using --type filter):

➜  client git:(pr/source-list) kn source list --type apiserversource -oyaml | grep kind | tail -1 
kind: ApiServerSourceList

➜  client git:(pr/source-list) kn source list --type apiserversource -oyaml | grep apiVersion | head -1
apiVersion: sources.eventing.knative.dev/v1alpha1

knative-prow-robot

@navidshaikh: 2 warnings.

In response to this:

/lint
Fixes #480

Proposed Changes

Add 'kn source list' listing the available sources COs

Use dynamic client to

find out available source types

find the COs in given namespace for each source type
➜  client git:(pr/source-list) kn source list
NAME        TYPE              RESOURCE                                        SINK
k8sevents   ApiServerSource   apiserversources.sources.eventing.knative.dev   svc:mysvc

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.

pkg/kn/commands/source/human_readable_flags.go

navidshaikh · 2020-02-14T07:48:58Z

Adding another example of kn source list with another column READY.

navidshaikh · 2020-02-14T09:26:53Z

With 3 additional columns: CONDITIONS, READY, REASON

kn source list
NAME              TYPE              RESOURCE                                        SINK        CONDITIONS   READY   REASON
k8sevents         ApiServerSource   apiserversources.sources.eventing.knative.dev   svc:mysvc   6 OK / 6     True    
k8sevents2        ApiServerSource   apiserversources.sources.eventing.knative.dev   svc:mysvc   2 OK / 5     False    : User system:serviceaccount:default:doesnt-exist cannot get, list, watch resource "events" in API group ""
my-cron-trigger   CronJobSource     cronjobsources.sources.eventing.knative.dev     svc:mysvc   7 OK / 7     True

navidshaikh · 2020-02-14T09:30:31Z

With only READY column

kn source list
NAME              TYPE              RESOURCE                                        SINK        READY
k8sevents         ApiServerSource   apiserversources.sources.eventing.knative.dev   svc:mysvc   True
k8sevents2        ApiServerSource   apiserversources.sources.eventing.knative.dev   svc:mysvc   False
my-cron-trigger   CronJobSource     cronjobsources.sources.eventing.knative.dev     svc:mysvc   True

IMO READY column is enough else the columns don't fit in screen, CONDITIONS and READY can be seen from describe instead.

navidshaikh · 2020-02-14T11:59:29Z

Added --type flag to filter source list based on source type(s). Updated PR description with examples.

navidshaikh · 2020-02-18T13:06:42Z

/lint

knative-prow-robot

@navidshaikh: 2 warnings.

In response to this:

/lint

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.

pkg/dynamic/client.go

navidshaikh · 2020-02-18T14:47:39Z

This is ready for review.
/cc @rhuss @daisy-ycguo

navidshaikh · 2020-02-18T15:11:16Z

/test pull-knative-client-integration-tests

no endpoints available for service "webhook"

navidshaikh · 2020-02-18T17:11:22Z

/retest

On Tue, 18 Feb 2020 at 9:06 PM, Knative Prow Robot ***@***.***> wrote: @navidshaikh <https://github.com/navidshaikh>: The following test *failed*, say /retest to rerun all failed tests: Test name Commit Details Rerun command pull-knative-client-integration-tests 4d74e8d <4d74e8d> link <https://prow.knative.dev/view/gcs/knative-prow/pr-logs/pull/knative_client/666/pull-knative-client-integration-tests/1229785508830253057> /test pull-knative-client-integration-tests Full PR test history <https://gubernator.knative.dev/pr/knative_client/666>. Your PR dashboard <https://gubernator.knative.dev/pr/navidshaikh>. Instructions for interacting with me using PR comments are available here <https://git.k8s.io/community/contributors/guide/pull-requests.md>. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra <https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:> repository. I understand the commands that are listed here <https://go.k8s.io/bot-commands>. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#666?email_source=notifications&email_token=AATE22ACJQRCZ4GBEPFXFN3RDP577A5CNFSM4KVCVIL2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEMCOBWQ#issuecomment-587522266>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AATE22BBJSL7JXWIU3HFEXLRDP577ANCNFSM4KVCVILQ> .

-- Navid Shaikh Composed on handheld device.

navidshaikh · 2020-02-19T08:20:08Z

/retest

smoketests: timeout: service 'hello' not ready after 600 seconds

rhuss

This a partial review for the API and how the list is fetched. Please find my comments below, I think we could improve the API.

Also what I'm a bit puzzled: Why does return ListSourceTypes an Unstructured and not a list CustomResourceDefinitions (which would make it much easier and type-safer to work with).

pkg/dynamic/client.go

navidshaikh · 2020-02-28T11:35:25Z

/retest

navidshaikh · 2020-02-28T12:19:56Z

/test pull-knative-client-integration-tests

async revision delete issue

rhuss · 2020-03-05T09:14:19Z

/retest

@navidshaikh there are some codegen errors, too

navidshaikh · 2020-03-09T10:41:39Z

/retest

--- FAIL: TestSourceApiServer (210.14s)

rhuss · 2020-03-09T16:51:38Z

@navidshaikh lso codegen error. Guess that somehow sneaked in by a latest merge.

knative-metrics-robot · 2020-03-10T05:02:52Z

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

File	Old Coverage	New Coverage	Delta
pkg/dynamic/client.go	83.3%	89.2%	5.9
pkg/dynamic/lib.go	Do not exist	100.0%
pkg/kn/commands/flags/listfilters.go	Do not exist	100.0%
pkg/kn/commands/source/human_readable_flags.go	87.0%	87.2%	0.2
pkg/kn/commands/source/list.go	Do not exist	70.6%

rhuss · 2020-03-10T07:18:30Z

pkg/dynamic/client.go

+			sourceList.SetGroupVersionKind(sList.GetObjectKind().GroupVersionKind())
+		}
+	}
+	// Clear the Group and Version for list if there are multiple types of source objects found


pkg/dynamic/lib.go

rhuss · 2020-03-10T07:36:29Z

test/e2e/source_apiserver_test.go

+	t.Log("list sources in YAML format")
+	output = test.sourceList(t, r, "-oyaml")
+	assert.Check(t, util.ContainsAll(output, "testapisource1", "ApiServerSource", "Service", "testsvc0"))
+


thanks for already adding E2E tests, but maybe we could later enhance them with some more cases:

Invalid filter option

Heterogenous list of different types with -o yaml (would involve to combine a pingsource + apiserver source creation)

rhuss

Thanks !

Looks good, maybe one could extend the E2E tests a bit. I opened #722 for this.

/lgtm

knative-prow-robot · 2020-03-10T07:40:54Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: navidshaikh, rhuss

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~~ [navidshaikh,rhuss]

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

knative-prow-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 14, 2020

googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Feb 14, 2020

knative-prow-robot reviewed Feb 14, 2020

View reviewed changes

pkg/kn/commands/source/human_readable_flags.go Outdated Show resolved Hide resolved

pkg/kn/commands/source/human_readable_flags.go Outdated Show resolved Hide resolved

knative-prow-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Feb 14, 2020

knative-prow-robot requested review from mattmoor and sixolet February 14, 2020 07:48

navidshaikh force-pushed the pr/source-list branch from 78efb62 to 2381f8b Compare February 14, 2020 11:58

navidshaikh force-pushed the pr/source-list branch from 2381f8b to 1f53d19 Compare February 18, 2020 12:12

knative-prow-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 18, 2020

navidshaikh force-pushed the pr/source-list branch from 1f53d19 to c57b36a Compare February 18, 2020 13:04

navidshaikh changed the title ~~WIP: feature: Add 'kn source list'~~ feat(source): Add 'kn source list' Feb 18, 2020

knative-prow-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 18, 2020

knative-prow-robot reviewed Feb 18, 2020

View reviewed changes

pkg/dynamic/client.go Outdated Show resolved Hide resolved

pkg/dynamic/client.go Outdated Show resolved Hide resolved

navidshaikh force-pushed the pr/source-list branch 3 times, most recently from 247fa41 to 4d74e8d Compare February 18, 2020 14:38

knative-prow-robot requested review from daisy-ycguo and rhuss February 18, 2020 14:47

rhuss requested changes Feb 21, 2020

View reviewed changes

knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 5, 2020

Merge branch 'master' into pr/source-list

329c5cc

knative-prow-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 5, 2020

knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 9, 2020

Merge branch 'master' of github.com:knative/client into pr/source-list

0cf155e

knative-prow-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 9, 2020

Update ApiServer source group to apiserversources.sources.knative.dev

d1b9808

Merge branch 'master' of github.com:knative/client into pr/source-list

ab78629

navidshaikh force-pushed the pr/source-list branch from 8e6c66e to ab78629 Compare March 9, 2020 16:56

knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 9, 2020

Merge branch 'master' of github.com:knative/client into pr/source-list

ebda70d

knative-prow-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 9, 2020

Merge branch 'master' into pr/source-list

7544457

mattmoor reviewed Mar 9, 2020

View reviewed changes

Fix typos and align

a1508a3

rhuss reviewed Mar 10, 2020

View reviewed changes

pkg/dynamic/lib.go Show resolved Hide resolved

rhuss reviewed Mar 10, 2020

View reviewed changes

rhuss approved these changes Mar 10, 2020

View reviewed changes

knative-prow-robot assigned rhuss Mar 10, 2020

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 10, 2020

knative-prow-robot merged commit ba7e14c into knative:master Mar 10, 2020

navidshaikh deleted the pr/source-list branch March 10, 2020 07:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(source): Add 'kn source list' #666

feat(source): Add 'kn source list' #666

navidshaikh commented Feb 14, 2020 •

edited

Loading

knative-prow-robot left a comment

navidshaikh commented Feb 14, 2020

navidshaikh commented Feb 14, 2020

navidshaikh commented Feb 14, 2020

navidshaikh commented Feb 14, 2020

navidshaikh commented Feb 18, 2020

knative-prow-robot left a comment

navidshaikh commented Feb 18, 2020

navidshaikh commented Feb 18, 2020

navidshaikh commented Feb 18, 2020 via email

navidshaikh commented Feb 19, 2020

rhuss left a comment

navidshaikh commented Feb 28, 2020

navidshaikh commented Feb 28, 2020

rhuss commented Mar 5, 2020

navidshaikh commented Mar 9, 2020

rhuss commented Mar 9, 2020

knative-metrics-robot commented Mar 10, 2020

rhuss Mar 10, 2020

rhuss Mar 10, 2020

rhuss left a comment

knative-prow-robot commented Mar 10, 2020

feat(source): Add 'kn source list' #666

feat(source): Add 'kn source list' #666

Conversation

navidshaikh commented Feb 14, 2020 • edited Loading

Proposed Changes

Default output:

Using (case insensitive) type filters:

See kind and apiVersion of List for machine readable format:

knative-prow-robot left a comment

Choose a reason for hiding this comment

Proposed Changes

navidshaikh commented Feb 14, 2020

navidshaikh commented Feb 14, 2020

navidshaikh commented Feb 14, 2020

navidshaikh commented Feb 14, 2020

navidshaikh commented Feb 18, 2020

knative-prow-robot left a comment

Choose a reason for hiding this comment

navidshaikh commented Feb 18, 2020

navidshaikh commented Feb 18, 2020

navidshaikh commented Feb 18, 2020 via email

navidshaikh commented Feb 19, 2020

rhuss left a comment

Choose a reason for hiding this comment

navidshaikh commented Feb 28, 2020

navidshaikh commented Feb 28, 2020

rhuss commented Mar 5, 2020

navidshaikh commented Mar 9, 2020

rhuss commented Mar 9, 2020

knative-metrics-robot commented Mar 10, 2020

rhuss Mar 10, 2020

Choose a reason for hiding this comment

rhuss Mar 10, 2020

Choose a reason for hiding this comment

rhuss left a comment

Choose a reason for hiding this comment

knative-prow-robot commented Mar 10, 2020

navidshaikh commented Feb 14, 2020 •

edited

Loading

See `kind` and `apiVersion` of `List` for machine readable format: