diff --git a/docs/content/en/docs/workflows/ci-cd.md b/docs/content/en/docs/workflows/ci-cd.md index 1d0a4ed9411..584080f8d00 100644 --- a/docs/content/en/docs/workflows/ci-cd.md +++ b/docs/content/en/docs/workflows/ci-cd.md @@ -36,6 +36,10 @@ healthy before proceeding with the next steps in the pipeline. `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`. {{}} To determine if a `Deployment` resource is up and running, Skaffold relies on `kubectl rollout status` to obtain its status. diff --git a/pkg/skaffold/runner/deploy.go b/pkg/skaffold/runner/deploy.go index 96cd5ed04be..652559ed4b9 100644 --- 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/runcontext/context.go b/pkg/skaffold/runner/runcontext/context.go index 6c1b0504662..aded74592f0 100644 --- a/pkg/skaffold/runner/runcontext/context.go +++ b/pkg/skaffold/runner/runcontext/context.go @@ -110,16 +110,26 @@ func (ps Pipelines) TestCases() []*latest.TestCase { return tests } -func (ps Pipelines) StatusCheck() *bool { - var sc *bool - // set the group status check to disabled if any pipeline has StatusCheck - // set to false. +func (ps Pipelines) StatusCheck() (*bool, error) { + var enabled, disabled bool for _, p := range ps.pipelines { - if p.Deploy.StatusCheck != nil && !*p.Deploy.StatusCheck { - sc = util.BoolPtr(false) + if p.Deploy.StatusCheck != nil { + if *p.Deploy.StatusCheck { + enabled = true + } else { + disabled = true + } } } - return sc + 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 { @@ -158,16 +168,19 @@ func (rc *RunContext) Deployers() []latest.DeployType { return rc.Pipelines.Depl func (rc *RunContext) TestCases() []*latest.TestCase { return rc.Pipelines.TestCases() } -func (rc *RunContext) StatusCheck() bool { +func (rc *RunContext) StatusCheck() (*bool, error) { scOpts := rc.Opts.StatusCheck.Value() - scConfig := rc.Pipelines.StatusCheck() + scConfig, err := rc.Pipelines.StatusCheck() + if err != nil { + return nil, err + } switch { case scOpts != nil: - return *scOpts + return util.BoolPtr(*scOpts), nil case scConfig != nil: - return *scConfig + return util.BoolPtr(*scConfig), nil default: - return true + return util.BoolPtr(true), nil } } diff --git a/pkg/skaffold/runner/runcontext/context_test.go b/pkg/skaffold/runner/runcontext/context_test.go index 201c02c04fe..11f63a4a3fa 100644 --- a/pkg/skaffold/runner/runcontext/context_test.go +++ b/pkg/skaffold/runner/runcontext/context_test.go @@ -19,6 +19,8 @@ package runcontext import ( "testing" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest" + "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.Pipeline{ + { + Deploy: latest.DeployConfig{ + StatusCheck: test.statusCheckP1, + }, + }, + { + Deploy: latest.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) + }) + } +}