remoteManifests don't replace image on deployment if digest specified

[spmason]

trafficstars

Expected behavior

When using remoteManifests, if the deployed manifest contains an image digest then the image is not replaced with the one built by skaffold

Actual behavior

The image is not replaced and a warning WARN[0002] image [foo] is not used by the deployment is printed

Information

• Skaffold version: v1.17.2
• Operating system: MacOS Mojave
• Contents of skaffold.yaml:

This is a basic reproduction I've cooked up

---
apiVersion: skaffold/v2beta10
kind: Config
metadata:
  name: nginx
build:
  # Abuse the fact that the image is already pushed with `:latest` to docker.io
  tagPolicy:
    sha256: {}
  artifacts:
  - image: nginx
    custom:
      # Skaffold will simply pick up the version of `nginx` with the `:latest` tag from docker.io
      buildCommand: true

deploy:
  kubectl:
    manifests: [""]
    remoteManifests:
    - skaffold-digest-bug:deployment/nginx

And deployment.yaml:

apiVersion: apps/v1
kind: Deployment
metadata:
  name: nginx
  labels:
    app: nginx
  namespace: skaffold-digest-bug
spec:
  replicas: 1
  selector:
    matchLabels:
      app: nginx
  template:
    metadata:
      labels:
        app: nginx
    spec:
      containers:
      - name: nginx
        image: nginx:1.14.2@sha256:4cf620a5c81390ee209398ecc18e5fb9dd0f5155cd82adcbae532fec94006fb9
        ports:
        - containerPort: 80

Steps to reproduce the behavior

1. Use the above skaffold.yaml and deployment.yaml files
2. kubectl create ns skaffold-digest-bug
3. kubectl apply -f deployment.yaml
4. skaffold run
5. You'll see the warning WARN[0002] image [nginx] is not used by the deployment as described
6. Edit deployment.yaml to remove the sha from the image (ie image: nginx:1.14.2)
7. kubectl apply -f deployment.yaml
8. skaffold run
9. The warning will not appear

It looks like the problem is here: https://github.com/GoogleContainerTools/skaffold/blame/v1.17.2/pkg/skaffold/kubernetes/manifest/images.go#L108

That check was added in https://github.com/GoogleContainerTools/skaffold/commit/e3fd35032997048c777f9957ad02c160932f05af - but the comment on the commit ("Ignore tags (not digests) in manifests") is directly contradicted by the code and the behaviour. By not adding to r.found it is ignoring digests...

Is this behaviour intentional somehow?

[briandealwis]

This behaviour is intentional @spmason, but we're looking to make this behaviour configurable.

[spmason]

Do you know the reasons for this @briandealwis ? I couldn't find any discussion about it

I'd think that at least in the case of remoteManifests there's a case that Skaffold should always overwrite the deployed image..

[briandealwis]

Sounds similar to #5855.

[tejal29]

Reducing the priority since we don't have it planned for q3.

[j2udev]

@briandealwis I am also curious why this is expected behavior. For reference we primarily use the helm deployer and we can no longer simply skaffold run and have the image overwritten. This is causing a great deal of confusion as it appears to be a regression. We'll be rolling our version back until we can hear more about this

[nkubala]

@spmason @j2udevelopment I believe this was fixed in https://github.com/GoogleContainerTools/skaffold/pull/6342. could one of you upgrade to skaffold v1.32.0 or later and verify this issue has been resolved?

[j2udev]

Hey @nkubala, I upgraded to Skaffold 1.33.0, deployed my service, made a code change, deployed my service again (even used --cache-artifacts=false for good measure), and am still seeing the error. It's worth mentioning that we are using the helm deployer primarily and the OP and the fix you've linked seem to be focused on the kubectl deployer. Here is an example of what our deploy section looks like (using skaffold/v2beta16 currently, but can upgrade if needed)

deploy:
  helm:
    releases:
      - name: my-service
        remoteChart: my-remote/chart-path
        version: 5.0.x
        artifactOverrides:
          image: my-service
        valuesFiles:
          - ./values/values.yaml

@briandealwis mentioned that you are looking to make it configurable and if there is new configuration that we need to set to achieve the desired behavior here I'm happy to do that!

For a little more information, I did a skaffold run in debug mode and this appears to be the relevant logs:

DEBU[0003] Running command: [helm --kube-context minikube get all my-service]  subtask=0 task=Deploy
INFO[0003] Release my-service not upgraded as it is remote...  subtask=0 task=Deploy
WARN[0003] image [my-service:976dc64549ad7169780f2ce4d911a6933c73373452d214461f1d164e1a4f11bc] is not used.  subtask=-1 task=DevLoop
WARN[0003] 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

[j2udev]

Any update on this? Should I maybe make a separate issue that explicitly mentions the helm deployer?

[tejal29]

Thanks @j2udevelopment. Like @briandealwis mentioned this behavior is intentional. We don't have plans to make configurable in upcoming H2 2022 cycle. We would love to get any community help on this.

[sinhasonalkumar]

Skaffold and helm works if the chart is local (chartPath: './micro-svc-helm-chart/'). It replaces the image consistently. But when using remote chart repo (remoteChart: sonal-ms-github-helm-repo/micro-svc-helm-chart) then it does not replaces the image. I am using Skaffold version : v1.39.1 and minikube version: v1.26.0 on MAC M1 (12.2.1)

---

This works nicely with chartPath

apiVersion: skaffold/v2beta29
kind: Config
metadata:
  name: m-svc-2
build:
  artifacts:
  - image: m-svc-2-image
    docker:
      dockerfile: Dockerfile
deploy:
  # kubectl:
  #   manifests:
  #   - ./k8s/deployment.yaml
  helm:
    releases:
    - name: m-svc-2
      #remoteChart: sonal-ms-github-helm-repo/micro-svc-helm-chart
      #version: 0.6.0
      chartPath: './micro-svc-helm-chart/'
      valuesFiles:
      - '../skaffold-k8s-state/helm-values/m-svc-2/local/values.yaml'
      artifactOverrides:
        container.image: m-svc-2-image
      imageStrategy:
        fqn: {}

---

Logs

Build [m-svc-2-image] succeeded
Tags used in deployment:
 - m-svc-2-image -> m-svc-2-image:a9b64bcf91998cea29f3e5454830255691641c6df3989479d60b7e107c4e3763
Starting deploy...
Release "m-svc-2" has been upgraded. Happy Helming!
NAME: m-svc-2
LAST DEPLOYED: Fri Jul 29 15:21:45 2022
NAMESPACE: default
STATUS: deployed
REVISION: 2
TEST SUITE: None
Waiting for deployments to stabilize...
 - deployment/m-svc-2 is ready.
Deployments stabilized in 2.065 seconds

---

This does not work with Remote Helm Chart Repo Repo

apiVersion: skaffold/v2beta29
kind: Config
metadata:
  name: m-svc-2
build:
  artifacts:
  - image: m-svc-2-image
    docker:
      dockerfile: Dockerfile
deploy:
  # kubectl:
  #   manifests:
  #   - ./k8s/deployment.yaml
  helm:
    releases:
    - name: m-svc-2
      remoteChart: sonal-ms-github-helm-repo/micro-svc-helm-chart
      version: 0.6.0
      #chartPath: './micro-svc-helm-chart/'
      valuesFiles:
      - '../skaffold-k8s-state/helm-values/m-svc-2/local/values.yaml'
      artifactOverrides:
        container.image: m-svc-2-image
      imageStrategy:
        fqn: {}

---

Logs

 => => naming to docker.io/library/m-svc-2-image:f51bba0-dirty                                                                                                               0.0s

Use 'docker scan' to run Snyk tests against images to find vulnerabilities and learn how to fix them
Build [m-svc-2-image] succeeded
Tags used in deployment:
 - m-svc-2-image -> m-svc-2-image:556789fca8ab8a7ee1b39cc2572d447c3b26323c7944d43c0e05b2209c05cce5
Starting deploy...
WARN[0130] image [m-svc-2-image:556789fca8ab8a7ee1b39cc2572d447c3b26323c7944d43c0e05b2209c05cce5] is not used.  subtask=-1 task=DevLoop
WARN[0130] 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...
 - deployment/m-svc-2 is ready.
Deployments stabilized in 1.067 second

[jdoylei]

It would be extremely helpful if Skaffold had an option to match a Skaffold configuration's build.artifacts[*].image:

build:
  artifacts:
    - image: registry.io/hello

against a manifest's containers[*].image even when it uses a digest:

      containers:
      - name: hello
        image: registry.io/hello:tag-for-documentation@sha256:584...

Our use case is that we want to have a base K8s configuration that references the images by digest, which a tool like Argo CD consumes directly, and then have overlay developer configurations that Skaffold consumes.

Our current workaround is to have overlay developer configurations rewrite the images they expect Skaffold to manage, using Kustomize. The Kustomize config replaces the digest and any tag with just ":latest", to make it acceptable to Skaffold:

images:
  - name: '*hello'
    newTag: latest

But it's really tedious including this image rewrite in every overlay developer configuration, particularly when this is Skaffold's area of expertise - identifying what images to handle.