Cert-manager: v0.9.0 helm upgrade "breaking" change

[jwalker343]

Issue:
I use helm (Specifically helmfile) to manage the deployment of cert-manager into my clusters. Yesterday I reviewed the release notes for cert-manager v0.9.0 and decided to upgrade from v0.8.1. The helm upgrade failed because of the label changes made here: https://github.com/jetstack/cert-manager/commit/ea60750b43e11e45a844cdb363d2ddfe8410e59f#diff-6568ead7c77f28deb5176d72313db142

The field spec.selector.matchLabels is immutable and therefore cannot be changed after a deployment has been created. I think this should be easy enough for anybody to bypass as long as they know to use helm upgrade --force.

Proposal:
Update the v0.9.0 release notes to include this change as part of the ACTION REQUIRED header to minimize issues other users may have when upgrading from 0.8 to 0.9.

[pierluigilenoci]

I've the same problem. I had to delete the chart (with purge) and install it from scratch.
UPGRADE FAILED Error: Deployment.apps "cert-manager-cainjector" is invalid: spec.selector: Invalid value: v1.LabelSelector{MatchLabels:map[string]string{"app.kubernetes.io/instance":"cert-manager", "app.kubernetes.io/managed-by":"Tiller", "app.kubernetes.io/name":"cainjector", "app":"cainjector"}, MatchExpressions:[]v1.LabelSelectorRequirement(nil)}: field is immutable && Deployment.apps "cert-manager" is invalid: spec.selector: Invalid value: v1.LabelSelector{MatchLabels:map[string]string{"app":"cert-manager", "app.kubernetes.io/instance":"cert-manager", "app.kubernetes.io/managed-by":"Tiller", "app.kubernetes.io/name":"cert-manager"}, MatchExpressions:[]v1.LabelSelectorRequirement(nil)}: field is immutable Error: UPGRADE FAILED: Deployment.apps "cert-manager-cainjector" is invalid: spec.selector: Invalid value: v1.LabelSelector{MatchLabels:map[string]string{"app.kubernetes.io/instance":"cert-manager", "app.kubernetes.io/managed-by":"Tiller", "app.kubernetes.io/name":"cainjector", "app":"cainjector"}, MatchExpressions:[]v1.LabelSelectorRequirement(nil)}: field is immutable && Deployment.apps "cert-manager" is invalid: spec.selector: Invalid value: v1.LabelSelector{MatchLabels:map[string]string{"app":"cert-manager", "app.kubernetes.io/instance":"cert-manager", "app.kubernetes.io/managed-by":"Tiller", "app.kubernetes.io/name":"cert-manager"}, MatchExpressions:[]v1.LabelSelectorRequirement(nil)}: field is immutable

[munnerz]

huh, that's strange. Both myself and @cpanato manually tested this to ensure that you could still upgrade, although come to think, at the time the apps/v1beta1 API version was being used instead of apps/v1, so perhaps this has changed between API versions?

This is very annoying 😬 does helm upgrade --force recreate the resource under the hood and avoid this issue?

Does kubectl replace instead of kubectl apply also resolve this for the static manifest based install?

/kind bug
/area deploy
/priority critical-urgent
/milestone v0.9

[jwalker343]

@munnerz Yes, helm upgrade --force will delete and recreate the resource if the patch fails to apply. In this case the deployment resources are deleted and recreated.

I have not tested static manifest installs, but it looks like kubectl replace --force would be the recommended command: Kubernetes docs

---

In my case, running helm upgrade without --force put the release in a botched state since some resources were created successfully that did not previously exist. I had to manually cleanup the ClusterRoles and ClusterRoleBindings that were added and then helm upgrade --force cleanly upgraded to v0.9.0.

Purging the release like @PierluigiLenociAkelius mentioned would ultimately work but likely has the unintended side affect of deleting any of your certificate CRDs, which was a no-go for me.

[munnerz]

unintended side affect of deleting any of your certificate CRDs, which was a no-go for me.

we don't actually include CRDs in the Helm chart, so it should be okay 😄

[Starefossen]

I feel this is a technicallity to say that just that since the CRDs are not included in the chart it is not concidered a breaking change. You can not run the Helm Chart without the CRDs and any breaking changes in those should at least be noted in the release.

[munnerz]

I feel this is a technicallity to say that just that since the CRDs are not included in the chart it is not concidered a breaking change.

I didn't suggest that it's not a breaking change - simply that you can uninstall the Helm chart and re-install it without deleting your existing CRDs 😄

[Starefossen]

Ahhh... my bad @munnerz., I missunderstood. Yes, that is what we did and it solved it perfectly!

--

Thanks for the hard work you put into making cert-manger ❤️

[pierluigilenoci]

Purging the release like @PierluigiLenociAkelius mentioned would ultimately work but likely has the unintended side affect of deleting any of your certificate CRDs, which was a no-go for me.

The procedure has not deleted the CRDs. But of course this is an "extreme measure", not a real solution. At least adding an "update note" is good to have.

[munnerz]

This has now been documented in our upgrade guide - closing this now 😄