Helm diff does not use reuse-values

[paulmayer]

SUMMARY

kubernetes.core.helm diff does not consider reuse-values to calculate the diff, resulting in inaccurate diffs.

ISSUE TYPE

• Bug Report

COMPONENT NAME

The helmdiff_check signature differs from upgrades using deploy. reuse_values, for example, is missing.

Recently merged changes (#670) do not fully align the behavior.

ANSIBLE VERSION

ansible [core 2.16.3]
  config file = None
  configured module search path = ['/home/ansible/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/local/lib/python3.11/site-packages/ansible
  ansible collection location = /home/ansible/.ansible/collections:/usr/share/ansible/collections
  executable location = /usr/local/bin/ansible
  python version = 3.11.5 (main, Sep 22 2023, 15:34:29) [GCC 8.5.0 20210514 (Red Hat 8.5.0-20)] (/usr/bin/python3)
  jinja version = 3.1.3
  libyaml = True

COLLECTION VERSION

# /home/ansible/.ansible/collections/ansible_collections
Collection      Version
--------------- -------
kubernetes.core 3.0.0  

# /usr/local/lib/python3.11/site-packages/ansible_collections
Collection      Version
--------------- -------
kubernetes.core 2.4.0

CONFIGURATION

CONFIG_FILE() = /home/ansible/playbooks/ansible.cfg
DEFAULT_MODULE_PATH(/home/ansible/playbooks/ansible.cfg) = ['/home/ansible/playbooks/library', '/home/ansible/playbooks/{{ ANSIBLE_HOME ~ "/plugins/modules', '/usr/share/ansible/plugins/modules" }}']

OS / ENVIRONMENT

STEPS TO REPRODUCE

• Create a basic chart

helm create mychart
cd mychart
rm -rf templates
rm values.yaml
mkdir templates

add templates/configmap.yaml:

apiVersion: v1
kind: ConfigMap
metadata:
  name: cmap
data:
  myvalue: {{ .Values.myvalue }}
  myothervalue: {{ .Values.myothervalue }}

add values.yaml:

myvalue: foo
myothervalue: bar

create playbook helm-diff.yml

---
- name: Reproduce wrong diff output
  hosts: localhost
  connection: local
  gather_facts: false
  tasks:
    - name: Create KinD cluster
      ansible.builtin.command: >-
        kind create cluster
          --wait 300s
          --name helm-diff-reproduce
          --kubeconfig /tmp/kubeconfig
      changed_when: true
      check_mode: false

    - name: Create testing namespace
      kubernetes.core.k8s:
        state: present
        definition:
          apiVersion: v1
          kind: Namespace
          metadata:
            name: testing
            labels:
              name: testing
        kubeconfig: /tmp/kubeconfig
      check_mode: false

    - name: Create helm release
      kubernetes.core.helm:
        state: present
        chart_ref: /path/to/mychart
        release_name: myrelease
        release_namespace: testing
        release_values:
          myvalue: foo1
          myothervalue: bar1
        kubeconfig: /tmp/kubeconfig
        wait: true
      check_mode: false

    - name: Upgrade helm release
      kubernetes.core.helm:
        chart_ref: /path/to/mychart
        reset_values: false
        reuse_values: true
        release_name: myrelease
        release_namespace: testing
        values:
          myvalue: foo1
        kubeconfig: /tmp/kubeconfig

run ansible-playbook helm-diff.yml --check --diff and observe output:

TASK [Upgrade helm release] **************************************************************************************************************************************************************************************************************************************************************
testing, cmap, ConfigMap (v1) has changed:
  # Source: mychart/templates/configmap.yaml
  apiVersion: v1
  kind: ConfigMap
  metadata:
    name: cmap
  data:
    myvalue: foo1
-   myothervalue: bar1
+   myothervalue: bar
changed: [localhost]

(as opposed to no expected diff - see also empty output of helm diff upgrade myrelease /tmp/mychart --reuse-values -n testing --set myvalue=foo1 --kubeconfig /tmp/kubeconfig)

run ansible-playbook helm-diff.yml and check the configmap resource with kubectl get configmap cmap -n testing --kubeconfig /tmp/kubeconfig -o yaml:

apiVersion: v1
data:
  myothervalue: bar1
  myvalue: foo1
kind: ConfigMap
metadata:
  annotations:
    meta.helm.sh/release-name: myrelease
    meta.helm.sh/release-namespace: testing
  creationTimestamp: "2024-02-23T07:23:29Z"
  labels:
    app.kubernetes.io/managed-by: Helm
  name: cmap
  namespace: testing
  resourceVersion: "2323"
  uid: 2234d738-bc12-4f0f-b573-df9b2223deb0

Shows correct myothervalue reused.

EXPECTED RESULTS

The diff output matches what would otherwise be achieved with helm diff upgrade <...> --reuse-values <...>

ACTUAL RESULTS

Output as if running helm diff upgrade <...> without --reuse-values

[abikouo]

Hi @paulmayer thanks for reporting this issue.
I have submit a PR #683 to fix the idempotency issue. Could you please give a try ?

[paulmayer]

Hi @abikouo :

I tested your branch against my small example and a more contrived real world one and everything looks good (I see that the test also checks whether the interaction between reuse_values and reset_values works as expected - something I haven't done). Thanks a lot for your work!

What I am not sure is whether there is a reason (design decision, convention, earlier issues,...) for the command composition (string concatenations with cmd in helm_diff and deploy_cmd in deploy) to use their own separate logic in either of the two functions (increasing the risk of such issues cropping up in the future).

Thanks again for the quick fix!

[abikouo]

Hi @paulmayer thanks for your feedback

Hi @abikouo :

I tested your branch against my small example and a more contrived real world one and everything looks good (I see that the test also checks whether the interaction between reuse_values and reset_values works as expected - something I haven't done). Thanks a lot for your work!

What I am not sure is whether there is a reason (design decision, convention, earlier issues,...) for the command composition (string concatenations with cmd in helm_diff and deploy_cmd in deploy) to use their own separate logic in either of the two functions (increasing the risk of such issues cropping up in the future).

There is no design decision for having these 2 separated, this is how the module was originally written, this can be subjected to future refactoring to create a single function dealing with diff and deploy options

Thanks again for the quick fix!