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

Introduce helm-extra-set-args command line parameter #402

Merged
merged 3 commits into from
Mar 23, 2022

Conversation

ilmax
Copy link
Contributor

@ilmax ilmax commented Mar 4, 2022

What this PR does / why we need it:
Often times it's required to override the values file when running chart-testing, we have helm-extra-args command line option, but that one is used also to uninstall the release so adding set parameters there results in an error in uninstalling the release (see #322, #212, #171)

Which issue this PR fixes
fixes #322, fixes #212, fixes #171

Special notes for your reviewer:

@ilmax ilmax changed the title introduce helm-extra-set-args Introduce helm-extra-set-args command line parameter Mar 4, 2022
@ilmax ilmax force-pushed the ilmax/add-helm-extra-set-flag branch 2 times, most recently from 431c57f to 2201116 Compare March 4, 2022 12:27
@cpanato
Copy link
Member

cpanato commented Mar 14, 2022

@ilmax thanks for the PR, there are some updates in the tests you need to adjust and also can you add some tests for this change as well?

@ilmax ilmax force-pushed the ilmax/add-helm-extra-set-flag branch from 58d2a73 to 3d30428 Compare March 15, 2022 15:08
Massimiliano Donini added 2 commits March 15, 2022 16:10
Signed-off-by: ilmax <[email protected]>

Signed-off-by: Massimiliano Donini <[email protected]>
- Fixed error when no values are passed in
- Added integration test

Signed-off-by: Massimiliano Donini <[email protected]>
@ilmax ilmax force-pushed the ilmax/add-helm-extra-set-flag branch from 3d30428 to feb4b66 Compare March 15, 2022 15:10
@ilmax
Copy link
Contributor Author

ilmax commented Mar 15, 2022

@cpanato I fixed the issue, and added an integration test where I fetch another version of nginx, let me know if you need me to do something more here, Thanks!

@cpanato
Copy link
Member

cpanato commented Mar 16, 2022

@ilmax thanks for the PR
can you generate the docs using the doc-gen?

go build -o ct-bin ./ct/main.go

$ ./ct-bin doc-gen
Generating docs...
Done.

then commit the changes?
thanks!

Signed-off-by: Massimiliano Donini <[email protected]>
@ilmax
Copy link
Contributor Author

ilmax commented Mar 16, 2022

@cpanato Sorry, I missed that, done and pushed!

@cpanato cpanato requested a review from jlegrone March 16, 2022 12:16
Copy link
Member

@cpanato cpanato left a comment

Choose a reason for hiding this comment

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

lgtm

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