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.

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

Adhere to semver #554

Closed
cwrau opened this issue Feb 29, 2024 · 20 comments · Fixed by #569
Closed

Adhere to semver #554

cwrau opened this issue Feb 29, 2024 · 20 comments · Fixed by #569

Comments

@cwrau
Copy link
Contributor

cwrau commented Feb 29, 2024

Describe the problem/challenge you have
Currently the velero helm chart kinda randomly decides which part of the version gets bumped when a change is merged, see #549 (comment). This means that every user has to look at every little (patch/minor) release to see if something broke for them.

Describe the solution you'd like
Adhere to semver.

Anything else you would like to add:
This helps users judge releases just by their version number and use tools to auto-update or to automatically create small PRs to update versions.

@qiuming-best
Copy link
Collaborator

Our chart would release for two cases:

  • Velero release
  • Chat itself updates

What about we make a rule like below:

Supposing the Velero version is X.Y.Z
X.Y determined the Major version of the chart
Such as 6 -> 1.13; 7 ->1.14;  8 -> 1.15
Z determined the Minor version of the chart
Such as 0 -> 1.13.0; 1->1.13.1; 2 -> 1.13.2
The patch version of the chart is determined by the chart itself.

Such as if Velero release V1.13.1
the first chart version for it would be 6.1.0

If we have one urgent breaking change, it will break the semver rule.

we could add a table in the README to record the relationship between chart versions and Velero versions.
We would still follow the aforementioned rules as a base. However, if a breaking change occurs, we would upgrade the major version and continue to follow the same rules. For example, initially:
6 -> 1.13; 7 ->1.14;
Then, if version 7 experiences a breaking change, we would upgrade the major version. At this point, it would be:
6 -> 1.13; 7 ->1.14; breaking change occurred 8 ->1.14;

@qiuming-best
Copy link
Collaborator

Any ideas about it? @jenting @reasonerjt @ywk253100

@cwrau
Copy link
Contributor Author

cwrau commented Mar 8, 2024

Our chart would release for two cases:

  • Velero release
  • Chat itself updates

What about we make a rule like below:

Supposing the Velero version is X.Y.Z
X.Y determined the Major version of the chart
Such as 6 -> 1.13; 7 ->1.14;  8 -> 1.15
Z determined the Minor version of the chart
Such as 0 -> 1.13.0; 1->1.13.1; 2 -> 1.13.2
The patch version of the chart is determined by the chart itself.

Such as if Velero release V1.13.1
the first chart version for it would be 6.1.0

If we have one urgent breaking change, it will break the semver rule.

we could add a table in the README to record the relationship between chart versions and Velero versions.
We would still follow the aforementioned rules as a base. However, if a breaking change occurs, we would upgrade the major version and continue to follow the same rules. For example, initially:
6 -> 1.13; 7 ->1.14;
Then, if version 7 experiences a breaking change, we would upgrade the major version. At this point, it would be:
6 -> 1.13; 7 ->1.14; breaking change occurred 8 ->1.14;

But, why? Why not just follow semver? 🤔

A velero patch update is a patch update for the chart, just because there is no lesser level of version.
A velero minor update is either a minor update for the chart, if the chart also exposes new functionality, or also just a patch update.
A velero major update is either a major update for the chart, if the chart can't catch the breaking change, or a minor update. (for example if the configuration format/syntax or a default changes, the chart could compensate for this, so the user doesn't have to do anything. That way that update is not a breaking change. This is optional of course, you can just release a major version)

@qiuming-best
Copy link
Collaborator

qiuming-best commented Mar 8, 2024

@cwrau Thanks for your response.

The main purpose of making the semver version rules is we want each Velero release version to have a range of independent chart versions and avoid conflicts in chart versions.

For Velero, it will release a version in the normal development cycle like v1.13.0
but Velero will release patch versions separately like V1.12.4, and it's released after v1.13.0. also, it will release v1.11.2
Helm charts need to release new versions along with all these Velero releases. and helm charts may update new releases with no Velero release. For Velero v1.12.4 we should allocate one proper chart version less than the version for Velero 1.13.0. For Velero v1.11.2, we should also allocate one proper chart version less than the version for Velero 1.12.4.

If our chart doesn't have a clear rule related to the Velero version, it would make the chart version disordered and even conflict.

@qiuming-best
Copy link
Collaborator

qiuming-best commented Mar 8, 2024

Now I've updated one new idea:

we should have two aspects that need to be considered.

  • The branch control
  • The version control

Branch control

For the branch control, we could have one new branch when Velero releases one new major version:
we could have the following rules:
such as:
Velero 1.13.0, we could create a new branch named velero-helm-chart-6
Velero 1.14.0, we could create a new branch named velero-helm-chart-7

For Velero minior release, we could still use the current branch but tag one label
such as:
Velero 1.13.0 we create the branch named velero-helm-chart-6, but tag one label, the label name would be velero-$chart-version

Velero 1.13.1, we use the branch named velero-helm-chart-6but tag one label, the label name would be velero-$chart-version

for the chart version, we could follow the rules in the next section.

If we have the branch control, we could release an older helm chart with the older branch.

For version control:

Our chart would release for two cases:

  • Velero release
  • Chat itself updates

What about we make a rule like below:

Supposing the Velero version is X.Y.Z
X.Y determined the Major version of the chart
Such as 1.13 -> 6; 1.14->7;  1.15 -> 8

Both Z and chart itself updates determined the patch version of the chart
Such as the following scenario:
Velero 1.13.0 release:
 1.13.0 -> 0; 
chat itself updates but not Velero
patch version ++ 
Velero 1.13.1 release:
patch version ++ 
the patch version now is 2

The minor version is 0 in general. it's a placeholder for breaking change
if no breaking change ->  minor version is 0
if has break change  -> minor version ++

@cwrau
Copy link
Contributor Author

cwrau commented Mar 8, 2024

Oh wow, you plan on having multiple Helm Chart "strains"? So multiple concurrent chart "versions"?

That of course complicates things, a lot

Why not just go forward with velero and follow the standard semver? 🤔

I would even go so far as to say that staying on an older version of velero is a special case and just updating is the normal way, so having no real way to update older velero versions is not a problem for the normal user.

This is of course easier if velero itself also follows semver, but if it doesn't, you either have to carefully read each and every release changelog or also open an issue "upstream" so they use semver as well 😅

Which looks like it's the case, as you say 1.13 -> 6 and 1.14 -> 7, so these minor versions of velero have breaking changes?


To summarise, if this chart isn't going to follow real semver, then there is no big gain in changing the versioning concept, as with the new concept I still can only auto-update patch versions, which won't update velero to the next minor version 😅

@qiuming-best
Copy link
Collaborator

qiuming-best commented Mar 11, 2024

Which looks like it's the case, as you say 1.13 -> 6 and 1.14 -> 7, so these minor versions of velero have breaking changes?

yes

Maybe if we want to support concurrent chart "versions", the idea I mentioned maybe work.
Or our helm chart just follows the standard semver rule not considering older velero releases is the easiest way as @cwrau mentioned.

@reasonerjt
Copy link
Contributor

@cwrau Would you mind giving a more concrete example?

Are you suggesting that the chart should bump up the major version in #549 ?

I would rather suggest that we should have an easier way to map between the version of velero chart and velero.
i.e. when velero bumps up the minor version , the chart will also bump up minor version and tag a release, when velero bumps up the patch version and tag a release.

@cwrau
Copy link
Contributor Author

cwrau commented Mar 11, 2024

Which looks like it's the case, as you say 1.13 -> 6 and 1.14 -> 7, so these minor versions of velero have breaking changes?

yes

Then, as I said, I see two ways to handle this in a semver way, either the one bumping the velero version inspects the changelog and determines if there is a breaking change or we open an issue upstream so they change their versioning to semver as well.

@cwrau Would you mind giving a more concrete example?

Are you suggesting that the chart should bump up the major version in #549 ?

Definitely, that way the user can see at a glance that there is a breaking change in that version, and, conversely, see that other versions are safe to upgrade to, as there is no breaking change in there.

I would rather suggest that we should have an easier way to map between the version of velero chart and velero. i.e. when velero bumps up the minor version , the chart will also bump up minor version and tag a release, when velero bumps up the patch version and tag a release.

If you really want to support multiple concurrent versions, I guess one could still use the major version for tracking the breaking upstream velero and follow semver. You'd just have to never do anything breaking in the old versions 🤔 That would be restricting, but I guess that should work.

And major reworks, or anything breaking really, would only happen in the latest version and bump the major version.
But of course, that way there wouldn't be a mapping rule between versions, as the major version could either be for a new velero version or any other breaking change.
But to be honest, the chart version has nothing to do with the velero version, that just complicates things, if you want to keep track of the velero version, there is the appVersion field in the Chart.yaml

@qiuming-best
Copy link
Collaborator

@cwrau Thanks for your advice.

So following semver rule is the prerequisite, also we want to support multiple concurrent versions.

If we want to meet the conditions above, the breaking changes of helm chart should bump up the major version and never do breaking changes for the older version, or it may lead to a higher chart version having a lower velero version.

There is no strong correlation between Chart versions and Velero versions as we still could get the Velero version through appVersion.

Semantic versioning is summarised below:

Given a version number MAJOR.MINOR.PATCH, increment the:

MAJOR version when you make incompatible API changes
MINOR version when you add functionality in a backward compatible manner
PATCH version when you make backward compatible bug fixes

So :

  • for every Velero minor version release, the helm chart bumps up the major version
  • for breaking changes in the helm chart, the helm chart bumps up the major version (which restricts breaking change only allowed for the latest version)
  • for the helm chart when having added functionality, bump up a minor version
  • for the helm chart when fixing bugs, bump up the patch version

Another thing we need to consider is cutting the branch when the Velero minor version is released, the details we could follow the rules I mentioned about Branch control, multiple branches are needed if we support multiple concurrent versions

@qiuming-best
Copy link
Collaborator

@jenting Do you have any idea about it?

@siegenthalerroger
Copy link

@cwrau I think a major sticking point that you're overseeing is the fact that Velero itself doesn't adhere to SemVer... so if the helm chart adheres to it, there will be major updates for every minor velero release. In that sense @qiuming-best's last response seems to cover that in the proposed versioning scheme.

@cwrau
Copy link
Contributor Author

cwrau commented Mar 14, 2024

@cwrau I think a major sticking point that you're overseeing is the fact that Velero itself doesn't adhere to SemVer... so if the helm chart adheres to it, there will be major updates for every minor velero release.

I didn't miss it, I would say it's not a problem? 🤷‍♂️

Just because they don't adhere to it doesn't mean we can't make the live of our users easier by adhering to it. 🤔

@siegenthalerroger
Copy link

@cwrau fair enough, just thought it would bare highlighting as it does imply that the helm chart's major version would change fairly often. tbc I'm a fan of having the helmchart adhere to semver ;)

@qiuming-best
Copy link
Collaborator

@cwrau @siegenthalerroger please take a look at the PR that I submitted, especially the instruction doc. Thanks

@TTRCmedia
Copy link

Any news on this?

Looking into the PR, some users are waiting to install a newer version with fixes they need (me, too) for quite a long time. Perhaps stay with the current versioning and build the chart the "old" way until the decision about semver is made?

@siegenthalerroger
Copy link

@TTRCmedia not sure what you're waiting for tbh, there's not going to be any retroactive changes anyway. None of the breaking changes have been disturbing enough to warrant waiting with the deployment of required fixes.

@TTRCmedia
Copy link

Oh sorry, I did not see the last comment before mine and the linked PR.

My comment was about this PR to build the chart for 1.13.1, as that version includes the fix for vmware-tanzu/velero#7445 we are waiting for.

@siegenthalerroger
Copy link

Ah, no problem. You can manually bump the velero version for patches if you need to (I have for my org) by overriding the value. You have to bump the plugin versions manually anyway so it's never bothered me if I have a pressing concern.

  values:
    image:
      tag: v1.13.2

@contributorr
Copy link

contributorr commented May 2, 2024

Ah, no problem. You can manually bump the velero version for patches if you need to (I have for my org) by overriding the value. You have to bump the plugin versions manually anyway so it's never bothered me if I have a pressing concern.

  values:
    image:
      tag: v1.13.2

Sure, you could do that, but then Helm would report wrong APP VERSION.

# helm get values velero -n velero -o yaml | grep -B4 tag:         
image:
  imagePullSecrets: []
  pullPolicy: IfNotPresent
  repository: velero/velero
  tag: v1.13.2

# helm list -n velero                                                                                                                
NAME  	NAMESPACE	REVISION	UPDATED                              	STATUS  	CHART       	APP VERSION
velero	velero   	2       	2024-05-02 14:30:17.585325 +0200 CEST	deployed	velero-6.0.0	1.13.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants