Helm deployer examples still reference artifactOverrides

[cmeury]

See: https://skaffold.dev/docs/pipeline-stages/deployers/helm/#image-configuration (file: docs-v2/content/en/docs/pipeline-stages/deployers/helm.md)

Coincidentally, it's also mentioned here and in some other places (maybe a global grep might be useful):

• examples/helm-deployment/README.md

Also, the alert at the top of the page mentions artifactsOverride when in fact the key was called artifactOverrides.

[alewis001]

With help from @cmeury I was able to get my migration from v1 to v2 to work. The migration required the changed below. Therefore It may be beneficial to others to include this information in the documentation when addressing this issue.

Changes

The image: mapping within the Helm templates is no longer {{ .Values.image }} but "{{ .Values.image.repository }}:{{ .Values.image.tag }}" and then include the following within the releases sequence of the manifest section in skaffold.yaml:

setValueTemplates:
  image.repository: '{{.IMAGE_REPO}}'
  image.tag: '{{.IMAGE_TAG}}@{{.IMAGE_DIGEST}}'

[cmeury]

As mentioned in the MR, artifactOverrides is still shown as examples on the page. Don't think this is resolved completely.

[chris13524]

Looks like imageStrategy was also removed? I was using the fqn mode which was the default. To workaround, I can use use setValueTemplates w/ a value of {{.IMAGE_REPO}}:{{.IMAGE_TAG}}@{{.IMAGE_DIGEST}}. But this becomes increasingly challenging since I have a dozen or more images, so I need to use N

It's unclear to me what Skaffold's new default imageStrategy is when not specifying setValueTemplates. Seems like it can infer the artifact based on the values file structure?

[toniopelo]

I don't understand how it's supposed to work either @chris13524, did you find out the logic behind the implementation (without imageStrategy ?).

cc @aaron-prindle, as mentionned above imageStrategy is not in the schema anymore and you PR doesn't correct that part. adding a little more guidance on how to have the helm classic values structure (as following) would be a lot of help :)

# example values file where skaffold should replace values for every image it builds

service1:
  image:
    repository:
    tag:

service2:
  image:
    repository:
    tag:

I'm using setValueTemplates but it's cumbersome when you have multiple images, and it also produce the following warning for every image which make me think it's not the intended way:

WARN[0025] image [...] is not used.  subtask=-1 task=DevLoop
WARN[0025] See helm documentation on how to replace image names with their actual tags: https://skaffold.dev/docs/pipeline-stages/deployers/helm/#image-configuration  subtask=-1 task=DevLoop

[falcorocks]

@toniopelo hey, I have a similar situation to yours, but I can't get the deploy tag to match that used in build. Can you provide me with a full example of your workaround?

# my values.yaml
base:
  image:
    repository: xxx/base
    tag: v1.7.5
    pullPolicy: IfNotPresent

webserver:
  image:
    repository: xxx/webserver
    tag: v1.7.5

# skaffold.yaml
apiVersion: skaffold/v3
kind: Config
metadata:
  name: myapp
build:
  artifacts:
    - image: xxx/base
      context: ./base
    - image: xxx/webserver
      context: .
deploy:
  helm:
    releases:
    - name: myapp
      createNamespace: true
      namespace: dev
      chartPath: .
      version: 0.1.0
      valuesFiles:
        - values.yaml
      setValueTemplates:
        base.image.repository: '{{.IMAGE_NAME}}'
        base.image.tag: '{{.IMAGE_TAG}}'
        webserver.image.repository: '{{.IMAGE_NAME2}}'
        webserver.image.tag: '{{.IMAGE_TAG2}}'

$ skaffold dev  
Listing files to watch...
 - xxx/base
 - xxx/webserver
Generating tags...
 - xxx/base -> xxx/base:v1.7.5 #notice that this is a git submodule with tree set at tag v1.7.5
 - xxx/webserver -> xxx/webserver:32ed75d-dirty
Tags used in deployment:
 - xxx/base -> xxx/base:2f007284f8587f4618c03921aa8972a8bfc0a726de9a925d87a4b270d4edd474
 - xxx/webserver -> xxx/webserver:bcd5006051826c35e7cd38336593eb6a07c7c48bab4fd0fa3ca4504d1469b9b9
# which then fails because of mismatch

[aaron-prindle]

Adding some more context here. The change to remove artifactOverrides (and subsequently imageStrategy) comes from the PR here which explains the rationale: #6949

This PR removes the Helm deployer's artifactOverrides configuration by using Skaffold as the Helm post-renderer to perform our manifest transformations, including image replacement. This simplifies configuring Helm projects and also avoiding resource churn (e.g., setting a label on a deployments will re-create its pods; #3133).

This change requires moving our minimum supported Helm version to 3.1.0. This change will break any users that use their own post-renderer hooks via deploy.helm.flags.

I think some of the context and important information was not conveyed in the current No more artifactOverrides note we have here - https://skaffold.dev/docs/pipeline-stages/deployers/helm/#configuring-your-helm-project-with-skaffold. I will update the note there and the examples to have some additional/more-complex solutions as well as explicitly callout that imageStrategy is also removed. The actual logic for migrating v1 artifactOverrides to v2 setValues (for imageStrategy:fqn) or setValues & setValueTemplates (for imageStrategy:helm) is here:
https://github.com/GoogleContainerTools/skaffold/blob/main/pkg/skaffold/schema/v2beta29/upgrade.go#L118-L143
(added in this PR #7707)

EDIT: from the additional responses below it seems that currently the logic there is not correct for all cases leaving users to manual change entries. Hoping to resolve this with continued conversation here to get a repro case

[aaron-prindle]

@falcorocks I believe the issue you are having is that the tag values:

base.image.tag: '{{.IMAGE_TAG}}'

should instead be

base.image.tag: '{{.IMAGE_TAG}}@{{.IMAGE_DIGEST}}'

[aaron-prindle]

@toniopelo my PR attempted to acccount for fqn and helm image strategies (see the in the linked code below): https://github.com/GoogleContainerTools/skaffold/blob/main/pkg/skaffold/schema/v2beta29/upgrade.go#L118-L143

Is there possibly an issue in the logic there?

[aaron-prindle]

Created #8057 to track adding more context and migration information to the relevant helm documentation

[aaron-prindle]

Also did users here find that skaffold fix (or the auto-upgrade) broke them on a v2.X.X binary using a v1 binary schema (confusingly a v1 binary schema meaning apiVersion: skaffold/v2beta*) and they required manual tweaks to their skaffold.yaml?

[alewis001]

Hi @aaron-prindle , yes skaffold fix didn't produce a working file. The output from fix used setValues but I had to change that to setValuesTemplate as per my earlier comment here: #8002 (comment)

I'll try to get the "before" and the "after fix" today if I can, unfortunately it's an awkward day where I won't be at my machine much 🙂

[aaron-prindle]

@alewis001 can you post a redacted version of the v1 skaffold.yaml that did not work in the thread here? That way we can try to fix this issue in skaffold fix. Thanks!

EDIT: I see you added some info regarding the "before" and "after fix" at the bottom of your message - that would be great thanks!

[alewis001]

@alewis001 can you post a redacted version of the v1 skaffold.yaml that did not work in the thread here? That way we can try to fix this issue in `skaffold fix

Ha, I just updated my previous comment before you posted. Yes, I'll try to get that today if I can. Unfortunately not at my computer much today so it might be tomorrow.

[falcorocks]

@falcorocks I believe the issue you are having is that the tag values:

base.image.tag: '{{.IMAGE_TAG}}'

should instead be

base.image.tag: '{{.IMAGE_TAG}}@{{.IMAGE_DIGEST}}'

thank you @aaron-prindle , but it's still won't work, even in a single image scenario. BTW I'm working with a local kubernetes cluster (k3d) could this be relevant?

# skaffold.yaml
apiVersion: skaffold/v3
kind: Config
metadata:
  name: crs-watcher
build:
  artifacts:
    - image: registry.gitlab.com/cyberrs/watcher
deploy:
  helm:
    releases:
    - name: crs-watcher
      namespace: staging
      createNamespace: true
      chartPath: .
      valuesFiles:
        - values.yaml
      setValueTemplates:
        image.repository: '{{.IMAGE_REPO}}'
        image.tag: '{{.IMAGE_TAG}}@{{.IMAGE_DIGEST}}'

# deployment.yaml
apiVersion: apps/v1
kind: Deployment
metadata:
  creationTimestamp: null
  labels:
    app: cyberrs-watcher
  name: {{ .Release.Name }}-watcher
  namespace: {{ .Release.Namespace }}
spec:
  replicas: 1
  selector:
    matchLabels:
      app: cyberrs-watcher
  template:
    metadata:
      creationTimestamp: null
      labels:
        app: cyberrs-watcher
    spec:
      containers:
      - image: {{ .Values.image.repository }}:{{ .Values.image.tag }}
        name: watcher
        resources: {}

# values.yaml
image:
  repository: registry.gitlab.com/cyberrs/watcher
  tag: latest

$ skaffold dev
Listing files to watch...
 - registry.gitlab.com/cyberrs/watcher
Generating tags...
 - registry.gitlab.com/cyberrs/watcher -> registry.gitlab.com/cyberrs/watcher:69dae48
Checking cache...
 - registry.gitlab.com/cyberrs/watcher: Found. Tagging
Tags used in deployment:
 - registry.gitlab.com/cyberrs/watcher -> registry.gitlab.com/cyberrs/watcher:9552842b0246863c7c151716c102822d5dd23ee17fe9c54f9dd26a9ea8e403e5
Starting deploy...
Loading images into k3d cluster nodes...
Images loaded in 84ns
Release "crs-watcher" has been upgraded. Happy Helming!
NAME: crs-watcher
LAST DEPLOYED: Thu Nov 10 10:46:02 2022
NAMESPACE: staging
STATUS: deployed
REVISION: 12
TEST SUITE: None
WARN[0006] image [registry.gitlab.com/cyberrs/watcher:9552842b0246863c7c151716c102822d5dd23ee17fe9c54f9dd26a9ea8e403e5] is not used.  subtask=-1 task=DevLoop
WARN[0006] See helm documentation on how to replace image names with their actual tags: https://skaffold.dev/docs/pipeline-stages/deployers/helm/#image-configuration  subtask=-1 task=DevLoop
Waiting for deployments to stabilize...
 - staging:deployment/crs-watcher-watcher: InspectFailed: Failed to apply default image tag "registry.gitlab.com/cyberrs/watcher:9552842b0246863c7c151716c102822d5dd23ee17fe9c54f9dd26a9ea8e403e5@": couldn't parse image reference "registry.gitlab.com/cyberrs/watcher:9552842b0246863c7c151716c102822d5dd23ee17fe9c54f9dd26a9ea8e403e5@": invalid reference format
    - staging:pod/crs-watcher-watcher-858d468b76-wvpb9: InspectFailed: Failed to apply default image tag "registry.gitlab.com/cyberrs/watcher:9552842b0246863c7c151716c102822d5dd23ee17fe9c54f9dd26a9ea8e403e5@": couldn't parse image reference "registry.gitlab.com/cyberrs/watcher:9552842b0246863c7c151716c102822d5dd23ee17fe9c54f9dd26a9ea8e403e5@": invalid reference format

[alewis001]

Hi,

Hopefully the attached files are useful. I have removed some profiles as they were just repeats with minor changes but I have left a single profile in each file.

Skaffold v1 (api version: v2beta16)

The Helm Template spec.template.spec.containers.image: {{ .Values.image }} in order to have the value populated by Skaffold v1 binary (this was as a result of my understanding of the docs and it worked so I assumed, rightly or wrongly, that I was doing the right thing).

When using Skaffold v2 binary, the Helm Template value was null.

skaffold.v2beta16.yaml.txt

Skaffold v2 (api version: v3)

This file was generated using skaffold fix. This did not change the behaviour of skaffold v2 binary, the image value remained null.

skaffold.v3.yaml.txt

Skaffold v2 (api version: v3) - Fixed

After guidance from @cmeury, I made the changes as I mentioned in this comment to produce this file.

I.e. Changed the Helm Template to spec.template.spec.containers.image: "{{ .Values.image.repository }}:{{ .Values.image.tag }}" and the following in the skaffold.yaml:

setValueTemplates:
  image.repository: '{{.IMAGE_REPO}}'
  image.tag: '{{.IMAGE_TAG}}@{{.IMAGE_DIGEST}}'

skaffold.v3-fixed.yaml.txt

[aaron-prindle]

Thanks for the information @alewis001. In reading through this I still trying to understand how the skaffold.v3.yaml.txt file produced null for the image. If I understand correctly, your project setup is (with respect to helm + skaffold) very similar to our example here:
https://github.com/GoogleContainerTools/skaffold/blob/main/examples/helm-deployment/skaffold.yaml

which does have it's image: field populated:

skaffold render output

apiVersion: apps/v1
kind: Deployment
metadata:
  labels:
    app: skaffold-helm
  name: skaffold-helm
spec:
  replicas: 2
  selector:
    matchLabels:
      app: skaffold-helm
  template:
    metadata:
      labels:
        app: skaffold-helm
    spec:
      containers:
      - image: gcr.io/aprindle-test-cluster/skaffold-helm:latest@sha256:9b1a4826a617ebd2d17527cebad1ca6584e470116235da13528553f7a07149eb
        name: skaffold-helm

Ideally we want to move away from using setValueTemplates in our migration logic as it is not-intuitive as a result of #5317 (#8008 shows an example of this). Can you possibly share any logs for the failing runs? Currently my understanding of the issues with migrating from v1 -> v2 using artifactOverrides is essentially that setValueTemplates doesn't work as the fix logic made incorrect assumptions about the env vars being correct for a given replacement (IMAGE_TAG is not sufficient for multi-config, need IMAGE_TAG, IMAGE_TAG2, IMAGE_TAGX specified which is non-intuitive and non-realistic UX as you have to know the artifact order) and currently the new solution (#8066) would be to instead do something like:

imageStrategy: fqn ->

setValueTemplates:
  image: image-name

imageStrategy: helm ->

setValueTemplates:
  image.repository: image-name
  image.tag: image-name

imageStrategy: helm+explicitRegistry ->

  setValueTemplates:
    image.regsitry: image-name
    image.repository: image-name
    image.tag: image-name

[alewis001]

It's the -p minikube that is causing the null. If I do skaffold render without -p minikube then the image: value is no longer null; however, it does not have the repo address or sha256 and instead it appears to be the build.artifacts.image value verbatim.

I also use the --default-repo argument when running commands such as run, render, etc. as I work with a number of clusters and different container registries. I don't know whether that explains the lack of repo and digest on the generated image value? I tried removing the tagPolicy section I have in my skaffold.yaml file and that didn't appear to affect the image: value.

I'll try to collect logs but as I'll have to go through them to redact them, it might take a while to produce. I'll be as quick as I can.

[alewis001]

A bit more digging, and using the output from skaffold fix rather than any manual edits...

In the values files for the Helm template, I didn't have a image attribute set as I was relying on skaffold always setting it.

The behaviour is:

• image not defined/set in Helm values files + -p <profile> = null in rendered output.
• image not defined/set in Helm values files + -p <profile> not set = value in skaffold file in rendered output but... no repo or digest.
• image defined in values file + -p <profile> = value from helm template values file.
• image defined in values file + -p <profile> not set = value in skaffold file in rendered output but... no repo or digest.

So it would appear that using -p <profile> causes skaffold to use whatever is in the values file(s) or null if not defined. If no profile is specified then it uses the artifact name in the skaffold file but is for some reason not populating the repo name and/or the digest.

[alewis001]

The odd behaviour of image value not including the repo or the tag is that the image value is in a StatefulSet and not a Deployment. If I change the helm template from SS to Deployment, the image: value is then correct.

I did try adding the profile argument back in once I discovered the SS/Deployment issue but as soon as the profile is included, the image: value is just set with whatever is set via Helm.

[aaron-prindle]

Ah thank you very much for the additional information, I believe I understand the issue now. Now that helm uses Skaffold as a --post-renderer to do our image replacement (eg: image: skaffold-helm-image -> image: gcr.io/aprindle-test-cluster/skaffold-helm:latest@sha256:9b1a4826a617ebd2d17527cebad1ca6584e470116235da13528553f7a07149eb). This replacement though is the same replacement used by skaffold direcltly which has allow/denylist logic to only touch certain k8s object/parts-of-schemas for replacement. I believe the fix here is to configure the allowlist/denylist in the use case of helm template .. --post-renderer=<skaffold-binary> to remove any allowlist/denylist to get the proper functionality for this case. The Deployment type is in this allowlist which is why it works there. I will included these changes in my WIP fix (#8066 ). Thanks @alewis001

[aaron-prindle]

I do believe the above is the issue (allowlist/denylist omitting the change when it should not be) but StatefulSet is in the allowlist for transformation so I would have thought it would have modified this value but perhaps it is in another location in the yaml:
https://github.com/GoogleContainerTools/skaffold/blob/main/pkg/skaffold/kubernetes/manifest/visitor.go#L67-L72

	{Group: "apps", Kind: "StatefulSet"}: {
		GroupKind: "StatefulSet.apps",
		Image:     []string{".*"},
		Labels:    []string{".*"},
		PodSpec:   []string{".*"},
	},

	{Group: "apps", Kind: "StatefulSet"}: {
		GroupKind: "StatefulSet.apps",
		Labels:    []string{".spec.volumeClaimTemplates.metadata.labels"},
	},

Still remains though that these lists should not be considerered at all when using skaffold as a --post-renderer

[aaron-prindle]

@alewis001 can you post a minified/redacted version of the StatefulSet that is having the issues? (can just be the fully qualified spec.foo.bar: skaffold-image key/value that is not working properly). Want to make sure my allow/denylist assumption is correct

EDIT: I believe you already shared this above - spec.template.spec.containers.image. Ok to ignore this message sorry, I am putting together my own repro project now based on the findings above. Will add more details here when I am finished and have some findings/fixes

[aaron-prindle]

@alewis001, there was an issue with how StatefulSet values were replaced which caused the discrepency you saw between StatefulSets and Deployments (an bug with using skaffold as a --post-renderer with StatefulSet). This is fixed now @ HEAD via #8085 . I couldn't repro any profile related issues with null so currently that isn't fixed (hoping it is related to the above issue)

[aaron-prindle]

A large doc update explaining how to use helm with Skaffold v2.X.Y is currently WIP here:
#8093

Feel free to add any comments there if you believe any other information should be added/clarified

[alewis001]

Thanks @aaron-prindle. I'll get the latest code and build skaffold and try out your fix. Hopefully I'll be able to confirm whether or not the profile issue is fixed as well.

[alewis001]

@aaron-prindle I've just tested with a locally built version v2.0.2-9-g4f314d4fa. I can confirm that the StatefulSet vs Deployment issue has indeed been fixed but the issue where the Helm value is used instead of the Skaffold generated value when a profile is specified is still present. I'll do some digging.

[alewis001]

The lack of Skaffold generated image: when using a profile appears to be caused by the following section in skaffold.yaml (see the attached files in my comment above)

  - op: add
    path: /manifests/helm/releases/0/setValues
    value:
      deployment:
        jvmStats: -Dcom.sun.management.jmxremote -Dcom.sun.management.jmxremote.authenticate=false
          -Dcom.sun.management.jmxremote.ssl=false -Dcom.sun.management.jmxremote.local.only=false
          -Dcom.sun.management.jmxremote.port=1099 -Dcom.sun.management.jmxremote.rmi.port=1099
          -Djava.rmi.server.hostname=127.0.0.1

Below are the different Helm commands depending on whether a profile is specified and whether or not the section above is included (Note: mychart and myimage are redacted values just in case my redaction causes any confusion):

Scenario	Helm Command	Image set by
no profile	helm --kube-context lke67841-ctx template mychart ./src/main/helm --post-renderer /Users/alewis/dev/go/bin/skaffold --set image=myimage --namespace default	Skaffold
with profile but no setValues section	helm --kube-context lke67841-ctx template mychart ./src/main/helm --post-renderer /Users/alewis/dev/go/bin/skaffold --set image=myimage -f ./src/main/helm/values.d/dev-linode.yaml --namespace default	Skaffold
with profile and setValues section	helm --kube-context lke67841-ctx template mychart ./src/main/helm --post-renderer /Users/alewis/dev/go/bin/skaffold --set deployment.jvmStats=-Dcom.sun.management.jmxremote -Dcom.sun.management.jmxremote.authenticate=false -Dcom.sun.management.jmxremote.ssl=false -Dcom.sun.management.jmxremote.local.only=false -Dcom.sun.management.jmxremote.port=1099 -Dcom.sun.management.jmxremote.rmi.port=1099 -Djava.rmi.server.hostname=127.0.0.1 -f ./src/main/helm/values.d/dev-linode.yaml --namespace default	Helm

If you need the actual logs, let me know and I'll try to redact them accordingly and get them back to you.

[aaron-prindle]

@alewis001, thanks for the information and your help here. I will look into the profile issue more today. I have created #8100 to track the profile issue specifically, we can move the continued discussion here to that issue.

[aaron-prindle]

I am closing this issue has the helm deployer documentation has been updated to have many more examples including using multiple images and what the equivalent skaffold.yaml should be in Skaffold v2 for each Skaffold v1 imageStrategy. Also the v2 docs now on longer reference artifactOverrides or imageStrategy with #8093

[aaron-prindle]

skaffold.dev should be updated with the improved docs later today. Our staging docs site:
https://skaffold-v2-latest.web.app/docs/pipeline-stages/deployers/helm/

for anyone wanted to view this earlier