Run chart releaser action from release branches only

[wongma7]

Is this a bug fix or adding new feature? fixes #858

What is this PR about? / Why do we need it?

"What happened? This would enable us to branch helm chart releases (e.g. have a different chart for 0.10.x vs 0.9.x).

And it would reduce confusion in the release process. Currently you release the image to registries by pushing a tag on the release branch but you release a corresponding helm chart by pushing something to the master branch.

See efs fix for details: kubernetes-sigs/aws-efs-csi-driver#333"

fyi @ayberk @krmichel

What testing is done?

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wongma7

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [wongma7]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[wongma7]

BTW, this is a double edged sword. because folks browsing master branch might expect the master branch chart to have been released: kubernetes-sigs/aws-efs-csi-driver#421

So I want others' input and careful consideration :D

[coveralls]

Pull Request Test Coverage Report for Build 1918

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 79.188%

---

Totals
Change from base Build 1917:	0.0%
Covered Lines:	1990
Relevant Lines:	2513

---

💛 - Coveralls

[wongma7]

I realize this means that the helm chart release will exist for x minutes before the image to which it is referring will exist in registries.

This is a problem we had in the past that this change would reintroduce.

I think the benefits outweigh it tho.

[wongma7]

(still thinking outloud)
Actually let me edit the release process to account for this.

[ayberk]

Hmm I can see the confusion around not having the latest chart released. My concern is that our chart hasn't been consistent enough to carry all the changes across release branches. Maybe we should do this after @krmichel's changes? When it's more stable?

[wongma7]

Maybe we should do this after @krmichel's changes? When it's more stable?

I'm okay with that.

/hold

[krmichel]

I also wonder if there will be confusion about chart versions since they don't generally match the versions of the actual images they use. Will the helm chart be released only from the most current release branch or any release branch? Will there be some method other than human caution and observation that will keep the chart versions straight across branches? Like what if the chart version was 0.27.0 or 1.1.0 on the branch for the 0.11 release of the image. Would it go up a minor in the release branch for 0.12? If so, what would the version of the chart be in the 0.11 release branch if a new feature was added to the chart after 0.12 was released. 0.28.0 or 1.2.0 would already be taken and setting it to either 0.27.1/1.1.1 would not accurately represent the change content, and 0.29.0/1.3.0 would falsely imply that the changes from 0.28.0/1.2.0 would be included in that chart version. It seems like since the chart is a pseudo-autonomous piece of software if you want to release it from branches it should have its own release branches independent of the image releases. Maybe chart-release-* as a convention. That would allow for bug fixes to older versions without the complications I was mentioning earlier. Thoughts?

[wongma7]

@krmichel

Will the helm chart be released only from the most current release branch or any release branch?

Only the most recent release branch, because my main motivation is to protect the helm chart from changes in master branch, not to maintain multiple parallel branches of the helm chart. This thinking goes for the driver as well, because we have never accepted PRs to any branch but master and the most recent release branch. This might change in the future after GA depending on whether we want to maintain multiple minor version tracks in order to support older kubernetes version, but so far we haven't needed to. For example, with the CSIDriver object going from v1beta1 to v1, you are introducing some logic in the chart itself to check the k8s version, so there's no need to maintain different chart tracks for v1beta1 and v1. Anyway, if we assume that we will only ever make changes to the most current release branch, then it mostly avoids the scenarios you described.

Like what if the chart version was 0.27.0 or 1.1.0 on the branch for the 0.11 release of the image. Would it go up a minor in the release branch for 0.12? If so, what would the version of the chart be in the 0.11 release branch if a new feature was added to the chart after 0.12 was released.

Ideally we would never backport a feature to an old version of the chart, only bugfixes in rare cases, so that would sidestep this scenario. So far, I have been bumping the chart minor version along with the app version. And sometimes we bump the chart minor version independent of the app version. Strictly speaking, as I have mentioned before, I guess we have been abusing semver & helm versioning conventions, but, I really don't know a better way to signal changes to users. At the least, to cut down on confusion, we'll need to start maintaining a chart changelog (someone is working on this in EFS repo). So users will be able to look at chart version, app version, and the changelog for each.

It seems like since the chart is a pseudo-autonomous piece of software if you want to release it from branches it should have its own release branches independent of the image releases. Maybe chart-release-* as a convention.

It might be the case that branching the chart will be the cleanest solution but IMO probably will add more confusion & maintenance burden. The chart still is tied to image releases by appVersion like the obvious example is if the chart has a value for toggling a flag that only exists in appVersion> x.y. Then it's reasonable to host the chart in the image's release-x.y branch.

[wongma7]

/hold cancel

I want to bring this back, #995 updated the release process so that we release the kustomize template in release-* branch only after verifying images exist. Similarly, it would make sense to only release the helm chart in release-* branch only after verifying images exist. I think this model has worked OK in efs repo, it requires some additional work from maintainers to cherry-pick helm changes but it is more stable for users

[k8s-ci-robot]

@wongma7: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Reopen this issue or PR with /reopen
• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

[k8s-ci-robot]

@k8s-triage-robot: Closed this PR.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Reopen this issue or PR with /reopen
• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.