Merge pull request #826 from bivas/fix-issue-360

add support for helm image convention vs fqn setting

Author: r2d4

examples/helm-deployment/skaffold-helm/values.yaml

@@ -2,10 +2,12 @@
 # This is a YAML-formatted file.
 # Declare variables to be passed into your templates.
 replicaCount: 1
-image:
-  repository: nginx
-  tag: stable
-  pullPolicy: IfNotPresent
+image: nginx:stable
+# This is the helm convention on declaring images
+# image:
+#   repository: nginx
+#   tag: stable
+#   pullPolicy: IfNotPresent
 service:
   name: nginx
   type: ClusterIP

pkg/skaffold/deploy/helm.go

@@ -129,7 +129,13 @@ func (h *HelmDeployer) deployRelease(out io.Writer, r v1alpha2.HelmRelease, buil
 	var setOpts []string
 	for k, v := range params {
 		setOpts = append(setOpts, "--set")
-		setOpts = append(setOpts, fmt.Sprintf("%s=%s", k, v.Tag))
+		if r.ImageStrategy.HelmImageConfig.HelmConventionConfig != nil {
+			tagSplit := strings.Split(v.Tag, ":")
+			imageRepositoryTag := fmt.Sprintf("%s.repository=%s,%s.tag=%s", k, tagSplit[0], k, tagSplit[1])
+			setOpts = append(setOpts, imageRepositoryTag)
+		} else {
+			setOpts = append(setOpts, fmt.Sprintf("%s=%s", k, v.Tag))
+		}
 	}
 
 	// First build dependencies.

pkg/skaffold/deploy/helm_test.go

@@ -54,7 +54,7 @@ var testDeployConfig = &v1alpha2.HelmDeploy{
 			Name:      "skaffold-helm",
 			ChartPath: "examples/test",
 			Values: map[string]string{
-				"image.tag": "skaffold-helm",
+				"image": "skaffold-helm",
 			},
 			Overrides: map[string]interface{}{
 				"foo": "bar",
@@ -66,13 +66,36 @@ var testDeployConfig = &v1alpha2.HelmDeploy{
 	},
 }
 
+var testDeployHelmStyleConfig = &v1alpha2.HelmDeploy{
+	Releases: []v1alpha2.HelmRelease{
+		{
+			Name:      "skaffold-helm",
+			ChartPath: "examples/test",
+			Values: map[string]string{
+				"image": "skaffold-helm",
+			},
+			Overrides: map[string]interface{}{
+				"foo": "bar",
+			},
+			SetValues: map[string]string{
+				"some.key": "somevalue",
+			},
+			ImageStrategy: v1alpha2.HelmImageStrategy{
+				HelmImageConfig: v1alpha2.HelmImageConfig{
+					HelmConventionConfig: &v1alpha2.HelmConventionConfig{},
+				},
+			},
+		},
+	},
+}
+
 var testDeployConfigParameterUnmatched = &v1alpha2.HelmDeploy{
 	Releases: []v1alpha2.HelmRelease{
 		{
 			Name:      "skaffold-helm",
 			ChartPath: "examples/test",
 			Values: map[string]string{
-				"image.tag": "skaffold-helm-unmatched",
+				"image": "skaffold-helm-unmatched",
 			},
 		},
 	},
@@ -84,7 +107,7 @@ var testDeployFooWithPackaged = &v1alpha2.HelmDeploy{
 			Name:      "foo",
 			ChartPath: "testdata/foo",
 			Values: map[string]string{
-				"image.tag": "foo",
+				"image": "foo",
 			},
 			Packaged: &v1alpha2.HelmPackaged{
 				Version:    "0.1.2",
@@ -222,13 +245,42 @@ func TestHelmDeploy(t *testing.T) {
 		{
 			description: "get failure should install not upgrade",
 			cmd: &MockHelm{
-				t:             t,
-				getResult:     fmt.Errorf("not found"),
+				t:         t,
+				getResult: fmt.Errorf("not found"),
+				installMatcher: func(cmd *exec.Cmd) bool {
+					expected := map[string]bool{fmt.Sprintf("image=%s", testBuilds[0].Tag): true}
+					for _, arg := range cmd.Args {
+						if expected[arg] {
+							return true
+						}
+					}
+					return false
+				},
 				upgradeResult: fmt.Errorf("should not have called upgrade"),
 			},
 			deployer: NewHelmDeployer(testDeployConfig, testKubeContext, testNamespace),
 			builds:   testBuilds,
 		},
+		{
+			description: "get failure should install not upgrade with helm image strategy",
+			cmd: &MockHelm{
+				t:         t,
+				getResult: fmt.Errorf("not found"),
+				installMatcher: func(cmd *exec.Cmd) bool {
+					builds := strings.Split(testBuilds[0].Tag, ":")
+					expected := map[string]bool{fmt.Sprintf("image.repository=%s,image.tag=%s", builds[0], builds[1]): true}
+					for _, arg := range cmd.Args {
+						if expected[arg] {
+							return true
+						}
+					}
+					return false
+				},
+				upgradeResult: fmt.Errorf("should not have called upgrade"),
+			},
+			deployer: NewHelmDeployer(testDeployHelmStyleConfig, testKubeContext, testNamespace),
+			builds:   testBuilds,
+		},
 		{
 			description: "get success should upgrade not install",
 			cmd: &MockHelm{
@@ -305,13 +357,18 @@ func TestHelmDeploy(t *testing.T) {
 	}
 }
 
+type CommandMatcher func(*exec.Cmd) bool
+
 type MockHelm struct {
 	t *testing.T
 
-	getResult     error
-	installResult error
-	upgradeResult error
-	depResult     error
+	getResult      error
+	getMatcher     CommandMatcher
+	installResult  error
+	installMatcher CommandMatcher
+	upgradeResult  error
+	upgradeMatcher CommandMatcher
+	depResult      error
 
 	packageOut    io.Reader
 	packageResult error
@@ -339,10 +396,19 @@ func (m *MockHelm) RunCmd(c *exec.Cmd) error {
 
 	switch c.Args[3] {
 	case "get":
+		if m.getMatcher != nil && !m.getMatcher(c) {
+			m.t.Errorf("get matcher failed to match cmd")
+		}
 		return m.getResult
 	case "install":
+		if m.installMatcher != nil && !m.installMatcher(c) {
+			m.t.Errorf("install matcher failed to match cmd")
+		}
 		return m.installResult
 	case "upgrade":
+		if m.upgradeMatcher != nil && !m.upgradeMatcher(c) {
+			m.t.Errorf("upgrade matcher failed to match cmd")
+		}
 		return m.upgradeResult
 	case "dep":
 		return m.depResult

pkg/skaffold/schema/v1alpha2/config.go

@@ -152,6 +152,7 @@ type HelmRelease struct {
 	Wait              bool                   `yaml:"wait"`
 	Overrides         map[string]interface{} `yaml:"overrides"`
 	Packaged          *HelmPackaged          `yaml:"packaged"`
+	ImageStrategy     HelmImageStrategy      `yaml:"imageStrategy"`
 }
 
 // HelmPackaged represents parameters for packaging helm chart.
@@ -163,6 +164,24 @@ type HelmPackaged struct {
 	AppVersion string `yaml:"appVersion"`
 }
 
+type HelmImageStrategy struct {
+	HelmImageConfig `yaml:",inline"`
+}
+
+type HelmImageConfig struct {
+	HelmFQNConfig        *HelmFQNConfig        `yaml:"fqn"`
+	HelmConventionConfig *HelmConventionConfig `yaml:"helm"`
+}
+
+// HelmFQNConfig represents image config to use the FullyQualifiedImageName as param to set
+type HelmFQNConfig struct {
+	Property string `yaml:"property"`
+}
+
+// HelmConventionConfig represents image config in the syntax of image.repository and image.tag
+type HelmConventionConfig struct {
+}
+
 // Artifact represents items that need to be built, along with the context in which
 // they should be built.
 type Artifact struct {