diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index e805b79d35..9c7a490f29 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -5,12 +5,29 @@ [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%"] @@ -35,7 +52,7 @@ | 🐛 | Retry update operation on an optimistic lock failure -| https://github.com/knative/client/pull/[#240] +| https://github.com/knative/client/pull/240[#240] | 🎁 | Add `kn route list` diff --git a/docs/cmd/kn_service_update.md b/docs/cmd/kn_service_update.md index c9fda2e651..ea73cd5971 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/pkg/kn/commands/service/service.go b/pkg/kn/commands/service/service.go index 9f719a9399..5c3adb10c1 100644 --- a/pkg/kn/commands/service/service.go +++ b/pkg/kn/commands/service/service.go @@ -15,7 +15,14 @@ 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" ) @@ -36,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 6310e87fb3..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 } diff --git a/pkg/kn/commands/service/service_update.go b/pkg/kn/commands/service/service_update.go index 6ea08ef6e5..39260977ef 100644 --- a/pkg/kn/commands/service/service_update.go +++ b/pkg/kn/commands/service/service_update.go @@ -26,6 +26,7 @@ import ( func NewServiceUpdateCommand(p *commands.KnParams) *cobra.Command { var editFlags ConfigurationEditFlags + var waitFlags commands.WaitFlags serviceUpdateCommand := &cobra.Command{ Use: "update NAME", @@ -56,7 +57,8 @@ func NewServiceUpdateCommand(p *commands.KnParams) *cobra.Command { var retries = 0 for { - service, err := client.GetService(args[0]) + name := args[0] + service, err := client.GetService(name) if err != nil { return err } @@ -76,6 +78,15 @@ func NewServiceUpdateCommand(p *commands.KnParams) *cobra.Command { } 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 } @@ -84,5 +95,6 @@ func NewServiceUpdateCommand(p *commands.KnParams) *cobra.Command { 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 0a98860065..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,6 +88,29 @@ 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() @@ -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) @@ -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) @@ -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) @@ -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") { @@ -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") { @@ -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") { 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/wait_flags.go b/pkg/kn/commands/wait_flags.go index 3536ffa3c2..838a1159d4 100644 --- a/pkg/kn/commands/wait_flags.go +++ b/pkg/kn/commands/wait_flags.go @@ -32,8 +32,8 @@ 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.", what) 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") }