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.

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

Improve error messages for deploy.kubeContext error cases #2993

2 changes: 1 addition & 1 deletion integration/examples/profiles/skaffold.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
apiVersion: skaffold/v1beta15
apiVersion: skaffold/v1beta16
kind: Config
build:
# only build and deploy "world-service" on main profile
Expand Down
54 changes: 38 additions & 16 deletions integration/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,35 +42,42 @@ func TestRun(t *testing.T) {
description: "getting-started",
dir: "examples/getting-started",
pods: []string{"getting-started"},
}, {
},
{
description: "nodejs",
dir: "examples/nodejs",
deployments: []string{"node"},
}, {
},
{
description: "structure-tests",
dir: "examples/structure-tests",
pods: []string{"getting-started"},
}, {
},
{
description: "microservices",
dir: "examples/microservices",
// See https://github.com/GoogleContainerTools/skaffold/issues/2372
args: []string{"--status-check=false"},
deployments: []string{"leeroy-app", "leeroy-web"},
}, {
},
{
description: "envTagger",
dir: "examples/tagging-with-environment-variables",
pods: []string{"getting-started"},
env: []string{"FOO=foo"},
}, {
},
{
description: "bazel",
dir: "examples/bazel",
pods: []string{"bazel"},
}, {
},
{
description: "Google Cloud Build",
dir: "examples/google-cloud-build",
pods: []string{"getting-started"},
gcpOnly: true,
}, {
},
{
description: "Google Cloud Build with sub folder",
dir: "testdata/gcb-sub-folder",
pods: []string{"getting-started"},
Expand All @@ -87,51 +94,66 @@ func TestRun(t *testing.T) {
dir: "examples/kaniko",
pods: []string{"getting-started-kaniko"},
gcpOnly: true,
}, {
},
{
description: "kaniko local",
dir: "examples/kaniko-local",
pods: []string{"getting-started-kaniko"},
gcpOnly: true,
}, {
},
{
description: "kaniko local with target",
dir: "testdata/kaniko-target",
pods: []string{"getting-started-kaniko"},
gcpOnly: true,
}, {
},
{
description: "kaniko local with sub folder",
dir: "testdata/kaniko-sub-folder",
pods: []string{"getting-started-kaniko"},
gcpOnly: true,
}, {
},
{
description: "kaniko microservices",
dir: "testdata/kaniko-microservices",
deployments: []string{"leeroy-app", "leeroy-web"},
gcpOnly: true,
}, {
},
{
description: "jib",
dir: "testdata/jib",
deployments: []string{"web"},
}, {
},
{
description: "jib in googlecloudbuild",
dir: "testdata/jib",
args: []string{"-p", "gcb"},
deployments: []string{"web"},
gcpOnly: true,
}, {
},
{
description: "jib gradle",
dir: "testdata/jib-gradle",
deployments: []string{"web"},
}, {
},
{
description: "jib gradle in googlecloudbuild",
dir: "testdata/jib-gradle",
args: []string{"-p", "gcb"},
deployments: []string{"web"},
gcpOnly: true,
}, {
},
{
description: "custom builder",
dir: "testdata/custom",
pods: []string{"bazel"},
},
{
description: "profiles",
dir: "examples/profiles",
args: []string{"-p", "minikube-profile"},
pods: []string{"hello-service"},
},
}
for _, test := range tests {
t.Run(test.description, func(t *testing.T) {
Expand Down
12 changes: 8 additions & 4 deletions pkg/skaffold/kubernetes/context/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,19 @@ var (
// Changing the kube-context of a running Skaffold process is not supported, so
// after the first call, the kube-context will be locked.
func UseKubeContext(cliValue, yamlValue string) {
newKubeContext := yamlValue
if cliValue != "" {
newKubeContext = cliValue
}
kubeContextOnce.Do(func() {
kubeContext = yamlValue
if cliValue != "" {
kubeContext = cliValue
}
kubeContext = newKubeContext
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
25 changes: 14 additions & 11 deletions pkg/skaffold/schema/profiles.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ import (
func ApplyProfiles(c *latest.SkaffoldConfig, opts cfg.SkaffoldOptions) error {
byName := profilesByName(c.Profiles)

profiles, hasContextActivatedProfile, err := activatedProfiles(c.Profiles, opts)
profiles, contextSpecificProfiles, err := activatedProfiles(c.Profiles, opts)
if err != nil {
return errors.Wrap(err, "finding auto-activated profiles")
}
Expand All @@ -55,10 +55,10 @@ func ApplyProfiles(c *latest.SkaffoldConfig, opts cfg.SkaffoldOptions) error {
}
}

return checkKubeContextConsistency(hasContextActivatedProfile, opts.KubeContext, c.Deploy.KubeContext)
return checkKubeContextConsistency(contextSpecificProfiles, opts.KubeContext, c.Deploy.KubeContext)
}

func checkKubeContextConsistency(isContextImmutable bool, cliContext, effectiveContext string) error {
func checkKubeContextConsistency(contextSpecificProfiles []string, cliContext, effectiveContext string) error {
// cli flag takes precedence
if cliContext != "" {
return nil
Expand All @@ -68,18 +68,21 @@ func checkKubeContextConsistency(isContextImmutable bool, cliContext, effectiveC
if err != nil {
return errors.Wrap(err, "getting current cluster context")
}
currentContext := kubeConfig.CurrentContext

// nothing to do
if effectiveContext == "" || effectiveContext == kubeConfig.CurrentContext || !isContextImmutable {
if effectiveContext == "" || effectiveContext == currentContext || len(contextSpecificProfiles) == 0 {
return nil
}

return fmt.Errorf("some activated profile contains kubecontext specific settings for a different than the effective kubecontext, please revise your profile activations")
return fmt.Errorf("profiles %q were activated by kube-context %q, but the effective kube-context is %q -- please revise your `profiles.activation` and `deploy.kubeContext` configurations", contextSpecificProfiles, currentContext, effectiveContext)
}

func activatedProfiles(profiles []latest.Profile, opts cfg.SkaffoldOptions) ([]string, bool, error) {
// activatedProfiles returns the activated profiles and activated profiles which are kube-context specific.
// The latter matters for error reporting when the effective kube-context changes.
func activatedProfiles(profiles []latest.Profile, opts cfg.SkaffoldOptions) ([]string, []string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this now returns a list of profiles and activated profile?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not quite. It returns a list of activated profiles and a list of activated profiles that are kube-context specific. I added a godoc to clarify this.

activated := opts.Profiles
hasContextActivatedProfile := false
var contextSpecificProfiles []string

// Auto-activated profiles
for _, profile := range profiles {
Expand All @@ -88,24 +91,24 @@ func activatedProfiles(profiles []latest.Profile, opts cfg.SkaffoldOptions) ([]s

env, err := isEnv(cond.Env)
if err != nil {
return nil, false, err
return nil, nil, err
}

kubeContext, err := isKubeContext(cond.KubeContext, opts)
if err != nil {
return nil, false, err
return nil, nil, err
}

if command && env && kubeContext {
if cond.KubeContext != "" {
hasContextActivatedProfile = true
contextSpecificProfiles = append(contextSpecificProfiles, profile.Name)
}
activated = append(activated, profile.Name)
}
}
}

return activated, hasContextActivatedProfile, nil
return activated, contextSpecificProfiles, nil
}

func isEnv(env string) (bool, error) {
Expand Down