-
Notifications
You must be signed in to change notification settings - Fork 140
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
helm: add support for history-max parameter #164
Conversation
The --history-max parameter allows the user to set a maximum amount of revisions per release to be kept in the history. By default helm keeps 10 revisions per release, which means that 10 secrets will be kept per release.
Codecov Report
@@ Coverage Diff @@
## main #164 +/- ##
=======================================
Coverage 24.02% 24.02%
=======================================
Files 1 1
Lines 154 154
Branches 29 29
=======================================
Hits 37 37
- Misses 111 112 +1
+ Partials 6 5 -1
Continue to review full report at Codecov.
|
When the history_max option is not set, the module will not pass '--history-max' to the CLI command. This ensures that the defaults of the helm CLI will alwasy be used.
Thanks for you review @Akasurde , I've added your remarks to the PR. Thanks! |
Thanks for reviewing @abikouo! |
Should be ok now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vvanouytsel Thank you for the PR! The --history-max
option is not supported for helm install
which is used when the replace
param is set. The history_max
and replace
parameters will need to be set to mutually exclusive. This can be done here:
kubernetes.core/plugins/modules/helm.py
Line 554 in b32e5cd
mutually_exclusive=[ |
kubernetes.core/plugins/modules/k8s.py
Line 65 in b32e5cd
- mutually exclusive with C(apply) |
The 'history_max' parameter is not available when using the 'helm install' command, it is only implemented for 'helm ugprade'. The 'replace' option uses the 'install' parameter, thus 'replace' and 'history_max' have to be mutually exclusive.
@gravesm Thanks for your review! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few small nits and then everything else looks good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @vvanouytsel!
Add helm dependency update SUMMARY Execute the helm dependency update under the hood when found dependencies block in Chart.yaml file. Support the execution of: Standalone dependency update by executing: helm dependency update CHART Inline dependency update when specifying the helm chart_repo_url by adding --dependency-update to the helm install command. ISSUE TYPE Feature Pull Request #191 COMPONENT NAME helm, helm_template ADDITIONAL INFORMATION There is a doc generated for history_max option for the helm module. I think that is not generated in the previous PR #164. There is others changes affect the docs/ folder when I run the collection_prep_add_docs -p . command. These changes are added in the last commit 64eab40. I let you decide rather we keep the commit or remove it. The --dependency-update insertion option is tested used a local helm chart repository create via docker. So here are the tasks that test this feature. Maybe if we create a GitHub repository for the helm chart, we can add this test code in the CI pipeline. # Test The update dependency with chart_repo_url - name: "Test chart without dependencies block and chart_repo_url defined" block: - name: "Test chart without dependencies block and chart_repo_url defined" helm: binary_path: "{{ helm_binary }}" name: test chart_ref: "ingress-nginx" chart_repo_url: https://kubernetes.github.io/ingress-nginx chart_version: "{{ chart_source_version | default(omit) }}" namespace: "{{ helm_namespace }}" create_namespace: yes register: release - assert: that: - "'--dependency-update' not in release.command" - "'upgrade' in release.command" success_msg: "Command does not contains '--dependency-update' options" fail_msg: "Command contains '--dependency-update' options" - name: "Test chart with dependencies block and chart_repo_url defined and replace True" block: - name: "Test chart with dependencies block and chart_repo_url defined and replace True" helm: binary_path: "{{ helm_binary }}" name: test1 chart_ref: "dep_up" chart_repo_url: http://repo:8080/charts chart_version: "{{ chart_source_version | default(omit) }}" namespace: "{{ helm_namespace }}" create_namespace: yes replace: true register: release - debug: var=release - assert: that: - "'--dependency-update' in release.command" - "'install' in release.command" success_msg: "Command contains '--dependency-update' options with helm install command" fail_msg: "Command not contains '--dependency-update' with helm install command" - name: "Test chart with dependencies block and chart_repo_url defined and replace False fails" block: - name: "Test chart with dependencies block and chart_repo_url defined and replace False fails" helm: binary_path: "{{ helm_binary }}" name: test2 chart_ref: "dep_up" chart_repo_url: http://repo:8080/charts chart_version: "{{ chart_source_version | default(omit) }}" namespace: "{{ helm_namespace }}" create_namespace: yes replace: false register: release ignore_errors: true - assert: that: - release.failed - release.msg == "'--dependency-update' hasn't been supported yet with 'helm upgrade'. Please use 'helm install' instead by adding 'replace' option" success_msg: "Command build fail when adding '--dependency-update' with the helm upgrade command" Reviewed-by: Mike Graves <[email protected]> Reviewed-by: Wissem BEN CHAABANE <[email protected]> Reviewed-by: Bikouo Aubin <None>
SUMMARY
The --history-max parameter allows the user to set a maximum amount of
revisions per release to be kept in the history.
By default helm keeps 10 revisions per release, which means that 10
secrets will be kept per release. If you have multiple helm releases, the amount of secrets created by Helm will quickly add up.
With supporting the 'history-max' CLI parameter via the ansible module, the user will have the posibility to determine how many revisions he wants to keep per release.
ISSUE TYPE
COMPONENT NAME
plugins/modules/helm.py
ADDITIONAL INFORMATION
Before change:
After change:
Without history_max set:
With history_max set: