Skip to content

Commit

Permalink
Merge pull request #2993 from corneliusweig/w/kubecontext/improve-errors
Browse files Browse the repository at this point in the history
Improve error messages for `deploy.kubeContext` error cases
  • Loading branch information
tejal29 authored Oct 9, 2019
2 parents 67328ad + dd595a9 commit 2810eee
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 31 deletions.
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) {
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

0 comments on commit 2810eee

Please sign in to comment.