Reduce default status check deadline to 2 mins (#3687)

* Reduce default status deadline to 2 minutes * reduce deploy status check to 2 mins * fix lint * rename function and address 1 comment
GoogleContainerTools · Feb 20, 2020 · b436574 · b436574
1 parent 77361dc
commit b436574
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 5 deletions.
diff --git a/pkg/skaffold/deploy/status_check.go b/pkg/skaffold/deploy/status_check.go
@@ -38,7 +38,7 @@ import (
 )
 
 var (
-	defaultStatusCheckDeadline = time.Duration(10) * time.Minute
+	defaultStatusCheckDeadline = 2 * time.Minute
 
 	// Poll period for checking set to 100 milliseconds
 	defaultPollPeriodInMilliseconds = 100
@@ -69,8 +69,11 @@ func StatusCheck(ctx context.Context, defaultLabeller *DefaultLabeller, runCtx *
 		return errors.Wrap(err, "getting Kubernetes client")
 	}
 
-	deadline := getDeadline(runCtx.Cfg.Deploy.StatusCheckDeadlineSeconds)
-	deployments, err := getDeployments(client, runCtx.Opts.Namespace, defaultLabeller, deadline)
+	deployments, err := getDeployments(client, runCtx.Opts.Namespace, defaultLabeller,
+		getDeadline(runCtx.Cfg.Deploy.StatusCheckDeadlineSeconds))
+
+	deadline := statusCheckMaxDeadline(runCtx.Cfg.Deploy.StatusCheckDeadlineSeconds, deployments)
+
 	if err != nil {
 		return errors.Wrap(err, "could not fetch deployments")
 	}
@@ -110,7 +113,7 @@ func getDeployments(client kubernetes.Interface, ns string, l *DefaultLabeller,
 	deployments := make([]Resource, 0, len(deps.Items))
 	for _, d := range deps.Items {
 		var deadline time.Duration
-		if d.Spec.ProgressDeadlineSeconds == nil || *d.Spec.ProgressDeadlineSeconds > int32(deadlineDuration.Seconds()) {
+		if d.Spec.ProgressDeadlineSeconds == nil {
 			deadline = deadlineDuration
 		} else {
 			deadline = time.Duration(*d.Spec.ProgressDeadlineSeconds) * time.Second
@@ -257,3 +260,16 @@ func (c *resourceCounter) markProcessed(err error) resourceCounter {
 		pods:        &podCp,
 	}
 }
+
+func statusCheckMaxDeadline(value int, deployments []Resource) time.Duration {
+	if value > 0 {
+		return time.Duration(value) * time.Second
+	}
+	d := time.Duration(0)
+	for _, r := range deployments {
+		if r.Deadline() > d {
+			d = r.Deadline()
+		}
+	}
+	return d
+}
diff --git a/pkg/skaffold/deploy/status_check_test.go b/pkg/skaffold/deploy/status_check_test.go
@@ -89,7 +89,7 @@ func TestGetDeployments(t *testing.T) {
 				},
 			},
 			expected: []Resource{
-				resource.NewDeployment("dep1", "test", time.Duration(200)*time.Second),
+				resource.NewDeployment("dep1", "test", time.Duration(300)*time.Second),
 			},
 		},
 		{
@@ -483,3 +483,35 @@ func TestResourceMarkProcessed(t *testing.T) {
 		})
 	}
 }
+
+func TestGetStatusCheckDeadline(t *testing.T) {
+	tests := []struct {
+		description string
+		value       int
+		deps        []Resource
+		expected    time.Duration
+	}{
+		{
+			description: "no value specified",
+			deps: []Resource{
+				resource.NewDeployment("dep1", "test", time.Duration(10)*time.Second),
+				resource.NewDeployment("dep2", "test", time.Duration(20)*time.Second),
+			},
+			expected: time.Duration(20) * time.Second,
+		},
+		{
+			description: "value specified less than all other resources",
+			value:       5,
+			deps: []Resource{
+				resource.NewDeployment("dep1", "test", time.Duration(10)*time.Second),
+				resource.NewDeployment("dep2", "test", time.Duration(20)*time.Second),
+			},
+			expected: time.Duration(5) * time.Second,
+		},
+	}
+	for _, test := range tests {
+		testutil.Run(t, test.description, func(t *testutil.T) {
+			t.CheckDeepEqual(test.expected, statusCheckMaxDeadline(test.value, test.deps))
+		})
+	}
+}