Skaffold: Image handling does not match values.yaml structure from helm create

[afrittoli]

The sample chart created by helm create uses nested variables for docker images.
The same is a common practice in stable charts as well:

image:
  repository: nginx
  tag: stable
  pullPolicy: IfNotPresent

The manifest template fetches the details of image from .Values accordingly:

      containers:
        - name: {{ .Chart.Name }}
           image: "{{ .Values.image.repository }}:{{ .Values.image.tag }}"
           imagePullPolicy: {{ .Values.image.pullPolicy }}

Skaffold overrides the image using the key specified in its pipeline configuration, and it appends the image_name:tag to it, which is not compatible with the value being split into repository and tag.

Expected behavior

The rendered manifest should be something like:

      containers:
        - name: my-chart
           image: image1:<tag-from-skaffold-driven-build>

Actual behavior

Instead what happens is:

      containers:
        - name: my-chart
           image: image1:<tag-from-skaffold-driven-build>:<default tag from chart values>

Information

Skaffold version: v0.2.0 (built from source)

$ git describe --tags
v0.2.0-91-g88530bd

Operating system: macOS Sierra 10.12.6
Content of skaffold.yaml:

build:
  tagPolicy: sha256
  artifacts:
  - imageName: image1
    workspace: image1
  local: {}
deploy:
  helm:
    releases:
    - name: dev
      chartPath: mychart
      namespace: default
      valuesFilePath: skaffold/dev-values.yaml
      values:
        service1_image.repository: image1
        service2_image.repository: image2

I appreciate that there is no fixed structure for how the image variable is defined, but the current behaviour does not help using skaffold with many stock charts, and it forces putting image name and tag together.

[r2d4]

Thanks for the descriptive issue. This is something I noticed when developing the helm deployer. I'm not sure what the best path forward is.

We should absolutely work with stock charts that separate image name and tag. I'm not sure the best way to expose this. The helm deployer could just inject tag only by default. How does that sound?

[afrittoli]

Thanks for the quick reply.

That sounds good. Probably skaffold should support both the current behaviour of appending the tag to the image as well as injecting .tag as well, perhaps with some extra config, so it doesn't break existing skaffold users.

One idea - a bit convoluted but it supports the existing syntax as well.
When matching images, if .tag is found as a value, the tag is injected in the corresponding key. Otherwise it is attached to the repo and name like today.

For example the following:

deploy:
  helm:
    releases:
    - name: dev
      chartPath: mychart
      namespace: default
      valuesFilePath: skaffold/dev-values.yaml
      values:
        service1_image.repository: image1
        service1_image.tag: image1.tag
        service2_image: image2

would lead skaffold to invoke helm with these parameters:

--set service1_image.repository=image1 --set service1_image.tag=<skaffold image1 build tag>
--set service2_image: image2:<skaffold image2 build tag>

[hemslo]

I find a workaround for current version:

change skaffold.yaml to

values:
  # image: skaffold-helm
  imageName: skaffold-helm

then change deployment.yaml

containers:
  - name: {{ .Chart.Name }}
    # image: {{ .Values.image }}
    image: {{ if .Values.imageName }}{{ .Values.imageName }}{{ else }}"{{ .Values.image.repository }}:{{ .Values.image.tag }}"{{ end }}
    # imagePullPolicy: {{ .Values.pullPolicy }}
    imagePullPolicy: {{ .Values.image.pullPolicy }}

I think the correct way is not to use tagger.GenerateFullyQualifiedImageName as tag, use image name and tag separately.

[wstrange]

Just to throw in one more wrinkle: We often split our image definition into three parts:

image: "{{ .Values.image.repository }}/{{.Values.image.component}}:{{ .Values.image.tag }}"

Where component is the application (nginx, my-app, etc.)

We do this so we can easily swap out the top level repository (say from quay.io to the docker hub).

[afrittoli]

The way that I deal with this issue right now is to have an umbrella chart and allow for override values compatible with skaffold, e.g. in values.yaml:

# overrideMyImageFullName: <registry>/<ns>/<image-name>:<tag>

And then I have a named template to handle it:

{{- define "my.image" -}}
{{- if .Values. overrideMyImageFullName -}}
{{- .Values. overrideMyImageFullName -}}
{{- else -}}
{{- printf "%s/%s:%s" .Values.my-image.repository .Values.my-image.name .Values.my-image.tag -}}
{{- end -}}
{{- end -}}

[bivas]

I've envisioned something in the config line of:

type HelmRelease struct {
    Name              string                 `yaml:"name"`
    ChartPath         string                 `yaml:"chartPath"`
    ValuesFilePath    string                 `yaml:"valuesFilePath"`
    Values            map[string]string      `yaml:"values,omitempty"`
    Namespace         string                 `yaml:"namespace"`
    Version           string                 `yaml:"version"`
    SetValues         map[string]string      `yaml:"setValues"`
    SetValueTemplates map[string]string      `yaml:"setValueTemplates"`
    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.
type HelmPackaged struct {
    // Version sets the version on the chart to this semver version.
    Version string `yaml:"version"`

    // AppVersion set the appVersion on the chart to this version
    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 {
    UseNameAsPrefix bool `yaml:"useNameAsPrefix"`
}

This will enable the following example deployment config:

deploy:
  helm:
    releases:
      - name: shell
        chartPath: stable/my-chart
        values:
          image: acme/my-container
        imageStrategy:
          helm: {}

will create the following CLI: helm install ... --set image.repository=acme/my-container --set image.tag=<generated>

and if we use:

imageStrategy:
  fqn: {}

CLI will be: helm install ... --set image=acme/my-container:<generated>

[enriched]

I don't know if there is a smarter/more efficient way to do this, but I ended up using this for a helm chart with a lot of different images that I wanted to easily set the tag and registry for CD purposes.

More variables could be added to the coalesce if there were other places where values would be stored. And instead of passing in a string variable as the second part of the list, it could be a dict for a "helm convention" .Values.image. But the general idea is, extract the registry, repository, and tag from the passed in string, and if they aren't there use the defaults.

{{- /*
This helper is used to add the .Values.defaultRegistry and .Values.defaultTag,
if they are not specified in the passed in image string. It is called like:
{{ template "my-chart.image" (list . .Values.image) }}
*/ -}}
{{- define "my-chart.image" -}}
{{- $root := index . 0 -}}
{{- $serviceImage  := index . 1 -}}
{{- $imageRegex := "(.+/)?([^:]+)(:.+)?" -}}
{{- $serviceImageRegistry := regexReplaceAll $imageRegex $serviceImage "${1}" -}}
{{- $serviceImageRepository := regexReplaceAll $imageRegex $serviceImage "${2}" -}}
{{- $serviceImageTag := regexReplaceAll $imageRegex $serviceImage "${3}" -}}
{{- $registry := required "Could not determine image registry" (trimSuffix "/" (coalesce $serviceImageRegistry $root.Values.defaultRegistry)) -}}
{{- $repository := required "Could not determine image repository" (coalesce $serviceImageRepository) -}}
{{- $tag := required "Could not determine image tag" (trimAll ":" (coalesce $serviceImageTag $root.Values.defaultTag)) -}}
{{- printf "%s/%s:%s" $registry $repository $tag -}}
{{- end -}}

[balopat]

@enriched can you open a separate issue for that?
@afrittoli I'm wondering if we can close this issue given that the helmConvention config is in place now?

[afrittoli]

Yes, thank you