Add logic for creating default deployer in runner

pkg/skaffold/deploy/status/status_check.go

@@ -42,7 +42,8 @@ import (
 )
 
 var (
-	defaultStatusCheckDeadline = 10 * time.Minute
+	// DefaultStatusCheckDeadline is the default timeout for resource status checks
+	DefaultStatusCheckDeadline = 10 * time.Minute
 
 	// Poll period for checking set to 1 second
 	defaultPollPeriodInMilliseconds = 1000
@@ -235,7 +236,7 @@ func getDeadline(d int) time.Duration {
 	if d > 0 {
 		return time.Duration(d) * time.Second
 	}
-	return defaultStatusCheckDeadline
+	return DefaultStatusCheckDeadline
 }
 
 func (s statusChecker) printStatusCheckSummary(out io.Writer, r *resource.Deployment, c counter) {

pkg/skaffold/runner/new.go

@@ -18,6 +18,7 @@ package runner
 
 import (
 	"context"
+	"errors"
 	"fmt"
 
 	"github.com/sirupsen/logrus"
@@ -34,6 +35,7 @@ import (
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/kubectl"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/kustomize"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/label"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/status"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/event"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/filemon"
 	pkgkubectl "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubectl"
@@ -44,6 +46,7 @@ import (
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/sync"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/test"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/trigger"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
 )
 
 // NewForConfig returns a new SkaffoldRunner for a SkaffoldConfig
@@ -233,6 +236,107 @@ func getSyncer(cfg sync.Config) sync.Syncer {
 	return sync.NewSyncer(cfg)
 }
 
+/*
+The "default deployer" is used in `skaffold apply`, which uses a `kubectl` deployer to actuate resources
+on a cluster regardless of provided deployer configuration in the skaffold.yaml.
+The default deployer will honor a select set of deploy configuration from an existing skaffold.yaml:
+	- deploy.StatusCheckDeadlineSeconds
+	- deploy.Logs.Prefix
+	- deploy.Kubectl.Flags
+	- deploy.Kubectl.DefaultNamespace
+	- deploy.Kustomize.Flags
+	- deploy.Kustomize.DefaultNamespace
+For a multi-config project, we do not currently support resolving conflicts between differing sets of this deploy configuration.
+Therefore, in this function we do implicit validation of the provided configuration, and fail if any conflict cannot be resolved.
+*/
+func getDefaultDeployer(runCtx *runcontext.RunContext, labels map[string]string) (deploy.Deployer, error) {
+	deployCfgs := runCtx.DeployConfigs()
+
+	var kFlags *latest.KubectlFlags
+	var logPrefix string
+	var defaultNamespace *string
+	var kubeContext string
+	statusCheckTimeout := -1
+
+	for _, d := range deployCfgs {
+		if d.KubeContext != "" {
+			if kubeContext != "" {
+				return nil, errors.New("cannot resolve active Kubernetes context - multiple contexts configured in skaffold.yaml")
+			}
+			kubeContext = d.KubeContext
+		}
+		if d.StatusCheckDeadlineSeconds != 0 && d.StatusCheckDeadlineSeconds != int(status.DefaultStatusCheckDeadline.Seconds()) {
+			if statusCheckTimeout != -1 && statusCheckTimeout != d.StatusCheckDeadlineSeconds {
+				return nil, fmt.Errorf("found multiple status check timeouts in skaffold.yaml (not supported in `skaffold apply`): %d, %d", statusCheckTimeout, d.StatusCheckDeadlineSeconds)
+			}
+			statusCheckTimeout = d.StatusCheckDeadlineSeconds
+		}
+		if d.Logs.Prefix != "" {
+			if logPrefix != "" && logPrefix != d.Logs.Prefix {
+				return nil, fmt.Errorf("found multiple log prefixes in skaffold.yaml (not supported in `skaffold apply`): %s, %s", logPrefix, d.Logs.Prefix)
+			}
+			logPrefix = d.Logs.Prefix
+		}
+		var currentDefaultNamespace *string
+		var currentKubectlFlags latest.KubectlFlags
+		if d.KubectlDeploy != nil {
+			currentDefaultNamespace = d.KubectlDeploy.DefaultNamespace
+			currentKubectlFlags = d.KubectlDeploy.Flags
+		}
+		if d.KustomizeDeploy != nil {
+			currentDefaultNamespace = d.KustomizeDeploy.DefaultNamespace
+			currentKubectlFlags = d.KustomizeDeploy.Flags
+		}
+		if kFlags == nil {
+			kFlags = &currentKubectlFlags
+		}
+		if err := validateKubectlFlags(kFlags, currentKubectlFlags); err != nil {
+			return nil, err
+		}
+		if currentDefaultNamespace != nil {
+			if defaultNamespace != nil && *defaultNamespace != *currentDefaultNamespace {
+				return nil, fmt.Errorf("found multiple namespaces in skaffold.yaml (not supported in `skaffold apply`): %s, %s", *defaultNamespace, *currentDefaultNamespace)
+			}
+			defaultNamespace = currentDefaultNamespace
+		}
+	}
+	if kFlags == nil {
+		kFlags = &latest.KubectlFlags{}
+	}
+	k := &latest.KubectlDeploy{
+		Flags:            *kFlags,
+		DefaultNamespace: defaultNamespace,
+	}
+	defaultDeployer, err := kubectl.NewDeployer(runCtx, labels, k)
+	if err != nil {
+		return nil, fmt.Errorf("instantiating default kubectl deployer: %w", err)
+	}
+	return defaultDeployer, nil
+}
+
+func validateKubectlFlags(flags *latest.KubectlFlags, additional latest.KubectlFlags) error {
+	errStr := "conflicting sets of kubectl deploy flags not supported in `skaffold apply` (flag: %s)"
+	if additional.DisableValidation != flags.DisableValidation {
+		return fmt.Errorf(errStr, additional.DisableValidation)
+	}
+	for _, flag := range additional.Apply {
+		if !util.StrSliceContains(flags.Apply, flag) {
+			return fmt.Errorf(errStr, flag)
+		}
+	}
+	for _, flag := range additional.Delete {
+		if !util.StrSliceContains(flags.Delete, flag) {
+			return fmt.Errorf(errStr, flag)
+		}
+	}
+	for _, flag := range additional.Global {
+		if !util.StrSliceContains(flags.Global, flag) {
+			return fmt.Errorf(errStr, flag)
+		}
+	}
+	return nil
+}
+
 func getDeployer(runCtx *runcontext.RunContext, labels map[string]string) (deploy.Deployer, error) {
 	deployerCfg := runCtx.Deployers()
 

pkg/skaffold/runner/new_test.go

@@ -124,3 +124,117 @@ func TestGetDeployer(tOuter *testing.T) {
 		}
 	})
 }
+
+func TestGetDefaultDeployer(tOuter *testing.T) {
+	testutil.Run(tOuter, "TestGetDeployer", func(t *testutil.T) {
+		tests := []struct {
+			name      string
+			cfgs      []latest.DeployType
+			expected  *kubectl.Deployer
+			shouldErr bool
+		}{
+			{
+				name: "one config with kubectl deploy",
+				cfgs: []latest.DeployType{{
+					KubectlDeploy: &latest.KubectlDeploy{},
+				}},
+				expected: t.RequireNonNilResult(kubectl.NewDeployer(&runcontext.RunContext{
+					Pipelines: runcontext.NewPipelines([]latest.Pipeline{{}}),
+				}, nil, &latest.KubectlDeploy{
+					Flags: latest.KubectlFlags{},
+				})).(*kubectl.Deployer),
+			},
+			{
+				name: "one config with kubectl deploy, with flags",
+				cfgs: []latest.DeployType{{
+					KubectlDeploy: &latest.KubectlDeploy{
+						Flags: latest.KubectlFlags{
+							Apply:  []string{"--foo"},
+							Global: []string{"--bar"},
+						},
+					},
+				}},
+				expected: t.RequireNonNilResult(kubectl.NewDeployer(&runcontext.RunContext{
+					Pipelines: runcontext.NewPipelines([]latest.Pipeline{{}}),
+				}, nil, &latest.KubectlDeploy{
+					Flags: latest.KubectlFlags{
+						Apply:  []string{"--foo"},
+						Global: []string{"--bar"},
+					},
+				})).(*kubectl.Deployer),
+			},
+			{
+				name: "two kubectl configs with mismatched flags should fail",
+				cfgs: []latest.DeployType{
+					{
+						KubectlDeploy: &latest.KubectlDeploy{
+							Flags: latest.KubectlFlags{
+								Apply: []string{"--foo"},
+							},
+						},
+					},
+					{
+						KubectlDeploy: &latest.KubectlDeploy{
+							Flags: latest.KubectlFlags{
+								Apply: []string{"--bar"},
+							},
+						},
+					},
+				},
+				shouldErr: true,
+			},
+			{
+				name: "one config with helm deploy",
+				cfgs: []latest.DeployType{{
+					HelmDeploy: &latest.HelmDeploy{},
+				}},
+				expected: t.RequireNonNilResult(kubectl.NewDeployer(&runcontext.RunContext{
+					Pipelines: runcontext.NewPipelines([]latest.Pipeline{{}}),
+				}, nil, &latest.KubectlDeploy{
+					Flags: latest.KubectlFlags{},
+				})).(*kubectl.Deployer),
+			},
+			{
+				name: "one config with kustomize deploy",
+				cfgs: []latest.DeployType{{
+					KustomizeDeploy: &latest.KustomizeDeploy{},
+				}},
+				expected: t.RequireNonNilResult(kubectl.NewDeployer(&runcontext.RunContext{
+					Pipelines: runcontext.NewPipelines([]latest.Pipeline{{}}),
+				}, nil, &latest.KubectlDeploy{
+					Flags: latest.KubectlFlags{},
+				})).(*kubectl.Deployer),
+			},
+		}
+
+		for _, test := range tests {
+			testutil.Run(tOuter, test.name, func(t *testutil.T) {
+				pipelines := []latest.Pipeline{}
+				for _, cfg := range test.cfgs {
+					pipelines = append(pipelines, latest.Pipeline{
+						Deploy: latest.DeployConfig{
+							DeployType: cfg,
+						},
+					})
+				}
+				deployer, err := getDefaultDeployer(&runcontext.RunContext{
+					Pipelines: runcontext.NewPipelines(pipelines),
+				}, nil)
+
+				t.CheckErrorAndFailNow(test.shouldErr, err)
+
+				// if we were expecting an error, this implies that the returned deployer is nil
+				// this error was checked in the previous call, so if we didn't fail there (i.e. the encountered error was correct),
+				// then the test is finished and we can continue.
+				if !test.shouldErr {
+					t.CheckTypeEquality(&kubectl.Deployer{}, deployer)
+
+					kDeployer := deployer.(*kubectl.Deployer)
+					if !reflect.DeepEqual(kDeployer, test.expected) {
+						t.Fail()
+					}
+				}
+			})
+		}
+	})
+}

pkg/skaffold/runner/runcontext/context.go

@@ -81,6 +81,14 @@ func (ps Pipelines) Artifacts() []*latest.Artifact {
 	return artifacts
 }
 
+func (ps Pipelines) DeployConfigs() []latest.DeployConfig {
+	var cfgs []latest.DeployConfig
+	for _, p := range ps.pipelines {
+		cfgs = append(cfgs, p.Deploy)
+	}
+	return cfgs
+}
+
 func (ps Pipelines) Deployers() []latest.DeployType {
 	var deployers []latest.DeployType
 	for _, p := range ps.pipelines {
@@ -127,6 +135,8 @@ func (rc *RunContext) PortForwardResources() []*latest.PortForwardResource {
 
 func (rc *RunContext) Artifacts() []*latest.Artifact { return rc.Pipelines.Artifacts() }
 
+func (rc *RunContext) DeployConfigs() []latest.DeployConfig { return rc.Pipelines.DeployConfigs() }
+
 func (rc *RunContext) Deployers() []latest.DeployType { return rc.Pipelines.Deployers() }
 
 func (rc *RunContext) TestCases() []*latest.TestCase { return rc.Pipelines.TestCases() }

testutil/util.go

@@ -140,6 +140,15 @@ func (t *T) CheckError(shouldErr bool, err error) {
 	CheckError(t.T, shouldErr, err)
 }
 
+// CheckErrorAndFailNow checks that the provided error complies with whether or not we expect an error
+// and fails the test execution immediately if it does not.
+// Useful for testing functions which return (obj interface{}, e error) and subsequent checks operate on `obj`
+// assuming that it is not nil.
+func (t *T) CheckErrorAndFailNow(shouldErr bool, err error) {
+	t.Helper()
+	CheckErrorAndFailNow(t.T, shouldErr, err)
+}
+
 // CheckErrorContains checks that an error is not nil and contains
 // a given message.
 func (t *T) CheckErrorContains(message string, err error) {
@@ -297,6 +306,13 @@ func CheckError(t *testing.T, shouldErr bool, err error) {
 	}
 }
 
+func CheckErrorAndFailNow(t *testing.T, shouldErr bool, err error) {
+	t.Helper()
+	if err := checkErr(shouldErr, err); err != nil {
+		t.Fatal(err)
+	}
+}
+
 func EnsureTestPanicked(t *testing.T) {
 	if recover() == nil {
 		t.Errorf("should have panicked")