Add kn route list command #202

navidshaikh · 2019-06-25T10:14:42Z

Fixes #201

Example:

# list all routes
kn route list
NAME    URL                                     AGE   CONDITIONS   TRAFFIC
hello   http://hello.default.apps-crc.testing   13m   1 OK / 3     100% -> hello-7w8z7
kn1     http://kn1.default.apps-crc.testing     22h   3 OK / 3     100% -> kn1-pf6jr
rt1     http://rt1.default.apps-crc.testing     22h   3 OK / 3     20% -> svc1-j7hqr, 80% -> svc1-j7hqr

Help section

kn route list --help           
List available routes.

Usage:
  kn route list NAME [flags]

Examples:

  # List all routes
  kn route list

  # List route 'web' in namespace 'dev'
  kn route list web -n dev

  # List all routes in yaml format
  kn route list -o yaml

Flags:
      --all-namespaces                If present, list the requested object(s) across all namespaces. Namespace in current context is ignored even if specified with --namespace.
      --allow-missing-template-keys   If true, ignore any errors in templates when a field or map key is missing in the template. Only applies to golang and jsonpath output formats. (default true)
  -h, --help                          help for list
  -n, --namespace string              List the requested object(s) in given namespace.
  -o, --output string                 Output format. One of: json|yaml|name|go-template|go-template-file|template|templatefile|jsonpath|jsonpath-file.
      --template string               Template string or path to template file to use when -o=go-template, -o=go-template-file. The template format is golang templates [http://golang.org/pkg/text/template/#pkg-overview].

Global Flags:
      --config string       config file (default is $HOME/.kn/config.yaml)
      --kubeconfig string   kubectl config file (default is $HOME/.kube/config)

knative-prow-robot · 2019-06-25T10:14:56Z

Hi @navidshaikh. Thanks for your PR.

I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

navidshaikh · 2019-06-25T10:16:24Z

/ok-to-test

knative-prow-robot · 2019-06-25T10:16:53Z

@navidshaikh: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/ok-to-test

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.

sixolet · 2019-06-25T17:18:27Z

/ok-to-test

rhuss · 2019-06-25T17:18:31Z

/ok-to-test

pkg/kn/commands/route/list_test.go

navidshaikh · 2019-06-26T11:41:05Z

Failure prompts

2019/06/26 07:37:17 Code coverage is below threshold (50%), fail presubmit workflow intentionally

will add more test coverage for pkg/kn/commands/route/list_flags.go

navidshaikh · 2019-06-26T15:03:07Z

/test pull-knative-client-integration-tests

navidshaikh · 2019-06-26T16:23:06Z

/test pull-knative-client-integration-tests

navidshaikh · 2019-06-27T12:37:04Z

e2e tests are failing with

--- FAIL: TestBasicWorkflow (148.82s)
basic_workflow_test.go:177: Expected output incorrect, expecting to include:
100% -> hello
Instead found:
NAME URL AGE CONDITIONS TRAFFIC
hello http://hello.kne2etests.example.com 21s 0 OK / 3

The traffic percent and revision name altogether is not showing up (I added a time.Sleep as well before executing route list command, age is 21s), also conditions for route are 0/3, this works well on my local cluster.

rhuss · 2019-06-27T15:06:41Z

The traffic percent and revision name altogether is not showing up (I added a time.Sleep as well before executing route list command, age is 21s), also conditions for route are 0/3, this works well on my local cluster.

Let us work on #54 now and get it integrated, then hopefully we can get rid a bit of the flakiness.

navidshaikh · 2019-06-28T15:04:02Z

/assign @rhuss @cppforlife @maximilien @sixolet

pkg/kn/commands/route/list.go

cppforlife · 2019-06-28T17:48:50Z

pkg/kn/commands/route/human_readable_flags.go

+}
+
+func calculateTraffic(targets []servingv1alpha1.TrafficTarget) string {
+	var traffic string


it seems that we keep on overriding traffic string in a loop?

@cppforlife: yup but with an update only if there is traffic split available

traffic = fmt.Sprintf("%s, %d%% -> %s", traffic, target.Percent, target.RevisionName)

im still confused, should it be traffic += ... instead since there could be multiple targets?

cppforlife · 2019-06-28T17:53:16Z

pkg/kn/commands/route/list_flags_test.go

+	"k8s.io/cli-runtime/pkg/genericclioptions"
+)
+
+func TestRoutListFlags(t *testing.T) {


i dont think we need this test. it's too close to implementation so it doesnt add confidence that this object is correct. additionally route list flags object is used in the command test, so it gets tested there indirectly in a more resilient manner.

I added this file's test, as without tests pk/kn/commands/route/list_flags.go was failing CI go test coverage threshold (must be > 50%).

should I be testing this in some other way?

pkg/kn/commands/route/route.go

pkg/kn/commands/test_helper.go

test/e2e-smoke-tests.sh

pkg/kn/commands/route/human_readable_flags.go

pkg/kn/commands/route/list_flags_test.go

navidshaikh · 2019-07-01T11:05:44Z

/test pull-knative-client-integration-tests

maximilien · 2019-07-01T12:44:15Z

Actually one last thing before lgtm. Can you squash the 14 commits into one? Unless you have good reason to have so many. Otherwise, looks good to me :)

navidshaikh · 2019-07-01T13:01:56Z

Can you squash the 14 commits into one? Unless you have good reason to have so many. Otherwise, looks good to me :)

The bot should do this while merging to master, for eg: check this commit squashed into one from 9.

maximilien · 2019-07-01T13:05:09Z

/lgtm

maximilien · 2019-07-01T13:05:38Z

Can you squash the 14 commits into one? Unless you have good reason to have so many. Otherwise, looks good to me :)

The bot should do this while merging to master, for eg: check this commit squashed into one from 9.

Thanks, great to know. Best 🍻

rhuss · 2019-07-01T13:22:17Z

The bot should do this while merging to master, for eg: check this commit squashed into one from 9.

Tru, but you will get the sum of all commit messages (even when you did some extra rounds which then has been reverted). In that case you can still squash on your own to have a clean single commit message, too.

- Accepts an argument name for listing particular route - Enables the machine readable output flags - Updates docs for kn route command group - Adds unit tests for route command group and route list - Adds integration tests for route list in basic workflow test - Updates tests and getting namespace - Adds more unit tests for code in pkg/kn/commands/route/list_flags.go - Adds route list command in smoke tests - Updates vendor/modules.txt - Clean up imports - Addresses review comments - replaces knative to Knative - uses reflect.DeepEqual for slice comparison - removes few code comments - removes irrelevant tests modifications from the PR

knative-metrics-robot · 2019-07-02T11:57:38Z

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
pkg/kn/commands/route/human_readable_flags.go	Do not exist	95.8%
pkg/kn/commands/route/list.go	Do not exist	76.7%
pkg/kn/commands/route/list_flags.go	Do not exist	66.7%
pkg/kn/commands/route/route.go	Do not exist	100.0%

rhuss · 2019-07-02T13:56:20Z

/lgtm

(re-adding @maximilien approval after squashing)

knative-prow-robot · 2019-07-02T18:24:42Z

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

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

googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Jun 25, 2019

knative-prow-robot requested review from cppforlife and evankanderson June 25, 2019 10:14

knative-prow-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jun 25, 2019

knative-prow-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jun 25, 2019

savitaashture reviewed Jun 26, 2019

View reviewed changes

pkg/kn/commands/route/list_test.go Outdated Show resolved Hide resolved

navidshaikh force-pushed the pr/routes-list branch from 0687da8 to 18da0ac Compare June 26, 2019 07:35

navidshaikh force-pushed the pr/routes-list branch from 641079e to 172bebd Compare June 28, 2019 14:28

knative-prow-robot assigned cppforlife, maximilien, rhuss and sixolet Jun 28, 2019

cppforlife reviewed Jun 28, 2019

View reviewed changes

pkg/kn/commands/route/list.go Outdated Show resolved Hide resolved

cppforlife reviewed Jun 28, 2019

View reviewed changes

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

cppforlife reviewed Jun 28, 2019

View reviewed changes

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

cppforlife reviewed Jun 28, 2019

View reviewed changes

test/e2e-smoke-tests.sh Outdated Show resolved Hide resolved

maximilien reviewed Jul 1, 2019

View reviewed changes

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

maximilien reviewed Jul 1, 2019

View reviewed changes

pkg/kn/commands/route/list_flags_test.go Outdated Show resolved Hide resolved

navidshaikh force-pushed the pr/routes-list branch from 172bebd to d7bbab4 Compare July 1, 2019 10:31

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

navidshaikh force-pushed the pr/routes-list branch from d7bbab4 to 175d9b5 Compare July 1, 2019 13:30

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

navidshaikh force-pushed the pr/routes-list branch from 175d9b5 to 45a8044 Compare July 2, 2019 11:55

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

rhuss approved these changes Jul 2, 2019

View reviewed changes

knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 2, 2019

knative-prow-robot merged commit aac0ec2 into knative:master Jul 2, 2019

navidshaikh deleted the pr/routes-list branch July 3, 2019 07:38

Kaustubh-pande pushed a commit to Kaustubh-pande/client that referenced this pull request Jul 12, 2023

[release-v1.9] Update kn-workflow to v0.30.3 (knative#202)

f7ecd70

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add kn route list command #202

Add kn route list command #202

navidshaikh commented Jun 25, 2019

knative-prow-robot commented Jun 25, 2019

navidshaikh commented Jun 25, 2019

knative-prow-robot commented Jun 25, 2019

sixolet commented Jun 25, 2019

rhuss commented Jun 25, 2019

navidshaikh commented Jun 26, 2019

navidshaikh commented Jun 26, 2019

navidshaikh commented Jun 26, 2019

navidshaikh commented Jun 27, 2019

rhuss commented Jun 27, 2019

navidshaikh commented Jun 28, 2019

cppforlife Jun 28, 2019

navidshaikh Jul 1, 2019

cppforlife Jul 2, 2019

cppforlife Jun 28, 2019

navidshaikh Jul 1, 2019 •

edited

Loading

navidshaikh commented Jul 1, 2019

maximilien commented Jul 1, 2019 •

edited

Loading

navidshaikh commented Jul 1, 2019

maximilien commented Jul 1, 2019

maximilien commented Jul 1, 2019 •

edited

Loading

rhuss commented Jul 1, 2019

knative-metrics-robot commented Jul 2, 2019

rhuss commented Jul 2, 2019

knative-prow-robot commented Jul 2, 2019

Add kn route list command #202

Add kn route list command #202

Conversation

navidshaikh commented Jun 25, 2019

knative-prow-robot commented Jun 25, 2019

navidshaikh commented Jun 25, 2019

knative-prow-robot commented Jun 25, 2019

sixolet commented Jun 25, 2019

rhuss commented Jun 25, 2019

navidshaikh commented Jun 26, 2019

navidshaikh commented Jun 26, 2019

navidshaikh commented Jun 26, 2019

navidshaikh commented Jun 27, 2019

rhuss commented Jun 27, 2019

navidshaikh commented Jun 28, 2019

cppforlife Jun 28, 2019

Choose a reason for hiding this comment

navidshaikh Jul 1, 2019

Choose a reason for hiding this comment

cppforlife Jul 2, 2019

Choose a reason for hiding this comment

cppforlife Jun 28, 2019

Choose a reason for hiding this comment

navidshaikh Jul 1, 2019 • edited Loading

Choose a reason for hiding this comment

navidshaikh commented Jul 1, 2019

maximilien commented Jul 1, 2019 • edited Loading

navidshaikh commented Jul 1, 2019

maximilien commented Jul 1, 2019

maximilien commented Jul 1, 2019 • edited Loading

rhuss commented Jul 1, 2019

knative-metrics-robot commented Jul 2, 2019

rhuss commented Jul 2, 2019

knative-prow-robot commented Jul 2, 2019

navidshaikh Jul 1, 2019 •

edited

Loading

maximilien commented Jul 1, 2019 •

edited

Loading

maximilien commented Jul 1, 2019 •

edited

Loading