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

feat: add new auto versionincrement field for helmchart resource #1593

Merged

Conversation

aroberts87
Copy link
Contributor

@aroberts87 aroberts87 commented Sep 13, 2023

The intent of this change is to allow helm charts to have a version bump that is proportional to the version bump done by a dependency, rather than just a static major, minor, or patch increment. This will allow potential major or breaking changes in subchart dependencies from being masked by a lesser increment done on the parent chart, as well as larger version increments on the parent from looking more daunting than the changes they contain of the subchart bump is a lesser type.

As an example, with a simple one parent / one child chart relationship and the parent chart's starting version of 1.0.0, the new behavior yields the following behavior.

Child Original Version Child Version Bump New Parent Chart Version
0.3.0 0.3.1 1.0.1
0.3.0 0.4.0 1.1.0
0.3.0 1.0.0 2.0.0

Coupled with the functionality added in #1541, it will now be possible to have the following behavior when dealing with parent charts that have multiple subcharts with simultaneous version bumps during the same updatecli invocation.

Child A Original Version Child B Original Version Child A Version Bump Child B Version Bump New Parent Chart Version
0.3.0 5.0.3 0.3.1 5.0.3 1.0.1
0.3.0 5.0.3 0.4.0 5.0.4 1.1.0
0.3.0 5.0.3 0.8.7 6.0.3 2.0.0

Test

To test this pull request, you can run the following commands:

cd pkg/plugins/resources/helm
go test

Additional Information

Tradeoff

Potential improvement

  • Based on my testing running apply with --push=false breaks the intended workflow as the originChartMetadata values do not appear to be updated with the cumulative updates between helmchart targets.

  • I'm not married to the term child-consistent for describing the version bump type, but at the same time could not think of another concise way to describe the intent that I felt would be comfortable using as parameter.

@olblak
Copy link
Member

olblak commented Sep 14, 2023

This is a great idea which I like very very much and could be useful for any value updated within the chart so not only dependencies

What would you think to rename "childdependent" to "auto"

And add additional logic in case CHILDDEPENDENT:

if both source and new information are semver compliance, then we guess the new version
if at least one of the two information is not semver, then we just bump the minor as we used to do

This means that a container tag update could also trigger the correct version bump if the tag is semver compliant

@olblak
Copy link
Member

olblak commented Sep 15, 2023

For information the failing check can be ignore by adding "childdependent" to .github/actions/spelling/allow.txt

@olblak olblak added enhancement New feature or request resource-helm labels Sep 15, 2023
@aroberts87
Copy link
Contributor Author

Hey @olblak! I like your proposals a lot and will make the adjustments. I apologize for the delay in response as well; I was away on vacation and am just catching back up.

Thanks for your suggestions!

@aroberts87 aroberts87 changed the title feat: add new child-dependent versionincrement field for helmchart resource feat: add new auto versionincrement field for helmchart resource Sep 21, 2023
the intent of this change is to allow helm charts to have a version bump that
corresponds with the highest level of bump done by a dependency rather than
just a static major, minor, or patch increment. this will allow potential major
or breaking changes in chart dependencies from being masked by a lesser increment
done on the parent chart.
update the `child-dependent` name to something a bit
more generic
Adapt the behavior of the `auto` version bump behavior to cause
a minor version bump if either old or new target version is not
a valid semver string. this will ensure that the legacy behavior
of the default minor version bump will continue.
@aroberts87 aroberts87 force-pushed the feat/dependency-consistent-version-bumps branch from fc6745d to f6cba80 Compare September 21, 2023 19:56
@olblak
Copy link
Member

olblak commented Sep 25, 2023

Hey @olblak! I like your proposals a lot and will make the adjustments. I apologize for the delay in response as well; I was away on vacation and am just catching back up.

Thanks for your suggestions!

Thanks for the contribution and don't worry for the delay, I was a few day off as well.
I'll try to review in the coming days and target the next minor version update

@olblak
Copy link
Member

olblak commented Sep 26, 2023

I like the idea behind the pullrequest very much, just a minor comment about downgrade scenarios

aroberts87 and others added 3 commits September 27, 2023 16:12
in the `auto` version bump scenario we will now check
if the new semver major, minor, and patch versions are
not equal to the old ones rather than simply if the new
version is greater than the old one. this will address
the possibility of a downgrade scenario.
add a catch-all block for the `auto` version bump setting
that will do a minor release bump if the old and new semver
major, minor, and patch versions are identical which implies
that the change must be in the metadata tags.
@olblak olblak added this to the 0.62.0 milestone Sep 28, 2023
@olblak olblak merged commit ee936eb into updatecli:main Sep 28, 2023
6 checks passed
@aroberts87 aroberts87 deleted the feat/dependency-consistent-version-bumps branch September 28, 2023 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request resource-helm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants