Skaffold: Updating Skaffold breaks redeploys since it changes `labels`

[avloss]

apiVersion: apps/v1
kind: Deployment
...
  labels:
...
    skaffold.dev/docker-api-version: "1.40"

If I now try to update this with another version of skaffold, I would get following exception

`selector` does not match template `labels`

https://github.com/kubernetes/kubernetes/issues/26202

Downgrading skaffold to the version which was used for original deploy does resolve the issue.

[balopat]

Thanks for opening @avloss - can you share more about your setup?
What is the version of skaffold you are using and upgrading to?
Can you share more logs? I'm not sure where this error message is coming from. Is this a skaffold run or deploy or dev?

I tried, but I can't reproduce it with our examples - skaffold redeploys the resources nicely with a new label.

[manofthepeace]

Hi, I cannot really redo it, but I can say I got the same issue while doing a gcp lab on qwiklabs. Basically the labs tells you to fetch latest skaffold, which fetches 0.41. Then we clone https://github.com/blipzimmerman/microservices-demo-1, run skaffold run, works, then we modify the "image" in one of the yaml, than skaffold run refuses to run because of the label change. I fetched an older version of skaffold, went down randomly to 0.39, then skaffold ran fine to redeploy with the change.

So it does not seem to be related necessarily to a skaffold update since I started with the latest. with a image change. only skaffold run was used.

[kfuglsang]

With 0.41 I'm experiencing the same issue now. First deploy worked but second deployment fails with this error. I'm using Azure AKS.

Didn't have this issue on Skaffold 0.30 since it didn't add the run-id label.

I noticed that the issue only occurs if the previous deployment successfully started. If its stuck with e.g. imagepullbackoff then a new deployment is allowed, even if the labels have changed.

[henriquebremenkanp]

I am experiencing this too on the latest v1.1.0.
Need to first to skaffold delete then do a skaffold run again.

[paultiplady]

I have reproduced this on v1.8.0 and 1.9.1 too. I think the problem is the labels that Skaffold is creating:

Labels:                 
                        app.kubernetes.io/managed-by=skaffold-v1.9.1
                        skaffold.dev/builder=local
                        skaffold.dev/cleanup=true
                        skaffold.dev/deployer=kustomize
                        skaffold.dev/docker-api-version=1.40
                        skaffold.dev/namespace=underwriting
                        skaffold.dev/profile.0=development
                        skaffold.dev/run-id=697ed2d1-26e1-413a-a5dc-44f4b2e37df7
                        skaffold.dev/tag-policy=git-commit
                        skaffold.dev/tail=true

This seems quite odd to me. It's not possible to apply a modification of a deployment's labels, for reasons documented in https://github.com/kubernetes/kubernetes/issues/26202. (In summary modifying label selectors would break the deployment's ability to clean up old replicasets, I believe). So in order to apply a Deployment you need to do pass --force to have Skaffold delete and recreate it.

I may be missing something fundamental, but I think the current skaffold behaviour of putting a bunch of mutable state in the labels is broken; there have been similar bugs in many Helm templates as well (see https://github.com/helm/helm/issues/1390 for example).

My suggestions would be:

1. All of this deploy metadata should be in annotations. You can't mutate deployment labels. Annotations are where you're supposed to put metadata in k8s.

2. If skaffold injects metadata, it should (at least configurably) only be included in the top-level deployment metadata (metadata.annotations), and omit injecting into the pod template's metadata (spec.template.metadata.annotations). That way you don't force a redeploy if the template hasn't changed.

While my pods _can_ be restarted at any time, I'd rather not have my RabbitMQ pod bounce on every deploy when there are no changes to the manifest.

I'm guessing that 2. is working as designed, i.e. the skaffold.dev/run-id is injected to add entropy so that you guarantee a redeploy, and other comments have said it's used for port-forwarding. But I think it's worth considering revisiting the design here. Perhaps that's a separate issue, I can open a new ticket for discussion there if you'd prefer.

Would a better "out of the box" behaviour be to use a fixed run-id for skaffold deploy, and randomly-generated one for skaffold dev? You should be able to generate a guaranteed-unique run ID deterministically using the resource type, name, and namespace.

(Note, I think you could use the same fix as https://github.com/GoogleContainerTools/skaffold/pull/2752 for the original issue, but it seems hacky to have to override internal labels to get working deploys).

[paultiplady]

Note that even if you override the run-id with a static value, we are still going to fail to deploy after upgrading skaffold or docker, or any other config changes that would modify the deploy metadata injected by skaffold.

[erulabs]

While my pods can be restarted at any time, I'd rather not have my RabbitMQ pod bounce on every deploy when there are no changes to the manifest.

2x this very much. In addition, when migrating to using skaffold when previously using kustomization (for example), one needs to re-name services to avoid the immutable field errors. Using an annotation would make migration to skaffold much easier.