Add StatusCheck field to skaffold.yaml (#4904) (#5706)

* Add StatusCheck field to skaffold.yaml (#4904) * Change StatusCheck field from bool to *bool * Fail if one pipeline has StatusCheck set to true and another has it set to false * Move new statusCheck field from v2beta15.json to v2beta16.json * Short circuit the `enabled && disabled` StatusCheck check
GoogleContainerTools · Apr 30, 2021 · ebe2f6d · ebe2f6d
1 parent 3c7ce0a
commit ebe2f6d
Show file tree

Hide file tree

Showing 7 changed files with 172 additions and 27 deletions.
diff --git a/docs/content/en/docs/workflows/ci-cd.md b/docs/content/en/docs/workflows/ci-cd.md
@@ -33,7 +33,13 @@ This feature can be very useful in Continuous Delivery pipelines to ensure that
 healthy before proceeding with the next steps in the pipeline.
 
 {{<alert title="Note">}}
-`healthcheck` is enabled by default; it can be disabled with the `--status-check=false` flag.
+`healthcheck` is enabled by default; it can be disabled with the `--status-check=false`
+flag, or by setting the `statusCheck` field of the deployment config stanza in
+the `skaffold.yaml` to false.
+
+If the `skaffold.yaml` contains multiple pipelines, it is invalid for one to
+have `statusCheck` explicitly set to `true` and a second to have `statusCheck`
+explicitly set to `false`.
 {{</alert>}}
 
 To determine if a `Deployment` resource is up and running, Skaffold relies on `kubectl rollout status` to obtain its status.
@@ -59,7 +65,6 @@ the time specified by [`progressDeadlineSeconds`](https://kubernetes.io/docs/con
 from the deployment configuration.
 
 If the `Deployment.spec.progressDeadlineSeconds` is not set, Skaffold will either wait for
-
 the time specified in the `statusCheckDeadlineSeconds` field of the deployment config stanza in the `skaffold.yaml`, or
 default to 10 minutes if this is not specified.
 

diff --git a/docs/content/en/schemas/v2beta16.json b/docs/content/en/schemas/v2beta16.json
@@ -1047,6 +1047,11 @@
           "description": "configures how container logs are printed as a result of a deployment.",
           "x-intellij-html-description": "configures how container logs are printed as a result of a deployment."
         },
+        "statusCheck": {
+          "type": "boolean",
+          "description": "*beta* enables waiting for deployments to stabilize.",
+          "x-intellij-html-description": "<em>beta</em> enables waiting for deployments to stabilize."
+        },
         "statusCheckDeadlineSeconds": {
           "type": "integer",
           "description": "*beta* deadline for deployments to stabilize in seconds.",
@@ -1058,6 +1063,7 @@
         "kpt",
         "kubectl",
         "kustomize",
+        "statusCheck",
         "statusCheckDeadlineSeconds",
         "kubeContext",
         "logs"

diff --git a/pkg/skaffold/runner/deploy.go b/pkg/skaffold/runner/deploy.go
@@ -201,7 +201,11 @@ func failIfClusterIsNotReachable() error {
 
 func (r *SkaffoldRunner) performStatusCheck(ctx context.Context, out io.Writer) error {
 	// Check if we need to perform deploy status
-	if !r.runCtx.StatusCheck() {
+	enabled, err := r.runCtx.StatusCheck()
+	if err != nil {
+		return err
+	}
+	if enabled != nil && !*enabled {
 		return nil
 	}
 

diff --git a/pkg/skaffold/runner/deploy_test.go b/pkg/skaffold/runner/deploy_test.go
@@ -42,34 +42,71 @@ import (
 func TestDeploy(t *testing.T) {
 	expectedOutput := "Waiting for deployments to stabilize..."
 	tests := []struct {
-		description string
-		testBench   *TestBench
-		statusCheck config.BoolOrUndefined
-		shouldErr   bool
-		shouldWait  bool
+		description       string
+		testBench         *TestBench
+		statusCheckFlag   *bool // --status-check CLI flag
+		statusCheckConfig *bool // skaffold.yaml Deploy.StatusCheck field
+		shouldErr         bool
+		shouldWait        bool
 	}{
 		{
-			description: "deploy shd perform status check",
+			description: "deploy shd perform status check when statusCheck flag is unspecified, in-config value is unspecified",
 			testBench:   &TestBench{},
-			statusCheck: config.NewBoolOrUndefined(nil),
 			shouldWait:  true,
 		},
 		{
-			description: "deploy shd perform status check",
-			testBench:   &TestBench{},
-			statusCheck: config.NewBoolOrUndefined(util.BoolPtr(true)),
-			shouldWait:  true,
+			description:       "deploy shd not perform status check when statusCheck flag is unspecified, in-config value is false",
+			testBench:         &TestBench{},
+			statusCheckConfig: util.BoolPtr(false),
 		},
 		{
-			description: "deploy shd not perform status check",
-			testBench:   &TestBench{},
-			statusCheck: config.NewBoolOrUndefined(util.BoolPtr(false)),
+			description:       "deploy shd perform status check when statusCheck flag is unspecified, in-config value is true",
+			testBench:         &TestBench{},
+			statusCheckConfig: util.BoolPtr(true),
+			shouldWait:        true,
+		},
+		{
+			description:     "deploy shd not perform status check when statusCheck flag is false, in-config value is unspecified",
+			testBench:       &TestBench{},
+			statusCheckFlag: util.BoolPtr(false),
+		},
+		{
+			description:       "deploy shd not perform status check when statusCheck flag is false, in-config value is false",
+			testBench:         &TestBench{},
+			statusCheckFlag:   util.BoolPtr(false),
+			statusCheckConfig: util.BoolPtr(false),
+		},
+		{
+			description:       "deploy shd not perform status check when statusCheck flag is false, in-config value is true",
+			testBench:         &TestBench{},
+			statusCheckFlag:   util.BoolPtr(false),
+			statusCheckConfig: util.BoolPtr(true),
+		},
+		{
+			description:     "deploy shd perform status check when statusCheck flag is true, in-config value is unspecified",
+			testBench:       &TestBench{},
+			statusCheckFlag: util.BoolPtr(true),
+			shouldWait:      true,
+		},
+		{
+			description:       "deploy shd perform status check when statusCheck flag is true, in-config value is false",
+			testBench:         &TestBench{},
+			statusCheckFlag:   util.BoolPtr(true),
+			statusCheckConfig: util.BoolPtr(false),
+			shouldWait:        true,
+		},
+		{
+			description:       "deploy shd perform status check when statusCheck flag is true, in-config value is true",
+			testBench:         &TestBench{},
+			statusCheckFlag:   util.BoolPtr(true),
+			statusCheckConfig: util.BoolPtr(true),
+			shouldWait:        true,
 		},
 		{
-			description: "deploy shd not perform status check when deployer is in error",
-			testBench:   &TestBench{deployErrors: []error{errors.New("deploy error")}},
-			shouldErr:   true,
-			statusCheck: config.NewBoolOrUndefined(util.BoolPtr(true)),
+			description:     "deploy shd not perform status check when deployer is in error",
+			testBench:       &TestBench{deployErrors: []error{errors.New("deploy error")}},
+			shouldErr:       true,
+			statusCheckFlag: util.BoolPtr(true),
 		},
 	}
 
@@ -82,7 +119,8 @@ func TestDeploy(t *testing.T) {
 			})
 
 			runner := createRunner(t, test.testBench, nil, []*latest_v1.Artifact{{ImageName: "img1"}, {ImageName: "img2"}}, nil)
-			runner.runCtx.Opts.StatusCheck = test.statusCheck
+			runner.runCtx.Opts.StatusCheck = config.NewBoolOrUndefined(test.statusCheckFlag)
+			runner.runCtx.Pipelines.All()[0].Deploy.StatusCheck = test.statusCheckConfig
 			out := new(bytes.Buffer)
 
 			err := runner.Deploy(context.Background(), out, []graph.Artifact{

diff --git a/pkg/skaffold/runner/runcontext/context.go b/pkg/skaffold/runner/runcontext/context.go
@@ -110,6 +110,28 @@ func (ps Pipelines) TestCases() []*latest_v1.TestCase {
 	return tests
 }
 
+func (ps Pipelines) StatusCheck() (*bool, error) {
+	var enabled, disabled bool
+	for _, p := range ps.pipelines {
+		if p.Deploy.StatusCheck != nil {
+			if *p.Deploy.StatusCheck {
+				enabled = true
+			} else {
+				disabled = true
+			}
+			if enabled && disabled {
+				return nil, fmt.Errorf("cannot explicitly enable StatusCheck in one pipeline and explicitly disable it in another pipeline, see https://skaffold.dev/docs/workflows/ci-cd/#waiting-for-skaffold-deployments-using-healthcheck")
+			}
+		}
+	}
+	// set the group status check to disabled if any pipeline has StatusCheck
+	// set to false.
+	if disabled {
+		return util.BoolPtr(false), nil
+	}
+	return util.BoolPtr(true), nil
+}
+
 func (ps Pipelines) StatusCheckDeadlineSeconds() int {
 	c := 0
 	// set the group status check deadline to maximum of any individually specified value
@@ -146,12 +168,20 @@ func (rc *RunContext) Deployers() []latest_v1.DeployType { return rc.Pipelines.D
 
 func (rc *RunContext) TestCases() []*latest_v1.TestCase { return rc.Pipelines.TestCases() }
 
-func (rc *RunContext) StatusCheck() bool {
-	sc := rc.Opts.StatusCheck.Value()
-	if sc == nil {
-		return true
+func (rc *RunContext) StatusCheck() (*bool, error) {
+	scOpts := rc.Opts.StatusCheck.Value()
+	scConfig, err := rc.Pipelines.StatusCheck()
+	if err != nil {
+		return nil, err
+	}
+	switch {
+	case scOpts != nil:
+		return util.BoolPtr(*scOpts), nil
+	case scConfig != nil:
+		return util.BoolPtr(*scConfig), nil
+	default:
+		return util.BoolPtr(true), nil
 	}
-	return *sc
 }
 
 func (rc *RunContext) StatusCheckDeadlineSeconds() int {

diff --git a/pkg/skaffold/runner/runcontext/context_test.go b/pkg/skaffold/runner/runcontext/context_test.go
@@ -19,6 +19,8 @@ package runcontext
 import (
 	"testing"
 
+	latest_v1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
 	"github.com/GoogleContainerTools/skaffold/testutil"
 )
 
@@ -89,3 +91,60 @@ func TestRunContext_UpdateNamespaces(t *testing.T) {
 		})
 	}
 }
+
+func TestPipelines_StatusCheck(t *testing.T) {
+	tests := []struct {
+		description   string
+		statusCheckP1 *bool
+		statusCheckP2 *bool
+		wantEnabled   *bool
+		wantErr       bool
+	}{
+		{
+			description: "both pipelines' statusCheck values are unspecified",
+			wantEnabled: util.BoolPtr(true),
+		},
+		{
+			description:   "one pipeline's statusCheck value is true",
+			statusCheckP1: util.BoolPtr(true),
+			wantEnabled:   util.BoolPtr(true),
+		},
+		{
+			description:   "one pipeline's statusCheck value is false",
+			statusCheckP1: util.BoolPtr(false),
+			wantEnabled:   util.BoolPtr(false),
+		},
+		{
+			description:   "one pipeline's statusCheck value is true, one pipeline's statusCheck value is false",
+			statusCheckP1: util.BoolPtr(true),
+			statusCheckP2: util.BoolPtr(false),
+			wantErr:       true,
+		},
+	}
+	for _, test := range tests {
+		testutil.Run(t, test.description, func(t *testutil.T) {
+			p := &Pipelines{
+				pipelines: []latest_v1.Pipeline{
+					{
+						Deploy: latest_v1.DeployConfig{
+							StatusCheck: test.statusCheckP1,
+						},
+					},
+					{
+						Deploy: latest_v1.DeployConfig{
+							StatusCheck: test.statusCheckP2,
+						},
+					},
+				},
+			}
+			gotEnabled, err := p.StatusCheck()
+			if err != nil && !test.wantErr {
+				t.Errorf("p.StatusCheck() got error %v, want no error", err)
+			}
+			if err == nil && test.wantErr {
+				t.Errorf("p.StatusCheck() got no error, want error")
+			}
+			t.CheckDeepEqual(test.wantEnabled, gotEnabled)
+		})
+	}
+}
diff --git a/pkg/skaffold/schema/latest/v1/config.go b/pkg/skaffold/schema/latest/v1/config.go
@@ -497,6 +497,9 @@ type TestCase struct {
 type DeployConfig struct {
 	DeployType `yaml:",inline"`
 
+	// StatusCheck *beta* enables waiting for deployments to stabilize.
+	StatusCheck *bool `yaml:"statusCheck,omitempty"`
+
 	// StatusCheckDeadlineSeconds *beta* is the deadline for deployments to stabilize in seconds.
 	StatusCheckDeadlineSeconds int `yaml:"statusCheckDeadlineSeconds,omitempty"`