diff --git a/cmd/skaffold/app/cmd/cmd.go b/cmd/skaffold/app/cmd/cmd.go index dfda8dbd9f6..c06297f998a 100644 --- a/cmd/skaffold/app/cmd/cmd.go +++ b/cmd/skaffold/app/cmd/cmd.go @@ -33,6 +33,7 @@ import ( sErrors "github.com/GoogleContainerTools/skaffold/pkg/skaffold/errors" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/instrumentation" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/instrumentation/prompt" + kubectx "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/context" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/output" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/server" @@ -95,6 +96,9 @@ func NewSkaffoldCommand(out, errOut io.Writer) *cobra.Command { return err } + // Setup kubeContext and kubeConfig + kubectx.ConfigureKubeConfig(opts.KubeConfig, opts.KubeContext) + // Start API Server shutdown, err := server.Initialize(opts) if err != nil { diff --git a/cmd/skaffold/app/cmd/runner.go b/cmd/skaffold/app/cmd/runner.go index f442d16e829..ed14d87f102 100644 --- a/cmd/skaffold/app/cmd/runner.go +++ b/cmd/skaffold/app/cmd/runner.go @@ -30,7 +30,6 @@ import ( "github.com/GoogleContainerTools/skaffold/pkg/skaffold/initializer" initConfig "github.com/GoogleContainerTools/skaffold/pkg/skaffold/initializer/config" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/instrumentation" - kubectx "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/context" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/output" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/parser" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner" @@ -80,9 +79,6 @@ func runContext(out io.Writer, opts config.SkaffoldOptions) (*runcontext.RunCont } setDefaultDeployer(configs) - // TODO: Should support per-config kubecontext. Right now we constrain all configs to define the same kubecontext. - kubectx.ConfigureKubeConfig(opts.KubeConfig, opts.KubeContext, configs[0].Deploy.KubeContext) - if err := validation.Process(configs); err != nil { return nil, nil, fmt.Errorf("invalid skaffold config: %w", err) } diff --git a/pkg/skaffold/deploy/helm/deploy.go b/pkg/skaffold/deploy/helm/deploy.go index 6fb6cc72a9f..8719cb65d9c 100644 --- a/pkg/skaffold/deploy/helm/deploy.go +++ b/pkg/skaffold/deploy/helm/deploy.go @@ -49,6 +49,7 @@ import ( "github.com/GoogleContainerTools/skaffold/pkg/skaffold/instrumentation" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/manifest" + kstatus "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/status" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/log" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/output" latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1" @@ -110,6 +111,7 @@ type Deployer struct { type Config interface { kubectl.Config + kstatus.Config IsMultiConfig() bool } @@ -141,7 +143,7 @@ func NewDeployer(cfg Config, labels map[string]string, provider deploy.Component accessor: provider.Accessor.GetKubernetesAccessor(podSelector), debugger: provider.Debugger.GetKubernetesDebugger(podSelector), logger: provider.Logger.GetKubernetesLogger(podSelector), - statusMonitor: provider.Monitor.GetKubernetesMonitor(), + statusMonitor: provider.Monitor.GetKubernetesMonitor(cfg), syncer: provider.Syncer.GetKubernetesSyncer(podSelector), originalImages: originalImages, kubeContext: cfg.GetKubeContext(), diff --git a/pkg/skaffold/deploy/kpt/kpt.go b/pkg/skaffold/deploy/kpt/kpt.go index 660f775778c..92954879831 100644 --- a/pkg/skaffold/deploy/kpt/kpt.go +++ b/pkg/skaffold/deploy/kpt/kpt.go @@ -44,6 +44,7 @@ import ( "github.com/GoogleContainerTools/skaffold/pkg/skaffold/instrumentation" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/manifest" + kstatus "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/status" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/log" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/output" latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1" @@ -92,6 +93,7 @@ type Deployer struct { type Config interface { kubectl.Config + kstatus.Config } // NewDeployer generates a new Deployer object contains the kptDeploy schema. @@ -103,7 +105,7 @@ func NewDeployer(cfg Config, labels map[string]string, provider deploy.Component accessor: provider.Accessor.GetKubernetesAccessor(podSelector), debugger: provider.Debugger.GetKubernetesDebugger(podSelector), logger: provider.Logger.GetKubernetesLogger(podSelector), - statusMonitor: provider.Monitor.GetKubernetesMonitor(), + statusMonitor: provider.Monitor.GetKubernetesMonitor(cfg), syncer: provider.Syncer.GetKubernetesSyncer(podSelector), insecureRegistries: cfg.GetInsecureRegistries(), labels: labels, diff --git a/pkg/skaffold/deploy/kubectl/cli.go b/pkg/skaffold/deploy/kubectl/cli.go index c676cbc4eb1..2adccc37436 100644 --- a/pkg/skaffold/deploy/kubectl/cli.go +++ b/pkg/skaffold/deploy/kubectl/cli.go @@ -32,6 +32,7 @@ import ( "github.com/GoogleContainerTools/skaffold/pkg/skaffold/instrumentation" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubectl" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/manifest" + kstatus "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/status" latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1" ) @@ -47,6 +48,7 @@ type CLI struct { type Config interface { kubectl.Config + kstatus.Config deploy.Config ForceDeploy() bool WaitForDeletions() config.WaitForDeletions diff --git a/pkg/skaffold/deploy/kubectl/kubectl.go b/pkg/skaffold/deploy/kubectl/kubectl.go index ba5283ecd69..5d8fed2108d 100644 --- a/pkg/skaffold/deploy/kubectl/kubectl.go +++ b/pkg/skaffold/deploy/kubectl/kubectl.go @@ -90,7 +90,7 @@ func NewDeployer(cfg Config, labels map[string]string, provider deploy.Component accessor: provider.Accessor.GetKubernetesAccessor(podSelector), debugger: provider.Debugger.GetKubernetesDebugger(podSelector), logger: provider.Logger.GetKubernetesLogger(podSelector), - statusMonitor: provider.Monitor.GetKubernetesMonitor(), + statusMonitor: provider.Monitor.GetKubernetesMonitor(cfg), syncer: provider.Syncer.GetKubernetesSyncer(podSelector), workingDir: cfg.GetWorkingDir(), globalConfig: cfg.GlobalConfig(), diff --git a/pkg/skaffold/deploy/kustomize/kustomize.go b/pkg/skaffold/deploy/kustomize/kustomize.go index 083d8d92f78..1e7a44571af 100644 --- a/pkg/skaffold/deploy/kustomize/kustomize.go +++ b/pkg/skaffold/deploy/kustomize/kustomize.go @@ -139,7 +139,7 @@ func NewDeployer(cfg kubectl.Config, labels map[string]string, provider deploy.C accessor: provider.Accessor.GetKubernetesAccessor(podSelector), debugger: provider.Debugger.GetKubernetesDebugger(podSelector), logger: provider.Logger.GetKubernetesLogger(podSelector), - statusMonitor: provider.Monitor.GetKubernetesMonitor(), + statusMonitor: provider.Monitor.GetKubernetesMonitor(cfg), syncer: provider.Syncer.GetKubernetesSyncer(podSelector), kubectl: kubectl, insecureRegistries: cfg.GetInsecureRegistries(), diff --git a/pkg/skaffold/kubernetes/context/context.go b/pkg/skaffold/kubernetes/context/context.go index 56c75e433a0..b35f922e4c5 100644 --- a/pkg/skaffold/kubernetes/context/context.go +++ b/pkg/skaffold/kubernetes/context/context.go @@ -44,21 +44,14 @@ var ( // When given, the firstCliValue always takes precedence over the yamlValue. // Changing the kube-context of a running Skaffold process is not supported, so // after the first call, the kube-context will be locked. -func ConfigureKubeConfig(cliKubeConfig, cliKubeContext, yamlKubeContext string) { - newKubeContext := yamlKubeContext - if cliKubeContext != "" { - newKubeContext = cliKubeContext - } +func ConfigureKubeConfig(cliKubeConfig, cliKubeContext string) { configureOnce.Do(func() { - kubeContext = newKubeContext + kubeContext = cliKubeContext kubeConfigFile = cliKubeConfig if kubeContext != "" { logrus.Infof("Activated kube-context %q", kubeContext) } }) - if kubeContext != newKubeContext { - logrus.Warn("Changing the kube-context is not supported after startup. Please restart Skaffold to take effect.") - } } // GetRestClientConfig returns a REST client config for API calls against the Kubernetes API. diff --git a/pkg/skaffold/kubernetes/context/context_test.go b/pkg/skaffold/kubernetes/context/context_test.go index 553d19886af..eb2c23122da 100644 --- a/pkg/skaffold/kubernetes/context/context_test.go +++ b/pkg/skaffold/kubernetes/context/context_test.go @@ -196,7 +196,7 @@ func TestGetRestClientConfig(t *testing.T) { func TestUseKubeContext(t *testing.T) { type invocation struct { - cliValue, yamlValue string + cliValue string } tests := []struct { name string @@ -208,24 +208,6 @@ func TestUseKubeContext(t *testing.T) { invocations: nil, expected: "", }, - { - name: "yaml value when no CLI value is given", - invocations: []invocation{{yamlValue: "context2"}}, - expected: "context2", - }, - { - name: "yaml value when no CLI value is given, first invocation persists", - invocations: []invocation{ - {yamlValue: "context-first"}, - {yamlValue: "context-second"}, - }, - expected: "context-first", - }, - { - name: "CLI value takes precedence", - invocations: []invocation{{cliValue: "context1", yamlValue: "context2"}}, - expected: "context1", - }, { name: "first CLI value takes precedence", invocations: []invocation{ @@ -234,45 +216,13 @@ func TestUseKubeContext(t *testing.T) { }, expected: "context-first", }, - { - name: "mixed CLI value and yaml value - I", - invocations: []invocation{ - {cliValue: "context-first"}, - {yamlValue: "context-second"}, - }, - expected: "context-first", - }, - { - name: "mixed CLI value and yaml value - II", - invocations: []invocation{ - {yamlValue: "context-first"}, - {cliValue: "context-second"}, - }, - expected: "context-first", - }, - { - name: "mixed CLI value and yaml value - III", - invocations: []invocation{ - {yamlValue: "context-first"}, - {cliValue: "context-second", yamlValue: "context-third"}, - }, - expected: "context-first", - }, - { - name: "mixed CLI value and yaml value - IV", - invocations: []invocation{ - {cliValue: "context-first", yamlValue: "context-second"}, - {cliValue: "context-third", yamlValue: "context-fourth"}, - }, - expected: "context-first", - }, } for _, test := range tests { testutil.Run(t, test.name, func(t *testutil.T) { kubeContext = "" for _, inv := range test.invocations { - ConfigureKubeConfig("", inv.cliValue, inv.yamlValue) + ConfigureKubeConfig("", inv.cliValue) } t.CheckDeepEqual(test.expected, kubeContext) diff --git a/pkg/skaffold/kubernetes/status/status_check.go b/pkg/skaffold/kubernetes/status/status_check.go index 8236ac416d2..b6b75937789 100644 --- a/pkg/skaffold/kubernetes/status/status_check.go +++ b/pkg/skaffold/kubernetes/status/status_check.go @@ -75,7 +75,7 @@ type Config interface { GetNamespaces() []string StatusCheckDeadlineSeconds() int Muted() config.Muted - StatusCheck() (*bool, error) + StatusCheck() *bool } // Monitor runs status checks for pods and deployments diff --git a/pkg/skaffold/runner/deployer.go b/pkg/skaffold/runner/deployer.go new file mode 100644 index 00000000000..7a6e4b5f040 --- /dev/null +++ b/pkg/skaffold/runner/deployer.go @@ -0,0 +1,206 @@ +/* +Copyright 2021 The Skaffold Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package runner + +import ( + "errors" + "fmt" + "strconv" + + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/helm" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/kpt" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/kubectl" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/kustomize" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/status" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext" + v1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/util" +) + +// deployerCtx encapsulates a given skaffold run context along with additional deployer constructs. +type deployerCtx struct { + *runcontext.RunContext + deploy v1.DeployConfig +} + +func (d *deployerCtx) GetKubeContext() string { + if d.deploy.KubeContext != "" { + return d.deploy.KubeContext + } + return d.RunContext.GetKubeContext() +} + +func (d *deployerCtx) StatusCheck() *bool { + // runcontext StatusCheck method returns the value set by the cli flag `--status-check` + // which overrides the value set in the individual configs. + if cliValue := d.RunContext.StatusCheck(); cliValue != nil { + return cliValue + } + return d.deploy.StatusCheck +} + +// GetDeployer creates a deployer from a given RunContext and deploy pipeline definitions. +func GetDeployer(runCtx *runcontext.RunContext, provider deploy.ComponentProvider, labels map[string]string) (deploy.Deployer, kubernetes.ImageListMux, error) { + var podSelectors kubernetes.ImageListMux + if runCtx.Opts.Apply { + return getDefaultDeployer(runCtx, provider, labels) + } + + deployerCfg := runCtx.Deployers() + + var deployers deploy.DeployerMux + for _, d := range deployerCfg { + dCtx := &deployerCtx{runCtx, d} + if d.HelmDeploy != nil { + h, podSelector, err := helm.NewDeployer(dCtx, labels, provider, d.HelmDeploy) + if err != nil { + return nil, nil, err + } + podSelectors = append(podSelectors, podSelector) + deployers = append(deployers, h) + } + + if d.KptDeploy != nil { + deployer, podSelector := kpt.NewDeployer(dCtx, labels, provider, d.KptDeploy) + podSelectors = append(podSelectors, podSelector) + deployers = append(deployers, deployer) + } + + if d.KubectlDeploy != nil { + deployer, podSelector, err := kubectl.NewDeployer(dCtx, labels, provider, d.KubectlDeploy) + if err != nil { + return nil, nil, err + } + podSelectors = append(podSelectors, podSelector) + deployers = append(deployers, deployer) + } + + if d.KustomizeDeploy != nil { + deployer, podSelector, err := kustomize.NewDeployer(dCtx, labels, provider, d.KustomizeDeploy) + if err != nil { + return nil, nil, err + } + podSelectors = append(podSelectors, podSelector) + deployers = append(deployers, deployer) + } + } + + return deployers, podSelectors, nil +} + +/* +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, provider deploy.ComponentProvider, labels map[string]string) (deploy.Deployer, kubernetes.ImageListMux, error) { + deployCfgs := runCtx.DeployConfigs() + + var kFlags *v1.KubectlFlags + var logPrefix string + var defaultNamespace *string + var kubeContext string + statusCheckTimeout := -1 + + for _, d := range deployCfgs { + if d.KubeContext != "" { + if kubeContext != "" && kubeContext != d.KubeContext { + return nil, 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, 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, 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 v1.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 = ¤tKubectlFlags + } + if err := validateKubectlFlags(kFlags, currentKubectlFlags); err != nil { + return nil, nil, err + } + if currentDefaultNamespace != nil { + if defaultNamespace != nil && *defaultNamespace != *currentDefaultNamespace { + return nil, nil, fmt.Errorf("found multiple namespaces in skaffold.yaml (not supported in `skaffold apply`): %s, %s", *defaultNamespace, *currentDefaultNamespace) + } + defaultNamespace = currentDefaultNamespace + } + } + if kFlags == nil { + kFlags = &v1.KubectlFlags{} + } + k := &v1.KubectlDeploy{ + Flags: *kFlags, + DefaultNamespace: defaultNamespace, + } + defaultDeployer, podSelector, err := kubectl.NewDeployer(runCtx, labels, provider, k) + if err != nil { + return nil, nil, fmt.Errorf("instantiating default kubectl deployer: %w", err) + } + return defaultDeployer, kubernetes.ImageListMux{podSelector}, nil +} + +func validateKubectlFlags(flags *v1.KubectlFlags, additional v1.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, strconv.FormatBool(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 +} diff --git a/pkg/skaffold/runner/deployer_test.go b/pkg/skaffold/runner/deployer_test.go new file mode 100644 index 00000000000..803ae15cc10 --- /dev/null +++ b/pkg/skaffold/runner/deployer_test.go @@ -0,0 +1,274 @@ +/* +Copyright 2021 The Skaffold Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package runner + +import ( + "reflect" + "testing" + + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/config" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/helm" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/kpt" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/kubectl" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/kustomize" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext" + latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/util" + "github.com/GoogleContainerTools/skaffold/testutil" +) + +func TestGetDeployer(tOuter *testing.T) { + testutil.Run(tOuter, "TestGetDeployer", func(t *testutil.T) { + tests := []struct { + description string + cfg latestV1.DeployType + helmVersion string + expected deploy.Deployer + apply bool + shouldErr bool + }{ + { + description: "no deployer", + expected: deploy.DeployerMux{}, + }, + { + description: "helm deployer with 3.0.0 version", + cfg: latestV1.DeployType{HelmDeploy: &latestV1.HelmDeploy{}}, + helmVersion: `version.BuildInfo{Version:"v3.0.0"}`, + expected: deploy.DeployerMux{ + &helm.Deployer{}, + }, + }, + { + description: "helm deployer with less than 3.0.0 version", + cfg: latestV1.DeployType{HelmDeploy: &latestV1.HelmDeploy{}}, + helmVersion: "2.0.0", + shouldErr: true, + }, + { + description: "kubectl deployer", + cfg: latestV1.DeployType{KubectlDeploy: &latestV1.KubectlDeploy{}}, + expected: deploy.DeployerMux{ + t.RequireNonNilResult(kubectl.NewDeployer(&runcontext.RunContext{ + Pipelines: runcontext.NewPipelines([]latestV1.Pipeline{{}}), + }, nil, deploy.NoopComponentProvider, &latestV1.KubectlDeploy{ + Flags: latestV1.KubectlFlags{}, + })).(deploy.Deployer), + }, + }, + { + description: "kustomize deployer", + cfg: latestV1.DeployType{KustomizeDeploy: &latestV1.KustomizeDeploy{}}, + expected: deploy.DeployerMux{ + t.RequireNonNilResult(kustomize.NewDeployer(&runcontext.RunContext{ + Pipelines: runcontext.NewPipelines([]latestV1.Pipeline{{}}), + }, nil, deploy.NoopComponentProvider, &latestV1.KustomizeDeploy{ + Flags: latestV1.KubectlFlags{}, + })).(deploy.Deployer), + }, + }, + { + description: "kpt deployer", + cfg: latestV1.DeployType{KptDeploy: &latestV1.KptDeploy{}}, + expected: deploy.DeployerMux{ + &kpt.Deployer{}, + }, + }, + { + description: "apply forces creation of kubectl deployer with kpt config", + cfg: latestV1.DeployType{KptDeploy: &latestV1.KptDeploy{}}, + apply: true, + expected: t.RequireNonNilResult(kubectl.NewDeployer(&runcontext.RunContext{ + Pipelines: runcontext.NewPipelines([]latestV1.Pipeline{{}}), + }, nil, deploy.NoopComponentProvider, &latestV1.KubectlDeploy{ + Flags: latestV1.KubectlFlags{}, + })).(deploy.Deployer), + }, + { + description: "apply forces creation of kubectl deployer with helm config", + cfg: latestV1.DeployType{HelmDeploy: &latestV1.HelmDeploy{}}, + helmVersion: `version.BuildInfo{Version:"v3.0.0"}`, + apply: true, + expected: t.RequireNonNilResult(kubectl.NewDeployer(&runcontext.RunContext{ + Pipelines: runcontext.NewPipelines([]latestV1.Pipeline{{}}), + }, nil, deploy.NoopComponentProvider, &latestV1.KubectlDeploy{ + Flags: latestV1.KubectlFlags{}, + })).(deploy.Deployer), + }, + { + description: "multiple deployers", + cfg: latestV1.DeployType{ + HelmDeploy: &latestV1.HelmDeploy{}, + KptDeploy: &latestV1.KptDeploy{}, + }, + helmVersion: `version.BuildInfo{Version:"v3.0.0"}`, + expected: deploy.DeployerMux{ + &helm.Deployer{}, + &kpt.Deployer{}, + }, + }, + } + for _, test := range tests { + testutil.Run(tOuter, test.description, func(t *testutil.T) { + if test.helmVersion != "" { + t.Override(&util.DefaultExecCommand, testutil.CmdRunWithOutput( + "helm version --client", + test.helmVersion, + )) + } + + deployer, _, err := GetDeployer(&runcontext.RunContext{ + Opts: config.SkaffoldOptions{ + Apply: test.apply, + }, + Pipelines: runcontext.NewPipelines([]latestV1.Pipeline{{ + Deploy: latestV1.DeployConfig{ + DeployType: test.cfg, + }, + }}), + }, deploy.NoopComponentProvider, nil) + + t.CheckError(test.shouldErr, err) + t.CheckTypeEquality(test.expected, deployer) + + if reflect.TypeOf(test.expected) == reflect.TypeOf(deploy.DeployerMux{}) { + expected := test.expected.(deploy.DeployerMux) + deployers := deployer.(deploy.DeployerMux) + t.CheckDeepEqual(len(expected), len(deployers)) + for i, v := range expected { + t.CheckTypeEquality(v, deployers[i]) + } + } + }) + } + }) +} + +func TestGetDefaultDeployer(tOuter *testing.T) { + testutil.Run(tOuter, "TestGetDeployer", func(t *testutil.T) { + tests := []struct { + name string + cfgs []latestV1.DeployType + expected *kubectl.Deployer + shouldErr bool + }{ + { + name: "one config with kubectl deploy", + cfgs: []latestV1.DeployType{{ + KubectlDeploy: &latestV1.KubectlDeploy{}, + }}, + expected: t.RequireNonNilResult(kubectl.NewDeployer(&runcontext.RunContext{ + Pipelines: runcontext.NewPipelines([]latestV1.Pipeline{{}}), + }, nil, deploy.NoopComponentProvider, &latestV1.KubectlDeploy{ + Flags: latestV1.KubectlFlags{}, + })).(*kubectl.Deployer), + }, + { + name: "one config with kubectl deploy, with flags", + cfgs: []latestV1.DeployType{{ + KubectlDeploy: &latestV1.KubectlDeploy{ + Flags: latestV1.KubectlFlags{ + Apply: []string{"--foo"}, + Global: []string{"--bar"}, + }, + }, + }}, + expected: t.RequireNonNilResult(kubectl.NewDeployer(&runcontext.RunContext{ + Pipelines: runcontext.NewPipelines([]latestV1.Pipeline{{}}), + }, nil, deploy.NoopComponentProvider, &latestV1.KubectlDeploy{ + Flags: latestV1.KubectlFlags{ + Apply: []string{"--foo"}, + Global: []string{"--bar"}, + }, + })).(*kubectl.Deployer), + }, + { + name: "two kubectl configs with mismatched flags should fail", + cfgs: []latestV1.DeployType{ + { + KubectlDeploy: &latestV1.KubectlDeploy{ + Flags: latestV1.KubectlFlags{ + Apply: []string{"--foo"}, + }, + }, + }, + { + KubectlDeploy: &latestV1.KubectlDeploy{ + Flags: latestV1.KubectlFlags{ + Apply: []string{"--bar"}, + }, + }, + }, + }, + shouldErr: true, + }, + { + name: "one config with helm deploy", + cfgs: []latestV1.DeployType{{ + HelmDeploy: &latestV1.HelmDeploy{}, + }}, + expected: t.RequireNonNilResult(kubectl.NewDeployer(&runcontext.RunContext{ + Pipelines: runcontext.NewPipelines([]latestV1.Pipeline{{}}), + }, nil, deploy.NoopComponentProvider, &latestV1.KubectlDeploy{ + Flags: latestV1.KubectlFlags{}, + })).(*kubectl.Deployer), + }, + { + name: "one config with kustomize deploy", + cfgs: []latestV1.DeployType{{ + KustomizeDeploy: &latestV1.KustomizeDeploy{}, + }}, + expected: t.RequireNonNilResult(kubectl.NewDeployer(&runcontext.RunContext{ + Pipelines: runcontext.NewPipelines([]latestV1.Pipeline{{}}), + }, nil, deploy.NoopComponentProvider, &latestV1.KubectlDeploy{ + Flags: latestV1.KubectlFlags{}, + })).(*kubectl.Deployer), + }, + } + + for _, test := range tests { + testutil.Run(tOuter, test.name, func(t *testutil.T) { + pipelines := []latestV1.Pipeline{} + for _, cfg := range test.cfgs { + pipelines = append(pipelines, latestV1.Pipeline{ + Deploy: latestV1.DeployConfig{ + DeployType: cfg, + }, + }) + } + deployer, _, err := getDefaultDeployer(&runcontext.RunContext{ + Pipelines: runcontext.NewPipelines(pipelines), + }, deploy.NoopComponentProvider, 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() + } + } + }) + } + }) +} diff --git a/pkg/skaffold/runner/runcontext/context.go b/pkg/skaffold/runner/runcontext/context.go index 7dd827e7787..135479d3670 100644 --- a/pkg/skaffold/runner/runcontext/context.go +++ b/pkg/skaffold/runner/runcontext/context.go @@ -96,10 +96,10 @@ func (ps Pipelines) DeployConfigs() []latestV1.DeployConfig { return cfgs } -func (ps Pipelines) Deployers() []latestV1.DeployType { - var deployers []latestV1.DeployType +func (ps Pipelines) Deployers() []latestV1.DeployConfig { + var deployers []latestV1.DeployConfig for _, p := range ps.pipelines { - deployers = append(deployers, p.Deploy.DeployType) + deployers = append(deployers, p.Deploy) } return deployers } @@ -112,28 +112,6 @@ func (ps Pipelines) TestCases() []*latestV1.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 @@ -166,26 +144,10 @@ func (rc *RunContext) Artifacts() []*latestV1.Artifact { return rc.Pipelines.Art func (rc *RunContext) DeployConfigs() []latestV1.DeployConfig { return rc.Pipelines.DeployConfigs() } -func (rc *RunContext) Deployers() []latestV1.DeployType { return rc.Pipelines.Deployers() } +func (rc *RunContext) Deployers() []latestV1.DeployConfig { return rc.Pipelines.Deployers() } func (rc *RunContext) TestCases() []*latestV1.TestCase { return rc.Pipelines.TestCases() } -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 - } -} - func (rc *RunContext) StatusCheckDeadlineSeconds() int { return rc.Pipelines.StatusCheckDeadlineSeconds() } @@ -226,6 +188,7 @@ func (rc *RunContext) RenderOnly() bool { return rc func (rc *RunContext) RenderOutput() string { return rc.Opts.RenderOutput } func (rc *RunContext) SkipRender() bool { return rc.Opts.SkipRender } func (rc *RunContext) SkipTests() bool { return rc.Opts.SkipTests } +func (rc *RunContext) StatusCheck() *bool { return rc.Opts.StatusCheck.Value() } func (rc *RunContext) Tail() bool { return rc.Opts.Tail } func (rc *RunContext) Trigger() string { return rc.Opts.Trigger } func (rc *RunContext) WaitForDeletions() config.WaitForDeletions { return rc.Opts.WaitForDeletions } diff --git a/pkg/skaffold/runner/runcontext/context_test.go b/pkg/skaffold/runner/runcontext/context_test.go index 3e773a1dc96..201c02c04fe 100644 --- a/pkg/skaffold/runner/runcontext/context_test.go +++ b/pkg/skaffold/runner/runcontext/context_test.go @@ -19,8 +19,6 @@ package runcontext import ( "testing" - latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/util" "github.com/GoogleContainerTools/skaffold/testutil" ) @@ -91,60 +89,3 @@ 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: []latestV1.Pipeline{ - { - Deploy: latestV1.DeployConfig{ - StatusCheck: test.statusCheckP1, - }, - }, - { - Deploy: latestV1.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/runner/v1/deploy_test.go b/pkg/skaffold/runner/v1/deploy_test.go index 7c9806ffc4e..d7340f8624b 100644 --- a/pkg/skaffold/runner/v1/deploy_test.go +++ b/pkg/skaffold/runner/v1/deploy_test.go @@ -30,6 +30,7 @@ import ( "github.com/GoogleContainerTools/skaffold/pkg/skaffold/graph" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubectl" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/client" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext" latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1" "github.com/GoogleContainerTools/skaffold/testutil" @@ -123,7 +124,7 @@ func TestSkaffoldDeployRenderOnly(t *testing.T) { KubeContext: "does-not-exist", } - deployer, _, err := getDeployer(runCtx, deploy.NoopComponentProvider, nil) + deployer, _, err := runner.GetDeployer(runCtx, deploy.NoopComponentProvider, nil) t.RequireNoError(err) r := SkaffoldRunner{ runCtx: runCtx, diff --git a/pkg/skaffold/runner/v1/new.go b/pkg/skaffold/runner/v1/new.go index 427ea51ff2f..8f04b6907b6 100644 --- a/pkg/skaffold/runner/v1/new.go +++ b/pkg/skaffold/runner/v1/new.go @@ -18,9 +18,7 @@ package v1 import ( "context" - "errors" "fmt" - "strconv" "github.com/sirupsen/logrus" @@ -29,10 +27,6 @@ import ( "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/cache" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/debug" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/helm" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/kpt" - "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/event" eventV2 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/event/v2" @@ -41,7 +35,6 @@ import ( "github.com/GoogleContainerTools/skaffold/pkg/skaffold/instrumentation" pkgkubectl "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubectl" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes" - kstatus "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/status" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/log" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext" @@ -52,7 +45,6 @@ import ( "github.com/GoogleContainerTools/skaffold/pkg/skaffold/tag" "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 @@ -99,11 +91,11 @@ func NewForConfig(runCtx *runcontext.RunContext) (*SkaffoldRunner, error) { Accessor: access.NewAccessorProvider(runCtx, labeller, kubectlCLI), Debugger: debug.NewDebugProvider(runCtx), Logger: log.NewLogProvider(runCtx, kubectlCLI), - Monitor: status.NewMonitorProvider(runCtx, labeller), + Monitor: status.NewMonitorProvider(labeller), Syncer: sync.NewSyncProvider(runCtx, kubectlCLI), } - deployer, podSelectors, err = getDeployer(runCtx, provider, labeller.Labels()) + deployer, podSelectors, err = runner.GetDeployer(runCtx, provider, labeller.Labels()) if err != nil { endTrace(instrumentation.TraceEndError(err)) return nil, fmt.Errorf("creating deployer: %w", err) @@ -233,151 +225,3 @@ func getTester(cfg test.Config, isLocalImage func(imageName string) (bool, error return tester, nil } - -/* -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, provider deploy.ComponentProvider, labels map[string]string) (deploy.Deployer, kubernetes.ImageListMux, error) { - deployCfgs := runCtx.DeployConfigs() - - var kFlags *latestV1.KubectlFlags - var logPrefix string - var defaultNamespace *string - var kubeContext string - statusCheckTimeout := -1 - - for _, d := range deployCfgs { - if d.KubeContext != "" { - if kubeContext != "" && kubeContext != d.KubeContext { - return nil, nil, errors.New("cannot resolve active Kubernetes context - multiple contexts configured in skaffold.yaml") - } - kubeContext = d.KubeContext - } - if d.StatusCheckDeadlineSeconds != 0 && d.StatusCheckDeadlineSeconds != int(kstatus.DefaultStatusCheckDeadline.Seconds()) { - if statusCheckTimeout != -1 && statusCheckTimeout != d.StatusCheckDeadlineSeconds { - return nil, 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, 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 latestV1.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 = ¤tKubectlFlags - } - if err := validateKubectlFlags(kFlags, currentKubectlFlags); err != nil { - return nil, nil, err - } - if currentDefaultNamespace != nil { - if defaultNamespace != nil && *defaultNamespace != *currentDefaultNamespace { - return nil, nil, fmt.Errorf("found multiple namespaces in skaffold.yaml (not supported in `skaffold apply`): %s, %s", *defaultNamespace, *currentDefaultNamespace) - } - defaultNamespace = currentDefaultNamespace - } - } - if kFlags == nil { - kFlags = &latestV1.KubectlFlags{} - } - k := &latestV1.KubectlDeploy{ - Flags: *kFlags, - DefaultNamespace: defaultNamespace, - } - defaultDeployer, podSelector, err := kubectl.NewDeployer(runCtx, labels, provider, k) - if err != nil { - return nil, nil, fmt.Errorf("instantiating default kubectl deployer: %w", err) - } - return defaultDeployer, kubernetes.ImageListMux{podSelector}, nil -} - -func validateKubectlFlags(flags *latestV1.KubectlFlags, additional latestV1.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, strconv.FormatBool(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, provider deploy.ComponentProvider, labels map[string]string) (deploy.Deployer, kubernetes.ImageListMux, error) { - var podSelectors kubernetes.ImageListMux - if runCtx.Opts.Apply { - return getDefaultDeployer(runCtx, provider, labels) - } - - deployerCfg := runCtx.Deployers() - - var deployers deploy.DeployerMux - for _, d := range deployerCfg { - if d.HelmDeploy != nil { - h, podSelector, err := helm.NewDeployer(runCtx, labels, provider, d.HelmDeploy) - if err != nil { - return nil, nil, err - } - podSelectors = append(podSelectors, podSelector) - deployers = append(deployers, h) - } - - if d.KptDeploy != nil { - deployer, podSelector := kpt.NewDeployer(runCtx, labels, provider, d.KptDeploy) - podSelectors = append(podSelectors, podSelector) - deployers = append(deployers, deployer) - } - - if d.KubectlDeploy != nil { - deployer, podSelector, err := kubectl.NewDeployer(runCtx, labels, provider, d.KubectlDeploy) - if err != nil { - return nil, nil, err - } - podSelectors = append(podSelectors, podSelector) - deployers = append(deployers, deployer) - } - - if d.KustomizeDeploy != nil { - deployer, podSelector, err := kustomize.NewDeployer(runCtx, labels, provider, d.KustomizeDeploy) - if err != nil { - return nil, nil, err - } - podSelectors = append(podSelectors, podSelector) - deployers = append(deployers, deployer) - } - } - - return deployers, podSelectors, nil -} diff --git a/pkg/skaffold/runner/v1/new_test.go b/pkg/skaffold/runner/v1/new_test.go index 7df2942f2a1..5f260694191 100644 --- a/pkg/skaffold/runner/v1/new_test.go +++ b/pkg/skaffold/runner/v1/new_test.go @@ -17,262 +17,15 @@ limitations under the License. package v1 import ( - "reflect" "testing" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/config" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/helm" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/kpt" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/kubectl" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/kustomize" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext" latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/util" "github.com/GoogleContainerTools/skaffold/testutil" ) -func TestGetDeployer(tOuter *testing.T) { - testutil.Run(tOuter, "TestGetDeployer", func(t *testutil.T) { - tests := []struct { - description string - cfg latestV1.DeployType - helmVersion string - expected deploy.Deployer - apply bool - shouldErr bool - }{ - { - description: "no deployer", - expected: deploy.DeployerMux{}, - }, - { - description: "helm deployer with 3.0.0 version", - cfg: latestV1.DeployType{HelmDeploy: &latestV1.HelmDeploy{}}, - helmVersion: `version.BuildInfo{Version:"v3.0.0"}`, - expected: deploy.DeployerMux{ - &helm.Deployer{}, - }, - }, - { - description: "helm deployer with less than 3.0.0 version", - cfg: latestV1.DeployType{HelmDeploy: &latestV1.HelmDeploy{}}, - helmVersion: "2.0.0", - shouldErr: true, - }, - { - description: "kubectl deployer", - cfg: latestV1.DeployType{KubectlDeploy: &latestV1.KubectlDeploy{}}, - expected: deploy.DeployerMux{ - t.RequireNonNilResult(kubectl.NewDeployer(&runcontext.RunContext{ - Pipelines: runcontext.NewPipelines([]latestV1.Pipeline{{}}), - }, nil, deploy.NoopComponentProvider, &latestV1.KubectlDeploy{ - Flags: latestV1.KubectlFlags{}, - })).(deploy.Deployer), - }, - }, - { - description: "kustomize deployer", - cfg: latestV1.DeployType{KustomizeDeploy: &latestV1.KustomizeDeploy{}}, - expected: deploy.DeployerMux{ - t.RequireNonNilResult(kustomize.NewDeployer(&runcontext.RunContext{ - Pipelines: runcontext.NewPipelines([]latestV1.Pipeline{{}}), - }, nil, deploy.NoopComponentProvider, &latestV1.KustomizeDeploy{ - Flags: latestV1.KubectlFlags{}, - })).(deploy.Deployer), - }, - }, - { - description: "kpt deployer", - cfg: latestV1.DeployType{KptDeploy: &latestV1.KptDeploy{}}, - expected: deploy.DeployerMux{ - &kpt.Deployer{}, - }, - }, - { - description: "apply forces creation of kubectl deployer with kpt config", - cfg: latestV1.DeployType{KptDeploy: &latestV1.KptDeploy{}}, - apply: true, - expected: t.RequireNonNilResult(kubectl.NewDeployer(&runcontext.RunContext{ - Pipelines: runcontext.NewPipelines([]latestV1.Pipeline{{}}), - }, nil, deploy.NoopComponentProvider, &latestV1.KubectlDeploy{ - Flags: latestV1.KubectlFlags{}, - })).(deploy.Deployer), - }, - { - description: "apply forces creation of kubectl deployer with helm config", - cfg: latestV1.DeployType{HelmDeploy: &latestV1.HelmDeploy{}}, - helmVersion: `version.BuildInfo{Version:"v3.0.0"}`, - apply: true, - expected: t.RequireNonNilResult(kubectl.NewDeployer(&runcontext.RunContext{ - Pipelines: runcontext.NewPipelines([]latestV1.Pipeline{{}}), - }, nil, deploy.NoopComponentProvider, &latestV1.KubectlDeploy{ - Flags: latestV1.KubectlFlags{}, - })).(deploy.Deployer), - }, - { - description: "multiple deployers", - cfg: latestV1.DeployType{ - HelmDeploy: &latestV1.HelmDeploy{}, - KptDeploy: &latestV1.KptDeploy{}, - }, - helmVersion: `version.BuildInfo{Version:"v3.0.0"}`, - expected: deploy.DeployerMux{ - &helm.Deployer{}, - &kpt.Deployer{}, - }, - }, - } - for _, test := range tests { - testutil.Run(tOuter, test.description, func(t *testutil.T) { - if test.helmVersion != "" { - t.Override(&util.DefaultExecCommand, testutil.CmdRunWithOutput( - "helm version --client", - test.helmVersion, - )) - } - - deployer, _, err := getDeployer(&runcontext.RunContext{ - Opts: config.SkaffoldOptions{ - Apply: test.apply, - }, - Pipelines: runcontext.NewPipelines([]latestV1.Pipeline{{ - Deploy: latestV1.DeployConfig{ - DeployType: test.cfg, - }, - }}), - }, deploy.NoopComponentProvider, nil) - - t.CheckError(test.shouldErr, err) - t.CheckTypeEquality(test.expected, deployer) - - if reflect.TypeOf(test.expected) == reflect.TypeOf(deploy.DeployerMux{}) { - expected := test.expected.(deploy.DeployerMux) - deployers := deployer.(deploy.DeployerMux) - t.CheckDeepEqual(len(expected), len(deployers)) - for i, v := range expected { - t.CheckTypeEquality(v, deployers[i]) - } - } - }) - } - }) -} - -func TestGetDefaultDeployer(tOuter *testing.T) { - testutil.Run(tOuter, "TestGetDeployer", func(t *testutil.T) { - tests := []struct { - name string - cfgs []latestV1.DeployType - expected *kubectl.Deployer - shouldErr bool - }{ - { - name: "one config with kubectl deploy", - cfgs: []latestV1.DeployType{{ - KubectlDeploy: &latestV1.KubectlDeploy{}, - }}, - expected: t.RequireNonNilResult(kubectl.NewDeployer(&runcontext.RunContext{ - Pipelines: runcontext.NewPipelines([]latestV1.Pipeline{{}}), - }, nil, deploy.NoopComponentProvider, &latestV1.KubectlDeploy{ - Flags: latestV1.KubectlFlags{}, - })).(*kubectl.Deployer), - }, - { - name: "one config with kubectl deploy, with flags", - cfgs: []latestV1.DeployType{{ - KubectlDeploy: &latestV1.KubectlDeploy{ - Flags: latestV1.KubectlFlags{ - Apply: []string{"--foo"}, - Global: []string{"--bar"}, - }, - }, - }}, - expected: t.RequireNonNilResult(kubectl.NewDeployer(&runcontext.RunContext{ - Pipelines: runcontext.NewPipelines([]latestV1.Pipeline{{}}), - }, nil, deploy.NoopComponentProvider, &latestV1.KubectlDeploy{ - Flags: latestV1.KubectlFlags{ - Apply: []string{"--foo"}, - Global: []string{"--bar"}, - }, - })).(*kubectl.Deployer), - }, - { - name: "two kubectl configs with mismatched flags should fail", - cfgs: []latestV1.DeployType{ - { - KubectlDeploy: &latestV1.KubectlDeploy{ - Flags: latestV1.KubectlFlags{ - Apply: []string{"--foo"}, - }, - }, - }, - { - KubectlDeploy: &latestV1.KubectlDeploy{ - Flags: latestV1.KubectlFlags{ - Apply: []string{"--bar"}, - }, - }, - }, - }, - shouldErr: true, - }, - { - name: "one config with helm deploy", - cfgs: []latestV1.DeployType{{ - HelmDeploy: &latestV1.HelmDeploy{}, - }}, - expected: t.RequireNonNilResult(kubectl.NewDeployer(&runcontext.RunContext{ - Pipelines: runcontext.NewPipelines([]latestV1.Pipeline{{}}), - }, nil, deploy.NoopComponentProvider, &latestV1.KubectlDeploy{ - Flags: latestV1.KubectlFlags{}, - })).(*kubectl.Deployer), - }, - { - name: "one config with kustomize deploy", - cfgs: []latestV1.DeployType{{ - KustomizeDeploy: &latestV1.KustomizeDeploy{}, - }}, - expected: t.RequireNonNilResult(kubectl.NewDeployer(&runcontext.RunContext{ - Pipelines: runcontext.NewPipelines([]latestV1.Pipeline{{}}), - }, nil, deploy.NoopComponentProvider, &latestV1.KubectlDeploy{ - Flags: latestV1.KubectlFlags{}, - })).(*kubectl.Deployer), - }, - } - - for _, test := range tests { - testutil.Run(tOuter, test.name, func(t *testutil.T) { - pipelines := []latestV1.Pipeline{} - for _, cfg := range test.cfgs { - pipelines = append(pipelines, latestV1.Pipeline{ - Deploy: latestV1.DeployConfig{ - DeployType: cfg, - }, - }) - } - deployer, _, err := getDefaultDeployer(&runcontext.RunContext{ - Pipelines: runcontext.NewPipelines(pipelines), - }, deploy.NoopComponentProvider, 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() - } - } - }) - } - }) -} - func TestIsImageLocal(t *testing.T) { tests := []struct { description string diff --git a/pkg/skaffold/status/provider.go b/pkg/skaffold/status/provider.go index 7bffc2f05cc..9265893155f 100644 --- a/pkg/skaffold/status/provider.go +++ b/pkg/skaffold/status/provider.go @@ -17,42 +17,34 @@ limitations under the License. package status import ( - "sync" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/label" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/status" ) type Provider interface { - GetKubernetesMonitor() Monitor + GetKubernetesMonitor(config status.Config) Monitor GetNoopMonitor() Monitor } type fullProvider struct { - kubernetesMonitor Monitor + k8sMonitor map[string]Monitor // keyed on KubeContext. TODO: make KubeContext a struct type. + labeller *label.DefaultLabeller } -var ( - provider *fullProvider - once sync.Once -) - -func NewMonitorProvider(config status.Config, l *label.DefaultLabeller) Provider { - once.Do(func() { - var m Monitor = &NoopMonitor{} - enabled, _ := config.StatusCheck() - if enabled == nil || *enabled { // assume enabled if value is nil - m = status.NewStatusMonitor(config, l) - } - provider = &fullProvider{ - kubernetesMonitor: m, - } - }) - return provider +func NewMonitorProvider(l *label.DefaultLabeller) Provider { + return &fullProvider{k8sMonitor: make(map[string]Monitor), labeller: l} } -func (p *fullProvider) GetKubernetesMonitor() Monitor { - return p.kubernetesMonitor +func (p *fullProvider) GetKubernetesMonitor(config status.Config) Monitor { + enabled := config.StatusCheck() + if enabled != nil && !*enabled { // assume disabled only if explicitly set to false + return &NoopMonitor{} + } + context := config.GetKubeContext() + if p.k8sMonitor[context] == nil { + p.k8sMonitor[context] = status.NewStatusMonitor(config, p.labeller) + } + return p.k8sMonitor[context] } func (p *fullProvider) GetNoopMonitor() Monitor { @@ -62,7 +54,7 @@ func (p *fullProvider) GetNoopMonitor() Monitor { // NoopProvider is used in tests type NoopProvider struct{} -func (p *NoopProvider) GetKubernetesMonitor() Monitor { +func (p *NoopProvider) GetKubernetesMonitor(config status.Config) Monitor { return &NoopMonitor{} } diff --git a/pkg/skaffold/status/provider_test.go b/pkg/skaffold/status/provider_test.go index df2215b409f..c8a2dd8099e 100644 --- a/pkg/skaffold/status/provider_test.go +++ b/pkg/skaffold/status/provider_test.go @@ -18,7 +18,6 @@ package status import ( "reflect" - "sync" "testing" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/config" @@ -48,9 +47,8 @@ func TestMonitorProvider(t *testing.T) { } for _, test := range tests { - once = sync.Once{} // reset once for tests testutil.Run(t, test.description, func(t *testutil.T) { - m := NewMonitorProvider(mockConfig{statusCheck: test.statusCheck}, nil).GetKubernetesMonitor() + m := NewMonitorProvider(nil).GetKubernetesMonitor(mockConfig{statusCheck: test.statusCheck}) t.CheckDeepEqual(test.isNoop, reflect.Indirect(reflect.ValueOf(m)).Type() == reflect.TypeOf(NoopMonitor{})) }) } @@ -61,7 +59,9 @@ type mockConfig struct { statusCheck *bool } -func (m mockConfig) StatusCheck() (*bool, error) { return m.statusCheck, nil } +func (m mockConfig) StatusCheck() *bool { return m.statusCheck } + +func (m mockConfig) GetKubeContext() string { return "" } func (m mockConfig) StatusCheckDeadlineSeconds() int { return 0 }