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

add reset_then_reuse_values support to helm module #802

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
+344 −3
Open

add reset_then_reuse_values support to helm module #802

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6ff5f87
add reset_then_reuse_values support to helm module
b0z02003 Jan 18, 2025
baed9a6
Fix and update Integration test for --reset-then-reuse-values
yurnov Jan 18, 2025
eced4f2
reset_then_reuse_values should fail when requirements are not met
b0z02003 Jan 19, 2025
b67a091
refactor module fail for helm_diff_version<3.9.12, helm_version<3.14.0
yurnov Jan 19, 2025
70b5fcd
update version_added to 5.2.0
yurnov Jan 21, 2025
2e66302
bump version_added to 6.0.0
yurnov Jan 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelogs/fragments/800-helm-add-reset_then_reuse_values-support.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
---
minor_changes:
- helm - add reset_then_reuse_values support to helm module (https://github.com/ansible-collections/kubernetes.core/issues/803).
21 changes: 21 additions & 0 deletions docs/kubernetes.core.helm_module.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,27 @@ Parameters
<div>If <em>reset_values</em> is set to <code>True</code>, this is ignored.</div>
</td>
</tr>
<tr>
<td colspan="2">
<div class="ansibleOptionAnchor" id="parameter-"></div>
<b>reset_then_reuse_values</b>
<a class="ansibleOptionLink" href="#parameter-" title="Permalink to this option"></a>
<div style="font-size: small">
<span style="color: purple">boolean</span>
</div>
<div style="font-style: italic; font-size: small; color: darkgreen">added in 5.1.0</div>
</td>
<td>
<ul style="margin: 0; padding: 0"><b>Choices:</b>
<li>no</li>
<li>yes</li>
</ul>
</td>
<td>
<div>When upgrading package, reset the values to the ones built into the chart, apply the last release&#x27;s values and merge in any overrides from parameters <em>release_values</em>, <em>values_files</em> or <em>set_values</em>.</div>
<div>If <em>reset_values</em> or <em>reuse_values</em> is set to <code>True</code>, this is ignored.</div>
</td>
</tr>
<tr>
<td colspan="2">
<div class="ansibleOptionAnchor" id="parameter-"></div>
Expand Down
40 changes: 40 additions & 0 deletions plugins/modules/helm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,16 @@
required: false
default: True
version_added: 3.0.0
reset_then_reuse_values:
description:
- When upgrading package, reset the values to the ones built into the chart, apply the last release's values and merge in any overrides from
parameters O(release_values), O(values_files) or O(set_values).
- If O(reset_values) or O(reuse_values) is set to V(True), this is ignored.
- This feature requires helm diff >= 3.9.12.
type: bool
required: false
default: False
version_added: 5.1.0

#Helm options
disable_hook:
Expand Down Expand Up @@ -509,6 +519,7 @@ def deploy(
set_value_args=None,
reuse_values=None,
reset_values=True,
reset_then_reuse_values=False,
):
"""
Install/upgrade/rollback release chart
Expand All @@ -526,6 +537,15 @@ def deploy(
if reuse_values is not None:
deploy_command += " --reuse-values=" + str(reuse_values)

if reset_then_reuse_values:
helm_version = module.get_helm_version()
if LooseVersion(helm_version) < LooseVersion("3.14.0"):
module.warn(
Copy link
Contributor

@yurnov yurnov Jan 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

it it ok to have worning, or better to change to fail like:

            module.fail_json(
                msg='helm support option --reset-then-reuse-values starting release >= 3.14.0',
                **opt_result

just to avoid a case when helm will runs without --reset-then-reuse-values with unexpected result in CI

Copy link
Author

@b0z02003 b0z02003 Jan 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

You are right, we should fail when trying to use that kind of unsupported feature, I updated the PR accordingly. However, I did not include the **opt_result part (nor any parameter other than msg) since the failure is not the result of a command execution. I also changed the message to better reflect the versioning status and to better match the messages I found in the collection, are you OK with that?

"helm support option --reset-then-reuse-values starting release >= 3.14.0"
)
else:
deploy_command += " --reset-then-reuse-values"

if wait:
deploy_command += " --wait"
if wait_timeout is not None:
Expand Down Expand Up @@ -642,6 +662,7 @@ def helmdiff_check(
set_value_args=None,
reuse_values=None,
reset_values=True,
reset_then_reuse_values=False,
):
"""
Use helm diff to determine if a release would change by upgrading a chart.
Expand Down Expand Up @@ -676,6 +697,20 @@ def helmdiff_check(
if reuse_values:
cmd += " --reuse-values"

if reset_then_reuse_values:
helm_diff_version = get_plugin_version("diff")
helm_version = module.get_helm_version()
if LooseVersion(helm_diff_version) < LooseVersion("3.9.12"):
Copy link
Contributor

Choose a reason for hiding this comment

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

I see a case when both requirement aren't meet, the only fist check will be executed and only reset_then_reuse_values requires helm diff >= 3.9.12, current version is {0} will be on fail message.

So, it better to check both conditions and have a message that contains helm, helm diff or both, depends on versions installed.

If you don't mind, I will prepare commit that improve this part

module.warn(
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

"helm diff support option --reset-then-reuse-values starting release >= 3.9.12"
)
elif LooseVersion(helm_version) < LooseVersion("3.14.0"):
module.warn(
"helm support option --reset-then-reuse-values starting release >= 3.14.0"
)
else:
cmd += " --reset-then-reuse-values"

rc, out, err = module.run_helm_command(cmd)
return (len(out.strip()) > 0, out.strip())

Expand Down Expand Up @@ -735,6 +770,7 @@ def argument_spec():
set_values=dict(type="list", elements="dict"),
reuse_values=dict(type="bool"),
reset_values=dict(type="bool", default=True),
reset_then_reuse_values=dict(type="bool", default=False),
)
)
return arg_spec
Expand Down Expand Up @@ -787,6 +823,7 @@ def main():
set_values = module.params.get("set_values")
reuse_values = module.params.get("reuse_values")
reset_values = module.params.get("reset_values")
reset_then_reuse_values = module.params.get("reset_then_reuse_values")

if update_repo_cache:
run_repo_update(module)
Expand Down Expand Up @@ -883,6 +920,7 @@ def main():
set_value_args=set_value_args,
reuse_values=reuse_values,
reset_values=reset_values,
reset_then_reuse_values=reset_then_reuse_values,
)
changed = True

Expand All @@ -908,6 +946,7 @@ def main():
set_value_args,
reuse_values=reuse_values,
reset_values=reset_values,
reset_then_reuse_values=reset_then_reuse_values,
)
if would_change and module._diff:
opt_result["diff"] = {"prepared": prepared}
Expand Down Expand Up @@ -943,6 +982,7 @@ def main():
set_value_args=set_value_args,
reuse_values=reuse_values,
reset_values=reset_values,
reset_then_reuse_values=reset_then_reuse_values,
)
changed = True

Expand Down
1 change: 1 addition & 0 deletions tests/integration/targets/helm/defaults/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,4 @@ test_namespace:
- "helm-from-url"
- "helm-reuse-values"
- "helm-chart-with-space-into-name"
- "helm-reset-then-reuse-values"
2 changes: 1 addition & 1 deletion tests/integration/targets/helm/tasks/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@
loop_control:
loop_var: helm_version
with_items:
- "v3.8.0"
- "v3.16.0"
3 changes: 3 additions & 0 deletions tests/integration/targets/helm/tasks/run_test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@
- name: test helm upgrade with reuse_values
include_tasks: test_helm_reuse_values.yml

- name: test helm upgrade with reset_then_reuse_values
include_tasks: test_helm_reset_then_reuse_values.yml

- name: test helm dependency update
include_tasks: test_up_dep.yml

Expand Down
75 changes: 75 additions & 0 deletions tests/integration/targets/helm/tasks/test_helm_reset_then_reuse_values.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
---
- name: Test helm reset_then_reuse_values
vars:
helm_namespace: "{{ test_namespace[11] }}"
chart_release_values:
replica:
replicaCount: 3
master:
count: 1
kind: Deployment
chart_reset_then_reuse_values:
replica:
replicaCount: 1
master:
count: 3
block:
- name: Initial chart installation
helm:
binary_path: "{{ helm_binary }}"
chart_ref: oci://registry-1.docker.io/bitnamicharts/redis
release_name: test-redis
release_namespace: "{{ helm_namespace }}"
create_namespace: true
release_values: "{{ chart_release_values }}"
register: install

- name: Get value set as string
helm_info:
binary_path: "{{ helm_binary }}"
release_name: test-redis
release_namespace: "{{ helm_namespace }}"
register: release_value

- name: Validate that chart values are as expected
assert:
that:
- install is changed
- '"--reset-then-reuse-values" not in install.command'
- release_value["status"]["values"] == chart_release_values

- name: Upgrade chart using reset_then_reuse_values=true
helm:
binary_path: "{{ helm_binary }}"
chart_ref: oci://registry-1.docker.io/bitnamicharts/redis
release_name: test-redis
release_namespace: "{{ helm_namespace }}"
reuse_values: false
reset_values: false
reset_then_reuse_values: true
release_values: "{{ chart_reset_then_reuse_values }}"
register: upgrade

- name: Get value set as string
helm_info:
binary_path: "{{ helm_binary }}"
release_name: test-redis
release_namespace: "{{ helm_namespace }}"
register: release_value

- name: Validate that chart values are as expected
assert:
that:
- upgrade is changed
- '"--reset-then-reuse-values" in upgrade.command'
- '"--reuse-values " not in upgrade.command'
- '"--reset-values" not in upgrade.command'
- release_value["status"]["values"] == chart_release_values | combine(chart_reset_then_reuse_values, recursive=true)

always:
- name: Remove helm namespace
k8s:
api_version: v1
kind: Namespace
name: "{{ helm_namespace }}"
state: absent
4 changes: 3 additions & 1 deletion tests/integration/targets/helm_diff/tasks/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
binary_path: "{{ helm_binary }}"
state: present
plugin_path: https://github.com/databus23/helm-diff
plugin_version: 3.4.0
plugin_version: 3.9.13

- name: Copy test chart
copy:
Expand Down Expand Up @@ -324,3 +324,5 @@
ignore_errors: true

- include_tasks: reuse_values.yml

- include_tasks: reset_then_reuse_values.yml
Loading