Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[velero] Adhere chart to semver #569

Merged
merged 1 commit into from
May 11, 2024
Merged

[velero] Adhere chart to semver #569

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/PULL_REQUEST_TEMPLATE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@
#### Checklist
[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]
- [ ] [DCO](https://github.com/helm/charts/blob/master/CONTRIBUTING.md#sign-your-work) signed
- [ ] Chart Version bumped
- [ ] Chart Version bumped, please refer to the [chart version instruction](../RELEASE-INSTRUCT.md#guidelines)
- [ ] Variables are documented in the values.yaml or README.md
- [ ] Title of the PR starts with chart name (e.g. `[velero]`)
6 changes: 3 additions & 3 deletions .github/workflows/lint-test.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,14 @@ jobs:
- name: Run chart-testing (list-changed)
id: list-changed
run: |
changed=$(ct list-changed --target-branch ${{ github.event.repository.default_branch }})
changed=$(ct list-changed --target-branch ${{ github.base_ref }})
if [[ -n "$changed" ]]; then
echo "changed=true" >> "$GITHUB_OUTPUT"
fi

- name: Run chart-testing (lint)
if: steps.list-changed.outputs.changed == 'true'
run: ct lint --target-branch ${{ github.event.repository.default_branch }}
run: ct lint --target-branch ${{ github.base_ref }}

- name: Create kind cluster
uses: helm/[email protected]
Expand All @@ -60,4 +60,4 @@ jobs:

- name: Run chart-testing (install+upgrade)
if: steps.list-changed.outputs.changed == 'true'
run: ct install --upgrade --target-branch ${{ github.event.repository.default_branch }}
run: ct install --upgrade --target-branch ${{ github.base_ref }}
1 change: 1 addition & 0 deletions .github/workflows/release.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ on:
push:
branches:
- main
- velero-helm-charts-v*

permissions:
contents: write
Expand Down
39 changes: 39 additions & 0 deletions RELEASE-INSTRUCT.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
Here is an instruction about the Velero helm charts release.

It involves Helm charts version control and branch control.

### Helm Charts Version Control

Our Helm charts are released under two scenarios:
- **Velero Release:** When Velero itself is released.

- **Chart Updates:** When there are updates specific to the Helm charts.

The version is defined in [Chart.yaml](charts/velero/Chart.yaml).

#### Guidelines
To comply with the [Semantic Versioning (semver)](https://semver.org/#summary) rule, follow these instructions:
- **Velero Release:**
- For each Velero minor version release, increase the major version of the Helm charts. For example, if Velero v1.13.0 is released, the Helm charts version becomes 6.0.0; for Velero v1.14.0, it becomes 7.0.0.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I don't think the helm chart itself follows the semver because the Velero bumps minor version (functionality change) but the corresponding helm chart changes the major version (breaking changes).

We could consider maintains the velero helm chart and Velero itself with this format . This makes the helm chart follows the semver as well, also we can easily check the helm chart upstream Velero version.

Copy link
Collaborator Author

@qiuming-best qiuming-best Apr 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jenting
I do not quite understand the format :
The upstream charts follow this versioning: 1.0.#+upX.Y.Z
Could you please give more details explanation?

Our main goal for this PR about versioning is to solve the conflicts in chart versions problem
Not sure the format you mentioned could avoid the conflicts in chart versions problem?

If it could avoid the conflicts, we also need to plan the helm chart version reasonably related format in 1.0.#+upX.Y.Z ?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jenting the reasoning behind the chart requiring a major bump for every minor velero bump is because velero itself doesn't follow semver. There are often changes in minor velero updates that require changes to be made to the deployment. To a certain extent the helm chart could catch these and make them non-breaking, however the decision here would be to forego that effort and just admit that the helm chart just "moves fast and breaks things" in a sense.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having looked at the rancher example, they actually have the same "ruleset" in a sense. The helm major version is always bumped for a minor rancher release (the 100.x.y examples), just with an additional +up....

Personally I don't think the additional +up is necessary, that's what the appVersion field in the Chart.yaml is for, and it's what we use to track the upstream version in our deployments.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if the Velero bump the major release? How should we tackle in the Velero helm chart?

My thought was

  • if the Velero bump patch/minor/major version, the helm chart itself should bump patch/minor/major version.
  • if the helm chart itself have to fix/feature/breaking change, bump the helm chart version ..-. For example: current version is 6.1.0-v1.13.0
    • The helm chart bug fix would be 6.1.1-v1.13.0
    • The helm chart feature enhance would be 6.2.0-v1.13.0
    • The helm chart breaking change would be 7.0.0-v1.13.0
    • The velero bug fix would be 6.1.1-v1.13.1
    • The velero feature enhance would be 6.2.0-v1.14.0
    • The velero bbreaking change would be 7.0.0-v2.0.0

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To the first question: just do a major bump as you'd expect.

To your suggestion: While that sounds fine, the issue raised was that the helm chart can sometimes manage to update to new velero minor versions without a breaking change, but sometimes it does. IMO, while the originally suggested change veers on the safe side (always doing major bumps for minor velero updates, just incase they have a breaking change), you could cover it just by making sure it major bumps if a breaking change is required.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood.


- For each Velero patch version release, increase the minor version. For instance, for Velero v1.13.1, the Helm charts version is 6.1.0; for Velero v1.13.2, it's 6.2.0.
- **Helm Charts Updates:**
- For breaking changes, increase the major version.

- For added functionality, increase the minor version.

- For bug fixes, increase the patch version.

#### Note:
Breaking changes are only allowed for the **latest Helm version**. This restriction ensures that older Helm versions do not have higher major versions than newer ones, preventing helm upgrade issues.

### Branch Control

Follow these rules for branch control:
- Our main branch should always align with the latest Velero major version. For example, if Velero v1.13 is the latest major version, any updates in the main branch should target Velero v1.13.

- Create a new branch prefixed with `velero-helm-charts-v` when Velero releases a new major version. For instance, if the main branch refers to Velero v1.13 and Velero v1.14 is released, name the new branch `velero-helm-charts-v7`. The branch version should match the major version of the Helm charts.

- Submit PRs for updates related to the newest Velero major version to the main branch. For updates related to older Velero major versions, submit PRs to the corresponding Helm charts branch (e.g., PRs for Velero v1.13.4 go to the `velero-helm-charts-v6` branch if it exists).

These guidelines ensure consistency and version control for our Helm charts releases.
Loading