refactor(service): Add a ProgressHandler for the Wait interface

docs/cmd/kn.md

@@ -6,8 +6,9 @@ Knative client
 
 Manage your Knative building blocks:
 
-* [Serving](https://github.com/knative/serving/tree/master): Manage your services and release new software to them.
-* [Eventing](https://github.com/knative/eventing/tree/master): Manage event subscriptions and channels. Connect event sources.
+Serving: Manage your services and release new software to them.
+Build: Create builds and keep track of their results.
+Eventing: Manage event subscriptions and channels. Connect up event sources.
 
 ### Options
 

pkg/kn/commands/revision/revision_list_test.go

@@ -26,6 +26,7 @@ import (
 	client_testing "k8s.io/client-go/testing"
 
 	"github.com/knative/client/pkg/kn/commands"
+
 	"github.com/knative/client/pkg/util"
 )
 

pkg/kn/commands/service/service.go

@@ -15,7 +15,12 @@
 package service
 
 import (
+	"fmt"
+	"io"
+
 	"github.com/knative/client/pkg/kn/commands"
+	"github.com/knative/client/pkg/wait"
+
 	"github.com/spf13/cobra"
 )
 
@@ -31,3 +36,14 @@ func NewServiceCommand(p *commands.KnParams) *cobra.Command {
 	serviceCmd.AddCommand(NewServiceUpdateCommand(p))
 	return serviceCmd
 }
+
+// Create a new service progress handler for waiting on a service to become ready.
+func newServiceWaitProgressHandler(stdout io.Writer, name string) wait.ProgressHandler {
+	return wait.NewSimpleProgressHandler(
+		stdout,
+		fmt.Sprintf("Waiting for service '%s' to become ready ... ", name),
+		".",
+		"ERROR",
+		"OK",
+	)
+}

pkg/kn/commands/service/service_create.go

@@ -104,7 +104,10 @@ func NewServiceCreateCommand(p *commands.KnParams) *cobra.Command {
 			}
 
 			if !waitFlags.Async {
-				err := client.WaitForService(name, time.Duration(waitFlags.TimeoutInSeconds)*time.Second, cmd.OutOrStdout())
+				err := client.WaitForService(
+					name,
+					time.Duration(waitFlags.TimeoutInSeconds)*time.Second,
+					newServiceWaitProgressHandler(cmd.OutOrStdout(), name))
 				if err != nil {
 					return err
 				}

pkg/serving/v1alpha1/client.go

@@ -16,7 +16,6 @@ package v1alpha1
 
 import (
 	"fmt"
-	"io"
 	"time"
 
 	"github.com/knative/pkg/apis"
@@ -52,8 +51,9 @@ type KnClient interface {
 	// Delete a service by name
 	DeleteService(name string) error
 
-	// Wait for a service to become ready, but not longer than provided timeout
-	WaitForService(name string, timeout time.Duration, out io.Writer) error
+	// Wait for a service to become ready, but not longer than provided timeout.
+	// Optionally a single progress handler can be given to print out progress
+	WaitForService(name string, timeout time.Duration, progressHandler ...wait.ProgressHandler) error
 
 	// Get a revision by name
 	GetRevision(name string) (*v1alpha1.Revision, error)
@@ -188,9 +188,9 @@ func (cl *knClient) DeleteService(serviceName string) error {
 }
 
 // Wait for a service to become ready, but not longer than provided timeout
-func (cl *knClient) WaitForService(name string, timeout time.Duration, out io.Writer) error {
+func (cl *knClient) WaitForService(name string, timeout time.Duration, progressHandlers ...wait.ProgressHandler) error {
 	waitForReady := newServiceWaitForReady(cl.client.Services(cl.namespace).Watch)
-	return waitForReady.Wait(name, timeout, out)
+	return waitForReady.Wait(name, timeout, progressHandlers...)
 }
 
 // Get a revision by name
@@ -281,7 +281,6 @@ func updateServingGvk(obj runtime.Object) error {
 // Can be used by `service_create` and `service_update`, hence this extra file
 func newServiceWaitForReady(watch wait.WatchFunc) wait.WaitForReady {
 	return wait.NewWaitForReady(
-		"service",
 		watch,
 		serviceConditionExtractor)
 }

pkg/serving/v1alpha1/client_test.go

@@ -15,7 +15,6 @@
 package v1alpha1
 
 import (
-	"bytes"
 	"fmt"
 	"testing"
 	"time"
@@ -357,8 +356,7 @@ func TestWaitForService(t *testing.T) {
 		})
 
 	t.Run("wait on a service to become ready with success", func(t *testing.T) {
-		buf := new(bytes.Buffer)
-		err := client.WaitForService(serviceName, 60*time.Second, buf)
+		err := client.WaitForService(serviceName, 60*time.Second, wait.NoopProgressHandler{})
 		assert.NilError(t, err)
 	})
 }

pkg/wait/progress_handler.go

@@ -0,0 +1,87 @@
+// 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 wait
+
+import (
+	"fmt"
+	"io"
+)
+
+// A callback which is informed about the progress of an operation
+type ProgressHandler interface {
+	// Called when the operation starts
+	Start()
+
+	// Called with a percentage indicating completion
+	// The argument will be in the range 0..100 if the progress percentage can be calculated
+	// or -1 if only an ongoing operation should be indicated
+	Tic(complete int)
+
+	// Called when operation finished sucessfully
+	Success()
+
+	// Called when the operation failed with the erro
+	Fail(err error)
+}
+
+// No operation progress handler which just stays silent
+type NoopProgressHandler struct{}
+
+func (w NoopProgressHandler) Start()     {}
+func (w NoopProgressHandler) Tic(int)    {}
+func (w NoopProgressHandler) Fail(error) {}
+func (w NoopProgressHandler) Success()   {}
+
+// Standard progress handler for printing out progress to a given writer
+type simpleProgressHandler struct {
+	out          io.Writer
+	startLabel   string
+	ticMark      string
+	errorLabel   string
+	successLabel string
+}
+
+// Create a new progress handler which writes out to the given stream.
+// The label will be printed right after start, error & success are terminal message depending on the outcome
+// In case of an error, the error itself is not printed as it is supposed that the calling function
+// will deal with the error and eventually print it out.
+func NewSimpleProgressHandler(out io.Writer, label string, tic string, error string, success string) ProgressHandler {
+	return &simpleProgressHandler{out, label, tic, error, success}
+}
+
+// Print our intial label
+func (w *simpleProgressHandler) Start() {
+	fmt.Fprint(w.out, w.startLabel)
+}
+
+// Tic progress
+func (w *simpleProgressHandler) Tic(complete int) {
+	if complete < 0 {
+		fmt.Fprint(w.out, w.ticMark)
+	} else {
+		fmt.Fprintf(w.out, " %d%%", complete)
+	}
+}
+
+// Printout ERROR label, but not the error.
+// The error will be printed out later anyway
+func (w *simpleProgressHandler) Fail(err error) {
+	fmt.Fprintln(w.out, w.errorLabel)
+}
+
+// Printout success label
+func (w *simpleProgressHandler) Success() {
+	fmt.Fprintln(w.out, w.successLabel)
+}

pkg/wait/progress_handler_test.go

@@ -0,0 +1,44 @@
+// 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 wait
+
+import (
+	"bytes"
+	"errors"
+	"strings"
+	"testing"
+
+	"gotest.tools/assert"
+)
+
+func TestSimpleProgressHandler(t *testing.T) {
+	buf := new(bytes.Buffer)
+	ph := NewSimpleProgressHandler(buf, "START", ".", "ERROR", "OK")
+	ph.Start()
+	assert.Assert(t, strings.Contains(buf.String(), "START"))
+	ph.Tic(-1)
+	assert.Assert(t, strings.Contains(buf.String(), "."))
+	ph.Tic(-1)
+	assert.Assert(t, strings.Contains(buf.String(), ".."))
+	ph.Tic(8)
+	assert.Assert(t, strings.Contains(buf.String(), " 8%"))
+	ph.Tic(42)
+	assert.Assert(t, strings.Contains(buf.String(), " 8% 42%"), buf.String())
+	ph.Fail(errors.New("foobar"))
+	assert.Assert(t, strings.Contains(buf.String(), "ERROR\n"))
+	assert.Assert(t, !strings.Contains(buf.String(), "foobar"))
+	ph.Success()
+	assert.Assert(t, strings.Contains(buf.String(), "OK\n"), buf.String())
+}

pkg/wait/wait_for_ready.go

@@ -31,16 +31,15 @@ import (
 type waitForReadyConfig struct {
 	watchFunc           WatchFunc
 	conditionsExtractor ConditionsExtractor
-	kind                string
 }
 
 // Interface used for waiting of a resource of a given name to reach a definitive
 // state in its "Ready" condition.
 type WaitForReady interface {
 
 	// Wait on resource the resource with this name until a given timeout
-	// and write status out on writer
-	Wait(name string, timeout time.Duration, out io.Writer) error
+	// Use an optional progresshandler to indicate progress (only the first progress handler is used)
+	Wait(name string, timeout time.Duration, progressHandlers ...ProgressHandler) error
 }
 
 // Create watch which is used when waiting for Ready condition
@@ -50,9 +49,8 @@ type WatchFunc func(opts v1.ListOptions) (watch.Interface, error)
 type ConditionsExtractor func(obj runtime.Object) (apis.Conditions, error)
 
 // Constructor with resource type specific configuration
-func NewWaitForReady(kind string, watchFunc WatchFunc, extractor ConditionsExtractor) WaitForReady {
+func NewWaitForReady(watchFunc WatchFunc, extractor ConditionsExtractor) WaitForReady {
 	return &waitForReadyConfig{
-		kind:                kind,
 		watchFunc:           watchFunc,
 		conditionsExtractor: extractor,
 	}
@@ -62,34 +60,45 @@ func NewWaitForReady(kind string, watchFunc WatchFunc, extractor ConditionsExtra
 // `watchFunc` creates the actual watch, `kind` is the type what your are watching for
 // (e.g. "service"), `timeout` is a timeout after which the watch should be cancelled if no
 // target state has been entered yet and `out` is used for printing out status messages
-func (w *waitForReadyConfig) Wait(name string, timeout time.Duration, out io.Writer) error {
+func (w *waitForReadyConfig) Wait(name string, timeout time.Duration, progressHandlers ...ProgressHandler) error {
 	opts := v1.ListOptions{
 		FieldSelector: fields.OneTermEqualSelector("metadata.name", name).String(),
 	}
 	addWatchTimeout(&opts, timeout)
 
-	fmt.Fprintf(out, "Waiting for %s '%s' to become ready ... ", w.kind, name)
-	flush(out)
+	var ph ProgressHandler
+	switch len(progressHandlers) {
+	case 0:
+		ph = NoopProgressHandler{}
+	case 1:
+		ph = progressHandlers[0]
+	default:
+		return fmt.Errorf("only one progresshandler allowed, but found %d (internal error)", len(progressHandlers))
+	}
+
+	ph.Start()
 
 	floatingTimeout := timeout
 	for {
 		start := time.Now()
 		retry, timeoutReached, err := w.waitForReadyCondition(opts, name, floatingTimeout)
 		if err != nil {
-			fmt.Fprintln(out)
+			ph.Fail(err)
 			return err
 		}
 		floatingTimeout = floatingTimeout - time.Since(start)
 		if timeoutReached || floatingTimeout < 0 {
-			return fmt.Errorf("timeout: %s '%s' not ready after %d seconds", w.kind, name, int(timeout/time.Second))
+			err := fmt.Errorf("timeout: '%s' not ready after %d seconds", name, int(timeout/time.Second))
+			ph.Fail(err)
+			return err
 		}
 
 		if retry {
 			// restart loop
 			continue
 		}
 
-		fmt.Fprintln(out, "OK")
+		ph.Success()
 		return nil
 	}
 }