Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix setting kubeContext in skaffold #6024

Merged
merged 2 commits into from
Jun 24, 2021
Merged

Fix setting kubeContext in skaffold #6024

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ddb968b
rebase
gsquared94 Jun 24, 2021
719304b
fix: support for multiple kubeContexts
gsquared94 Jun 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions cmd/skaffold/app/cmd/cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -95,6 +96,9 @@ func NewSkaffoldCommand(out, errOut io.Writer) *cobra.Command {
return err
}

// Setup kubeContext and kubeConfig
kubectx.ConfigureKubeConfig(opts.KubeConfig, opts.KubeContext)

Comment on lines +99 to +101
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel this should happen only for dev, run and deploy commands.
Not sure if this is the current behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unfortunately we evaluate the global config for all skaffold commands when we evaluate config.ShouldDisplaySurveyPrompt. We need to interpret the resolved kubeContext prior to evaluating the global config. So we need to do this check here also.

// Start API Server
shutdown, err := server.Initialize(opts)
if err != nil {
Expand Down
4 changes: 0 additions & 4 deletions cmd/skaffold/app/cmd/runner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
Expand Down
4 changes: 3 additions & 1 deletion pkg/skaffold/deploy/helm/deploy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -110,6 +111,7 @@ type Deployer struct {

type Config interface {
kubectl.Config
kstatus.Config
IsMultiConfig() bool
}

Expand Down Expand Up @@ -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(),
Expand Down
4 changes: 3 additions & 1 deletion pkg/skaffold/deploy/kpt/kpt.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -92,6 +93,7 @@ type Deployer struct {

type Config interface {
kubectl.Config
kstatus.Config
}

// NewDeployer generates a new Deployer object contains the kptDeploy schema.
Expand All @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions pkg/skaffold/deploy/kubectl/cli.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -47,6 +48,7 @@ type CLI struct {

type Config interface {
kubectl.Config
kstatus.Config
deploy.Config
ForceDeploy() bool
WaitForDeletions() config.WaitForDeletions
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/deploy/kubectl/kubectl.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/deploy/kustomize/kustomize.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
11 changes: 2 additions & 9 deletions pkg/skaffold/kubernetes/context/context.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In an ideal world, pkg/skaffold should be treated like a library, and these values should be computed in cmd/skaffold and provided into the library.

There are also the KUBECONFIG environment variable that should influencing this decision (and note that it is a PATH-like variable that can specify multiple files).

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.
Expand Down
54 changes: 2 additions & 52 deletions pkg/skaffold/kubernetes/context/context_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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{
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/kubernetes/status/status_check.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading