search

vmware-tanzu / helm-charts

Contains Helm charts for Kubernetes related open source tools
https://vmware-tanzu.github.io/helm-charts/
Apache License 2.0
254 stars 363 forks source link

Adhere to semver #554

Closed cwrau closed 6 months ago

cwrau commented 9 months ago

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 https://github.com/vmware-tanzu/helm-charts/issues/549#issuecomment-1958387077. 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 commented 8 months ago

Our chart would release for two cases:

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 commented 8 months ago

Any ideas about it? @jenting @reasonerjt @ywk253100

cwrau commented 8 months ago

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 commented 8 months ago

@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 commented 8 months ago

Now I've updated one new idea:

we should have two aspects that need to be considered.

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:

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

@cwrau Would you mind giving a more concrete example?

Are you suggesting that the chart should bump up the major version in https://github.com/vmware-tanzu/helm-charts/issues/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 commented 8 months ago

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 commented 8 months ago

@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 :

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 commented 8 months ago

@jenting Do you have any idea about it?

siegenthalerroger commented 8 months ago

@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 commented 8 months ago

@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 commented 8 months ago

@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 commented 7 months ago

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

TTRCmedia commented 7 months ago

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 commented 7 months ago

@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 commented 7 months ago

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 https://github.com/vmware-tanzu/velero/pull/7445 we are waiting for.

siegenthalerroger commented 7 months ago

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 commented 7 months ago

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