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

Add support for setting default-repo in global config #1057

Merged
merged 18 commits into from
Oct 23, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions cmd/skaffold/app/cmd/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ func AddRunDevFlags(cmd *cobra.Command) {
cmd.Flags().BoolVar(&opts.Notification, "toot", false, "Emit a terminal beep after the deploy is complete")
cmd.Flags().StringArrayVarP(&opts.Profiles, "profile", "p", nil, "Activate profiles by name")
cmd.Flags().StringVarP(&opts.Namespace, "namespace", "n", "", "Run deployments in the specified namespace")
cmd.Flags().StringVarP(&opts.DefaultRepo, "default-repo", "d", "", "Default repository value (overrides global config)")
}

func SetUpLogs(out io.Writer, level string) error {
Expand Down
25 changes: 17 additions & 8 deletions cmd/skaffold/app/cmd/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ func TestReadConfig(t *testing.T) {
}

func TestSetAndUnsetConfig(t *testing.T) {
dummyContext := "dummy_context"
var tests = []struct {
expectedSetCfg *Config
expectedUnsetCfg *Config
Expand Down Expand Up @@ -103,7 +104,11 @@ func TestSetAndUnsetConfig(t *testing.T) {
key: "not_a_real_value",
shouldErrSet: true,
expectedSetCfg: &Config{
ContextConfigs: []*ContextConfig{{}},
ContextConfigs: []*ContextConfig{
{
Kubecontext: dummyContext,
},
},
},
},
{
Expand All @@ -130,8 +135,18 @@ func TestSetAndUnsetConfig(t *testing.T) {
c, _ := yaml.Marshal(*emptyConfig)
cfg, teardown := testutil.TempFile(t, "config", c)

defer func() {
// reset all config context for next test
teardown()
kubecontext = dummyContext
configFile = ""
global = false
}()

// setup config context
kubecontext = test.kubecontext
if test.kubecontext != "" {
kubecontext = test.kubecontext
}
configFile = cfg
global = test.global

Expand All @@ -155,12 +170,6 @@ func TestSetAndUnsetConfig(t *testing.T) {
t.Error(cfgErr)
}
testutil.CheckErrorAndDeepEqual(t, false, err, test.expectedUnsetCfg, newConfig)

// reset all config context for next test
teardown()
kubecontext = ""
configFile = ""
global = false
})
}
}
3 changes: 1 addition & 2 deletions cmd/skaffold/app/cmd/config/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ func NewCmdList(out io.Writer) *cobra.Command {
Short: "List all values set in the global skaffold config",
Args: cobra.ExactArgs(0),
RunE: func(cmd *cobra.Command, args []string) error {
resolveKubectlContext()
return runList(out)
},
}
Expand All @@ -55,7 +54,7 @@ func runList(out io.Writer) error {
return errors.Wrap(err, "marshaling config")
}
} else {
config, err := getConfigForKubectx()
config, err := GetConfigForKubectx()
if err != nil {
return err
}
Expand Down
1 change: 0 additions & 1 deletion cmd/skaffold/app/cmd/config/set.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ func NewCmdSet(out io.Writer) *cobra.Command {
Short: "Set a value in the global skaffold config",
Args: cobra.ExactArgs(2),
RunE: func(cmd *cobra.Command, args []string) error {
resolveKubectlContext()
if err := setConfigValue(args[0], args[1]); err != nil {
return err
}
Expand Down
51 changes: 48 additions & 3 deletions cmd/skaffold/app/cmd/config/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package config

import (
"fmt"
"io/ioutil"
"path/filepath"

Expand Down Expand Up @@ -59,7 +60,10 @@ func resolveConfigFile() error {
return util.VerifyOrCreateFile(configFile)
}

// ReadConfigForFile reads the specified file and returns the contents
// parsed into a Config struct.
func ReadConfigForFile(filename string) (*Config, error) {
fmt.Printf("reading config for file %s\n", filename)
Copy link
Contributor

Choose a reason for hiding this comment

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

ux-nit: this feels like a bit of clutter on the screen - can we make it available only for debug logging?

contents, err := ioutil.ReadFile(filename)
if err != nil {
return nil, errors.Wrap(err, "reading global config")
Expand All @@ -78,10 +82,12 @@ func readConfig() (*Config, error) {
return ReadConfigForFile(configFile)
}

// return the specific config to be modified based on the provided kube context.
// either returns the config corresponding to the provided or current context,
// GetConfigForKubectx returns the specific config to be modified based on the
// provided kube context.
// Either returns the config corresponding to the provided or current context,
// or the global config if that is specified (or if no current context is set).
func getConfigForKubectx() (*ContextConfig, error) {
func GetConfigForKubectx() (*ContextConfig, error) {
resolveKubectlContext()
cfg, err := readConfig()
if err != nil {
return nil, err
Expand All @@ -98,7 +104,17 @@ func getConfigForKubectx() (*ContextConfig, error) {
return nil, nil
}

// GetGlobalConfig returns the global config values
func GetGlobalConfig() (*ContextConfig, error) {
cfg, err := readConfig()
if err != nil {
return nil, err
}
return cfg.Global, nil
}

func getOrCreateConfigForKubectx() (*ContextConfig, error) {
resolveKubectlContext()
cfg, err := readConfig()
if err != nil {
return nil, err
Expand Down Expand Up @@ -129,3 +145,32 @@ func getOrCreateConfigForKubectx() (*ContextConfig, error) {

return newCfg, nil
}

func GetDefaultRepo(cliValue string) (string, error) {
// CLI flag takes precedence. If no default-repo specified from a flag,
// retrieve the value from the global config.
if cliValue != "" {
return cliValue, nil
}
cfg, err := GetConfigForKubectx()
if err != nil {
return "", errors.Wrap(err, "retrieving global config")
}
var defaultRepo string
if cfg != nil {
defaultRepo = cfg.DefaultRepo
}
if defaultRepo == "" {
// if we don't have a defaultRepo value set for the current context,
// retrieve the global config and use this value as a fallback
cfg, err := GetGlobalConfig()
if err != nil {
return "", errors.Wrap(err, "retrieving global config")
}
if cfg != nil {
defaultRepo = cfg.DefaultRepo
}
}

return defaultRepo, nil
}
25 changes: 25 additions & 0 deletions cmd/skaffold/app/cmd/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@ limitations under the License.
package cmd

import (
configutil "github.com/GoogleContainerTools/skaffold/cmd/skaffold/app/cmd/config"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
"github.com/pkg/errors"
)

Expand All @@ -41,10 +43,33 @@ func newRunner(opts *config.SkaffoldOptions) (*runner.SkaffoldRunner, *latest.Sk
return nil, nil, errors.Wrap(err, "applying profiles")
}

defaultRepo, err := configutil.GetDefaultRepo(opts.DefaultRepo)
if err != nil {
return nil, nil, errors.Wrap(err, "getting default repo")
}

if err = applyDefaultRepoSubstitution(config, defaultRepo); err != nil {
return nil, nil, errors.Wrap(err, "substituting default repos")
}

runner, err := runner.NewForConfig(opts, config)
if err != nil {
return nil, nil, errors.Wrap(err, "creating runner")
}

return runner, config, nil
}

func applyDefaultRepoSubstitution(config *latest.SkaffoldPipeline, defaultRepo string) error {
if defaultRepo == "" {
// noop
return nil
}
for _, artifact := range config.Build.Artifacts {
artifact.ImageName = util.SubstituteDefaultRepoIntoImage(defaultRepo, artifact.ImageName)
}
for _, testCase := range config.Test {
testCase.ImageName = util.SubstituteDefaultRepoIntoImage(defaultRepo, testCase.ImageName)
}
return nil
}
1 change: 1 addition & 0 deletions pkg/skaffold/config/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ type SkaffoldOptions struct {
Trigger string
CustomLabels []string
WatchPollInterval int
DefaultRepo string
}

// Labels returns a map of labels to be applied to all deployed
Expand Down
17 changes: 0 additions & 17 deletions pkg/skaffold/deploy/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,20 +111,3 @@ func (m *multiDeployer) Cleanup(ctx context.Context, w io.Writer) error {

return nil
}

func joinTagsToBuildResult(builds []build.Artifact, params map[string]string) (map[string]build.Artifact, error) {
imageToBuildResult := map[string]build.Artifact{}
for _, build := range builds {
imageToBuildResult[build.ImageName] = build
}

paramToBuildResult := map[string]build.Artifact{}
for param, imageName := range params {
build, ok := imageToBuildResult[imageName]
if !ok {
return nil, fmt.Errorf("No build present for %s", imageName)
}
paramToBuildResult[param] = build
}
return paramToBuildResult, nil
}
24 changes: 22 additions & 2 deletions pkg/skaffold/deploy/helm.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,17 @@ type HelmDeployer struct {

kubeContext string
namespace string
defaultRepo string
}

// NewHelmDeployer returns a new HelmDeployer for a DeployConfig filled
// with the needed configuration for `helm`
func NewHelmDeployer(cfg *latest.HelmDeploy, kubeContext string, namespace string) *HelmDeployer {
func NewHelmDeployer(cfg *latest.HelmDeploy, kubeContext string, namespace string, defaultRepo string) *HelmDeployer {
return &HelmDeployer{
HelmDeploy: cfg,
kubeContext: kubeContext,
namespace: namespace,
defaultRepo: defaultRepo,
}
}

Expand Down Expand Up @@ -125,7 +127,7 @@ func (h *HelmDeployer) deployRelease(ctx context.Context, out io.Writer, r lates
color.Red.Fprintf(out, "Helm release %s not installed. Installing...\n", releaseName)
isInstalled = false
}
params, err := joinTagsToBuildResult(builds, r.Values)
params, err := h.joinTagsToBuildResult(builds, r.Values)
if err != nil {
return nil, errors.Wrap(err, "matching build results to chart values")
}
Expand Down Expand Up @@ -336,6 +338,24 @@ func (h *HelmDeployer) deleteRelease(ctx context.Context, out io.Writer, r lates
return nil
}

func (h *HelmDeployer) joinTagsToBuildResult(builds []build.Artifact, params map[string]string) (map[string]build.Artifact, error) {
imageToBuildResult := map[string]build.Artifact{}
for _, build := range builds {
imageToBuildResult[build.ImageName] = build
}

paramToBuildResult := map[string]build.Artifact{}
for param, imageName := range params {
newImageName := util.SubstituteDefaultRepoIntoImage(h.defaultRepo, imageName)
build, ok := imageToBuildResult[newImageName]
if !ok {
return nil, fmt.Errorf("No build present for %s", imageName)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return nil, fmt.Errorf("No build present for %s", imageName)
return nil, fmt.Errorf("no build present for %s", imageName)

nit: errors shouldn't start capitalized https://github.com/golang/go/wiki/CodeReviewComments#error-strings

}
paramToBuildResult[param] = build
}
return paramToBuildResult, nil
}

func evaluateReleaseName(nameTemplate string) (string, error) {
tmpl, err := util.ParseEnvTemplate(nameTemplate)
if err != nil {
Expand Down
Loading