diff --git a/cmd/skaffold/app/cmd/config/set_test.go b/cmd/skaffold/app/cmd/config/set_test.go index 3e97732d81f..a89eb44b672 100644 --- a/cmd/skaffold/app/cmd/config/set_test.go +++ b/cmd/skaffold/app/cmd/config/set_test.go @@ -227,6 +227,80 @@ func TestSetAndUnsetConfig(t *testing.T) { }, }, }, + { + description: "set kind disable load", + key: "kind-disable-load", + value: "true", + kubecontext: "this_is_a_context", + expectedSetCfg: &config.GlobalConfig{ + ContextConfigs: []*config.ContextConfig{ + { + Kubecontext: "this_is_a_context", + KindDisableLoad: util.BoolPtr(true), + }, + }, + }, + expectedUnsetCfg: &config.GlobalConfig{ + ContextConfigs: []*config.ContextConfig{ + { + Kubecontext: "this_is_a_context", + }, + }, + }, + }, + { + description: "set global kind disable load", + key: "kind-disable-load", + value: "true", + global: true, + expectedSetCfg: &config.GlobalConfig{ + Global: &config.ContextConfig{ + KindDisableLoad: util.BoolPtr(true), + }, + ContextConfigs: []*config.ContextConfig{}, + }, + expectedUnsetCfg: &config.GlobalConfig{ + Global: &config.ContextConfig{}, + ContextConfigs: []*config.ContextConfig{}, + }, + }, + { + description: "set k3d disable load", + key: "k3d-disable-load", + value: "true", + kubecontext: "this_is_a_context", + expectedSetCfg: &config.GlobalConfig{ + ContextConfigs: []*config.ContextConfig{ + { + Kubecontext: "this_is_a_context", + K3dDisableLoad: util.BoolPtr(true), + }, + }, + }, + expectedUnsetCfg: &config.GlobalConfig{ + ContextConfigs: []*config.ContextConfig{ + { + Kubecontext: "this_is_a_context", + }, + }, + }, + }, + { + description: "set global k3d disable load", + key: "k3d-disable-load", + value: "true", + global: true, + expectedSetCfg: &config.GlobalConfig{ + Global: &config.ContextConfig{ + K3dDisableLoad: util.BoolPtr(true), + }, + ContextConfigs: []*config.ContextConfig{}, + }, + expectedUnsetCfg: &config.GlobalConfig{ + Global: &config.ContextConfig{}, + ContextConfigs: []*config.ContextConfig{}, + }, + }, } for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { diff --git a/pkg/skaffold/build/local/local_test.go b/pkg/skaffold/build/local/local_test.go index 8d88f7326e2..a153657e447 100644 --- a/pkg/skaffold/build/local/local_test.go +++ b/pkg/skaffold/build/local/local_test.go @@ -278,12 +278,12 @@ func TestNewBuilder(t *testing.T) { dummyDaemon := dummyLocalDaemon{} tests := []struct { - description string - shouldErr bool - expectedPush bool - localBuild latest.LocalBuild - localClusterFn func(string, string, bool) (bool, error) - localDockerFn func(docker.Config) (docker.LocalDaemon, error) + description string + shouldErr bool + expectedPush bool + cluster config.Cluster + localBuild latest.LocalBuild + localDockerFn func(docker.Config) (docker.LocalDaemon, error) }{ { description: "failed to get docker client", @@ -293,14 +293,11 @@ func TestNewBuilder(t *testing.T) { shouldErr: true, }, { - description: "pushImages becomes !localCluster when local:push is not defined", + description: "pushImages becomes cluster.PushImages when local:push is not defined", localDockerFn: func(docker.Config) (docker.LocalDaemon, error) { return dummyDaemon, nil }, - localClusterFn: func(string, string, bool) (b bool, e error) { - b = false //because this is false and localBuild.push is nil - return - }, + cluster: config.Cluster{PushImages: true}, expectedPush: true, }, { @@ -308,10 +305,7 @@ func TestNewBuilder(t *testing.T) { localDockerFn: func(docker.Config) (docker.LocalDaemon, error) { return dummyDaemon, nil }, - localClusterFn: func(string, string, bool) (b bool, e error) { - b = false - return - }, + cluster: config.Cluster{PushImages: true}, localBuild: latest.LocalBuild{ Push: util.BoolPtr(false), }, @@ -324,12 +318,10 @@ func TestNewBuilder(t *testing.T) { if test.localDockerFn != nil { t.Override(&docker.NewAPIClient, test.localDockerFn) } - if test.localClusterFn != nil { - t.Override(&getLocalCluster, test.localClusterFn) - } builder, err := NewBuilder(&mockConfig{ - local: test.localBuild, + local: test.localBuild, + cluster: test.cluster, }) t.CheckError(test.shouldErr, err) @@ -442,6 +434,7 @@ type mockConfig struct { runcontext.RunContext // Embedded to provide the default values. local latest.LocalBuild mode config.RunMode + cluster config.Cluster } func (c *mockConfig) Pipeline() latest.Pipeline { @@ -453,3 +446,7 @@ func (c *mockConfig) Pipeline() latest.Pipeline { func (c *mockConfig) Mode() config.RunMode { return c.mode } + +func (c *mockConfig) GetCluster() config.Cluster { + return c.cluster +} diff --git a/pkg/skaffold/build/local/types.go b/pkg/skaffold/build/local/types.go index 1cb705ac928..e55973254c2 100644 --- a/pkg/skaffold/build/local/types.go +++ b/pkg/skaffold/build/local/types.go @@ -57,18 +57,13 @@ type Builder struct { artifactStore build.ArtifactStore } -// external dependencies are wrapped -// into private functions for testability - -var getLocalCluster = config.GetLocalCluster - type Config interface { docker.Config Pipeline() latest.Pipeline GlobalConfig() string GetKubeContext() string - DetectMinikube() bool + GetCluster() config.Cluster SkipTests() bool Mode() config.RunMode NoPruneChildren() bool @@ -82,19 +77,12 @@ func NewBuilder(cfg Config) (*Builder, error) { return nil, fmt.Errorf("getting docker client: %w", err) } - // TODO(https://github.com/GoogleContainerTools/skaffold/issues/3668): - // remove minikubeProfile from here and instead detect it by matching the - // kubecontext API Server to minikube profiles - - localCluster, err := getLocalCluster(cfg.GlobalConfig(), cfg.MinikubeProfile(), cfg.DetectMinikube()) - if err != nil { - return nil, fmt.Errorf("getting localCluster: %w", err) - } + cluster := cfg.GetCluster() var pushImages bool if cfg.Pipeline().Build.LocalBuild.Push == nil { - pushImages = !localCluster - logrus.Debugf("push value not present, defaulting to %t because localCluster is %t", pushImages, localCluster) + pushImages = cluster.PushImages + logrus.Debugf("push value not present, defaulting to %t because cluster.PushImages is %t", pushImages, cluster.PushImages) } else { pushImages = *cfg.Pipeline().Build.LocalBuild.Push } @@ -106,7 +94,7 @@ func NewBuilder(cfg Config) (*Builder, error) { cfg: cfg, kubeContext: cfg.GetKubeContext(), localDocker: localDocker, - localCluster: localCluster, + localCluster: cluster.Local, pushImages: pushImages, tryImportMissing: tryImportMissing, skipTests: cfg.SkipTests(), diff --git a/pkg/skaffold/config/global_config.go b/pkg/skaffold/config/global_config.go index f48cde680ce..db748fd09c3 100644 --- a/pkg/skaffold/config/global_config.go +++ b/pkg/skaffold/config/global_config.go @@ -34,6 +34,8 @@ type ContextConfig struct { DebugHelpersRegistry string `yaml:"debug-helpers-registry,omitempty"` UpdateCheck *bool `yaml:"update-check,omitempty"` Survey *SurveyConfig `yaml:"survey,omitempty"` + KindDisableLoad *bool `yaml:"kind-disable-load,omitempty"` + K3dDisableLoad *bool `yaml:"k3d-disable-load,omitempty"` } // SurveyConfig is the survey config information diff --git a/pkg/skaffold/config/types.go b/pkg/skaffold/config/types.go index 1a27e908b28..08a335f873d 100644 --- a/pkg/skaffold/config/types.go +++ b/pkg/skaffold/config/types.go @@ -58,3 +58,9 @@ func (m Muted) MuteDeploy() bool { return m.mute("deploy") } func (m Muted) mute(phase string) bool { return util.StrSliceContains(m.Phases, phase) || util.StrSliceContains(m.Phases, "all") } + +type Cluster struct { + Local bool + PushImages bool + LoadImages bool +} diff --git a/pkg/skaffold/config/util.go b/pkg/skaffold/config/util.go index e76897c57d0..067fc92ef47 100644 --- a/pkg/skaffold/config/util.go +++ b/pkg/skaffold/config/util.go @@ -169,27 +169,6 @@ func GetDefaultRepo(configFile string, cliValue *string) (string, error) { return cfg.DefaultRepo, nil } -func GetLocalCluster(configFile string, minikubeProfile string, detectMinikubeCluster bool) (bool, error) { - if minikubeProfile != "" { - return true, nil - } - cfg, err := GetConfigForCurrentKubectx(configFile) - if err != nil { - return false, err - } - // when set, the local-cluster config takes precedence - if cfg.LocalCluster != nil { - logrus.Infof("Using local-cluster=%v from config", *cfg.LocalCluster) - return *cfg.LocalCluster, nil - } - - config, err := kubectx.CurrentConfig() - if err != nil { - return true, err - } - return isDefaultLocal(config.CurrentContext, detectMinikubeCluster), nil -} - func GetInsecureRegistries(configFile string) ([]string, error) { cfg, err := GetConfigForCurrentKubectx(configFile) if err != nil { @@ -214,23 +193,51 @@ func GetDebugHelpersRegistry(configFile string) (string, error) { return constants.DefaultDebugHelpersRegistry, nil } -func isDefaultLocal(kubeContext string, detectMinikubeCluster bool) bool { - if kubeContext == constants.DefaultMinikubeContext || +func GetCluster(configFile string, minikubeProfile string, detectMinikube bool) (Cluster, error) { + cfg, err := GetConfigForCurrentKubectx(configFile) + if err != nil { + return Cluster{}, err + } + + kubeContext := cfg.Kubecontext + isKindCluster, isK3dCluster := IsKindCluster(kubeContext), IsK3dCluster(kubeContext) + + var local bool + switch { + case minikubeProfile != "": + local = true + + case cfg.LocalCluster != nil: + logrus.Infof("Using local-cluster=%t from config", *cfg.LocalCluster) + local = *cfg.LocalCluster + + case kubeContext == constants.DefaultMinikubeContext || kubeContext == constants.DefaultDockerForDesktopContext || kubeContext == constants.DefaultDockerDesktopContext || - IsKindCluster(kubeContext) || - IsK3dCluster(kubeContext) { - return true - } - if detectMinikubeCluster { - return cluster.GetClient().IsMinikube(kubeContext) + isKindCluster || isK3dCluster: + local = true + + case detectMinikube: + local = cluster.GetClient().IsMinikube(kubeContext) + + default: + local = false } - return false -} -// IsImageLoadingRequired checks if the cluster requires loading images into it -func IsImageLoadingRequired(kubeContext string) bool { - return IsKindCluster(kubeContext) || IsK3dCluster(kubeContext) + kindDisableLoad := cfg.KindDisableLoad != nil && *cfg.KindDisableLoad + k3dDisableLoad := cfg.K3dDisableLoad != nil && *cfg.K3dDisableLoad + + // load images for local kind/k3d cluster unless explicitly disabled + loadImages := local && ((isKindCluster && !kindDisableLoad) || (isK3dCluster && !k3dDisableLoad)) + + // push images for remote cluster or local kind/k3d cluster with image loading disabled + pushImages := !local || (isKindCluster && kindDisableLoad) || (isK3dCluster && k3dDisableLoad) + + return Cluster{ + Local: local, + LoadImages: loadImages, + PushImages: pushImages, + }, nil } // IsKindCluster checks that the given `kubeContext` is talking to `kind`. diff --git a/pkg/skaffold/config/util_test.go b/pkg/skaffold/config/util_test.go index f501eb31825..fce9ad524af 100644 --- a/pkg/skaffold/config/util_test.go +++ b/pkg/skaffold/config/util_test.go @@ -265,60 +265,110 @@ func TestIsUpdateCheckEnabled(t *testing.T) { } } -func TestIsDefaultLocal(t *testing.T) { - tests := []struct { - context string - expectedLocal bool - }{ - {context: "kind-other", expectedLocal: true}, - {context: "kind@kind", expectedLocal: true}, - {context: "k3d-k3s-default", expectedLocal: true}, - {context: "docker-for-desktop", expectedLocal: true}, - {context: "minikube", expectedLocal: true}, - {context: "docker-desktop", expectedLocal: true}, - {context: "anything-else", expectedLocal: false}, - {context: "kind@blah", expectedLocal: false}, - {context: "other-kind", expectedLocal: false}, - {context: "not-k3d", expectedLocal: false}, - } - for _, test := range tests { - testutil.Run(t, "", func(t *testutil.T) { - t.Override(&cluster.GetClient, func() cluster.Client { return fakeClient{} }) - - local := isDefaultLocal(test.context, true) - t.CheckDeepEqual(test.expectedLocal, local) - local = isDefaultLocal(test.context, false) - t.CheckDeepEqual(test.expectedLocal, local) - }) - } -} - type fakeClient struct{} func (fakeClient) IsMinikube(kubeContext string) bool { return kubeContext == "minikube" } func (fakeClient) MinikubeExec(...string) (*exec.Cmd, error) { return nil, nil } -func TestIsImageLoadingRequired(t *testing.T) { +func TestGetCluster(t *testing.T) { tests := []struct { - context string - expectedImageLoadingRequired bool + description string + cfg *ContextConfig + profile string + expected Cluster }{ - {context: "kind-other", expectedImageLoadingRequired: true}, - {context: "kind@kind", expectedImageLoadingRequired: true}, - {context: "k3d-k3s-default", expectedImageLoadingRequired: true}, - {context: "docker-for-desktop", expectedImageLoadingRequired: false}, - {context: "minikube", expectedImageLoadingRequired: false}, - {context: "docker-desktop", expectedImageLoadingRequired: false}, - {context: "anything-else", expectedImageLoadingRequired: false}, - {context: "kind@blah", expectedImageLoadingRequired: false}, - {context: "other-kind", expectedImageLoadingRequired: false}, - {context: "not-k3d", expectedImageLoadingRequired: false}, + { + description: "kind", + cfg: &ContextConfig{Kubecontext: "kind-other"}, + expected: Cluster{Local: true, LoadImages: true, PushImages: false}, + }, + { + description: "kind with local-cluster=false", + cfg: &ContextConfig{Kubecontext: "kind-other", LocalCluster: util.BoolPtr(false)}, + expected: Cluster{Local: false, LoadImages: false, PushImages: true}, + }, + { + description: "kind with kind-disable-load=true", + cfg: &ContextConfig{Kubecontext: "kind-other", KindDisableLoad: util.BoolPtr(true)}, + expected: Cluster{Local: true, LoadImages: false, PushImages: true}, + }, + { + description: "kind with legacy name", + cfg: &ContextConfig{Kubecontext: "kind@kind"}, + expected: Cluster{Local: true, LoadImages: true, PushImages: false}, + }, + { + description: "k3d", + cfg: &ContextConfig{Kubecontext: "k3d-k3s-default"}, + expected: Cluster{Local: true, LoadImages: true, PushImages: false}, + }, + { + description: "k3d with local-cluster=false", + cfg: &ContextConfig{Kubecontext: "k3d-k3s-default", LocalCluster: util.BoolPtr(false)}, + expected: Cluster{Local: false, LoadImages: false, PushImages: true}, + }, + { + description: "k3d with disable-load=true", + cfg: &ContextConfig{Kubecontext: "k3d-k3s-default", K3dDisableLoad: util.BoolPtr(true)}, + expected: Cluster{Local: true, LoadImages: false, PushImages: true}, + }, + { + description: "docker-for-desktop", + cfg: &ContextConfig{Kubecontext: "docker-for-desktop"}, + expected: Cluster{Local: true, LoadImages: false, PushImages: false}, + }, + { + description: "minikube", + cfg: &ContextConfig{Kubecontext: "minikube"}, + expected: Cluster{Local: true, LoadImages: false, PushImages: false}, + }, + { + description: "docker-desktop", + cfg: &ContextConfig{Kubecontext: "docker-desktop"}, + expected: Cluster{Local: true, LoadImages: false, PushImages: false}, + }, + { + description: "generic cluster with local-cluster=true", + cfg: &ContextConfig{Kubecontext: "some-cluster", LocalCluster: util.BoolPtr(true)}, + expected: Cluster{Local: true, LoadImages: false, PushImages: false}, + }, + { + description: "generic cluster with minikube profile", + cfg: &ContextConfig{Kubecontext: "some-cluster"}, + profile: "someprofile", + expected: Cluster{Local: true, LoadImages: false, PushImages: false}, + }, + { + description: "generic cluster", + cfg: &ContextConfig{Kubecontext: "anything-else"}, + expected: Cluster{Local: false, LoadImages: false, PushImages: true}, + }, + { + description: "not a legacy kind cluster", + cfg: &ContextConfig{Kubecontext: "kind@blah"}, + expected: Cluster{Local: false, LoadImages: false, PushImages: true}, + }, + { + description: "not a kind cluster", + cfg: &ContextConfig{Kubecontext: "other-kind"}, + expected: Cluster{Local: false, LoadImages: false, PushImages: true}, + }, + { + description: "not a k3d cluster", + cfg: &ContextConfig{Kubecontext: "not-k3d"}, + expected: Cluster{Local: false, LoadImages: false, PushImages: true}, + }, } for _, test := range tests { - testutil.Run(t, "", func(t *testutil.T) { - imageLoadingRequired := IsImageLoadingRequired(test.context) + testutil.Run(t, test.description, func(t *testutil.T) { + t.Override(&GetConfigForCurrentKubectx, func(string) (*ContextConfig, error) { return test.cfg, nil }) + t.Override(&cluster.GetClient, func() cluster.Client { return fakeClient{} }) + + cluster, _ := GetCluster("dummyname", test.profile, true) + t.CheckDeepEqual(test.expected, cluster) - t.CheckDeepEqual(test.expectedImageLoadingRequired, imageLoadingRequired) + cluster, _ = GetCluster("dummyname", test.profile, false) + t.CheckDeepEqual(test.expected, cluster) }) } } diff --git a/pkg/skaffold/runner/deploy.go b/pkg/skaffold/runner/deploy.go index e0ea6e9ff54..e02cf9c9d9a 100644 --- a/pkg/skaffold/runner/deploy.go +++ b/pkg/skaffold/runner/deploy.go @@ -59,7 +59,7 @@ See https://skaffold.dev/docs/pipeline-stages/taggers/#how-tagging-works`) return fmt.Errorf("unable to connect to Kubernetes: %w", err) } - if r.imagesAreLocal && config.IsImageLoadingRequired(r.runCtx.GetKubeContext()) { + if r.imagesAreLocal && r.runCtx.Cluster.LoadImages { err := r.loadImagesIntoCluster(ctx, out, artifacts) if err != nil { return err diff --git a/pkg/skaffold/runner/runcontext/context.go b/pkg/skaffold/runner/runcontext/context.go index 4214728f566..ad81a165aa9 100644 --- a/pkg/skaffold/runner/runcontext/context.go +++ b/pkg/skaffold/runner/runcontext/context.go @@ -41,6 +41,7 @@ type RunContext struct { Namespaces []string WorkingDir string InsecureRegistries map[string]bool + Cluster config.Cluster } func (rc *RunContext) GetKubeContext() string { return rc.KubeContext } @@ -48,6 +49,7 @@ func (rc *RunContext) GetNamespaces() []string { return rc.Namesp func (rc *RunContext) Pipeline() latest.Pipeline { return rc.Cfg } func (rc *RunContext) GetInsecureRegistries() map[string]bool { return rc.InsecureRegistries } func (rc *RunContext) GetWorkingDir() string { return rc.WorkingDir } +func (rc *RunContext) GetCluster() config.Cluster { return rc.Cluster } func (rc *RunContext) AddSkaffoldLabels() bool { return rc.Opts.AddSkaffoldLabels } func (rc *RunContext) AutoBuild() bool { return rc.Opts.AutoBuild } @@ -67,7 +69,6 @@ func (rc *RunContext) GetKubeConfig() string { return rc.Opt func (rc *RunContext) GetKubeNamespace() string { return rc.Opts.Namespace } func (rc *RunContext) GlobalConfig() string { return rc.Opts.GlobalConfig } func (rc *RunContext) MinikubeProfile() string { return rc.Opts.MinikubeProfile } -func (rc *RunContext) DetectMinikube() bool { return rc.Opts.DetectMinikube } func (rc *RunContext) Muted() config.Muted { return rc.Opts.Muted } func (rc *RunContext) NoPruneChildren() bool { return rc.Opts.NoPruneChildren } func (rc *RunContext) Notification() bool { return rc.Opts.Notification } @@ -114,6 +115,14 @@ func GetRunContext(opts config.SkaffoldOptions, cfg latest.Pipeline) (*RunContex insecureRegistries[r] = true } + // TODO(https://github.com/GoogleContainerTools/skaffold/issues/3668): + // remove minikubeProfile from here and instead detect it by matching the + // kubecontext API Server to minikube profiles + cluster, err := config.GetCluster(opts.GlobalConfig, opts.MinikubeProfile, opts.DetectMinikube) + if err != nil { + return nil, fmt.Errorf("getting cluster: %w", err) + } + return &RunContext{ Opts: opts, Cfg: cfg, @@ -121,6 +130,7 @@ func GetRunContext(opts config.SkaffoldOptions, cfg latest.Pipeline) (*RunContex KubeContext: kubeContext, Namespaces: namespaces, InsecureRegistries: insecureRegistries, + Cluster: cluster, }, nil }