diff --git a/.github/ISSUE_TEMPLATE/ask-question.md b/.github/ISSUE_TEMPLATE/ask-question.md new file mode 100644 index 0000000000..82ee5426da --- /dev/null +++ b/.github/ISSUE_TEMPLATE/ask-question.md @@ -0,0 +1,23 @@ +--- +name: Question +about: Ask a question about knative/client +title: '' +labels: kind/question +assignees: '' + +--- + +## In what area(s)? + + + + + +## Ask your question here: + diff --git a/.github/ISSUE_TEMPLATE/bug-report.md b/.github/ISSUE_TEMPLATE/bug-report.md new file mode 100644 index 0000000000..22b28a0c0d --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug-report.md @@ -0,0 +1,44 @@ +--- +name: Bug report +about: Report a bug in knative/client +title: '' +labels: kind/bug +assignees: '' + +--- + +## In what area(s)? + + + + + +## What version of Knative Client? + +> Paste output of 'kn version' + +## What version of Knative Serving running on your cluster? + +> 0.5.x +> 0.6.x +> 0.7.x +> 0.8.x + +## Expected Behavior + + + + +## Actual Behavior + + + + +## Steps to Reproduce the Problem + + diff --git a/.github/ISSUE_TEMPLATE/feature-request.md b/.github/ISSUE_TEMPLATE/feature-request.md new file mode 100644 index 0000000000..52f29b2184 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/feature-request.md @@ -0,0 +1,24 @@ +--- +name: Feature Request +about: Create a feature request for knative/client +title: '' +labels: kind/feature +assignees: '' + +--- + +## In what area(s)? + + + + + +## Describe the feature: + diff --git a/.github/pull-request-template.md b/.github/pull-request-template.md new file mode 100644 index 0000000000..99ad38da07 --- /dev/null +++ b/.github/pull-request-template.md @@ -0,0 +1,23 @@ + + +Fixes # + +## Proposed Changes + +* +* +* + +**Release Note** + + + +```release-note + +``` diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc new file mode 100644 index 0000000000..9c7a490f29 --- /dev/null +++ b/CHANGELOG.adoc @@ -0,0 +1,186 @@ +# Changelog + +// Template: +//// +[cols="1,10,3", options="header", width="100%"] +|=== +| | Description | PR + +| ๐ŸŽ๐Ÿ›๐Ÿงฝ๐Ÿ—‘๏ธ +| +| https://github.com/knative/client/pull/[#] +|=== +//// + +## v0.8.0 (unreleased) + +[cols="1,10,3", options="header", width="100%"] +|=== +| | Description | PR + +| ๐ŸŽ +| Wait for service to become ready with `kn service update` (same behaviour as for `kn service create`) +| https://github.com/knative/client/pull/271[#271] + +| ๐Ÿ› +| Better error handling when providing wrong kubeconfig option +| https://github.com/knative/client/pull/222[#222] + +|=== + +## v0.2.0 (2019-07-10) + +[cols="1,10,3", options="header", width="100%"] +|=== +| | Description | PR + +| ๐Ÿ› +| Show URL instead of address when listing services +| https://github.com/knative/client/pull/247[#247] + +| ๐ŸŽ +| Add `kn service list ` and `kn revision list ` +| https://github.com/knative/client/pull/150[#150] + +| ๐Ÿ› +| Dynamically set GroupVersionKind via schema lookup +| https://github.com/knative/client/pull/134[#134] + +| ๐Ÿงฝ +| Introduce a `KnClient` interface +| https://github.com/knative/client/pull/134[#134] + +| ๐Ÿ› +| Retry update operation on an optimistic lock failure +| https://github.com/knative/client/pull/240[#240] + +| ๐ŸŽ +| Add `kn route list` +| https://github.com/knative/client/pull/202[#202] + +| ๐Ÿงฝ +| Improved error message when no command is given +| https://github.com/knative/client/pull/218[#218] + +| ๐ŸŽ +| Add gotest.tools testing support +| https://github.com/knative/client/pull/218[#218] + +| ๐ŸŽ +| Add second test run against latest released Knative serving version +| https://github.com/knative/client/pull/170[#170] + +| ๐ŸŽ๏ธ +| Add `--port` to `kn service create` and `kn service update` +| https://github.com/knative/client/pull/191[#191] + +| ๐ŸŽ +| Add `kn revision delete` +| https://github.com/knative/client/pull/207[#207] + +| ๐ŸŽ +| Add goimport to `build.sh` +| https://github.com/knative/client/pull/186[#186] + +| ๐Ÿงฝ +| Wait for service to become ready with `kn service create` +| https://github.com/knative/client/pull/156[#156] + +| ๐ŸŽ +| Add shell based smoke tests +| https://github.com/knative/client/pull/183[#183] + +| ๐Ÿงฝ +| Use current namespace from `.kube/config` as default +| https://github.com/knative/client/pull/172[#172] + +| ๐Ÿงฝ +| Add `--force` to `kn service create` for replacing existing service +| https://github.com/knative/client/pull/79[#79] + +| ๐Ÿงฝ +| Add `kn revision list --service ` +| https://github.com/knative/client/pull/194[#194] + +| ๐Ÿงฝ +| Add success message to `kn service update` +| https://github.com/knative/client/pull/169[#169] + +| ๐ŸŽ +| Add mandatory license check to `build.sh` +| https://github.com/knative/client/pull/187[#187] + +| ๐ŸŽ +| Add Golang based E2E tests +| https://github.com/knative/client/pull/121[#121] + +| ๐Ÿงฝ +| Rename `kn revision get` to `kn revision list` +| https://github.com/knative/client/pull/180[#180] + +| ๐Ÿงฝ +| Rename `kn service get` to `kn service list` +| https://github.com/knative/client/pull/179[#179] + +| ๐Ÿงฝ +| Refactoring to use sub-packages +| https://github.com/knative/client/pull/66[#66] + +| ๐ŸŽ +| Add `--test`, `--fast`, `--update` to `build.sh` +| https://github.com/knative/client/pull/149[#149] + +| ๐Ÿงฝ๏ธ +| Update to Knative serving 0.6.0 +| https://github.com/knative/client/pull/129[#129] + +| ๐ŸŽ +| Add Zsh completion +| https://github.com/knative/client/pull/132[#132] + + +| ๐ŸŽ +| Add autoscale & concurrency options for `service create` and `service update` (`--min-scale`, `--max-scale`, `--concurrency-limit`, `--concurrency-target`) +| https://github.com/knative/client/pull/157[#157] + +| ๐ŸŽ +| Add `--watch` for `build.sh` to enter a compile-watch loop +| https://github.com/knative/client/pull/160[#160] + +|=== + +## v0.1.0 (2019-05-17) + +[cols="1,10,3", options="header", width="100%"] +|=== +| | Description | PR + +| ๐ŸŽ +| Add --force for `service create` +| https://github.com/knative/client/pull/79[#79] + +| ๐Ÿ› +| Fix info messages after `service create` and `service delete` +| https://github.com/knative/client/pull/95[#95] + +| ๐ŸŽ +| Add `revision get` +| https://github.com/knative/client/pull/97[#97] + +| ๐ŸŽ +| Add `service get` +| https://github.com/knative/client/pull/90[#90] + +|=== + +''' +_Legend_ : ๐ŸŽ Feature - ๐Ÿ› Fix - ๐Ÿงฝ Update - ๐Ÿ—‘๏ธ Remove + +//// +--------------------------------------------- +Ignore PRs: + +12 +45 +--------------------------------------------- +//// diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index fa8e2ade85..eb77257b72 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -73,7 +73,7 @@ You can link that script into a directory within your search `$PATH`. This allow * `build.sh` - Compile, test, generate docs and format source code * `build.sh -f` - Compile only * `build.sh -f -t` - Compile & test -* `build.sh -u` - Update dependencies before compiling +* `build.sh -c` - Update dependencies, regenerate documentation and format source files * `build.sh -w` - Enter watch mode for automatic recompilation * `build.sh -w -t` - Enter watch mode for automatic recompilation & running tests diff --git a/conventions/cli.md b/conventions/cli.md new file mode 100644 index 0000000000..db68aa7037 --- /dev/null +++ b/conventions/cli.md @@ -0,0 +1,71 @@ +# Guidelines for `kn` Commands + +Commands are generally of the form `kn `; the resource kind +forms a command group for all the operations you might want to do with that kind +of resource. + +Commands that directly concern more than one resource kind may be categorized +with one of the relevant resources, or may get their own top-level verb +(eg. `connect`). + +Top-level commands concerning the operation of `kn` itself, like `help` and +`version` are also okay. + +## Resource + +The .knative.dev Kind, singluar and lowercase. For example, `service` for +`serving.knative.dev/service` or `trigger` for `eventing.knative.dev/trigger`. + +## Verb + +If the thing the user's doing can be described by the following commands, these +should be the name of the verb: + +* `describe` prints detailed information about a single resource. It can include + status information of related or child resources, too. + +* `list` prints summary information about all resources of a type, possibly + filtered by parent or label selector. + +* `create` creates a resource. Accepts a `--force` flag to create-or-replace. + +* `update` updates a resource based on the changes the user would like to make. + +* `delete` deletes a resource + +For a given resource there should be parallelism between arguments to `create` +and `update` as much as possible. + +Other domain-specific verbs are possible on a case-by-case basis, like +`set-traffic` for a Knative Serving Service. + +## Arguments + +### Positionals + +Where there's a single target resource, the resource name should be a positional +argument. It needs to be of the resource type we're talking about, eg. `kn +revision` subcommands the positional must be naming a revision. + +```bash +kn service create foo --image gcr.io/things/stuff:tag +``` +In this case `foo` is positional, and refers to the service to create. + +### Flags + +* `--force` is a flag on all create commands, and will replace the resource if + it already exists (otherwise this is an error). The resource will be *mutated* + to have a spec exactly like the resource that would otherwise be created. It + is not deleted and recreated. + +* When a flag sets a particular field on create or update, it should be a short + name for the field, without necessarily specifying how it's nested. For + example, `--image=img.repo/asdf` in Knative Serving sets + `spec.template.containers[0].image` + +#### Output + +Commands that output information should support `--output` with a shorthand of +`-o` to choose how to frame their output, and `--template` for supplying +templates to output styles that use them. diff --git a/docs/cmd/kn.md b/docs/cmd/kn.md index d3f2f6bc7f..44c5ab7c4d 100644 --- a/docs/cmd/kn.md +++ b/docs/cmd/kn.md @@ -21,7 +21,6 @@ Manage your Knative building blocks: ### SEE ALSO -* [kn completion](kn_completion.md) - Output shell completion code (default Bash) * [kn plugin](kn_plugin.md) - Plugin command group * [kn revision](kn_revision.md) - Revision command group * [kn route](kn_route.md) - Route command group diff --git a/docs/cmd/kn_completion.md b/docs/cmd/kn_completion.md deleted file mode 100644 index 4c9215cb04..0000000000 --- a/docs/cmd/kn_completion.md +++ /dev/null @@ -1,30 +0,0 @@ -## kn completion - -Output shell completion code (default Bash) - -### Synopsis - -Output shell completion code (default Bash) - -``` -kn completion [flags] -``` - -### Options - -``` - -h, --help help for completion - --zsh Generates completion code for Zsh shell. -``` - -### Options inherited from parent commands - -``` - --config string kn config file (default is $HOME/.kn/config.yaml) - --kubeconfig string kubectl config file (default is $HOME/.kube/config) -``` - -### SEE ALSO - -* [kn](kn.md) - Knative client - diff --git a/docs/cmd/kn_revision_list.md b/docs/cmd/kn_revision_list.md index 7b0608af79..3d356f1978 100644 --- a/docs/cmd/kn_revision_list.md +++ b/docs/cmd/kn_revision_list.md @@ -7,7 +7,7 @@ List available revisions. List revisions for a given service. ``` -kn revision list [flags] +kn revision list [name] [flags] ``` ### Examples @@ -19,6 +19,12 @@ kn revision list [flags] # List revisions for a service 'svc1' in namespace 'myapp' kn revision list -s svc1 -n myapp + + # List all revisions in JSON output format + kn revision list -o json + + # List revision 'web' + kn revision list web ``` ### Options diff --git a/docs/cmd/kn_route.md b/docs/cmd/kn_route.md index a058e309bd..46696f7060 100644 --- a/docs/cmd/kn_route.md +++ b/docs/cmd/kn_route.md @@ -26,5 +26,6 @@ kn route [flags] ### SEE ALSO * [kn](kn.md) - Knative client +* [kn route describe](kn_route_describe.md) - Describe available route. * [kn route list](kn_route_list.md) - List available routes. diff --git a/docs/cmd/kn_route_describe.md b/docs/cmd/kn_route_describe.md new file mode 100644 index 0000000000..f1da10275f --- /dev/null +++ b/docs/cmd/kn_route_describe.md @@ -0,0 +1,33 @@ +## kn route describe + +Describe available route. + +### Synopsis + +Describe available route. + +``` +kn route describe NAME [flags] +``` + +### Options + +``` + --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 describe + -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. (default "yaml") + --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]. +``` + +### Options inherited from parent commands + +``` + --config string kn config file (default is $HOME/.kn/config.yaml) + --kubeconfig string kubectl config file (default is $HOME/.kube/config) +``` + +### SEE ALSO + +* [kn route](kn_route.md) - Route command group + diff --git a/docs/cmd/kn_service_create.md b/docs/cmd/kn_service_create.md index d89f9ef96f..4f0a2bed05 100644 --- a/docs/cmd/kn_service_create.md +++ b/docs/cmd/kn_service_create.md @@ -54,7 +54,7 @@ kn service create NAME --image IMAGE [flags] -p, --port int32 The port where application listens on. --requests-cpu string The requested CPU (e.g., 250m). --requests-memory string The requested memory (e.g., 64Mi). - --wait-timeout int Seconds to wait before giving up on waiting for service to be ready (default: 60). (default 60) + --wait-timeout int Seconds to wait before giving up on waiting for service to be ready. (default 60) ``` ### Options inherited from parent commands diff --git a/docs/cmd/kn_service_list.md b/docs/cmd/kn_service_list.md index 322e7e14db..9d7b08eeda 100644 --- a/docs/cmd/kn_service_list.md +++ b/docs/cmd/kn_service_list.md @@ -7,7 +7,21 @@ List available services. List available services. ``` -kn service list [flags] +kn service list [name] [flags] +``` + +### Examples + +``` + + # List all services + kn service list + + # List all services in JSON output format + kn service list -o json + + # List service 'web' + kn service list web ``` ### Options diff --git a/docs/cmd/kn_service_update.md b/docs/cmd/kn_service_update.md index 3116b08686..707a37652e 100644 --- a/docs/cmd/kn_service_update.md +++ b/docs/cmd/kn_service_update.md @@ -27,6 +27,7 @@ kn service update NAME [flags] ### Options ``` + --async Update service and don't wait for it to become ready. --concurrency-limit int Hard Limit of concurrent requests to be processed by a single replica. --concurrency-target int Recommendation for when to scale up based on the concurrent number of incoming request. Defaults to --concurrency-limit when given. -e, --env stringArray Environment variable to set. NAME=value; you may provide this flag any number of times to set multiple environment variables. @@ -40,6 +41,7 @@ kn service update NAME [flags] -p, --port int32 The port where application listens on. --requests-cpu string The requested CPU (e.g., 250m). --requests-memory string The requested memory (e.g., 64Mi). + --wait-timeout int Seconds to wait before giving up on waiting for service to be ready. (default 60) ``` ### Options inherited from parent commands diff --git a/go.sum b/go.sum index 330faf3a73..29a03df64c 100644 --- a/go.sum +++ b/go.sum @@ -342,6 +342,7 @@ golang.org/x/time v0.0.0-20190308202827-9d24e82272b4 h1:SvFZT6jyqRaOeXpc5h/JSfZe golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/tools v0.0.0-20180221164845-07fd8470d635/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20180828015842-6cd1fcedba52/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= +golang.org/x/tools v0.0.0-20181011042414-1f849cf54d09 h1:6Cq5LXQ/D2J5E7sYJemWSQApczOzY1rxSp8TWloyxIY= golang.org/x/tools v0.0.0-20181011042414-1f849cf54d09/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20190114222345-bf090417da8b/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20190226205152-f727befe758c/go.mod h1:9Yl7xja0Znq3iFh3HoIrodX9oNMXvdceNzlUR8zjMvY= diff --git a/hack/build.sh b/hack/build.sh index ebf5ccc3a3..e34b6c2488 100755 --- a/hack/build.sh +++ b/hack/build.sh @@ -47,40 +47,60 @@ run() { fi if $(has_flag --watch -w); then - watch - # No exit, needs to be stopped with CTRL-C anyways - fi + # Build and test first + go_build + go_test - if $(has_flag -u --update); then - # Update dependencies - update_deps + # Go in endless loop, to be stopped with CTRL-C + watch fi - if ! $(has_flag --fast -f); then + # Fast mode: Only compile and maybe run test + if $(has_flag --fast -f); then + go_build - # Format source code and cleanup imports - source_format + if $(has_flag --test -t); then + go_test + fi + exit 0 + fi - # Generate docs - # Check for license headers - check_license + # Run only tests + if $(has_flag --test -t); then + go_test + exit 0 + fi - # Auto generate cli docs - generate_docs + # Run only codegen + if $(has_flag --codegen -c); then + codegen + exit 0 fi - # Run build + # Default flow + codegen go_build - - # Run tests - if $(has_flag --test -t) || ! $(has_flag --fast -f); then - go_test - fi + go_test echo "โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€" ./kn version } + +codegen() { + # Update dependencies + update_deps + + # Format source code and cleanup imports + source_format + + # Check for license headers + check_license + + # Auto generate cli docs + generate_docs +} + go_fmt() { echo "๐Ÿงน ${S}Format" find $(echo $source_dirs) -name "*.go" -print0 | xargs -0 gofmt -s -w @@ -91,7 +111,7 @@ source_format() { which goimports >/dev/null 2>&1 if [ $? -ne 0 ]; then echo "โœ‹ No 'goimports' found. Please use" - echo "โœ‹ go get golang.org/x/tools/cmd/goimports" + echo "โœ‹ go install golang.org/x/tools/cmd/goimports" echo "โœ‹ to enable import cleanup. Import cleanup skipped." # Run go fmt instead @@ -112,8 +132,14 @@ go_build() { go_test() { local test_output=$(mktemp /tmp/kn-client-test-output.XXXXXX) - local red="" - local reset="" + + local red="" + local reset="" + # Use color only when a terminal is set + if [ -t 1 ]; then + red="" + reset="" + fi echo "๐Ÿงช ${S}Test" set +e @@ -240,10 +266,9 @@ Usage: $(basename $BASH_SOURCE) [... options ...] with the following options: --f --fast Only compile (without formatting, testing, doc generation) +-f --fast Only compile (without dep update, formatting, testing, doc gen) -t --test Run tests when used with --fast or --watch --i --imports Organize and cleanup imports --u --update Update dependencies before compiling +-c --codegen Runs formatting, doc gen and update without compiling/testing -w --watch Watch for source changes and recompile in fast mode -h --help Display this help message --verbose More output @@ -256,10 +281,12 @@ ln -s $(basedir)/hack/build.sh /usr/local/bin/kn_build.sh Examples: -* Compile, format, tests, docs: build.sh -* Compile only: build.sh --fast -* Compile with tests: build.sh -f -t -* Automatice recompilation: build.sh --watch +* Update deps, format, license check, + gen docs, compile, test: ........... build.sh +* Compile only: ...................... build.sh --fast +* Run only tests: .................... build.sh --test +* Compile with tests: ................ build.sh -f -t +* Automatic recompilation: ........... build.sh --watch EOT } diff --git a/hack/release.md b/hack/release.md deleted file mode 100644 index 4d77c801eb..0000000000 --- a/hack/release.md +++ /dev/null @@ -1,71 +0,0 @@ -# Creating a new Knative Client release - -The `release.sh` script automates the creation of Knative Client releases, either -nightly or versioned ones. - -By default, the script creates a nightly release but does not publish it -anywhere. - -## Common flags for cutting releases - -The following flags affect the behavior of the script, no matter the type of the -release. - -- `--skip-tests` Do not run tests before building the release. Otherwise, build, - unit and end-to-end tests are run and they all must pass for the release to be - built. -- `--tag-release`, `--notag-release` Tag (or not) the generated binaries with - either `vYYYYMMDD-` (for nightly releases) or `vX.Y.Z` for - versioned releases. _For versioned releases, a tag is always added._ -- `--release-gcs` Defines the GCS bucket where the binaries will be stored. By - default, this is `knative-nightly/client`. This flag is ignored if the release - is not being published. -- `--publish`, `--nopublish` Whether the generated binaries should be published - to the GCS bucket or not. If yes, the `--release-gcs` flag can be used to change - the default value of the GCS used. If no, the binaries will be written to the - local disk only (in the repository root directory). - -## Creating nightly releases - -Nightly releases are built against the current git tree. The behavior of the -script is defined by the common flags. You must have write access to the GCS -bucket the release will be pushed to, unless `--nopublish` is used. - -Examples: - -```bash -# Create and publish a nightly, tagged release. -./hack/release.sh --publish --tag-release - -# Create release, but don't test, publish or tag it. -./hack/release.sh --skip-tests --nopublish --notag-release -``` - -## Creating versioned releases - -_Note: only Knative admins can create versioned releases._ - -To specify a versioned release to be cut, you must use the `--version` flag. -Versioned releases are usually built against a branch in the Knative Client -repository, specified by the `--branch` flag. - -- `--version` Defines the version of the release, and must be in the form - `X.Y.Z`, where X, Y and Z are numbers. -- `--branch` Defines the branch in Knative Client repository from which the - release will be built. If not passed, the `master` branch at HEAD will be - used. This branch must be created before the script is executed, and must be - in the form `release-X.Y`, where X and Y must match the numbers used in the - version passed in the `--version` flag. This flag has no effect unless - `--version` is also passed. -- `--release-notes` Points to a markdown file containing a description of the - release. This is optional but highly recommended. It has no effect unless - `--version` is also passed. - -If this is the first time you're cutting a versioned release, you'll be prompted -for your GitHub username, password, and possibly 2-factor authentication -challenge before the release is published. - -The release will be published in the _Releases_ page of the Knative Client -repository, with the title _Knative Client release vX.Y.Z_ and the given release -notes. It will also be tagged _vX.Y.Z_ (both on GitHub and as a git annotated -tag). diff --git a/hack/release.sh b/hack/release.sh index c48917e0a3..e10654744e 100755 --- a/hack/release.sh +++ b/hack/release.sh @@ -14,6 +14,9 @@ # See the License for the specific language governing permissions and # limitations under the License. +# Documentation about this script and how to use it can be found +# at https://github.com/knative/test-infra/tree/master/ci + source $(dirname $0)/../vendor/github.com/knative/test-infra/scripts/release.sh source $(dirname $0)/build-flags.sh diff --git a/hack/update-deps.sh b/hack/update-deps.sh deleted file mode 100755 index 1e8e980303..0000000000 --- a/hack/update-deps.sh +++ /dev/null @@ -1,23 +0,0 @@ -#!/usr/bin/env bash - -# Copyright 2019 The Knative Authors -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -set -o errexit -set -o nounset -set -o pipefail - -# GO111MODULE used so that it works no matter if someone has this repo -# cloned under $GOPATH or not -GO111MODULE=on go mod vendor diff --git a/hack/verify-codegen.sh b/hack/verify-codegen.sh index 158ea36216..4e2c938052 100755 --- a/hack/verify-codegen.sh +++ b/hack/verify-codegen.sh @@ -20,33 +20,17 @@ set -o pipefail source $(dirname $0)/../vendor/github.com/knative/test-infra/scripts/library.sh -readonly TMP_DIFFROOT="$(mktemp -d ${REPO_ROOT_DIR}/tmpdiffroot.XXXXXX)" +# Needed later +go install golang.org/x/tools/cmd/goimports -cleanup() { - rm -rf "${TMP_DIFFROOT}" -} - -trap "cleanup" EXIT SIGINT - -cleanup - -# Save working tree state -mkdir -p "${TMP_DIFFROOT}/vendor" -cp -aR "${REPO_ROOT_DIR}/go.sum" "${REPO_ROOT_DIR}/vendor" "${TMP_DIFFROOT}" - -"${REPO_ROOT_DIR}/hack/update-deps.sh" -echo "Diffing ${REPO_ROOT_DIR} against freshly update dependencies" -ret=0 -diff -Naupr --no-dereference "${REPO_ROOT_DIR}/vendor" "${TMP_DIFFROOT}/vendor" || ret=1 - -# Restore working tree state -rm -fr "${REPO_ROOT_DIR}/go.sum" "${REPO_ROOT_DIR}/vendor" -cp -aR "${TMP_DIFFROOT}"/* "${REPO_ROOT_DIR}" - -if [[ $ret -eq 0 ]] -then - echo "${REPO_ROOT_DIR} up to date." +"${REPO_ROOT_DIR}"/hack/build.sh --codegen +if output="$(git status --porcelain)" && [ -z "$output" ]; then + echo "${REPO_ROOT_DIR} is up to date." else - echo "ERROR: ${REPO_ROOT_DIR} is out of date. Please run ./hack/update-deps.sh" + echo "ERROR: Modified files found:" + git status --porcelain + echo "ERROR: Diff" + git diff + echo "ERROR: ${REPO_ROOT_DIR} is out of date. Please run ./hack/build.sh -u and commit." exit 1 fi diff --git a/pkg/kn/commands/completion.go b/pkg/kn/commands/completion.go index 0c8a13843f..365a5814e9 100644 --- a/pkg/kn/commands/completion.go +++ b/pkg/kn/commands/completion.go @@ -28,8 +28,9 @@ func NewCompletionCommand(p *KnParams) *cobra.Command { var completionFlags CompletionFlags completionCmd := &cobra.Command{ - Use: "completion", - Short: "Output shell completion code (default Bash)", + Use: "completion", + Short: "Output shell completion code (default Bash)", + Hidden: true, // Don't show this in help listing. Run: func(cmd *cobra.Command, args []string) { if completionFlags.Zsh { cmd.Root().GenZshCompletion(os.Stdout) diff --git a/pkg/kn/commands/namespaced.go b/pkg/kn/commands/namespaced.go index e37fa3ebf5..414976b170 100644 --- a/pkg/kn/commands/namespaced.go +++ b/pkg/kn/commands/namespaced.go @@ -63,12 +63,17 @@ func (params *KnParams) GetNamespace(cmd *cobra.Command) (string, error) { return namespace, nil } +// CurrentNamespace returns the current namespace which is either provided as option or picked up from kubeconfig func (params *KnParams) CurrentNamespace() (string, error) { + var err error if params.fixedCurrentNamespace != "" { return params.fixedCurrentNamespace, nil } if params.ClientConfig == nil { - params.ClientConfig = params.GetClientConfig() + params.ClientConfig, err = params.GetClientConfig() + if err != nil { + return "", err + } } name, _, err := params.ClientConfig.Namespace() return name, err diff --git a/pkg/kn/commands/plugin/plugin_handler.go b/pkg/kn/commands/plugin/plugin_handler.go index edfc048cec..64b97c84fa 100644 --- a/pkg/kn/commands/plugin/plugin_handler.go +++ b/pkg/kn/commands/plugin/plugin_handler.go @@ -19,8 +19,11 @@ import ( "fmt" "os" "os/exec" + "path/filepath" "strings" "syscall" + + "github.com/knative/client/pkg/kn/commands" ) // PluginHandler is capable of parsing command line arguments @@ -53,14 +56,29 @@ func NewDefaultPluginHandler(validPrefixes []string) *DefaultPluginHandler { // Lookup implements PluginHandler func (h *DefaultPluginHandler) Lookup(filename string) (string, bool) { + var pluginPath string + var err error for _, prefix := range h.ValidPrefixes { - path, err := exec.LookPath(fmt.Sprintf("%s-%s", prefix, filename)) - if err != nil || len(path) == 0 { - continue + pluginPath = fmt.Sprintf("%s-%s", prefix, filename) + if commands.Cfg.LookupPluginsInPath { + pluginPath, err = exec.LookPath(pluginPath) + if err != nil || len(pluginPath) == 0 { + continue + } + } else { + pluginDir, err := ExpandPath(commands.Cfg.PluginsDir) + if err != nil { + return "", false + } + + pluginPath = filepath.Join(pluginDir, pluginPath) + _, err = os.Stat(pluginPath) + if os.IsNotExist(err) { + continue + } } - return path, true + return pluginPath, true } - return "", false } diff --git a/pkg/kn/commands/plugin/plugin_handler_test.go b/pkg/kn/commands/plugin/plugin_handler_test.go index 58f9523c46..eccac1f123 100644 --- a/pkg/kn/commands/plugin/plugin_handler_test.go +++ b/pkg/kn/commands/plugin/plugin_handler_test.go @@ -21,6 +21,7 @@ import ( "path/filepath" "testing" + "github.com/knative/client/pkg/kn/commands" "gotest.tools/assert" ) @@ -51,8 +52,7 @@ func TestPluginHandler(t *testing.T) { pluginPath = CreateTestPluginInPath(t, "kn-"+pluginName, KnTestPluginScript, FileModeExecutable, tmpPathDir) assert.Assert(t, pluginPath != "") - err = os.Setenv("PATH", tmpPathDir) - assert.Assert(t, err == nil) + commands.Cfg.PluginsDir = pluginPath } t.Run("#NewDefaultPluginHandler", func(t *testing.T) { @@ -64,28 +64,38 @@ func TestPluginHandler(t *testing.T) { }) t.Run("#Lookup", func(t *testing.T) { - t.Run("returns the first filepath matching prefix", func(t *testing.T) { - setup(t) - defer cleanup(t) - beforeEach(t) - - path, exists := pluginHandler.Lookup(pluginName) - assert.Assert(t, path != "", fmt.Sprintf("no path when Lookup(%s)", pluginName)) - assert.Assert(t, exists == true, fmt.Sprintf("could not Lookup(%s)", pluginName)) + t.Run("when plugin in pluginsDir", func(t *testing.T) { + t.Run("returns the first filepath matching prefix", func(t *testing.T) { + setup(t) + defer cleanup(t) + beforeEach(t) + + path, exists := pluginHandler.Lookup(pluginName) + assert.Assert(t, path != "", fmt.Sprintf("no path when Lookup(%s)", pluginName)) + assert.Assert(t, exists == true, fmt.Sprintf("could not Lookup(%s)", pluginName)) + }) + + t.Run("returns empty filepath when no matching prefix found", func(t *testing.T) { + setup(t) + defer cleanup(t) + + path, exists := pluginHandler.Lookup("bogus-plugin-name") + assert.Assert(t, path == "", fmt.Sprintf("unexpected plugin: kn-bogus-plugin-name")) + assert.Assert(t, exists == false, fmt.Sprintf("unexpected plugin: kn-bogus-plugin-name")) + }) }) - t.Run("returns empty filepath when no matching prefix found", func(t *testing.T) { - setup(t) - defer cleanup(t) + t.Run("when plugin in $PATH and lookupPluginsInPath true", func(t *testing.T) { + //TODO + }) - path, exists := pluginHandler.Lookup("bogus-plugin-name") - assert.Assert(t, path == "", fmt.Sprintf("unexpected plugin: kn-bogus-plugin-name")) - assert.Assert(t, exists == false, fmt.Sprintf("unexpected plugin: kn-bogus-plugin-name")) + t.Run("when plugin in $PATH and lookupPluginsInPath false", func(t *testing.T) { + //TODO }) }) t.Run("#Execute", func(t *testing.T) { - t.Run("fails while executing the executable with args and environment", func(t *testing.T) { + t.Run("fails executing bogus plugin name", func(t *testing.T) { setup(t) defer cleanup(t) beforeEach(t) diff --git a/pkg/kn/commands/plugin/plugin_list.go b/pkg/kn/commands/plugin/plugin_list.go index 48286b5abb..45f4343f76 100644 --- a/pkg/kn/commands/plugin/plugin_list.go +++ b/pkg/kn/commands/plugin/plugin_list.go @@ -75,6 +75,17 @@ Available plugin files are those that are: return pluginListCommand } +func ExpandPath(path string) (string, error) { + if strings.Contains(path, "~") { + var err error + path, err = expandHomeDir(path) + if err != nil { + return "", err + } + } + return path, nil +} + // Private func (o *PluginFlags) complete(cmd *cobra.Command) error { @@ -83,14 +94,9 @@ func (o *PluginFlags) complete(cmd *cobra.Command) error { SeenPlugins: make(map[string]string, 0), } - pluginPath := commands.Cfg.PluginsDir - - if strings.Contains(pluginPath, "~") { - var err error - pluginPath, err = expandHomeDir(pluginPath) - if err != nil { - return err - } + pluginPath, err := ExpandPath(commands.Cfg.PluginsDir) + if err != nil { + return err } if commands.Cfg.LookupPluginsInPath { diff --git a/pkg/kn/commands/revision/revision_list.go b/pkg/kn/commands/revision/revision_list.go index dd2e0c9201..cb60b525cc 100644 --- a/pkg/kn/commands/revision/revision_list.go +++ b/pkg/kn/commands/revision/revision_list.go @@ -29,7 +29,7 @@ func NewRevisionListCommand(p *commands.KnParams) *cobra.Command { revisionListFlags := NewRevisionListFlags() revisionListCommand := &cobra.Command{ - Use: "list", + Use: "list [name]", Short: "List available revisions.", Long: "List revisions for a given service.", Example: ` @@ -37,7 +37,13 @@ func NewRevisionListCommand(p *commands.KnParams) *cobra.Command { kn revision list # List revisions for a service 'svc1' in namespace 'myapp' - kn revision list -s svc1 -n myapp`, + kn revision list -s svc1 -n myapp + + # List all revisions in JSON output format + kn revision list -o json + + # List revision 'web' + kn revision list web`, RunE: func(cmd *cobra.Command, args []string) error { namespace, err := p.GetNamespace(cmd) if err != nil { @@ -65,7 +71,7 @@ func NewRevisionListCommand(p *commands.KnParams) *cobra.Command { return nil } } else { - revisionList, err = client.ListRevisions() + revisionList, err = getRevisionInfo(args, client) if err != nil { return err } @@ -74,7 +80,6 @@ func NewRevisionListCommand(p *commands.KnParams) *cobra.Command { return nil } } - printer, err := revisionListFlags.ToPrinter() if err != nil { return err @@ -90,3 +95,19 @@ func NewRevisionListCommand(p *commands.KnParams) *cobra.Command { revisionListFlags.AddFlags(revisionListCommand) return revisionListCommand } + +func getRevisionInfo(args []string, client v1alpha12.KnClient) (*v1alpha1.RevisionList, error) { + var ( + revisionList *v1alpha1.RevisionList + err error + ) + switch len(args) { + case 0: + revisionList, err = client.ListRevisions() + case 1: + revisionList, err = client.ListRevisions(v1alpha12.WithName(args[0])) + default: + return nil, fmt.Errorf("'kn revision list' accepts maximum 1 argument") + } + return revisionList, err +} diff --git a/pkg/kn/commands/revision/revision_list_test.go b/pkg/kn/commands/revision/revision_list_test.go index af0b86c2f5..b02a038f90 100644 --- a/pkg/kn/commands/revision/revision_list_test.go +++ b/pkg/kn/commands/revision/revision_list_test.go @@ -61,6 +61,19 @@ func TestRevisionListEmpty(t *testing.T) { } } +func TestRevisionListEmptyByName(t *testing.T) { + action, _, err := fakeRevisionList([]string{"revision", "list", "name"}, &v1alpha1.RevisionList{}) + if err != nil { + t.Error(err) + return + } + if action == nil { + t.Errorf("No action") + } else if !action.Matches("list", "revisions") { + t.Errorf("Bad action %v", action) + } +} + func TestRevisionListDefaultOutput(t *testing.T) { revision1 := createMockRevisionWithParams("foo-abcd", "foo") revision2 := createMockRevisionWithParams("bar-wxyz", "bar") @@ -123,6 +136,29 @@ func TestRevisionListForService(t *testing.T) { assert.Assert(t, util.ContainsAll(output[0], "No", "revisions", "svc3"), "no revisions") } +func TestRevisionListOneOutput(t *testing.T) { + revision := createMockRevisionWithParams("foo-abcd", "foo") + RevisionList := &v1alpha1.RevisionList{Items: []v1alpha1.Revision{*revision}} + action, output, err := fakeRevisionList([]string{"revision", "list", "foo-abcd"}, RevisionList) + if err != nil { + t.Fatal(err) + } + if action == nil { + t.Errorf("No action") + } else if !action.Matches("list", "revisions") { + t.Errorf("Bad action %v", action) + } + + assert.Assert(t, util.ContainsAll(output[0], "NAME", "SERVICE", "AGE", "CONDITIONS", "READY", "REASON")) + assert.Assert(t, util.ContainsAll(output[1], "foo", "foo-abcd")) +} + +func TestRevisionListOutputWithTwoRevName(t *testing.T) { + RevisionList := &v1alpha1.RevisionList{Items: []v1alpha1.Revision{}} + _, _, err := fakeRevisionList([]string{"revision", "list", "foo-abcd", "bar-abcd"}, RevisionList) + assert.ErrorContains(t, err, "'kn revision list' accepts maximum 1 argument") +} + func createMockRevisionWithParams(name, svcName string) *v1alpha1.Revision { revision := &v1alpha1.Revision{ TypeMeta: metav1.TypeMeta{ diff --git a/pkg/kn/commands/route/route.go b/pkg/kn/commands/route/route.go index 707d6dcff1..bb603c14cd 100644 --- a/pkg/kn/commands/route/route.go +++ b/pkg/kn/commands/route/route.go @@ -25,5 +25,6 @@ func NewRouteCommand(p *commands.KnParams) *cobra.Command { Short: "Route command group", } routeCmd.AddCommand(NewRouteListCommand(p)) + routeCmd.AddCommand(NewRouteDescribeCommand(p)) return routeCmd } diff --git a/pkg/kn/commands/route/route_describe.go b/pkg/kn/commands/route/route_describe.go new file mode 100644 index 0000000000..8e68aff67c --- /dev/null +++ b/pkg/kn/commands/route/route_describe.go @@ -0,0 +1,64 @@ +// Copyright ยฉ 2019 The Knative Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package route + +import ( + "errors" + + "github.com/knative/client/pkg/kn/commands" + "github.com/spf13/cobra" + "k8s.io/cli-runtime/pkg/genericclioptions" +) + +// NewRouteDescribeCommand represents 'kn route describe' command +func NewRouteDescribeCommand(p *commands.KnParams) *cobra.Command { + routeDescribePrintFlags := genericclioptions.NewPrintFlags("").WithDefaultOutput("yaml") + routeDescribeCommand := &cobra.Command{ + Use: "describe NAME", + Short: "Describe available route.", + RunE: func(cmd *cobra.Command, args []string) error { + if len(args) < 1 { + return errors.New("requires the route name.") + } + namespace, err := p.GetNamespace(cmd) + if err != nil { + return err + } + + client, err := p.NewClient(namespace) + if err != nil { + return err + } + + describeRoute, err := client.GetRoute(args[0]) + if err != nil { + return err + } + + printer, err := routeDescribePrintFlags.ToPrinter() + if err != nil { + return err + } + err = printer.PrintObj(describeRoute, cmd.OutOrStdout()) + if err != nil { + return err + } + return nil + }, + } + commands.AddNamespaceFlags(routeDescribeCommand.Flags(), false) + routeDescribePrintFlags.AddFlags(routeDescribeCommand) + return routeDescribeCommand +} diff --git a/pkg/kn/commands/route/route_describe_test.go b/pkg/kn/commands/route/route_describe_test.go new file mode 100644 index 0000000000..efa853ac34 --- /dev/null +++ b/pkg/kn/commands/route/route_describe_test.go @@ -0,0 +1,130 @@ +// Copyright ยฉ 2019 The Knative Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package route + +import ( + "encoding/json" + "strings" + "testing" + + "github.com/knative/client/pkg/kn/commands" + "github.com/knative/serving/pkg/apis/serving/v1alpha1" + "gotest.tools/assert" + "k8s.io/apimachinery/pkg/api/equality" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + client_testing "k8s.io/client-go/testing" + "sigs.k8s.io/yaml" +) + +func fakeRouteDescribe(args []string, response *v1alpha1.Route) (action client_testing.Action, output string, err error) { + knParams := &commands.KnParams{} + cmd, fakeRoute, buf := commands.CreateTestKnCommand(NewRouteCommand(knParams), knParams) + fakeRoute.AddReactor("*", "*", + func(a client_testing.Action) (bool, runtime.Object, error) { + action = a + return true, response, nil + }) + cmd.SetArgs(args) + err = cmd.Execute() + if err != nil { + return + } + output = buf.String() + return +} + +func TestCompletion(t *testing.T) { + var expectedRoute v1alpha1.Route + + setup := func(t *testing.T) { + expectedRoute = v1alpha1.Route{ + TypeMeta: metav1.TypeMeta{ + Kind: "Route", + APIVersion: "knative.dev/v1alpha1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "default", + }, + } + } + + t.Run("requires the route name", func(t *testing.T) { + _, _, err := fakeRouteDescribe([]string{"route", "describe"}, &v1alpha1.Route{}) + assert.Assert(t, err != nil) + assert.Assert(t, strings.Contains(err.Error(), "requires the route name.")) + }) + + t.Run("describe a valid route with default output", func(t *testing.T) { + setup(t) + + action, output, err := fakeRouteDescribe([]string{"route", "describe", "foo"}, &expectedRoute) + assert.Assert(t, err == nil) + assert.Assert(t, action != nil) + assert.Assert(t, action.Matches("get", "routes")) + + jsonData, err := yaml.YAMLToJSON([]byte(output)) + assert.Assert(t, err == nil) + + var returnedRoute v1alpha1.Route + err = json.Unmarshal(jsonData, &returnedRoute) + assert.Assert(t, err == nil) + assert.Assert(t, equality.Semantic.DeepEqual(expectedRoute, returnedRoute)) + }) + + t.Run("describe a valid route with special output", func(t *testing.T) { + t.Run("yaml", func(t *testing.T) { + setup(t) + + action, output, err := fakeRouteDescribe([]string{"route", "describe", "foo", "-oyaml"}, &expectedRoute) + assert.Assert(t, err == nil) + assert.Assert(t, action != nil) + assert.Assert(t, action.Matches("get", "routes")) + + jsonData, err := yaml.YAMLToJSON([]byte(output)) + assert.Assert(t, err == nil) + + var returnedRoute v1alpha1.Route + err = json.Unmarshal(jsonData, &returnedRoute) + assert.Assert(t, err == nil) + assert.Assert(t, equality.Semantic.DeepEqual(expectedRoute, returnedRoute)) + }) + + t.Run("json", func(t *testing.T) { + setup(t) + + action, output, err := fakeRouteDescribe([]string{"route", "describe", "foo", "-ojson"}, &expectedRoute) + assert.Assert(t, err == nil) + assert.Assert(t, action != nil) + assert.Assert(t, action.Matches("get", "routes")) + + var returnedRoute v1alpha1.Route + err = json.Unmarshal([]byte(output), &returnedRoute) + assert.Assert(t, err == nil) + assert.Assert(t, equality.Semantic.DeepEqual(expectedRoute, returnedRoute)) + }) + + t.Run("name", func(t *testing.T) { + setup(t) + + action, output, err := fakeRouteDescribe([]string{"route", "describe", "foo", "-oname"}, &expectedRoute) + assert.Assert(t, err == nil) + assert.Assert(t, action != nil) + assert.Assert(t, action.Matches("get", "routes")) + assert.Assert(t, strings.Contains(output, expectedRoute.Name)) + }) + }) +} diff --git a/pkg/kn/commands/service/configuration_edit_flags.go b/pkg/kn/commands/service/configuration_edit_flags.go index a80072e40e..e92f7f9838 100644 --- a/pkg/kn/commands/service/configuration_edit_flags.go +++ b/pkg/kn/commands/service/configuration_edit_flags.go @@ -66,7 +66,7 @@ func (p *ConfigurationEditFlags) AddCreateFlags(command *cobra.Command) { func (p *ConfigurationEditFlags) Apply(service *servingv1alpha1.Service, cmd *cobra.Command) error { - template, err := servinglib.GetRevisionTemplate(service) + template, err := servinglib.RevisionTemplateOfService(service) if err != nil { return err } diff --git a/pkg/kn/commands/service/human_readable_flags.go b/pkg/kn/commands/service/human_readable_flags.go index e6ed6b9731..4bf8be582b 100644 --- a/pkg/kn/commands/service/human_readable_flags.go +++ b/pkg/kn/commands/service/human_readable_flags.go @@ -25,8 +25,8 @@ import ( // ServiceListHandlers adds print handlers for service list command func ServiceListHandlers(h hprinters.PrintHandler) { kServiceColumnDefinitions := []metav1beta1.TableColumnDefinition{ - {Name: "Name", Type: "string", Description: "Name of the knative service."}, - {Name: "Domain", Type: "string", Description: "Domain name of the knative service."}, + {Name: "Name", Type: "string", Description: "Name of the Knative service."}, + {Name: "Url", Type: "string", Description: "URL of the Knative service."}, //{Name: "LastCreatedRevision", Type: "string", Description: "Name of last revision created."}, //{Name: "LastReadyRevision", Type: "string", Description: "Name of last ready revision."}, {Name: "Generation", Type: "integer", Description: "Sequence number of 'Generation' of the service that was last processed by the controller."}, @@ -57,7 +57,7 @@ func printKServiceList(kServiceList *servingv1alpha1.ServiceList, options hprint // printKService populates the knative service table rows func printKService(kService *servingv1alpha1.Service, options hprinters.PrintOptions) ([]metav1beta1.TableRow, error) { name := kService.Name - domain := kService.Status.RouteStatusFields.DeprecatedDomain + url := kService.Status.URL //lastCreatedRevision := kService.Status.LatestCreatedRevisionName //lastReadyRevision := kService.Status.LatestReadyRevisionName generation := kService.Status.ObservedGeneration @@ -71,7 +71,7 @@ func printKService(kService *servingv1alpha1.Service, options hprinters.PrintOpt } row.Cells = append(row.Cells, name, - domain, + url, //lastCreatedRevision, //lastReadyRevision, generation, diff --git a/pkg/kn/commands/service/service.go b/pkg/kn/commands/service/service.go index 7a4ff8ef2a..5c3adb10c1 100644 --- a/pkg/kn/commands/service/service.go +++ b/pkg/kn/commands/service/service.go @@ -15,10 +15,22 @@ package service import ( + "io" + "time" + "github.com/knative/client/pkg/kn/commands" + serving_kn_v1alpha1 "github.com/knative/client/pkg/serving/v1alpha1" + + "fmt" + "github.com/spf13/cobra" ) +const ( + // How often to retry in case of an optimistic lock error when replacing a service (--force) + MaxUpdateRetries = 3 +) + func NewServiceCommand(p *commands.KnParams) *cobra.Command { serviceCmd := &cobra.Command{ Use: "service", @@ -31,3 +43,16 @@ func NewServiceCommand(p *commands.KnParams) *cobra.Command { serviceCmd.AddCommand(NewServiceUpdateCommand(p)) return serviceCmd } + +func waitForService(client serving_kn_v1alpha1.KnClient, serviceName string, out io.Writer, timeout int) error { + fmt.Fprintf(out, "Waiting for service '%s' to become ready ... ", serviceName) + flush(out) + + err := client.WaitForService(serviceName, time.Duration(timeout)*time.Second) + if err != nil { + fmt.Fprintln(out) + return err + } + fmt.Fprintln(out, "OK") + return nil +} diff --git a/pkg/kn/commands/service/service_create.go b/pkg/kn/commands/service/service_create.go index dcf5b48431..6d7f575792 100644 --- a/pkg/kn/commands/service/service_create.go +++ b/pkg/kn/commands/service/service_create.go @@ -18,7 +18,6 @@ import ( "errors" "fmt" "io" - "time" "github.com/knative/client/pkg/kn/commands" "github.com/knative/client/pkg/serving/v1alpha1" @@ -105,16 +104,11 @@ func NewServiceCreateCommand(p *commands.KnParams) *cobra.Command { if !waitFlags.Async { out := cmd.OutOrStdout() - fmt.Fprintf(out, "Waiting for service '%s' to become ready ... ", name) - flush(out) - - err := client.WaitForService(name, time.Duration(waitFlags.TimeoutInSeconds)*time.Second) + err := waitForService(client, name, out, waitFlags.TimeoutInSeconds) if err != nil { - fmt.Fprintln(out) return err } - fmt.Fprintln(out, "OK") - return showUrl(client, name, namespace, cmd.OutOrStdout()) + return showUrl(client, name, namespace, out) } return nil @@ -122,7 +116,7 @@ func NewServiceCreateCommand(p *commands.KnParams) *cobra.Command { } commands.AddNamespaceFlags(serviceCreateCommand.Flags(), false) editFlags.AddCreateFlags(serviceCreateCommand) - waitFlags.AddConditionWaitFlags(serviceCreateCommand, 60, "service") + waitFlags.AddConditionWaitFlags(serviceCreateCommand, 60, "Create", "service") return serviceCreateCommand } @@ -147,17 +141,25 @@ func createService(client v1alpha1.KnClient, service *serving_v1alpha1_api.Servi } func replaceService(client v1alpha1.KnClient, service *serving_v1alpha1_api.Service, namespace string, out io.Writer) error { - existingService, err := client.GetService(service.Name) - if err != nil { - return err - } - service.ResourceVersion = existingService.ResourceVersion - err = client.UpdateService(service) - if err != nil { - return err + var retries = 0 + for { + existingService, err := client.GetService(service.Name) + if err != nil { + return err + } + service.ResourceVersion = existingService.ResourceVersion + err = client.UpdateService(service) + if err != nil { + // Retry to update when a resource version conflict exists + if api_errors.IsConflict(err) && retries < MaxUpdateRetries { + retries++ + continue + } + return err + } + fmt.Fprintf(out, "Service '%s' successfully replaced in namespace '%s'.\n", service.Name, namespace) + return nil } - fmt.Fprintf(out, "Service '%s' successfully replaced in namespace '%s'.\n", service.Name, namespace) - return nil } func serviceExists(client v1alpha1.KnClient, name string, namespace string) (bool, error) { diff --git a/pkg/kn/commands/service/service_create_test.go b/pkg/kn/commands/service/service_create_test.go index b9674513e3..8bafc37906 100644 --- a/pkg/kn/commands/service/service_create_test.go +++ b/pkg/kn/commands/service/service_create_test.go @@ -121,7 +121,7 @@ func TestServiceCreateImage(t *testing.T) { } else if !action.Matches("create", "services") { t.Fatalf("Bad action %v", action) } - template, err := servinglib.GetRevisionTemplate(created) + template, err := servinglib.RevisionTemplateOfService(created) if err != nil { t.Fatal(err) } else if template.Spec.DeprecatedContainer.Image != "gcr.io/foo/bar:baz" { @@ -140,7 +140,7 @@ func TestServiceCreateImageSync(t *testing.T) { } else if !action.Matches("create", "services") { t.Fatalf("Bad action %v", action) } - template, err := servinglib.GetRevisionTemplate(created) + template, err := servinglib.RevisionTemplateOfService(created) if err != nil { t.Fatal(err) } @@ -170,7 +170,7 @@ func TestServiceCreateEnv(t *testing.T) { "A": "DOGS", "B": "WOLVES"} - template, err := servinglib.GetRevisionTemplate(created) + template, err := servinglib.RevisionTemplateOfService(created) actualEnvVars, err := servinglib.EnvToMap(template.Spec.DeprecatedContainer.Env) if err != nil { t.Fatal(err) @@ -202,7 +202,7 @@ func TestServiceCreateWithRequests(t *testing.T) { corev1.ResourceMemory: parseQuantity(t, "64Mi"), } - template, err := servinglib.GetRevisionTemplate(created) + template, err := servinglib.RevisionTemplateOfService(created) if err != nil { t.Fatal(err) @@ -228,7 +228,7 @@ func TestServiceCreateWithLimits(t *testing.T) { corev1.ResourceMemory: parseQuantity(t, "1024Mi"), } - template, err := servinglib.GetRevisionTemplate(created) + template, err := servinglib.RevisionTemplateOfService(created) if err != nil { t.Fatal(err) @@ -257,7 +257,7 @@ func TestServiceCreateRequestsLimitsCPU(t *testing.T) { corev1.ResourceCPU: parseQuantity(t, "1000m"), } - template, err := servinglib.GetRevisionTemplate(created) + template, err := servinglib.RevisionTemplateOfService(created) if err != nil { t.Fatal(err) @@ -294,7 +294,7 @@ func TestServiceCreateRequestsLimitsMemory(t *testing.T) { corev1.ResourceMemory: parseQuantity(t, "1024Mi"), } - template, err := servinglib.GetRevisionTemplate(created) + template, err := servinglib.RevisionTemplateOfService(created) if err != nil { t.Fatal(err) @@ -324,7 +324,7 @@ func TestServiceCreateMaxMinScale(t *testing.T) { t.Fatalf("Bad action %v", action) } - template, err := servinglib.GetRevisionTemplate(created) + template, err := servinglib.RevisionTemplateOfService(created) if err != nil { t.Fatal(err) } @@ -371,7 +371,7 @@ func TestServiceCreateRequestsLimitsCPUMemory(t *testing.T) { corev1.ResourceMemory: parseQuantity(t, "1024Mi"), } - template, err := servinglib.GetRevisionTemplate(created) + template, err := servinglib.RevisionTemplateOfService(created) if err != nil { t.Fatal(err) @@ -421,7 +421,7 @@ func TestServiceCreateImageForce(t *testing.T) { } else if !action.Matches("update", "services") { t.Fatalf("Bad action %v", action) } - template, err := servinglib.GetRevisionTemplate(created) + template, err := servinglib.RevisionTemplateOfService(created) if err != nil { t.Fatal(err) } else if template.Spec.DeprecatedContainer.Image != "gcr.io/foo/bar:v2" { @@ -450,7 +450,7 @@ func TestServiceCreateEnvForce(t *testing.T) { "A": "CATS", "B": "LIONS"} - template, err := servinglib.GetRevisionTemplate(created) + template, err := servinglib.RevisionTemplateOfService(created) actualEnvVars, err := servinglib.EnvToMap(template.Spec.DeprecatedContainer.Env) if err != nil { t.Fatal(err) diff --git a/pkg/kn/commands/service/service_list.go b/pkg/kn/commands/service/service_list.go index 9930277904..b3498c705a 100644 --- a/pkg/kn/commands/service/service_list.go +++ b/pkg/kn/commands/service/service_list.go @@ -18,6 +18,8 @@ import ( "fmt" "github.com/knative/client/pkg/kn/commands" + v1alpha12 "github.com/knative/client/pkg/serving/v1alpha1" + "github.com/knative/serving/pkg/apis/serving/v1alpha1" "github.com/spf13/cobra" ) @@ -26,8 +28,17 @@ func NewServiceListCommand(p *commands.KnParams) *cobra.Command { serviceListFlags := NewServiceListFlags() serviceListCommand := &cobra.Command{ - Use: "list", + Use: "list [name]", Short: "List available services.", + Example: ` + # List all services + kn service list + + # List all services in JSON output format + kn service list -o json + + # List service 'web' + kn service list web`, RunE: func(cmd *cobra.Command, args []string) error { namespace, err := p.GetNamespace(cmd) if err != nil { @@ -37,7 +48,7 @@ func NewServiceListCommand(p *commands.KnParams) *cobra.Command { if err != nil { return err } - serviceList, err := client.ListServices() + serviceList, err := getServiceInfo(args, client) if err != nil { return err } @@ -61,3 +72,19 @@ func NewServiceListCommand(p *commands.KnParams) *cobra.Command { serviceListFlags.AddFlags(serviceListCommand) return serviceListCommand } + +func getServiceInfo(args []string, client v1alpha12.KnClient) (*v1alpha1.ServiceList, error) { + var ( + serviceList *v1alpha1.ServiceList + err error + ) + switch len(args) { + case 0: + serviceList, err = client.ListServices() + case 1: + serviceList, err = client.ListServices(v1alpha12.WithName(args[0])) + default: + return nil, fmt.Errorf("'kn service list' accepts maximum 1 argument") + } + return serviceList, err +} diff --git a/pkg/kn/commands/service/service_list_test.go b/pkg/kn/commands/service/service_list_test.go index ec3feede32..990bddd8fe 100644 --- a/pkg/kn/commands/service/service_list_test.go +++ b/pkg/kn/commands/service/service_list_test.go @@ -18,14 +18,16 @@ import ( "strings" "testing" - "github.com/knative/client/pkg/kn/commands" - "github.com/knative/client/pkg/util" + "github.com/knative/pkg/apis" duckv1beta1 "github.com/knative/pkg/apis/duck/v1beta1" "github.com/knative/serving/pkg/apis/serving/v1alpha1" "gotest.tools/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" client_testing "k8s.io/client-go/testing" + + "github.com/knative/client/pkg/kn/commands" + "github.com/knative/client/pkg/util" ) func fakeServiceList(args []string, response *v1alpha1.ServiceList) (action client_testing.Action, output []string, err error) { @@ -60,9 +62,22 @@ func TestListEmpty(t *testing.T) { } } +func TestGetEmpty(t *testing.T) { + action, _, err := fakeServiceList([]string{"service", "list", "name"}, &v1alpha1.ServiceList{}) + if err != nil { + t.Error(err) + return + } + if action == nil { + t.Errorf("No action") + } else if !action.Matches("list", "services") { + t.Errorf("Bad action %v", action) + } +} + func TestServiceListDefaultOutput(t *testing.T) { - service1 := createMockServiceWithParams("foo", "foo.default.example.com", 1) - service2 := createMockServiceWithParams("bar", "bar.default.example.com", 2) + service1 := createMockServiceWithParams("foo", "http://foo.default.example.com", 1) + service2 := createMockServiceWithParams("bar", "http://bar.default.example.com", 2) serviceList := &v1alpha1.ServiceList{Items: []v1alpha1.Service{*service1, *service2}} action, output, err := fakeServiceList([]string{"service", "list"}, serviceList) if err != nil { @@ -73,12 +88,36 @@ func TestServiceListDefaultOutput(t *testing.T) { } else if !action.Matches("list", "services") { t.Errorf("Bad action %v", action) } - assert.Check(t, util.ContainsAll(output[0], "NAME", "DOMAIN", "GENERATION", "AGE", "CONDITIONS", "READY", "REASON")) + assert.Check(t, util.ContainsAll(output[0], "NAME", "URL", "GENERATION", "AGE", "CONDITIONS", "READY", "REASON")) assert.Check(t, util.ContainsAll(output[1], "foo", "foo.default.example.com", "1")) assert.Check(t, util.ContainsAll(output[2], "bar", "bar.default.example.com", "2")) } -func createMockServiceWithParams(name, domain string, generation int64) *v1alpha1.Service { +func TestServiceGetOneOutput(t *testing.T) { + service := createMockServiceWithParams("foo", "foo.default.example.com", 1) + serviceList := &v1alpha1.ServiceList{Items: []v1alpha1.Service{*service}} + action, output, err := fakeServiceList([]string{"service", "list", "foo"}, serviceList) + if err != nil { + t.Fatal(err) + } + if action == nil { + t.Errorf("No action") + } else if !action.Matches("list", "services") { + t.Errorf("Bad action %v", action) + } + assert.Check(t, util.ContainsAll(output[0], "NAME", "URL", "GENERATION", "AGE", "CONDITIONS", "READY", "REASON")) + assert.Check(t, util.ContainsAll(output[1], "foo", "foo.default.example.com", "1")) +} + +func TestServiceGetWithTwoSrvName(t *testing.T) { + service := createMockServiceWithParams("foo", "foo.default.example.com", 1) + serviceList := &v1alpha1.ServiceList{Items: []v1alpha1.Service{*service}} + _, _, err := fakeServiceList([]string{"service", "list", "foo", "bar"}, serviceList) + assert.ErrorContains(t, err, "'kn service list' accepts maximum 1 argument") +} + +func createMockServiceWithParams(name, urlS string, generation int64) *v1alpha1.Service { + url, _ := apis.ParseURL(urlS) service := &v1alpha1.Service{ TypeMeta: metav1.TypeMeta{ Kind: "Service", @@ -95,7 +134,7 @@ func createMockServiceWithParams(name, domain string, generation int64) *v1alpha Status: duckv1beta1.Status{ ObservedGeneration: generation}, RouteStatusFields: v1alpha1.RouteStatusFields{ - DeprecatedDomain: domain, + URL: url, }, }, } diff --git a/pkg/kn/commands/service/service_update.go b/pkg/kn/commands/service/service_update.go index da4711b98b..39260977ef 100644 --- a/pkg/kn/commands/service/service_update.go +++ b/pkg/kn/commands/service/service_update.go @@ -18,12 +18,15 @@ import ( "errors" "fmt" - "github.com/knative/client/pkg/kn/commands" "github.com/spf13/cobra" + api_errors "k8s.io/apimachinery/pkg/api/errors" + + "github.com/knative/client/pkg/kn/commands" ) func NewServiceUpdateCommand(p *commands.KnParams) *cobra.Command { var editFlags ConfigurationEditFlags + var waitFlags commands.WaitFlags serviceUpdateCommand := &cobra.Command{ Use: "update NAME", @@ -52,28 +55,46 @@ func NewServiceUpdateCommand(p *commands.KnParams) *cobra.Command { return err } - service, err := client.GetService(args[0]) - if err != nil { - return err - } - service = service.DeepCopy() + var retries = 0 + for { + name := args[0] + service, err := client.GetService(name) + if err != nil { + return err + } + service = service.DeepCopy() - err = editFlags.Apply(service, cmd) - if err != nil { - return err - } + err = editFlags.Apply(service, cmd) + if err != nil { + return err + } - err = client.UpdateService(service) - if err != nil { - return err - } + err = client.UpdateService(service) + if err != nil { + // Retry to update when a resource version conflict exists + if api_errors.IsConflict(err) && retries < MaxUpdateRetries { + retries++ + continue + } + return err + } + + if !waitFlags.Async { + out := cmd.OutOrStdout() + err := waitForService(client, name, out, waitFlags.TimeoutInSeconds) + if err != nil { + return err + } + } - fmt.Fprintf(cmd.OutOrStdout(), "Service '%s' updated in namespace '%s'.\n", args[0], namespace) - return nil + fmt.Fprintf(cmd.OutOrStdout(), "Service '%s' updated in namespace '%s'.\n", args[0], namespace) + return nil + } }, } commands.AddNamespaceFlags(serviceUpdateCommand.Flags(), false) editFlags.AddUpdateFlags(serviceUpdateCommand) + waitFlags.AddConditionWaitFlags(serviceUpdateCommand, 60, "Update", "service") return serviceUpdateCommand } diff --git a/pkg/kn/commands/service/service_update_test.go b/pkg/kn/commands/service/service_update_test.go index 4902a2007a..76aed4bae2 100644 --- a/pkg/kn/commands/service/service_update_test.go +++ b/pkg/kn/commands/service/service_update_test.go @@ -21,17 +21,23 @@ import ( "strings" "testing" + "gotest.tools/assert" + "github.com/knative/client/pkg/kn/commands" servinglib "github.com/knative/client/pkg/serving" + "github.com/knative/client/pkg/util" + "github.com/knative/client/pkg/wait" + "github.com/knative/serving/pkg/apis/serving/v1alpha1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/watch" client_testing "k8s.io/client-go/testing" ) -func fakeServiceUpdate(original *v1alpha1.Service, args []string) ( +func fakeServiceUpdate(original *v1alpha1.Service, args []string, sync bool) ( action client_testing.Action, updated *v1alpha1.Service, output string, @@ -55,6 +61,24 @@ func fakeServiceUpdate(original *v1alpha1.Service, args []string) ( func(a client_testing.Action) (bool, runtime.Object, error) { return true, original, nil }) + if sync { + fakeServing.AddWatchReactor("services", + func(a client_testing.Action) (bool, watch.Interface, error) { + watchAction := a.(client_testing.WatchAction) + _, found := watchAction.GetWatchRestrictions().Fields.RequiresExactMatch("metadata.name") + if !found { + return true, nil, errors.New("no field selector on metadata.name found") + } + w := wait.NewFakeWatch(getServiceEvents("test-service")) + w.Start() + return true, w, nil + }) + fakeServing.AddReactor("get", "services", + func(a client_testing.Action) (bool, runtime.Object, error) { + return true, &v1alpha1.Service{}, nil + }) + } + cmd.SetArgs(args) err = cmd.Execute() if err != nil { @@ -64,10 +88,33 @@ func fakeServiceUpdate(original *v1alpha1.Service, args []string) ( return } +func TestServiceUpdateImageSync(t *testing.T) { + orig := newEmptyService() + + template, err := servinglib.RevisionTemplateOfService(orig) + if err != nil { + t.Fatal(err) + } + + servinglib.UpdateImage(template, "gcr.io/foo/bar:baz") + + action, updated, output, err := fakeServiceUpdate(orig, []string{ + "service", "update", "foo", "--image", "gcr.io/foo/quux:xyzzy", "--namespace", "bar"}, true) + + assert.NilError(t, err) + assert.Assert(t, action.Matches("update", "services")) + + template, err = servinglib.RevisionTemplateOfService(updated) + assert.NilError(t, err) + + assert.Equal(t, template.Spec.DeprecatedContainer.Image, "gcr.io/foo/quux:xyzzy") + assert.Assert(t, util.ContainsAll(strings.ToLower(output), "update", "foo", "service", "namespace", "bar", "ok", "waiting")) +} + func TestServiceUpdateImage(t *testing.T) { orig := newEmptyService() - template, err := servinglib.GetRevisionTemplate(orig) + template, err := servinglib.RevisionTemplateOfService(orig) if err != nil { t.Fatal(err) } @@ -75,7 +122,7 @@ func TestServiceUpdateImage(t *testing.T) { servinglib.UpdateImage(template, "gcr.io/foo/bar:baz") action, updated, output, err := fakeServiceUpdate(orig, []string{ - "service", "update", "foo", "--image", "gcr.io/foo/quux:xyzzy", "--namespace", "bar"}) + "service", "update", "foo", "--image", "gcr.io/foo/quux:xyzzy", "--namespace", "bar", "--async"}, false) if err != nil { t.Fatal(err) @@ -83,7 +130,7 @@ func TestServiceUpdateImage(t *testing.T) { t.Fatalf("Bad action %v", action) } - template, err = servinglib.GetRevisionTemplate(updated) + template, err = servinglib.RevisionTemplateOfService(updated) if err != nil { t.Fatal(err) } else if template.Spec.DeprecatedContainer.Image != "gcr.io/foo/quux:xyzzy" { @@ -104,7 +151,7 @@ func TestServiceUpdateMaxMinScale(t *testing.T) { action, updated, _, err := fakeServiceUpdate(original, []string{ "service", "update", "foo", - "--min-scale", "1", "--max-scale", "5", "--concurrency-target", "10", "--concurrency-limit", "100"}) + "--min-scale", "1", "--max-scale", "5", "--concurrency-target", "10", "--concurrency-limit", "100", "--async"}, false) if err != nil { t.Fatal(err) @@ -112,7 +159,7 @@ func TestServiceUpdateMaxMinScale(t *testing.T) { t.Fatalf("Bad action %v", action) } - template, err := servinglib.GetRevisionTemplate(updated) + template, err := servinglib.RevisionTemplateOfService(updated) if err != nil { t.Fatal(err) } @@ -161,7 +208,7 @@ func TestServiceUpdateEnv(t *testing.T) { }, } - template, err := servinglib.GetRevisionTemplate(orig) + template, err := servinglib.RevisionTemplateOfService(orig) if err != nil { t.Fatal(err) } @@ -169,7 +216,7 @@ func TestServiceUpdateEnv(t *testing.T) { servinglib.UpdateImage(template, "gcr.io/foo/bar:baz") action, updated, _, err := fakeServiceUpdate(orig, []string{ - "service", "update", "foo", "-e", "TARGET=Awesome"}) + "service", "update", "foo", "-e", "TARGET=Awesome", "--async"}, false) if err != nil { t.Fatal(err) @@ -181,7 +228,7 @@ func TestServiceUpdateEnv(t *testing.T) { Value: "Awesome", } - template, err = servinglib.GetRevisionTemplate(updated) + template, err = servinglib.RevisionTemplateOfService(updated) if err != nil { t.Fatal(err) } else if template.Spec.DeprecatedContainer.Image != "gcr.io/foo/bar:baz" { @@ -195,7 +242,7 @@ func TestServiceUpdateRequestsLimitsCPU(t *testing.T) { service := createMockServiceWithResources(t, "250", "64Mi", "1000m", "1024Mi") action, updated, _, err := fakeServiceUpdate(service, []string{ - "service", "update", "foo", "--requests-cpu", "500m", "--limits-cpu", "1000m"}) + "service", "update", "foo", "--requests-cpu", "500m", "--limits-cpu", "1000m", "--async"}, false) if err != nil { t.Fatal(err) } else if !action.Matches("update", "services") { @@ -211,7 +258,7 @@ func TestServiceUpdateRequestsLimitsCPU(t *testing.T) { corev1.ResourceMemory: resource.MustParse("1024Mi"), } - newTemplate, err := servinglib.GetRevisionTemplate(updated) + newTemplate, err := servinglib.RevisionTemplateOfService(updated) if err != nil { t.Fatal(err) } else { @@ -233,7 +280,7 @@ func TestServiceUpdateRequestsLimitsMemory(t *testing.T) { service := createMockServiceWithResources(t, "100m", "64Mi", "1000m", "1024Mi") action, updated, _, err := fakeServiceUpdate(service, []string{ - "service", "update", "foo", "--requests-memory", "128Mi", "--limits-memory", "2048Mi"}) + "service", "update", "foo", "--requests-memory", "128Mi", "--limits-memory", "2048Mi", "--async"}, false) if err != nil { t.Fatal(err) } else if !action.Matches("update", "services") { @@ -249,7 +296,7 @@ func TestServiceUpdateRequestsLimitsMemory(t *testing.T) { corev1.ResourceMemory: resource.MustParse("2048Mi"), } - newTemplate, err := servinglib.GetRevisionTemplate(updated) + newTemplate, err := servinglib.RevisionTemplateOfService(updated) if err != nil { t.Fatal(err) } else { @@ -273,7 +320,7 @@ func TestServiceUpdateRequestsLimitsCPU_and_Memory(t *testing.T) { action, updated, _, err := fakeServiceUpdate(service, []string{ "service", "update", "foo", "--requests-cpu", "500m", "--limits-cpu", "2000m", - "--requests-memory", "128Mi", "--limits-memory", "2048Mi"}) + "--requests-memory", "128Mi", "--limits-memory", "2048Mi", "--async"}, false) if err != nil { t.Fatal(err) } else if !action.Matches("update", "services") { @@ -289,7 +336,7 @@ func TestServiceUpdateRequestsLimitsCPU_and_Memory(t *testing.T) { corev1.ResourceMemory: resource.MustParse("2048Mi"), } - newTemplate, err := servinglib.GetRevisionTemplate(updated) + newTemplate, err := servinglib.RevisionTemplateOfService(updated) if err != nil { t.Fatal(err) } else { @@ -354,7 +401,7 @@ func createMockServiceWithResources(t *testing.T, requestCPU, requestMemory, lim }, } - template, err := servinglib.GetRevisionTemplate(service) + template, err := servinglib.RevisionTemplateOfService(service) if err != nil { t.Fatal(err) } diff --git a/pkg/kn/commands/service/wait_args.go b/pkg/kn/commands/service/wait_args.go deleted file mode 100644 index 8d985c26b6..0000000000 --- a/pkg/kn/commands/service/wait_args.go +++ /dev/null @@ -1,43 +0,0 @@ -// Copyright ยฉ 2019 The Knative Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package service - -import ( - "fmt" - - "github.com/knative/client/pkg/wait" - "github.com/knative/pkg/apis" - serving_v1alpha1_api "github.com/knative/serving/pkg/apis/serving/v1alpha1" - serving_v1alpha1_client "github.com/knative/serving/pkg/client/clientset/versioned/typed/serving/v1alpha1" - "k8s.io/apimachinery/pkg/runtime" -) - -// Create wait arguments for a Knative service which can be used to wait for -// a create/update options to be finished -// Can be used by `service_create` and `service_update`, hence this extra file -func newServiceWaitForReady(client serving_v1alpha1_client.ServingV1alpha1Interface, namespace string) wait.WaitForReady { - return wait.NewWaitForReady( - "service", - client.Services(namespace).Watch, - serviceConditionExtractor) -} - -func serviceConditionExtractor(obj runtime.Object) (apis.Conditions, error) { - service, ok := obj.(*serving_v1alpha1_api.Service) - if !ok { - return nil, fmt.Errorf("%v is not a service", obj) - } - return apis.Conditions(service.Status.Conditions), nil -} diff --git a/pkg/kn/commands/types.go b/pkg/kn/commands/types.go index e2b51dd775..d54ce335da 100644 --- a/pkg/kn/commands/types.go +++ b/pkg/kn/commands/types.go @@ -15,7 +15,11 @@ package commands import ( + "errors" + "fmt" "io" + "os" + "path/filepath" serving_kn_v1alpha1 "github.com/knative/client/pkg/serving/v1alpha1" serving_v1alpha1_client "github.com/knative/serving/pkg/client/clientset/versioned/typed/serving/v1alpha1" @@ -59,11 +63,17 @@ func (params *KnParams) newClient(namespace string) (serving_kn_v1alpha1.KnClien return serving_kn_v1alpha1.NewKnServingClient(client, namespace), nil } +// GetConfig returns Serving Client func (params *KnParams) GetConfig() (serving_v1alpha1_client.ServingV1alpha1Interface, error) { + var err error + if params.ClientConfig == nil { - params.ClientConfig = params.GetClientConfig() + params.ClientConfig, err = params.GetClientConfig() + if err != nil { + return nil, err + } } - var err error + config, err := params.ClientConfig.ClientConfig() if err != nil { return nil, err @@ -71,10 +81,28 @@ func (params *KnParams) GetConfig() (serving_v1alpha1_client.ServingV1alpha1Inte return serving_v1alpha1_client.NewForConfig(config) } -func (params *KnParams) GetClientConfig() clientcmd.ClientConfig { +// GetClientConfig gets ClientConfig from KubeCfgPath +func (params *KnParams) GetClientConfig() (clientcmd.ClientConfig, error) { loadingRules := clientcmd.NewDefaultClientConfigLoadingRules() - if len(params.KubeCfgPath) > 0 { + if len(params.KubeCfgPath) == 0 { + return clientcmd.NewNonInteractiveDeferredLoadingClientConfig(loadingRules, &clientcmd.ConfigOverrides{}), nil + } + + _, err := os.Stat(params.KubeCfgPath) + if err == nil { loadingRules.ExplicitPath = params.KubeCfgPath + return clientcmd.NewNonInteractiveDeferredLoadingClientConfig(loadingRules, &clientcmd.ConfigOverrides{}), nil + } + + if !os.IsNotExist(err) { + return nil, err + } + + paths := filepath.SplitList(params.KubeCfgPath) + if len(paths) > 1 { + return nil, errors.New(fmt.Sprintf("Can not find config file. '%s' looks like a path. "+ + "Please use the env var KUBECONFIG if you want to check for multiple configuration files", params.KubeCfgPath)) + } else { + return nil, errors.New(fmt.Sprintf("Config file '%s' can not be found", params.KubeCfgPath)) } - return clientcmd.NewNonInteractiveDeferredLoadingClientConfig(loadingRules, &clientcmd.ConfigOverrides{}) } diff --git a/pkg/kn/commands/types_test.go b/pkg/kn/commands/types_test.go new file mode 100644 index 0000000000..2ea70296e8 --- /dev/null +++ b/pkg/kn/commands/types_test.go @@ -0,0 +1,106 @@ +// Copyright ยฉ 2019 The Knative Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package commands + +import ( + "fmt" + "os" + "strings" + "testing" + + "github.com/knative/client/pkg/util" + "gotest.tools/assert" + "k8s.io/client-go/tools/clientcmd" + clientcmdapi "k8s.io/client-go/tools/clientcmd/api" +) + +type getConfigTestCase struct { + clientConfig clientcmd.ClientConfig + expectedErrString string +} + +func TestGetConfig(t *testing.T) { + for i, tc := range []getConfigTestCase{ + { + clientcmd.NewDefaultClientConfig(clientcmdapi.Config{}, &clientcmd.ConfigOverrides{}), + "no configuration has been provided", + }, + } { + p := &KnParams{ + ClientConfig: tc.clientConfig} + + _, err := p.GetConfig() + + switch len(tc.expectedErrString) { + case 0: + if err != nil { + t.Errorf("%d: unexpected error: %s", i, err.Error()) + } + default: + if err == nil { + t.Errorf("%d: wrong error detected: %s (expected) != %s (actual)", i, tc.expectedErrString, err) + } + if !strings.Contains(err.Error(), tc.expectedErrString) { + t.Errorf("%d: wrong error detected: %s (expected) != %s (actual)", i, tc.expectedErrString, err.Error()) + } + } + } +} + +type typeTestCase struct { + kubeCfgPath string + explicitPath string + expectedError string +} + +func TestGetClientConfig(t *testing.T) { + multiConfigs := fmt.Sprintf("%s%s%s", "/testing/assets/kube-config-01.yml", string(os.PathListSeparator), "/testing/assets/kube-config-02.yml") + + multiConfigs = multiConfigs + for _, tc := range []typeTestCase{ + { + "", + clientcmd.NewDefaultClientConfigLoadingRules().ExplicitPath, + "", + }, + { + "/testing/assets/kube-config-01.yml", + "", + fmt.Sprintf("Config file '%s' can not be found", "/testing/assets/kube-config-01.yml"), + }, + { + multiConfigs, + "", + fmt.Sprintf("Can not find config file. '%s' looks like a path. Please use the env var KUBECONFIG if you want to check for multiple configuration files", multiConfigs), + }, + } { + p := &KnParams{ + KubeCfgPath: tc.kubeCfgPath, + } + + clientConfig, err := p.GetClientConfig() + if tc.expectedError != "" { + assert.Assert(t, util.ContainsAll(err.Error(), tc.expectedError)) + } else { + assert.Assert(t, err == nil, err) + } + + if clientConfig != nil { + configAccess := clientConfig.ConfigAccess() + + assert.Assert(t, configAccess.GetExplicitFile() == tc.explicitPath) + } + } +} diff --git a/pkg/kn/commands/wait_flags.go b/pkg/kn/commands/wait_flags.go index ecbf7cee92..838a1159d4 100644 --- a/pkg/kn/commands/wait_flags.go +++ b/pkg/kn/commands/wait_flags.go @@ -32,10 +32,10 @@ type WaitFlags struct { // Add flags which influence the sync/async behaviour when creating or updating // resources. Set `waitDefault` argument if the default behaviour is synchronous. // Use `what` for describing what is waited for. -func (p *WaitFlags) AddConditionWaitFlags(command *cobra.Command, waitTimeoutDefault int, what string) { - waitUsage := fmt.Sprintf("Create %s and don't wait for it to become ready.", what) +func (p *WaitFlags) AddConditionWaitFlags(command *cobra.Command, waitTimeoutDefault int, action string, what string) { + waitUsage := fmt.Sprintf("%s %s and don't wait for it to become ready.", action, what) command.Flags().BoolVar(&p.Async, "async", false, waitUsage) - timeoutUsage := fmt.Sprintf("Seconds to wait before giving up on waiting for %s to be ready (default: %d).", what, waitTimeoutDefault) + timeoutUsage := fmt.Sprintf("Seconds to wait before giving up on waiting for %s to be ready.", what) command.Flags().IntVar(&p.TimeoutInSeconds, "wait-timeout", waitTimeoutDefault, timeoutUsage) } diff --git a/pkg/kn/commands/wait_flags_test.go b/pkg/kn/commands/wait_flags_test.go index e72d8924ef..b6ae64e9c6 100644 --- a/pkg/kn/commands/wait_flags_test.go +++ b/pkg/kn/commands/wait_flags_test.go @@ -41,7 +41,7 @@ func TestAddWaitForReadyFlags(t *testing.T) { flags := &WaitFlags{} cmd := cobra.Command{} - flags.AddConditionWaitFlags(&cmd, 60, "service") + flags.AddConditionWaitFlags(&cmd, 60, "Create", "service") err := cmd.ParseFlags(tc.args) if err != nil && !tc.isParseErrorExpected { @@ -66,7 +66,7 @@ func TestAddWaitUsageMessage(t *testing.T) { flags := &WaitFlags{} cmd := cobra.Command{} - flags.AddConditionWaitFlags(&cmd, 60, "blub") + flags.AddConditionWaitFlags(&cmd, 60, "bla", "blub") if !strings.Contains(cmd.UsageString(), "blub") { t.Error("no type returned in usage") } diff --git a/pkg/serving/config_changes.go b/pkg/serving/config_changes.go index b4bbebddfa..7fdee53729 100644 --- a/pkg/serving/config_changes.go +++ b/pkg/serving/config_changes.go @@ -27,7 +27,7 @@ import ( // vars. Does not touch any environment variables not mentioned, but it can add // new env vars and change the values of existing ones. func UpdateEnvVars(template *servingv1alpha1.RevisionTemplateSpec, vars map[string]string) error { - container, err := extractContainer(template) + container, err := ContainerOfRevisionTemplate(template) if err != nil { return err } @@ -78,7 +78,7 @@ func EnvToMap(vars []corev1.EnvVar) (map[string]string, error) { // Update a given image func UpdateImage(template *servingv1alpha1.RevisionTemplateSpec, image string) error { - container, err := extractContainer(template) + container, err := ContainerOfRevisionTemplate(template) if err != nil { return err } @@ -88,7 +88,7 @@ func UpdateImage(template *servingv1alpha1.RevisionTemplateSpec, image string) e // UpdateContainerPort updates container with a give port func UpdateContainerPort(template *servingv1alpha1.RevisionTemplateSpec, port int32) error { - container, err := extractContainer(template) + container, err := ContainerOfRevisionTemplate(template) if err != nil { return err } @@ -99,7 +99,7 @@ func UpdateContainerPort(template *servingv1alpha1.RevisionTemplateSpec, port in } func UpdateResources(template *servingv1alpha1.RevisionTemplateSpec, requestsResourceList corev1.ResourceList, limitsResourceList corev1.ResourceList) error { - container, err := extractContainer(template) + container, err := ContainerOfRevisionTemplate(template) if err != nil { return err } @@ -124,26 +124,6 @@ func UpdateResources(template *servingv1alpha1.RevisionTemplateSpec, requestsRes // ======================================================================================= -func usesOldV1alpha1ContainerField(revision *servingv1alpha1.RevisionTemplateSpec) bool { - return revision.Spec.DeprecatedContainer != nil -} - -func extractContainer(template *servingv1alpha1.RevisionTemplateSpec) (*corev1.Container, error) { - if usesOldV1alpha1ContainerField(template) { - return template.Spec.DeprecatedContainer, nil - } - containers := template.Spec.Containers - if len(containers) == 0 { - return nil, fmt.Errorf("internal: no container set in spec.template.spec.containers") - } - if len(containers) > 1 { - return nil, fmt.Errorf("internal: can't extract container for updating environment"+ - " variables as the configuration contains "+ - "more than one container (i.e. %d containers)", len(containers)) - } - return &containers[0], nil -} - func updateEnvVarsFromMap(env []corev1.EnvVar, vars map[string]string) []corev1.EnvVar { set := make(map[string]bool) for i := range env { diff --git a/pkg/serving/revision_template.go b/pkg/serving/revision_template.go new file mode 100644 index 0000000000..becff47631 --- /dev/null +++ b/pkg/serving/revision_template.go @@ -0,0 +1,48 @@ +// Copyright ยฉ 2019 The Knative Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package serving + +import ( + "fmt" + + servingv1alpha1 "github.com/knative/serving/pkg/apis/serving/v1alpha1" + corev1 "k8s.io/api/core/v1" +) + +func ContainerOfRevisionTemplate(template *servingv1alpha1.RevisionTemplateSpec) (*corev1.Container, error) { + return ContainerOfRevisionSpec(&template.Spec) +} + +func ContainerOfRevisionSpec(revisionSpec *servingv1alpha1.RevisionSpec) (*corev1.Container, error) { + if usesOldV1alpha1ContainerField(revisionSpec) { + return revisionSpec.DeprecatedContainer, nil + } + containers := revisionSpec.Containers + if len(containers) == 0 { + return nil, fmt.Errorf("internal: no container set in spec.template.spec.containers") + } + if len(containers) > 1 { + return nil, fmt.Errorf("internal: can't extract container for updating environment"+ + " variables as the configuration contains "+ + "more than one container (i.e. %d containers)", len(containers)) + } + return &containers[0], nil +} + +// ======================================================================================= + +func usesOldV1alpha1ContainerField(revisionSpec *servingv1alpha1.RevisionSpec) bool { + return revisionSpec.DeprecatedContainer != nil +} diff --git a/pkg/serving/service.go b/pkg/serving/service.go index 22a3b2c6e3..54c7f76d7b 100644 --- a/pkg/serving/service.go +++ b/pkg/serving/service.go @@ -25,7 +25,7 @@ import ( // 'old' v1alpha1 fields are looked up. // The returned revision template can be updated in place. // An error is returned if no revision template could be extracted -func GetRevisionTemplate(service *servingv1alpha1.Service) (*servingv1alpha1.RevisionTemplateSpec, error) { +func RevisionTemplateOfService(service *servingv1alpha1.Service) (*servingv1alpha1.RevisionTemplateSpec, error) { // Try v1beta1 field first if service.Spec.Template != nil { return service.Spec.Template, nil diff --git a/pkg/serving/v1alpha1/client.go b/pkg/serving/v1alpha1/client.go index f08a7d82dc..5e12ee9197 100644 --- a/pkg/serving/v1alpha1/client.go +++ b/pkg/serving/v1alpha1/client.go @@ -63,6 +63,9 @@ type KnClient interface { // Delete a revision DeleteRevision(name string) error + // Get a route by its unique name + GetRoute(name string) (*v1alpha1.Route, error) + // List routes ListRoutes(opts ...ListConfig) (*v1alpha1.RouteList, error) } @@ -219,6 +222,19 @@ func (cl *knClient) ListRevisions(config ...ListConfig) (*v1alpha1.RevisionList, return updateServingGvkForRevisionList(revisionList) } +// Get a route by its unique name +func (cl *knClient) GetRoute(name string) (*v1alpha1.Route, error) { + route, err := cl.client.Routes(cl.namespace).Get(name, v1.GetOptions{}) + if err != nil { + return nil, err + } + err = serving.UpdateGroupVersionKind(route, v1alpha1.SchemeGroupVersion) + if err != nil { + return nil, err + } + return route, nil +} + // List routes func (cl *knClient) ListRoutes(config ...ListConfig) (*v1alpha1.RouteList, error) { routeList, err := cl.client.Routes(cl.namespace).List(ListConfigs(config).toListOptions()) diff --git a/pkg/serving/v1alpha1/client_test.go b/pkg/serving/v1alpha1/client_test.go index 8ff7f4c185..b8cc355f26 100644 --- a/pkg/serving/v1alpha1/client_test.go +++ b/pkg/serving/v1alpha1/client_test.go @@ -285,6 +285,39 @@ func TestListRevisionForService(t *testing.T) { }) } +func TestGetRoute(t *testing.T) { + serving, client := setup() + routeName := "test-route" + + serving.AddReactor("get", "routes", + func(a client_testing.Action) (bool, runtime.Object, error) { + route := newRoute(routeName) + name := a.(client_testing.GetAction).GetName() + // Sanity check + assert.Assert(t, name != "") + assert.Equal(t, testNamespace, a.GetNamespace()) + if name == routeName { + return true, route, nil + } + return true, nil, errors.NewNotFound(v1alpha1.Resource("route"), name) + }) + + t.Run("get known route by name returns route", func(t *testing.T) { + route, err := client.GetRoute(routeName) + assert.NilError(t, err) + assert.Equal(t, routeName, route.Name, "route name should be equal") + validateGroupVersionKind(t, route) + }) + + t.Run("get unknown route name returns error", func(t *testing.T) { + nonExistingRouteName := "r@ute-that-d$es-n#t-exist" + route, err := client.GetRoute(nonExistingRouteName) + assert.Assert(t, route == nil, "no route should be returned") + assert.ErrorContains(t, err, "not found") + assert.ErrorContains(t, err, nonExistingRouteName) + }) +} + func TestListRoutes(t *testing.T) { serving, client := setup() diff --git a/test/e2e-smoke-tests.sh b/test/e2e-smoke-tests.sh index 315766b8b6..a1b7ba88e5 100755 --- a/test/e2e-smoke-tests.sh +++ b/test/e2e-smoke-tests.sh @@ -47,7 +47,7 @@ header "Running smoke tests" kubectl create ns $KN_E2E_SMOKE_TESTS_NAMESPACE || fail_test ./kn service create svc1 --async --image gcr.io/knative-samples/helloworld-go -e TARGET=Knative -n $KN_E2E_SMOKE_TESTS_NAMESPACE || fail_test -./kn service create hello --image gcr.io/knative-samples/helloworld-go -e TARGET=Knative -n $KN_E2E_SMOKE_TESTS_NAMESPACE || fail_test +./kn service create hello --image gcr.io/knative-samples/helloworld-go -e TARGET=Knative -n $KN_E2E_SMOKE_TESTS_NAMESPACE --wait-timeout 240 || fail_test ./kn service list hello -n $KN_E2E_SMOKE_TESTS_NAMESPACE -n $KN_E2E_SMOKE_TESTS_NAMESPACE || fail_test ./kn service update hello --env TARGET=kn -n $KN_E2E_SMOKE_TESTS_NAMESPACE || fail_test ./kn revision list hello -n $KN_E2E_SMOKE_TESTS_NAMESPACE || fail_test diff --git a/test/e2e/basic_workflow_test.go b/test/e2e/basic_workflow_test.go index 864b65af7d..c11b5c429a 100644 --- a/test/e2e/basic_workflow_test.go +++ b/test/e2e/basic_workflow_test.go @@ -25,80 +25,99 @@ import ( "gotest.tools/assert" ) +var ( + e env + k kn +) + +const ( + KnDefaultTestImage string = "gcr.io/knative-samples/helloworld-go" +) + +func Setup(t *testing.T) func(t *testing.T) { + e = buildEnv(t) + k = kn{t, e.Namespace, Logger{}} + CreateTestNamespace(t, e.Namespace) + return Teardown +} + +func Teardown(t *testing.T) { + DeleteTestNamespace(t, e.Namespace) +} + func TestBasicWorkflow(t *testing.T) { - test := NewE2eTest(t) - test.Setup(t) - defer test.Teardown(t) + teardown := Setup(t) + defer teardown(t) t.Run("returns no service before running tests", func(t *testing.T) { - test.testServiceListEmpty(t) + testServiceListEmpty(t, k) }) t.Run("create hello service and returns no error", func(t *testing.T) { - test.testServiceCreate(t, "hello") + testServiceCreate(t, k, "hello") }) t.Run("returns valid info about hello service", func(t *testing.T) { - test.testServiceList(t, "hello") - test.testServiceDescribe(t, "hello") + testServiceList(t, k, "hello") + testServiceDescribe(t, k, "hello") }) t.Run("update hello service's configuration and returns no error", func(t *testing.T) { - test.testServiceUpdate(t, "hello", []string{"--env", "TARGET=kn", "--port", "8888"}) + testServiceUpdate(t, k, "hello", []string{"--env", "TARGET=kn", "--port", "8888"}) }) t.Run("create another service and returns no error", func(t *testing.T) { - test.testServiceCreate(t, "svc2") + testServiceCreate(t, k, "svc2") }) t.Run("returns a list of revisions associated with hello and svc2 services", func(t *testing.T) { - test.testRevisionListForService(t, "hello") - test.testRevisionListForService(t, "svc2") + testRevisionListForService(t, k, "hello") + testRevisionListForService(t, k, "svc2") }) t.Run("returns a list of routes associated with hello and svc2 services", func(t *testing.T) { - test.testRouteList(t) - test.testRouteListWithArgument(t, "hello") + testRouteList(t, k) + testRouteListWithArgument(t, k, "hello") }) t.Run("delete hello and svc2 services and returns no error", func(t *testing.T) { - test.testServiceDelete(t, "hello") - test.testServiceDelete(t, "svc2") + testServiceDelete(t, k, "hello") + testServiceDelete(t, k, "svc2") }) t.Run("returns no service after completing tests", func(t *testing.T) { - test.testServiceListEmpty(t) + testServiceListEmpty(t, k) }) } // Private test functions -func (test *e2eTest) testServiceListEmpty(t *testing.T) { - out, err := test.kn.RunWithOpts([]string{"service", "list"}, runOpts{NoNamespace: false}) +func testServiceListEmpty(t *testing.T, k kn) { + out, err := k.RunWithOpts([]string{"service", "list"}, runOpts{NoNamespace: false}) assert.NilError(t, err) assert.Check(t, util.ContainsAll(out, "No resources found.")) } -func (test *e2eTest) testServiceCreate(t *testing.T, serviceName string) { - out, err := test.kn.RunWithOpts([]string{"service", "create", +func testServiceCreate(t *testing.T, k kn, serviceName string) { + out, err := k.RunWithOpts([]string{"service", "create", fmt.Sprintf("%s", serviceName), "--image", KnDefaultTestImage}, runOpts{NoNamespace: false}) assert.NilError(t, err) - assert.Check(t, util.ContainsAll(out, "Service", serviceName, "successfully created in namespace", test.kn.namespace, "OK")) + assert.Check(t, util.ContainsAll(out, "Service", serviceName, "successfully created in namespace", k.namespace, "OK")) } -func (test *e2eTest) testServiceList(t *testing.T, serviceName string) { - out, err := test.kn.RunWithOpts([]string{"service", "list", serviceName}, runOpts{NoNamespace: false}) +func testServiceList(t *testing.T, k kn, serviceName string) { + out, err := k.RunWithOpts([]string{"service", "list", serviceName}, runOpts{NoNamespace: false}) assert.NilError(t, err) expectedOutput := fmt.Sprintf("%s", serviceName) assert.Check(t, util.ContainsAll(out, expectedOutput)) } -func (test *e2eTest) testRevisionListForService(t *testing.T, serviceName string) { - out, err := test.kn.RunWithOpts([]string{"revision", "list", "-s", serviceName}, runOpts{NoNamespace: false}) +func testRevisionListForService(t *testing.T, k kn, serviceName string) { + out, err := k.RunWithOpts([]string{"revision", "list", "-s", serviceName}, runOpts{NoNamespace: false}) assert.NilError(t, err) outputLines := strings.Split(out, "\n") @@ -110,8 +129,8 @@ func (test *e2eTest) testRevisionListForService(t *testing.T, serviceName string } } -func (test *e2eTest) testServiceDescribe(t *testing.T, serviceName string) { - out, err := test.kn.RunWithOpts([]string{"service", "describe", serviceName}, runOpts{NoNamespace: false}) +func testServiceDescribe(t *testing.T, k kn, serviceName string) { + out, err := k.RunWithOpts([]string{"service", "describe", serviceName}, runOpts{NoNamespace: false}) assert.NilError(t, err) expectedOutputHeader := `apiVersion: serving.knative.dev/v1alpha1 @@ -120,37 +139,37 @@ metadata:` expectedOutput := `generation: 1 name: %s namespace: %s` - expectedOutput = fmt.Sprintf(expectedOutput, serviceName, test.kn.namespace) + expectedOutput = fmt.Sprintf(expectedOutput, serviceName, k.namespace) assert.Check(t, util.ContainsAll(out, expectedOutputHeader, expectedOutput)) } -func (test *e2eTest) testServiceUpdate(t *testing.T, serviceName string, args []string) { - out, err := test.kn.RunWithOpts(append([]string{"service", "update", serviceName}, args...), runOpts{NoNamespace: false}) +func testServiceUpdate(t *testing.T, k kn, serviceName string, args []string) { + out, err := k.RunWithOpts(append([]string{"service", "update", serviceName}, args...), runOpts{NoNamespace: false}) assert.NilError(t, err) expectedOutput := fmt.Sprintf("Service '%s' updated", serviceName) assert.Check(t, util.ContainsAll(out, expectedOutput)) } -func (test *e2eTest) testRouteList(t *testing.T) { - out, err := test.kn.RunWithOpts([]string{"route", "list"}, runOpts{}) +func testRouteList(t *testing.T, k kn) { + out, err := k.RunWithOpts([]string{"route", "list"}, runOpts{}) assert.NilError(t, err) expectedHeaders := []string{"NAME", "URL", "AGE", "CONDITIONS", "TRAFFIC"} assert.Check(t, util.ContainsAll(out, expectedHeaders...)) } -func (test *e2eTest) testRouteListWithArgument(t *testing.T, routeName string) { - out, err := test.kn.RunWithOpts([]string{"route", "list", routeName}, runOpts{}) +func testRouteListWithArgument(t *testing.T, k kn, routeName string) { + out, err := k.RunWithOpts([]string{"route", "list", routeName}, runOpts{}) assert.NilError(t, err) expectedOutput := fmt.Sprintf("100%% -> %s", routeName) assert.Check(t, util.ContainsAll(out, routeName, expectedOutput)) } -func (test *e2eTest) testServiceDelete(t *testing.T, serviceName string) { - out, err := test.kn.RunWithOpts([]string{"service", "delete", serviceName}, runOpts{NoNamespace: false}) +func testServiceDelete(t *testing.T, k kn, serviceName string) { + out, err := k.RunWithOpts([]string{"service", "delete", serviceName}, runOpts{NoNamespace: false}) assert.NilError(t, err) - assert.Check(t, util.ContainsAll(out, "Service", serviceName, "successfully deleted in namespace", test.kn.namespace)) + assert.Check(t, util.ContainsAll(out, "Service", serviceName, "successfully deleted in namespace", k.namespace)) } diff --git a/test/e2e/common.go b/test/e2e/common.go index 09f2dcc68e..f9aeadd12c 100644 --- a/test/e2e/common.go +++ b/test/e2e/common.go @@ -23,7 +23,6 @@ import ( "regexp" "strings" "testing" - "time" ) type runOpts struct { @@ -36,53 +35,22 @@ type runOpts struct { Redact bool } -const ( - KnDefaultTestImage string = "gcr.io/knative-samples/helloworld-go" - MaxRetries int = 10 - RetrySleepDuration time.Duration = 30 * time.Second -) - -type e2eTest struct { - env env - kn kn -} - -func NewE2eTest(t *testing.T) *e2eTest { - return &e2eTest{ - env: buildEnv(t), - } -} - -// Setup set up an enviroment for kn integration test returns the Teardown cleanup function -func (test *e2eTest) Setup(t *testing.T) { - test.env.Namespace = fmt.Sprintf("%s%d", test.env.Namespace, namespaceCount) - namespaceCount++ - test.kn = kn{t, test.env.Namespace, Logger{}} - test.CreateTestNamespace(t, test.env.Namespace) -} - -// Teardown clean up -func (test *e2eTest) Teardown(t *testing.T) { - test.DeleteTestNamespace(t, test.env.Namespace) -} - // CreateTestNamespace creates and tests a namesspace creation invoking kubectl -func (test *e2eTest) CreateTestNamespace(t *testing.T, namespace string) { - logger := Logger{} - expectedOutputRegexp := fmt.Sprintf("namespace?.+%s.+created", namespace) - out, err := createNamespace(t, namespace, MaxRetries, logger) +func CreateTestNamespace(t *testing.T, namespace string) { + kubectl := kubectl{t, Logger{}} + out, err := kubectl.RunWithOpts([]string{"create", "namespace", namespace}, runOpts{}) if err != nil { - logger.Fatalf("Could not create namespace, giving up") + t.Fatalf(fmt.Sprintf("Error executing 'kubectl create namespace' command. Error: %s", err.Error())) } - // check that last output indeed show created namespace + expectedOutputRegexp := fmt.Sprintf("namespace?.+%s.+created", namespace) if !matchRegexp(t, expectedOutputRegexp, out) { t.Fatalf("Expected output incorrect, expecting to include:\n%s\n Instead found:\n%s\n", expectedOutputRegexp, out) } } // CreateTestNamespace deletes and tests a namesspace deletion invoking kubectl -func (test *e2eTest) DeleteTestNamespace(t *testing.T, namespace string) { +func DeleteTestNamespace(t *testing.T, namespace string) { kubectl := kubectl{t, Logger{}} out, err := kubectl.RunWithOpts([]string{"delete", "namespace", namespace}, runOpts{}) if err != nil { @@ -95,61 +63,7 @@ func (test *e2eTest) DeleteTestNamespace(t *testing.T, namespace string) { } } -// WaitForNamespaceDeleted wait until namespace is deleted -func (test *e2eTest) WaitForNamespaceDeleted(t *testing.T, namespace string) { - logger := Logger{} - deleted := checkNamespaceDeleted(t, namespace, MaxRetries, logger) - if !deleted { - t.Fatalf(fmt.Sprintf("Error deleting namespace %s, timed out", namespace)) - } -} - // Private functions -func checkNamespaceDeleted(t *testing.T, namespace string, maxRetries int, logger Logger) bool { - kubectlGetNamespace := func() (string, error) { - kubectl := kubectl{t, logger} - return kubectl.RunWithOpts([]string{"get", "namespace"}, runOpts{}) - } - - retries := 0 - for retries < MaxRetries { - output, _ := kubectlGetNamespace() - if !strings.Contains(output, namespace) { - return true - } - - retries++ - logger.Debugf("Namespace is terminating, waiting %ds, and trying again: %d of %d\n", int(RetrySleepDuration.Seconds()), retries, maxRetries) - time.Sleep(RetrySleepDuration) - } - - return true -} - -func createNamespace(t *testing.T, namespace string, maxRetries int, logger Logger) (string, error) { - kubectlCreateNamespace := func() (string, error) { - kubectl := kubectl{t, logger} - return kubectl.RunWithOpts([]string{"create", "namespace", namespace}, runOpts{AllowError: true}) - } - - var ( - retries int - err error - out string - ) - - for retries < maxRetries { - out, err := kubectlCreateNamespace() - if err == nil { - return out, nil - } - retries++ - logger.Debugf("Could not create namespace, waiting %ds, and trying again: %d of %d\n", int(RetrySleepDuration.Seconds()), retries, maxRetries) - time.Sleep(RetrySleepDuration) - } - - return out, err -} func runCLIWithOpts(cli string, args []string, opts runOpts, logger Logger) (string, error) { logger.Debugf("Running '%s'...\n", cmdCLIDesc(cli, args)) diff --git a/test/e2e/env.go b/test/e2e/env.go index 438669b2ac..7af9fcf7ba 100644 --- a/test/e2e/env.go +++ b/test/e2e/env.go @@ -26,8 +26,6 @@ type env struct { const defaultKnE2ENamespace = "kne2etests" -var namespaceCount = 0 - func buildEnv(t *testing.T) env { env := env{ Namespace: os.Getenv("KN_E2E_NAMESPACE"), diff --git a/test/e2e/revision_workflow_test.go b/test/e2e/revision_workflow_test.go index b6905656ad..f9a713a436 100644 --- a/test/e2e/revision_workflow_test.go +++ b/test/e2e/revision_workflow_test.go @@ -25,34 +25,31 @@ import ( ) func TestRevisionWorkflow(t *testing.T) { - test := NewE2eTest(t) - test.Setup(t) - defer test.Teardown(t) + teardown := Setup(t) + defer teardown(t) t.Run("create hello service and returns no error", func(t *testing.T) { - test.testServiceCreate(t, "hello") + testServiceCreate(t, k, "hello") }) t.Run("delete latest revision from hello service and returns no error", func(t *testing.T) { - test.testDeleteRevision(t, "hello") + testDeleteRevision(t, k, "hello") }) t.Run("delete hello service and returns no error", func(t *testing.T) { - test.testServiceDelete(t, "hello") + testServiceDelete(t, k, "hello") }) } -// Private - -func (test *e2eTest) testDeleteRevision(t *testing.T, serviceName string) { - revName, err := test.kn.RunWithOpts([]string{"revision", "list", "-o=jsonpath={.items[0].metadata.name}"}, runOpts{}) +func testDeleteRevision(t *testing.T, k kn, serviceName string) { + revName, err := k.RunWithOpts([]string{"revision", "list", "-o=jsonpath={.items[0].metadata.name}"}, runOpts{}) assert.NilError(t, err) if strings.Contains(revName, "No resources found.") { t.Errorf("Could not find revision name.") } - out, err := test.kn.RunWithOpts([]string{"revision", "delete", revName}, runOpts{}) + out, err := k.RunWithOpts([]string{"revision", "delete", revName}, runOpts{}) assert.NilError(t, err) - assert.Check(t, util.ContainsAll(out, "Revision", revName, "deleted", "namespace", test.kn.namespace)) + assert.Check(t, util.ContainsAll(out, "Revision", revName, "deleted", "namespace", k.namespace)) } diff --git a/test/e2e/service_options_test.go b/test/e2e/service_options_test.go index 0088b79264..9922ecc945 100644 --- a/test/e2e/service_options_test.go +++ b/test/e2e/service_options_test.go @@ -25,53 +25,50 @@ import ( ) func TestServiceOptions(t *testing.T) { - test := NewE2eTest(t) - test.Setup(t) - defer test.Teardown(t) + teardown := Setup(t) + defer teardown(t) t.Run("create hello service with concurrency options and returns no error", func(t *testing.T) { - test.testServiceCreateWithOptions(t, "hello", []string{"--concurrency-limit", "250", "--concurrency-target", "300"}) + testServiceCreateWithOptions(t, k, "hello", []string{"--concurrency-limit", "250", "--concurrency-target", "300"}) }) t.Run("returns valid concurrency options for hello service", func(t *testing.T) { - test.testServiceDescribeConcurrencyLimit(t, "hello", "250") - test.testServiceDescribeConcurrencyTarget(t, "hello", "300") + testServiceDescribeConcurrencyLimit(t, k, "hello", "250") + testServiceDescribeConcurrencyTarget(t, k, "hello", "300") }) t.Run("update concurrency limit for hello service and returns no error", func(t *testing.T) { - test.testServiceUpdate(t, "hello", []string{"--concurrency-limit", "300"}) + testServiceUpdate(t, k, "hello", []string{"--concurrency-limit", "300"}) }) t.Run("returns correct concurrency limit for hello service", func(t *testing.T) { - test.testServiceDescribeConcurrencyLimit(t, "hello", "300") + testServiceDescribeConcurrencyLimit(t, k, "hello", "300") }) t.Run("delete hello service and returns no error", func(t *testing.T) { - test.testServiceDelete(t, "hello") + testServiceDelete(t, k, "hello") }) } -// Private - -func (test *e2eTest) testServiceCreateWithOptions(t *testing.T, serviceName string, options []string) { +func testServiceCreateWithOptions(t *testing.T, k kn, serviceName string, options []string) { command := []string{"service", "create", serviceName, "--image", KnDefaultTestImage} command = append(command, options...) - out, err := test.kn.RunWithOpts(command, runOpts{NoNamespace: false}) + out, err := k.RunWithOpts(command, runOpts{NoNamespace: false}) assert.NilError(t, err) - assert.Check(t, util.ContainsAll(out, "Service", serviceName, "successfully created in namespace", test.kn.namespace, "OK")) + assert.Check(t, util.ContainsAll(out, "Service", serviceName, "successfully created in namespace", k.namespace, "OK")) } -func (test *e2eTest) testServiceDescribeConcurrencyLimit(t *testing.T, serviceName, concurrencyLimit string) { - out, err := test.kn.RunWithOpts([]string{"service", "describe", serviceName}, runOpts{NoNamespace: false}) +func testServiceDescribeConcurrencyLimit(t *testing.T, k kn, serviceName, concurrencyLimit string) { + out, err := k.RunWithOpts([]string{"service", "describe", serviceName}, runOpts{NoNamespace: false}) assert.NilError(t, err) expectedOutput := fmt.Sprintf("containerConcurrency: %s", concurrencyLimit) assert.Check(t, util.ContainsAll(out, expectedOutput)) } -func (test *e2eTest) testServiceDescribeConcurrencyTarget(t *testing.T, serviceName, concurrencyTarget string) { - out, err := test.kn.RunWithOpts([]string{"service", "describe", serviceName}, runOpts{NoNamespace: false}) +func testServiceDescribeConcurrencyTarget(t *testing.T, k kn, serviceName, concurrencyTarget string) { + out, err := k.RunWithOpts([]string{"service", "describe", serviceName}, runOpts{NoNamespace: false}) assert.NilError(t, err) expectedOutput := fmt.Sprintf("autoscaling.knative.dev/target: \"%s\"", concurrencyTarget)