Merge pull request #2811 from tejal29/refactor-break-1

Print status check summary when a status check is completed.
GoogleContainerTools · Sep 9, 2019 · 9460f60 · 9460f60
2 parents 9d0fcd6 + b91d599
commit 9460f60
Show file tree

Hide file tree

Showing 4 changed files with 103 additions and 7 deletions.
diff --git a/pkg/skaffold/deploy/status_check.go b/pkg/skaffold/deploy/status_check.go
@@ -19,10 +19,13 @@ package deploy
 import (
 	"context"
 	"fmt"
+	"io"
 	"strings"
 	"sync"
+	"sync/atomic"
 	"time"
 
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/color"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -43,7 +46,16 @@ var (
 	executeRolloutStatus = getRollOutStatus
 )
 
-func StatusCheck(ctx context.Context, defaultLabeller *DefaultLabeller, runCtx *runcontext.RunContext) error {
+const (
+	tabHeader = " -"
+)
+
+type counter struct {
+	total   int
+	pending int32
+}
+
+func StatusCheck(ctx context.Context, defaultLabeller *DefaultLabeller, runCtx *runcontext.RunContext, out io.Writer) error {
 	client, err := pkgkubernetes.Client()
 	if err != nil {
 		return errors.Wrap(err, "getting kubernetes client")
@@ -59,14 +71,17 @@ func StatusCheck(ctx context.Context, defaultLabeller *DefaultLabeller, runCtx *
 	wg := sync.WaitGroup{}
 	// Its safe to use sync.Map without locks here as each subroutine adds a different key to the map.
 	syncMap := &sync.Map{}
-	kubeCtl := kubectl.NewFromRunContext(runCtx)
+
+	c := newCounter(len(dMap))
 
 	for dName, deadlineDuration := range dMap {
 		wg.Add(1)
 		go func(dName string, deadlineDuration time.Duration) {
 			defer wg.Done()
-			err := pollDeploymentRolloutStatus(ctx, kubeCtl, dName, deadlineDuration)
+			err := pollDeploymentRolloutStatus(ctx, kubectl.NewFromRunContext(runCtx), dName, deadlineDuration)
 			syncMap.Store(dName, err)
+			pending := c.markProcessed()
+			printStatusCheckSummary(dName, pending, c.total, err, out)
 		}(dName, deadlineDuration)
 	}
 
@@ -118,7 +133,7 @@ func pollDeploymentRolloutStatus(ctx context.Context, k *kubectl.CLI, dName stri
 }
 
 func getSkaffoldDeployStatus(m *sync.Map) error {
-	errorStrings := []string{}
+	var errorStrings []string
 	m.Range(func(k, v interface{}) bool {
 		if t, ok := v.(error); ok {
 			errorStrings = append(errorStrings, fmt.Sprintf("deployment %s failed due to %s", k, t.Error()))
@@ -143,3 +158,39 @@ func getDeadline(d int) time.Duration {
 	}
 	return defaultStatusCheckDeadline
 }
+
+func printStatusCheckSummary(dName string, pending int, total int, err error, out io.Writer) {
+	status := fmt.Sprintf("%s deployment/%s", tabHeader, dName)
+	if err != nil {
+		status = fmt.Sprintf("%s failed.%s Error: %s.",
+			status,
+			trimNewLine(getPendingMessage(pending, total)),
+			trimNewLine(err.Error()),
+		)
+	} else {
+		status = fmt.Sprintf("%s is ready.%s", status, getPendingMessage(pending, total))
+	}
+	color.Default.Fprintln(out, status)
+}
+
+func getPendingMessage(pending int, total int) string {
+	if pending > 0 {
+		return fmt.Sprintf(" [%d/%d deployment(s) still pending]", pending, total)
+	}
+	return ""
+}
+
+func trimNewLine(msg string) string {
+	return strings.TrimSuffix(msg, "\n")
+}
+
+func newCounter(i int) *counter {
+	return &counter{
+		total:   i,
+		pending: int32(i),
+	}
+}
+
+func (c *counter) markProcessed() int {
+	return int(atomic.AddInt32(&c.pending, -1))
+}
diff --git a/pkg/skaffold/deploy/status_check_test.go b/pkg/skaffold/deploy/status_check_test.go
@@ -17,6 +17,7 @@ limitations under the License.
 package deploy
 
 import (
+	"bytes"
 	"context"
 	"errors"
 	"fmt"
@@ -225,6 +226,7 @@ func TestPollDeploymentRolloutStatus(t *testing.T) {
 		testutil.Run(t, test.description, func(t *testutil.T) {
 			t.Override(&defaultPollPeriodInMilliseconds, 10)
 			t.Override(&util.DefaultExecCommand, test.commands)
+
 			cli := &kubectl.CLI{KubeContext: testKubeContext, Namespace: "test"}
 			err := pollDeploymentRolloutStatus(context.Background(), cli, "dep", time.Duration(test.duration)*time.Millisecond)
 			t.CheckError(test.shouldErr, err)
@@ -328,3 +330,45 @@ func TestGetRollOutStatus(t *testing.T) {
 		})
 	}
 }
+
+func TestPrintSummaryStatus(t *testing.T) {
+	tests := []struct {
+		description string
+		pending     int32
+		err         error
+		expected    string
+	}{
+		{
+			description: "no deployment left and current is in success",
+			pending:     0,
+			err:         nil,
+			expected:    " - deployment/dep is ready.\n",
+		},
+		{
+			description: "no deployment left and current is in error",
+			pending:     0,
+			err:         errors.New("context deadline expired"),
+			expected:    " - deployment/dep failed. Error: context deadline expired.\n",
+		},
+		{
+			description: "more than 1 deployment left and current is in success",
+			pending:     4,
+			err:         nil,
+			expected:    " - deployment/dep is ready. [4/10 deployment(s) still pending]\n",
+		},
+		{
+			description: "more than 1 deployment left and current is in error",
+			pending:     8,
+			err:         errors.New("context deadline expired"),
+			expected:    " - deployment/dep failed. [8/10 deployment(s) still pending] Error: context deadline expired.\n",
+		},
+	}
+
+	for _, test := range tests {
+		testutil.Run(t, test.description, func(t *testutil.T) {
+			out := new(bytes.Buffer)
+			printStatusCheckSummary("dep", int(test.pending), 10, test.err, out)
+			t.CheckDeepEqual(test.expected, out.String())
+		})
+	}
+}
diff --git a/pkg/skaffold/runner/deploy.go b/pkg/skaffold/runner/deploy.go
@@ -50,7 +50,7 @@ func (r *SkaffoldRunner) performStatusCheck(ctx context.Context, out io.Writer)
 	if r.runCtx.Opts.StatusCheck {
 		start := time.Now()
 		color.Default.Fprintln(out, "Waiting for deployments to stabilize")
-		err := statusCheck(ctx, r.defaultLabeller, r.runCtx)
+		err := statusCheck(ctx, r.defaultLabeller, r.runCtx, out)
 		if err != nil {
 			return err
 		}

diff --git a/pkg/skaffold/runner/deploy_test.go b/pkg/skaffold/runner/deploy_test.go
@@ -20,6 +20,7 @@ import (
 	"bytes"
 	"context"
 	"errors"
+	"io"
 	"io/ioutil"
 	"strings"
 	"testing"
@@ -59,7 +60,7 @@ func TestDeploy(t *testing.T) {
 		},
 	}
 
-	dummyStatusCheck := func(ctx context.Context, l *deploy.DefaultLabeller, runCtx *runcontext.RunContext) error {
+	dummyStatusCheck := func(context.Context, *deploy.DefaultLabeller, *runcontext.RunContext, io.Writer) error {
 		return nil
 	}
 	for _, test := range tests {
@@ -109,7 +110,7 @@ func TestDeployNamespace(t *testing.T) {
 		},
 	}
 
-	dummyStatusCheck := func(ctx context.Context, l *deploy.DefaultLabeller, runCtx *runcontext.RunContext) error {
+	dummyStatusCheck := func(context.Context, *deploy.DefaultLabeller, *runcontext.RunContext, io.Writer) error {
 		return nil
 	}
 	for _, test := range tests {