add reset_then_reuse_values support to helm module

[b0z02003]

SUMMARY

Starting with version 3.14.0, Helm supports --reset-then-reuse-values. As discussed on the original PR. This greatly improves on --reuse-values as it allows to avoid templates errors when new features are added to an upgraded chart.

Closes #803

ISSUE TYPE

• Feature Pull Request

COMPONENT NAME

helm

ADDITIONAL INFORMATION

This PR is greatly 'inspired' by #575 and because I wasn't sure how I could provide additional tests for it, I actually copied those build previously for --reuse-values (as it is an improvement on this feature.

[softwarefactory-project-zuul]

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/83f476caa6754ad180048e444dda5b9a

✔️ ansible-galaxy-importer SUCCESS in 4m 32s
✔️ build-ansible-collection SUCCESS in 6m 08s

[softwarefactory-project-zuul]

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/d599dcd8a95847fca57c251d33d9e188

✔️ ansible-galaxy-importer SUCCESS in 5m 13s
✔️ build-ansible-collection SUCCESS in 5m 28s

[softwarefactory-project-zuul]

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/a99a1a55ac8b48279f1a348cb9958051

✔️ ansible-galaxy-importer SUCCESS in 4m 51s
✔️ build-ansible-collection SUCCESS in 5m 50s

[abikouo]

@b0z02003 Thanks for submitting this PR.
In addition to the requested changes, you must include the new test files in main.yml of the corresponding integration test targets.

[softwarefactory-project-zuul]

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/7d875e50571f4e85b2e7deec96a15936

✔️ ansible-galaxy-importer SUCCESS in 4m 48s
✔️ build-ansible-collection SUCCESS in 5m 24s

[b0z02003]

@abikouo Thanks for the suggestions ! I've tried to improve on them to also check for helm version and added the integration tests (that was a big oversight on my part, my bad).

[softwarefactory-project-zuul]

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/2bfd564abf3947a59fcc353b78b7ba27

✔️ ansible-galaxy-importer SUCCESS in 4m 14s
✔️ build-ansible-collection SUCCESS in 5m 53s

[abikouo]

This will address linters issues

[softwarefactory-project-zuul]

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/84be6f6970494700bc7fdf1b82d924d1

✔️ ansible-galaxy-importer SUCCESS in 4m 18s
✔️ build-ansible-collection SUCCESS in 5m 18s

[softwarefactory-project-zuul]

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/106403b4b0114e9f818fda2fdbc69018

✔️ ansible-galaxy-importer SUCCESS in 3m 50s
✔️ build-ansible-collection SUCCESS in 5m 25s

[abikouo]

@b0z02003 Some tests are failing because you need to rebase your branch with the main branch. However, you'll also need to fix the new tests you introduced.

[softwarefactory-project-zuul]

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/39451f82bfb74feb9f681f056d4c06a3

✔️ ansible-galaxy-importer SUCCESS in 3m 56s
✔️ build-ansible-collection SUCCESS in 5m 15s

[softwarefactory-project-zuul]

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/ac91308cd52644e99e0b2208a44cc0c2

✔️ ansible-galaxy-importer SUCCESS in 3m 49s
✔️ build-ansible-collection SUCCESS in 5m 44s

[yurnov]

I see some race conditions:

TASK [helm : Initial chart installation] ***************************************
task path: /home/runner/collections/ansible_collections/kubernetes/core/tests/integration/targets/helm/tasks/test_helm_reset_then_reuse_values.yml:17
Saturday 18 January 2025  18:31:30 +0000 (0:00:00.039)       0:03:29.157 ****** 
......
fatal: [localhost]: FAILED! => {
    "changed": false,
    "command": "/tmp/helm/linux-amd64/helm upgrade -i --reset-values --create-namespace -f=/tmp/tmpvpofk4e1.yml test-redis 'oci://registry-1.docker.io/bitnamicharts/redis'",
    "invocation": {
        "module_args": {
            "api_key": null,
            "atomic": false,
            "binary_path": "/tmp/helm/linux-amd64/helm",
            "ca_cert": null,
            "chart_ref": "oci://registry-1.docker.io/bitnamicharts/redis",
            "chart_repo_url": null,
            "chart_version": null,
            "context": null,
            "create_namespace": true,
            "dependency_update": false,
            "disable_hook": false,
            "force": false,
            "history_max": null,
            "host": null,
            "kubeconfig": null,
            "post_renderer": null,
            "purge": true,
            "release_name": "test-redis",
            "release_namespace": "helm-reuse-values",
            "release_state": "present",
            "release_values": {
                "master": {
                    "count": 1,
                    "kind": "Deployment"
                },
                "replica": {
                    "replicaCount": 3
                }
            },
            "replace": false,
            "reset_then_reuse_values": false,
            "reset_values": true,
            "reuse_values": null,
            "set_values": null,
            "skip_crds": false,
            "timeout": null,
            "update_repo_cache": false,
            "validate_certs": true,
            "values_files": [],
            "wait": false,
            "wait_timeout": null
        }
    },
    "msg": "Failure when executing Helm command. Exited 1.\nstdout: \nstderr: Error: UPGRADE FAILED: create: failed to create: secrets \"sh.helm.release.v1.test-redis.v3\" is forbidden: unable to create new content in namespace helm-reuse-values because it is being terminated\n",
    "stderr": "Error: UPGRADE FAILED: create: failed to create: secrets \"sh.helm.release.v1.test-redis.v3\" is forbidden: unable to create new content in namespace helm-reuse-values because it is being terminated\n",
    "stderr_lines": [
        "Error: UPGRADE FAILED: create: failed to create: secrets \"sh.helm.release.v1.test-redis.v3\" is forbidden: unable to create new content in namespace helm-reuse-values because it is being terminated"
    ],
    "stdout": "",
    "stdout_lines": []
}

[softwarefactory-project-zuul]

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/9fac1a62b16942a2ba01960e65454e29

✔️ ansible-galaxy-importer SUCCESS in 5m 05s
✔️ build-ansible-collection SUCCESS in 5m 26s

[yurnov]

I see some race conditions:

I have fixed this race condition and all other parts of the integration test for this PR and added a test to ensure that a warning appears in case of older versions of Helm and Helm diff, so the CI part is fully OK.

However, I still have a not resolved question:
https://github.com/b0z02003/kubernetes.core/blob/baed9a658fa538476d415978406c1cd579fe83d3/plugins/modules/helm.py#L540C1-L547C59

Should we have just a warning message and skips of the unsupported option --reset-then-reuse-values here? It potentially may lead to unexpected results with just warning that easy to didn't catch in CI/automation, so, probably it better to have something like:

    if reset_then_reuse_values:
        helm_version = module.get_helm_version()
        if LooseVersion(helm_version) < LooseVersion("3.14.0"):
            module.fail_json(
                msg='helm support option --reset-then-reuse-values starting release >= 3.14.0',
                **opt_result
            )
        else:
            deploy_command += " --reset-then-reuse-values"

And same here:
https://github.com/b0z02003/kubernetes.core/blob/baed9a658fa538476d415978406c1cd579fe83d3/plugins/modules/helm.py#L700C1-L712C48

I'm not sure, looking for suggestions from repo maintainers and @b0z02003.

Att @gravesm, @abikouo

[yurnov]

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

[b0z02003]

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?

[yurnov]

same here

[softwarefactory-project-zuul]

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/36535ed7b9cc4ca28452ef201bdb5730

✔️ ansible-galaxy-importer SUCCESS in 3m 50s
✔️ build-ansible-collection SUCCESS in 5m 16s

[yurnov]

Hi @b0z02003, you committed to testing in CI https://github.com/ansible-collections/kubernetes.core/actions/runs/12853271360/job/35836239457, and all tests passed.

[yurnov]

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

[yurnov]

Hi @abikouo, @gravesm,

would you please review this PR? The integration test is completed here.

[softwarefactory-project-zuul]

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/bbb23e1d841746b8a3f4d8a0c13fc2b4

✔️ ansible-galaxy-importer SUCCESS in 4m 06s
✔️ build-ansible-collection SUCCESS in 5m 30s

[softwarefactory-project-zuul]

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/a68f5f226348420abeccc75214d4e431

✔️ ansible-galaxy-importer SUCCESS in 4m 46s
✔️ build-ansible-collection SUCCESS in 5m 16s

[softwarefactory-project-zuul]

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/0b1cede427d04cbe9a2d4d92b3ca7731

✔️ ansible-galaxy-importer SUCCESS in 4m 24s
✔️ build-ansible-collection SUCCESS in 5m 18s