Charts: [stable/*] Create and document charts policy for breaking change upgrades

[scottrigby]

Current status

Any breaking (backwards incompatible) changes to a chart should:

1. Bump the MAJOR version (following semver)
2. In the README, under a section called "Upgrading", describe the manual steps necessary to upgrade to the new (specified) MAJOR version

• [x] Decide on policy
• [x] Document policy

Original summary

Hi everyone, After @unguiculus reached started a Slack Charts maintainers DM about #5576, an unresolved problem emerged. We don't have a kubernetes/charts policy for how to handle breaking change upgrades. ## Considerations That PR made it clear we have two main choices: 1. Add logic to the templates for every possible upgrade step (from every past to new version of an app). - 🚫 This was deemed to be not only an inelegant way to handle this problem on the whole, but also would make charts more difficult to maintain in the future. 2. Follow semver to denote breaking changes (bump MAJOR version, even if the upstream project's `appVersion` itself did not), and somehow clearly communicate the required upgrade steps to chart users who are upgrading from relevant previous versions to the current one (the steps themselves will vary by application and old/new version). - ✅ This is the preferred option. ## Next steps We plan to meet about determining some helpful guidelines about this. Possibilities discussed were UPGRADE_NOTES/CHANGELOG files for charts, or adding a new section to the README (perhaps under new section header such as `Upgrading`, just above `Installing the chart` so users are more likely to see it?). The ultimate goal of this issue is to track how we decide upon and document a consistent strategy for communicating additional info about breaking changes to users who want to upgrade from past versions of a chart.

[scottrigby]

@unguiculus @viglesiasce @prydonius @mattfarina @lachie83 please let me know if I missed anything.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Any further update will cause the issue/pull request to no longer be considered stale. Thank you for your contributions.

[stale[bot]]

This issue is being automatically closed due to inactivity.

[scottrigby]

We should keep this issue open till we document this.

[desaintmartin]

Here is an issue where several charts are affected and need breaking changes: https://github.com/helm/charts/issues/7680

[juan131]

Hi @scottrigby

I agree with adding the instructions on a new Upgrading section. We can use sth like:

## Upgrading

### To Y.Y.Y

Backwards compatibility is not guaranteed unless you modify the labels used on the chart's deployments.
Use the workaround below to upgrade from versions previous to Y.Y.Y:

<Workaround here>

[javsalgar]

I tried this workaround before upgrading and fixed it:

kubectl patch deployments <deployment-name> --type=json -p='[{"op": "remove", "path": "/spec/selector/matchLabels/chart"}]'

[desaintmartin]

I tried this workaround before upgrading and fixed it:

kubectl patch deployments <deployment-name> --type=json -p='[{"op": "remove", "path": "/spec/selector/matchLabels/chart"}]'

I've just tried it on a apps/v1beta2 Statefulset (rabbitmq), and the outcome is still the same:

The StatefulSet "my-rabbitmq" is invalid: spec: Forbidden: updates to statefulset spec for fields other than 'replicas', 'template', and 'updateStrategy' are forbidden.

Was your Deployment using >=v1beta2 as well?

For >=apps/v1beta2, the only way I've found is to:
k delete statefulsets.apps --cascade=false my-rabbitmq
Then upgrade.

[javsalgar]

After checking, I confirm that the way @desaintmartin proposed is the one that works for StatefulSets.

[desaintmartin]

And after checking, I confirm that the "kubectl patch" command line does NOT work either on Deployment apps/v1beta2 and later

[desaintmartin]

I've tried to do a PR for redis as an example, with its own "how to upgrade" in its readme.
https://github.com/helm/charts/pull/7686

[javsalgar]

In these charts where deployments <= apps/v1beta2, the patch command works so the guide written in #7813 and #7785 are applicable

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Any further update will cause the issue/pull request to no longer be considered stale. Thank you for your contributions.

[stale[bot]]

This issue is being automatically closed due to inactivity.

[scottrigby]

I should close this - we've adopted the README policy. I'll update the summary above.

[scottrigby]

Eh, review guidelines still need to be updated