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.

Sign up for GitHub

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

Jump to bottom

helm_template: add optional show_only and release_namespace arguments #388

Merged
merged 2 commits into from
Mar 1, 2022
Merged

helm_template: add optional show_only and release_namespace arguments #388

merged 2 commits into from
Mar 1, 2022

Conversation

cvstebut
Copy link
Contributor

SUMMARY

This PR adds the "show_only" and "release_namespace" as optional arguments to the helm_template module.
It does some work towards #313.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

changelogs/fragments/313-helm-template-add-support-for-show-only-and-release-namespace.yml
plugins/modules/helm_template.py
tests/unit/modules/test_helm_template.py

ADDITIONAL INFORMATION

The PR does include unit tests instead of integration test.

Reasoning:

The existing integration tests already include a task based on helm_template. So we know that the module does a proper job of using the command line generated inside the module to call helm.
As I trust helm itself to "do its job" correctly, all that should be necessary is to test the correct generation of the command line itself. The included unit tests hopefully do a proper job.

With regards of the pretty long testing times for the module, I really prefer unit tests, if at all possible.

Please let me know if this fits.

@ansible-zuul
Copy link

ansible-zuul bot commented Feb 19, 2022

Build succeeded.

✔️ noop SUCCESS in 0s

Warning:
Failed to update check run ansible/check: 500 Server Error: Internal Server Error for url: https://api.github.com/repos/ansible-collections/kubernetes.core/check-runs/5260611390

@softwarefactory-project-zuul
Copy link

Build failed.

@cvstebut
Copy link
Contributor Author

I might be wrong, but the errors in ansible/check look more like configuration issues in the testing pipeline.
Please advise, if you think I can do something from my side.

@cvstebut
Copy link
Contributor Author

recheck

@softwarefactory-project-zuul
Copy link

Build failed.

@gravesm
Copy link
Member

gravesm commented Feb 21, 2022

@cvstebut The sanity failure for the 2.11 test is unrelated to your changes here, and will likely have to be fixed in ansible. Not sure yet what the time frame for that fix is.

@cvstebut
Copy link
Contributor Author

@gravesm Thanks for letting me know! And also many thanks to those working on the testing pipelines!

@cvstebut @Akasurde
Update show_only description
b3468f1 
Co-authored-by: Abhijeet Kasurde <akasurde@redhat.com>
@softwarefactory-project-zuul
Copy link

Build succeeded.

@cvstebut
Copy link
Contributor Author

Hi,

I am currently working on adding further optional parameters to helm_template:

  • name (str: sets helm template NAME, thus setting the release name to be used in the rendered templates)
  • release_name (bool: sets helm template --release-name parameter. The release_name will then be used in the output_dir path)
  • disable_hook (bool: sets helm template --no_hooks parameter)

Should I extend this PR or wait for this PR to be merged and start the next one?
Waiting would also allow to "step back" while all the work on the integration tests is being done.

@gravesm
Copy link
Member

gravesm commented Mar 1, 2022

I think we can go ahead and merge this. Since this just has unit tests, it shouldn't be affected by any changes to the integration tests. Thanks for your work on it!

@gravesm gravesm added the mergeit label Mar 1, 2022
@goneri
Copy link
Member

goneri commented Mar 1, 2022

The ansible-ee-tests-stable-2.12 has failed. I just pushed ansible/ansible-zuul-jobs#1369 to ignore it. Once this PR is merged, we will able to retrigger another gating job with the regate key word.

@goneri
Copy link
Member

goneri commented Mar 1, 2022

regate

@goneri
Copy link
Member

goneri commented Mar 1, 2022

My previous PR didn't work. I've pushed this one ansible/ansible-zuul-jobs#1370 which this time turn the job non-voting globally.

@softwarefactory-project-zuul

This comment was marked as outdated.

Sign in to view
@goneri
Copy link
Member

goneri commented Mar 1, 2022

regate

@softwarefactory-project-zuul
Copy link

Build succeeded (gate pipeline).

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit 73499d9 into ansible-collections:main Mar 1, 2022
@cvstebut cvstebut deleted the helm_template_add_show_only_and_release_namespace branch March 2, 2022 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Akasurde Akasurde Akasurde left review comments

@gravesm gravesm gravesm approved these changes

Assignees
No one assigned
Labels
mergeit
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@cvstebut @gravesm @goneri @Akasurde