Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

release: detect change of charts base values #182

Merged
merged 1 commit into from
Jan 8, 2020

Conversation

stefansedich
Copy link
Contributor

@stefansedich stefansedich commented Jan 7, 2020

Fix #174

From my investigation I found that upon running the dry-run we compare to see if the release values have changed but never actually checked to see if the charts base values themselves actually changed.

This adds a checksum of the chart values to the Chart type which in turn leads to the chart comparison to see a difference resulting in a release to occur when the base charts values are changed.

I also did some slight refactoring to move the existing Checksum logic into the helm package and changed any existing stuff to use that.

@hiddeco let me know what other changes you envisage here, one thing we could do and I am unsure about is return the actual values on the Chart and not just a checksum, but given we are not using them for anything else the checksum might be the simplest route.

Copy link
Member

@hiddeco hiddeco left a comment

Choose a reason for hiding this comment

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

Awesome! I wrote an end-to-end test to confirm it works.

Small comment about import aliases. And to keep the history clean, can you please smash your commits into one descriptive commit and rebase the branch instead of merging in master? Thanks ⭐

@stefansedich stefansedich force-pushed the chart-values-checksum branch from a155205 to d4c4168 Compare January 8, 2020 00:53
@stefansedich
Copy link
Contributor Author

stefansedich commented Jan 8, 2020

@hiddeco should be rebased now, my bad I am so used to using the squash feature in GitHub now I often forget to rebase PRs.

As far as the comment on import aliases I did not see any comment? I am not doing anything special myself there I assume it is vim-go doing it for me as it was something it updated upon saving/fmt the file for me.

Thanks for the test too I was going to talk to you about this, but this gives me a good guide for the future on e2e tests.

@hiddeco hiddeco force-pushed the chart-values-checksum branch from d4c4168 to 0f03935 Compare January 8, 2020 09:48
By returning values as part of the `helm.Chart` object, so that the
comparison done during dry-run will take into account the charts
base values which ultimately results in ensure we perofrm a release
when the values change.
@hiddeco hiddeco force-pushed the chart-values-checksum branch from 5759aac to 94d20aa Compare January 8, 2020 10:02
@hiddeco hiddeco changed the title Handle releasing upon changes to chart base values release: detect change of charts base values Jan 8, 2020
@hiddeco hiddeco changed the title release: detect change of charts base values release: detect change of charts base values Jan 8, 2020
@hiddeco hiddeco merged commit f059e29 into fluxcd:master Jan 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Changing values.yaml in chart does not result in new deployment
2 participants