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

Implement values override merging #8

Merged
merged 3 commits into from
Apr 26, 2024
Merged

Implement values override merging #8

merged 3 commits into from
Apr 26, 2024

Conversation

AverageMarcus
Copy link
Member

@AverageMarcus AverageMarcus commented Apr 26, 2024

What does this PR do?

Changes the values loading logic to merge any provided values on top of the default values provided in this module.

Also clusterbuilder updated to take in a slice of values overrides that are layered ontop of the default values and the needed refactoring to support this.

In the future I anticipate the standup CLI to be able to apply "patches" to the values to configure functionality without needing the user to provide values files. For example, a CLI flag to disable the use of spot instances could be implemented by having a patch file that overrides the CAPA values like we currently do in our Tekton Task.

Towards: https://github.com/giantswarm/giantswarm/issues/30661

Checklist

  • CHANGELOG.md has been updated

@AverageMarcus AverageMarcus requested a review from a team as a code owner April 26, 2024 13:17
Signed-off-by: Marcus Noble <[email protected]>
Signed-off-by: Marcus Noble <[email protected]>
@AverageMarcus AverageMarcus merged commit 73082b3 into main Apr 26, 2024
4 checks passed
@AverageMarcus AverageMarcus deleted the merge_values branch April 26, 2024 15:13
Copy link

@pipo02mix pipo02mix left a comment

Choose a reason for hiding this comment

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

LGTM nice unit tests!

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 this pull request may close these issues.

3 participants