lib/ansible/utils/vars.py - def merge_hash - fix for append / prepend when merging vars are equal #79312

R37ribution · 2022-11-04T16:51:57Z

SUMMARY

Resolves #79293

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

resolves combine() filter plugin not appending / prepending properly when the two vars to be combined are equal

ADDITIONAL INFORMATION

test.yaml playbook used to reproduce issue

---
- hosts: localhost
  gather_facts: false
  vars:
    "data":
      "profile_list":
        - "test"
        #- "Profile1"
    "new_data":
      "profile_list":
        - "test"
        #- "Profile2"
  tasks:
    - name: combine
      set_fact:
        data: "{{ data | default({}, true) | combine(new_data, recursive=True, list_merge='append') }}"
    - name: print
      debug:
        var: data

Before change:

$ ansible-playbook test.yaml
[DEPRECATION WARNING]: Ansible will require Python 3.8 or newer on the controller starting with Ansible 2.12. Current version: 3.6.8 (default, Nov 16 2020,
16:55:22) [GCC 4.8.5 20150623 (Red Hat 4.8.5-44)]. This feature will be removed from ansible-core in version 2.12. Deprecation warnings can be disabled by
setting deprecation_warnings=False in ansible.cfg.
[WARNING]: No inventory was parsed, only implicit localhost is available
[WARNING]: provided hosts list is empty, only localhost is available. Note that the implicit localhost does not match 'all'

PLAY [localhost] ********************************************************************************************************************************************

TASK [combine] **********************************************************************************************************************************************
ok: [localhost]

TASK [print] ************************************************************************************************************************************************
ok: [localhost] => {
    "data": {
        "profile_list": [
            "test"
        ]
    }
}

PLAY RECAP **************************************************************************************************************************************************
localhost                  : ok=2    changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0

$

After change:

$ ansible-playbook test.yaml
[DEPRECATION WARNING]: Ansible will require Python 3.8 or newer on the controller starting with Ansible 2.12. Current version: 3.6.8 (default, Nov 16 2020,
16:55:22) [GCC 4.8.5 20150623 (Red Hat 4.8.5-44)]. This feature will be removed from ansible-core in version 2.12. Deprecation warnings can be disabled by
setting deprecation_warnings=False in ansible.cfg.
[WARNING]: No inventory was parsed, only implicit localhost is available
[WARNING]: provided hosts list is empty, only localhost is available. Note that the implicit localhost does not match 'all'

PLAY [localhost] ********************************************************************************************************************************************

TASK [combine] **********************************************************************************************************************************************
ok: [localhost]

TASK [print] ************************************************************************************************************************************************
ok: [localhost] => {
    "data": {
        "profile_list": [
            "test",
            "test"
        ]
    }
}

PLAY RECAP **************************************************************************************************************************************************
localhost                  : ok=2    changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0

$

bcoca

tests and changelog? https://docs.ansible.com/ansible/latest/community/contributions.html

…when-vars-are-equal.yml - add changelog fragment for PR ansible#79312

R37ribution · 2022-11-04T22:13:14Z

tests and changelog? https://docs.ansible.com/ansible/latest/community/contributions.html

@bcoca I believe I have resolved your concerns. Can you have another look? Thanks, Aaron

bcoca · 2022-11-08T15:10:12Z

lib/ansible/utils/vars.py

    # (this `if` can be remove without impact on the function
    #  except performance)
-    if x == {} or x == y:
+    if x == {} or (x == y and list_merge != 'append' and list_merge != 'prepend'):


list_merge not in ('append', 'prepend')

can you also add a test to prevent regressions?

I thought you might ask about not in - I'll get it added.

Hey @bcoca, I made the code change. As far as the test goes, I'll need to dig into that as I'm not familiar with the creation of tests yet in this repo.

can you also add a test to prevent regressions?

@bcoca Do you have any guides on how to create regression tests?

bcoca · 2022-11-08T15:10:52Z

lib/ansible/utils/vars.py

    # (this `if` can be remove without impact on the function
    #  except performance)
-    if x == {} or x == y:
+    if x == {} or (x == y and list_merge != 'append' and list_merge != 'prepend'):


can you also add a test to prevent regressions?

bcoca · 2022-11-08T15:31:45Z

note: not for this PR, but perpend_unique/append_unique might be worth adding to keep the case in which the preexisting behavior is desired.

bcoca · 2022-11-08T20:15:16Z

https://docs.ansible.com/ansible/latest/dev_guide/testing.html

webknjaz · 2024-01-02T22:23:08Z

The branch needs rebasing.

…when-vars-are-equal.yml - add changelog fragment for PR ansible#79312

R37ribution · 2024-07-18T22:54:07Z

The branch needs rebasing.

@webknjaz I have rebased with ...

$ git remote add upstream https://github.com/ansible/ansible.git

$ git remote -v
origin  https://www.github.com/R37ribution/ansible.git (fetch)
origin  https://www.github.com/R37ribution/ansible.git (push)
upstream        https://github.com/ansible/ansible.git (fetch)
upstream        https://github.com/ansible/ansible.git (push)

$ git fetch upstream
remote: Enumerating objects: 112214, done.
remote: Counting objects: 100% (35157/35157), done.
remote: Compressing objects: 100% (464/464), done.
remote: Total 112214 (delta 34694), reused 35132 (delta 34683), pack-reused 77057
Receiving objects: 100% (112214/112214), 52.63 MiB | 14.81 MiB/s, done.
Resolving deltas: 100% (75630/75630), completed with 8987 local objects.
From https://github.com/ansible/ansible
 * [new branch]            devel                   -> upstream/devel
 * [new branch]            mazer_role_loader       -> upstream/mazer_role_loader
 * [new branch]            milestone               -> upstream/milestone
 * [new branch]            release1.5.0            -> upstream/release1.5.0
 * [new branch]            release1.5.1            -> upstream/release1.5.1
<SNIP>

$ git rebase upstream/devel
Successfully rebased and updated refs/heads/R37ribution/issue-79293.

$ git push --force
warning: redirecting to https://github.com/R37ribution/ansible.git/
Enumerating objects: 24, done.
Counting objects: 100% (24/24), done.
Delta compression using up to 8 threads
Compressing objects: 100% (15/15), done.
Writing objects: 100% (17/17), 1.79 KiB | 25.00 KiB/s, done.
Total 17 (delta 11), reused 0 (delta 0), pack-reused 0 (from 0)
remote: Resolving deltas: 100% (11/11), completed with 6 local objects.
To https://www.github.com/R37ribution/ansible.git
 + f7d81372d7...016f134e2d R37ribution/issue-79293 -> R37ribution/issue-79293 (forced update)

$

R37ribution · 2024-07-18T23:44:47Z

https://docs.ansible.com/ansible/latest/dev_guide/testing.html

@bcoca I have ran the following tests according to the guide...

ansible-test sanity

Rocky 8.10 [success] - ansible-test sanity-rocky810.txt

Rocky 8.9 [failure] - ansible-test sanity-rocky89-failed.txt

Successfully installed PyYAML-6.0.1 astroid-3.1.0 dill-0.3.8 isort-5.13.2 mccabe-0.7.0 platformdirs-4.2.0 pylint-3.1.0 tomli-2.0.1 tomlkit-0.12.4 typing_extensions-4.10.0
ERROR: Found 1 pylint issue(s) which need to be resolved:
ERROR: test/sanity/ignore.txt:68:1: ansible-test: Ignoring 'deprecated-class' on 'lib/ansible/utils/collection_loader/_collection_finder.py' is unnecessary
See documentation for help: https://docs.ansible.com/ansible-core/devel/dev_guide/testing/sanity/pylint.html

ansible-test units

Rocky 8.10 [failure]

[root@CHQS-EniCEJ55G6 ansible-pr-testing]# ansible-test units
WARNING: Skipping unit tests on Python 3.8 because it could not be found.
WARNING: Skipping unit tests on Python 3.9 because it could not be found.
WARNING: Skipping unit tests on Python 3.10 because it could not be found.
WARNING: Skipping unit tests on Python 3.13 because it could not be found.
Unit test modules with Python 3.11
ERROR: usage: __main__.py [options] [file_or_dir] [file_or_dir] [...]
__main__.py: error: unrecognized arguments: -n test/units/modules/test_apt.py test/units/modules/test_apt_key.py test/units/modules/test_async_wrapper.py test/units/modules/test_copy.py test/units/modules/test_hostname.py test/units/modules/test_iptables.py test/units/modules/test_known_hosts.py test/units/modules/test_pip.py test/units/modules/test_service.py test/units/modules/test_service_facts.py test/units/modules/test_systemd.py test/units/modules/test_unarchive.py test/units/modules/test_uri.py
  inifile: /root/ansible-pr-testing/test/lib/ansible_test/_data/pytest/config/default.ini
  rootdir: /root/ansible-pr-testing

FATAL: Command "pytest -r a -n auto --color yes -p no:cacheprovider -c /root/ansible-pr-testing/test/lib/ansible_test/_data/pytest/config/default.ini --junit-xml /root/ansible-pr-testing/test/results/junit/python3.11-modules-units.xml --strict-markers --rootdir /root/ansible-pr-testing --confcutdir /root/ansible-pr-testing --durations=25 test/units/modules/test_apt.py test/units/modules/test_apt_key.py test/units/modules/test_async_wrapper.py test/units/modules/test_copy.py test/units/modules/test_hostname.py test/units/modules/test_iptables.py test/units/modules/test_known_hosts.py test/units/modules/test_pip.py test/units/modules/test_service.py test/units/modules/test_service_facts.py test/units/modules/test_systemd.py test/units/modules/test_unarchive.py test/units/modules/test_uri.py" returned exit status 4.
[root@CHQS-EniCEJ55G6 ansible-pr-testing]#

Rocky 8.9 [failure] -

[root@CHQS-EniCEJ55G6 ansible-pr-testing]# ansible-test units
WARNING: Skipping unit tests on Python 3.8 because it could not be found.
WARNING: Skipping unit tests on Python 3.9 because it could not be found.
WARNING: Skipping unit tests on Python 3.10 because it could not be found.
WARNING: Skipping unit tests on Python 3.12 because it could not be found.
WARNING: Skipping unit tests on Python 3.13 because it could not be found.
Unit test modules with Python 3.11
ERROR: usage: __main__.py [options] [file_or_dir] [file_or_dir] [...]
__main__.py: error: unrecognized arguments: -n test/units/modules/test_apt.py test/units/modules/test_apt_key.py test/units/modules/test_async_wrapper.py test/units/modules/test_copy.py test/units/modules/test_hostname.py test/units/modules/test_iptables.py test/units/modules/test_known_hosts.py test/units/modules/test_pip.py test/units/modules/test_service.py test/units/modules/test_service_facts.py test/units/modules/test_systemd.py test/units/modules/test_unarchive.py test/units/modules/test_uri.py
  inifile: /root/ansible-pr-testing/test/lib/ansible_test/_data/pytest/config/default.ini
  rootdir: /root/ansible-pr-testing

FATAL: Command "pytest -r a -n auto --color yes -p no:cacheprovider -c /root/ansible-pr-testing/test/lib/ansible_test/_data/pytest/config/default.ini --junit-xml /root/ansible-pr-testing/test/results/junit/python3.11-modules-units.xml --strict-markers --rootdir /root/ansible-pr-testing --confcutdir /root/ansible-pr-testing --durations=25 test/units/modules/test_apt.py test/units/modules/test_apt_key.py test/units/modules/test_async_wrapper.py test/units/modules/test_copy.py test/units/modules/test_hostname.py test/units/modules/test_iptables.py test/units/modules/test_known_hosts.py test/units/modules/test_pip.py test/units/modules/test_service.py test/units/modules/test_service_facts.py test/units/modules/test_systemd.py test/units/modules/test_unarchive.py test/units/modules/test_uri.py" returned exit status 4.
[root@CHQS-EniCEJ55G6 ansible-pr-testing]#

ansible-test integration -v ping
- Rocky 8.10 [success, Exception: boom] - ansible-test integration -v ping-rocky810.txt
- Rocky 8.9 [success, Exception: boom] - ansible-test integration -v ping-rocky89.txt

ansible --version

Rocky 8.10:

[root@CHQS-EniCEJ55G6 ansible-pr-testing]# cat /etc/redhat-release
Rocky Linux release 8.10 (Green Obsidian)
[root@CHQS-EniCEJ55G6 ansible-pr-testing]# uname -r
5.15.146.1-microsoft-standard-WSL2
[root@CHQS-EniCEJ55G6 ansible-pr-testing]# ansible --version
[WARNING]: You are running the development version of Ansible. You should only run Ansible from "devel" if you are modifying the Ansible engine, or trying out features under
development. This is a rapidly changing source of code and can become unstable at any point.
ansible [core 2.18.0.dev0] (testing_PR79312 016f134e2d) last updated 2024/07/18 18:29:00 (GMT -400)
  config file = /etc/ansible/ansible.cfg
  configured module search path = ['/root/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /root/ansible-pr-testing/lib/ansible
  ansible collection location = /root/.ansible/collections:/usr/share/ansible/collections
  executable location = /root/ansible-pr-testing/bin/ansible
  python version = 3.12.3 (main, Jul  2 2024, 20:57:30) [GCC 8.5.0 20210514 (Red Hat 8.5.0-22)] (/usr/bin/python)
  jinja version = 3.1.4
  libyaml = True
[root@CHQS-EniCEJ55G6 ansible-pr-testing]#

Rocky 8.9:

[root@CHQS-EniCEJ55G6 ansible-pr-testing]# cat /etc/redhat-release
Rocky Linux release 8.9 (Green Obsidian)
[root@CHQS-EniCEJ55G6 ansible-pr-testing]# uname -r
5.15.146.1-microsoft-standard-WSL2
[root@CHQS-EniCEJ55G6 ansible-pr-testing]# ansible --version
[WARNING]: You are running the development version of Ansible. You should only run Ansible from "devel" if you are modifying the Ansible engine, or trying out features under
development. This is a rapidly changing source of code and can become unstable at any point.
ansible [core 2.18.0.dev0] (testing_PR79312 016f134e2d) last updated 2024/07/18 19:10:02 (GMT -400)
  config file = /etc/ansible/ansible.cfg
  configured module search path = ['/root/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /root/ansible-pr-testing/lib/ansible
  ansible collection location = /root/.ansible/collections:/usr/share/ansible/collections
  executable location = /root/ansible-pr-testing/bin/ansible
  python version = 3.11.5 (main, Oct 25 2023, 16:19:59) [GCC 8.5.0 20210514 (Red Hat 8.5.0-20)] (/usr/bin/python)
  jinja version = 3.1.4
  libyaml = True
[root@CHQS-EniCEJ55G6 ansible-pr-testing]#

webknjaz · 2024-07-22T13:21:14Z

FTR the VyOS failure is unrelated.

webknjaz · 2024-07-23T14:42:35Z

This needs to be rebased to unblock the CI.

…ppend / prepend

…when-vars-are-equal.yml - add changelog fragment for PR ansible#79312

…ange

R37ribution · 2024-07-24T15:24:30Z

This needs to be rebased to unblock the CI.

done!

R37ribution · 2024-07-24T15:25:38Z

The last piece to getting this completed AFAIK is getting the regression test in place that @bcoca requested. If someone has a guide or even an example of a similar regression test, I can get that implemented in this PR.

ansibot added affects_2.15 bug This issue/PR relates to a bug. needs_triage Needs a first human triage before being processed. new_contributor This PR is the first contribution by a new community member. small_patch labels Nov 4, 2022

bcoca requested changes Nov 4, 2022

View reviewed changes

ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Nov 4, 2022

R37ribution marked this pull request as draft November 4, 2022 21:02

ansibot added the WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. label Nov 4, 2022

R37ribution added a commit to R37ribution/ansible that referenced this pull request Nov 4, 2022

ansible#79293: changelogs/fragments/79312-fix-combine-append-prepend-…

d8d1573

…when-vars-are-equal.yml - add changelog fragment for PR ansible#79312

R37ribution added a commit to R37ribution/ansible that referenced this pull request Nov 4, 2022

ansible#79293: changelogs/fragments/79312-fix-combine-append-prepend-…

70bf94b

…when-vars-are-equal.yml - add changelog fragment for PR ansible#79312

R37ribution force-pushed the R37ribution/issue-79293 branch from d8d1573 to 70bf94b Compare November 4, 2022 21:35

ansibot removed the small_patch label Nov 4, 2022

R37ribution marked this pull request as ready for review November 4, 2022 22:12

R37ribution requested a review from bcoca November 4, 2022 22:13

ansibot removed the WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. label Nov 4, 2022

bcoca reviewed Nov 8, 2022

View reviewed changes

bcoca requested changes Nov 8, 2022

View reviewed changes

bcoca removed the needs_triage Needs a first human triage before being processed. label Nov 8, 2022

ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Nov 8, 2022

ansibot added stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. stale_review Updates were made after the last review and the last review is more than 7 days old. labels Nov 16, 2022

s-hertel self-assigned this May 24, 2023

ansibot removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. stale_review Updates were made after the last review and the last review is more than 7 days old. labels Jun 17, 2023

ansibot added the stale_review Updates were made after the last review and the last review is more than 7 days old. label Jul 12, 2023

This comment was marked as resolved.

Sign in to view

webknjaz added the ci_verified Changes made in this PR are causing tests to fail. label Jan 2, 2024

webknjaz added the ci_verified Changes made in this PR are causing tests to fail. label Jan 5, 2024

ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Jan 12, 2024

R37ribution added a commit to R37ribution/ansible that referenced this pull request Jul 18, 2024

ansible#79293: changelogs/fragments/79312-fix-combine-append-prepend-…

ad84a91

…when-vars-are-equal.yml - add changelog fragment for PR ansible#79312

R37ribution force-pushed the R37ribution/issue-79293 branch from 6401f65 to f7d8137 Compare July 18, 2024 21:07

ansibot removed ci_verified Changes made in this PR are causing tests to fail. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Jul 18, 2024

R37ribution force-pushed the R37ribution/issue-79293 branch 2 times, most recently from f7d8137 to 016f134 Compare July 18, 2024 21:25

R37ribution added a commit to R37ribution/ansible that referenced this pull request Jul 18, 2024

ansible#79293: changelogs/fragments/79312-fix-combine-append-prepend-…

ff48fd1

…when-vars-are-equal.yml - add changelog fragment for PR ansible#79312

R37ribution added 3 commits July 24, 2024 09:43

ansible#79293: lib/ansible/utils/vars.py - def merge_hash - fix for a…

83c5ee2

…ppend / prepend

ansible#79293: changelogs/fragments/79312-fix-combine-append-prepend-…

031105c

…when-vars-are-equal.yml - add changelog fragment for PR ansible#79312

ansible#79293: lib/ansible/utils/vars.py - def merge_hash - syntax ch…

c1219c9

…ange

R37ribution force-pushed the R37ribution/issue-79293 branch from 016f134 to c1219c9 Compare July 24, 2024 13:44

ansibot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Jul 24, 2024

This comment was marked as abuse.

Sign in to view

ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Aug 7, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lib/ansible/utils/vars.py - def merge_hash - fix for append / prepend when merging vars are equal #79312

lib/ansible/utils/vars.py - def merge_hash - fix for append / prepend when merging vars are equal #79312

R37ribution commented Nov 4, 2022

bcoca left a comment •

edited

Loading

R37ribution commented Nov 4, 2022

bcoca Nov 8, 2022 •

edited

Loading

bcoca Nov 8, 2022

R37ribution Nov 8, 2022

R37ribution Nov 8, 2022

R37ribution Jul 18, 2024

bcoca Nov 8, 2022

bcoca commented Nov 8, 2022

bcoca commented Nov 8, 2022

This comment was marked as resolved.

This comment was marked as resolved.

webknjaz commented Jan 2, 2024

R37ribution commented Jul 18, 2024

R37ribution commented Jul 18, 2024

webknjaz commented Jul 22, 2024

webknjaz commented Jul 23, 2024

This comment was marked as abuse.

R37ribution commented Jul 24, 2024

R37ribution commented Jul 24, 2024

lib/ansible/utils/vars.py - def merge_hash - fix for append / prepend when merging vars are equal #79312

Are you sure you want to change the base?

lib/ansible/utils/vars.py - def merge_hash - fix for append / prepend when merging vars are equal #79312

Conversation

R37ribution commented Nov 4, 2022

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

test.yaml playbook used to reproduce issue

Before change:

After change:

bcoca left a comment • edited Loading

Choose a reason for hiding this comment

R37ribution commented Nov 4, 2022

bcoca Nov 8, 2022 • edited Loading

Choose a reason for hiding this comment

bcoca Nov 8, 2022

Choose a reason for hiding this comment

R37ribution Nov 8, 2022

Choose a reason for hiding this comment

R37ribution Nov 8, 2022

Choose a reason for hiding this comment

R37ribution Jul 18, 2024

Choose a reason for hiding this comment

bcoca Nov 8, 2022

Choose a reason for hiding this comment

bcoca commented Nov 8, 2022

bcoca commented Nov 8, 2022

This comment was marked as resolved.

This comment was marked as resolved.

webknjaz commented Jan 2, 2024

R37ribution commented Jul 18, 2024

R37ribution commented Jul 18, 2024

webknjaz commented Jul 22, 2024

webknjaz commented Jul 23, 2024

This comment was marked as abuse.

R37ribution commented Jul 24, 2024

R37ribution commented Jul 24, 2024

bcoca left a comment •

edited

Loading

bcoca Nov 8, 2022 •

edited

Loading