Skip to content

fix(vmware_guest_network): fix idempotency and diff issue due to the …#2547

Open
ryanb74 wants to merge 3 commits into
ansible-collections:mainfrom
ryanb74:fix-guestvmwarenetwork-idempotency
Open

fix(vmware_guest_network): fix idempotency and diff issue due to the …#2547
ryanb74 wants to merge 3 commits into
ansible-collections:mainfrom
ryanb74:fix-guestvmwarenetwork-idempotency

Conversation

@ryanb74

@ryanb74 ryanb74 commented Mar 4, 2026

Copy link
Copy Markdown

…before state not showing vlan id at all while after state was showing vlan id

SUMMARY

When running the guest_vmware_network module, changes are always reported due to the vlan_id not being present in the before_diff, which messes up the comparison between before and after state.

This PR fixes this issue.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

vmware_guest_network module

ADDITIONAL INFORMATION

Running the module with the following parameters, in check mode with diff mode enabled :

- name: "Set Network on VM {{ hostname }}"
  community.vmware.vmware_guest_network:
    hostname: '{{ vcenterhost }}'
    username: "{{ vcenter_username }}"
    password: "{{ vcenter_password }}"
    datacenter: "{{ datacenter }}"
    name: '{{ hostname }}'
    network_name: "{{ network_name }}"
    mac_address: "{{ mac_address}}"
    state: "present"	

Before changes :

TASK [vms_network : Set Network on VM virtual_machine_name] [CHECK MODE] ***************************************************************************************************************************************
--- before
+++ after
@@ -1,4 +1,11 @@
 {
+    "MAC_ADDR": {
+        "label": "check_mode_adapter",
+        "mac_address": "MAC_ADDR",
+        "network_name": "NETWORK_NAME",
+        "unit_number": 40000,
+        "vlan_id": 1006
+    },
     "MAC_ADDR": {
         "allow_guest_ctl": true,
         "connected": false,

changed: [localhost] => (item=NETWORK_NAME [MAC_ADDR]) => {
    "ansible_loop_var": "item",
    "changed": true,
    "item": "NETWORK_NAME [MAC_ADDR]",
    "network_info": [
        {
            "allow_guest_ctl": true,
            "connected": false,
            "device_type": "vmxnet3",
            "label": "Network adapter 1",
            "mac_address": "MAC_ADDR",
            "network_name": "NETWORK_NAME",
            "start_connected": true,
            "switch": "SWITCH_NAME",
            "unit_number": 7,
            "wake_onlan": false
        },
        {
            "label": "check_mode_adapter",
            "mac_address": "MAC_ADDR",
            "network_name": "NETWORK_NAME",
            "unit_number": 40000,
            "vlan_id": 1006
        }
    ]
}

After changes :

TASK [vms_network : Set Network on VM virtual_machine_name] [CHECK MODE] ***************************************************************************************************************************************
ok: [localhost] => (item=NETWORK_NAME [MAC_ADDR]) => {
    "ansible_loop_var": "item",
    "changed": false,
    "item": "NETWORK_NAME [MAC_ADDR]",
    "network_info": [
        {
            "allow_guest_ctl": true,
            "connected": false,
            "device_type": "vmxnet3",
            "label": "Network adapter 1",
            "mac_address": "MAC_ADDR",
            "network_name": "NETWORK_NAME",
            "start_connected": true,
            "switch": "SWITCH_NAME",
            "unit_number": 7,
            "vlan_id": 1006,
            "wake_onlan": false
        }
    ]
}

We can see that after the change, the module doesn't report "changed" anymore and that there is no diff displayed because no difference was found.

@softwarefactory-project-zuul

Copy link
Copy Markdown

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

✔️ ansible-tox-linters SUCCESS in 6m 19s
✔️ ansible-galaxy-importer SUCCESS in 5m 10s (non-voting)
✔️ build-ansible-collection SUCCESS in 6m 46s

@ryanb74 ryanb74 marked this pull request as draft March 4, 2026 12:45
@ryanb74

ryanb74 commented Mar 4, 2026

Copy link
Copy Markdown
Author

Looks like I need to work a bit more on that one, I actually missed an issue :

MAC address case counts and now there are two NICS for the virtual machine I tested, with the same MAC address but one in lowercase and the other uppercase, we can see there in the "before" section of the description of this PR

@ryanb74

ryanb74 commented Mar 4, 2026

Copy link
Copy Markdown
Author

According to the following article, mac addresses should always be entered lowercase in vmware :
https://knowledge.broadcom.com/external/article/334929/best-practices-for-assigning-static-mac.html

I enhanced the mac_address argument documentation to precise that it is case sensitive.

It seems like vmware accepts uppercase mac addresses to configure network adapters, so some people may have uppercase addresses in their systems, maybe the case could even be mixed ???

I wonder if we shouldn't add some piece of code to normalize the user input to whatever comes from the current state of the nic in vmware

ryanb74 added 2 commits March 4, 2026 14:21
…before state not showing vlan id at all while after state was showing vlan id
@ryanb74 ryanb74 force-pushed the fix-guestvmwarenetwork-idempotency branch from 9e5157e to 37d641d Compare March 4, 2026 13:22
@ryanb74

ryanb74 commented Mar 4, 2026

Copy link
Copy Markdown
Author

I tested the code again and confirm that it looks good and that the code that adds the vlan id in the before diff has the expected impact as long as we use the proper case for mac address.

@ryanb74 ryanb74 marked this pull request as ready for review March 4, 2026 13:26
@softwarefactory-project-zuul

Copy link
Copy Markdown

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

✔️ ansible-tox-linters SUCCESS in 5m 25s
✔️ ansible-galaxy-importer SUCCESS in 4m 52s (non-voting)
✔️ build-ansible-collection SUCCESS in 7m 39s

@ryanb74

ryanb74 commented Mar 25, 2026

Copy link
Copy Markdown
Author

Hi @mariolenz, any update regarding this PR ?
Thanks

@ryanb74 ryanb74 force-pushed the fix-guestvmwarenetwork-idempotency branch from 37d641d to 6f38f9f Compare April 8, 2026 07:21
@softwarefactory-project-zuul

Copy link
Copy Markdown

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

✔️ ansible-tox-linters SUCCESS in 5m 23s
✔️ ansible-galaxy-importer SUCCESS in 5m 30s (non-voting)
✔️ build-ansible-collection SUCCESS in 6m 42s

@sonarqubecloud

Copy link
Copy Markdown

@mariolenz mariolenz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm not 100% sure if I understand this bugfix, but to the best of my knowledge it LGTM.

@mariolenz mariolenz added bug This issue/PR relates to a bug mergeit labels Jul 6, 2026
@centosinfra-prod-github-app

Copy link
Copy Markdown

Build succeeded (gate pipeline).
https://gateway-cloud-softwarefactory.apps.ocp.cloud.ci.centos.org/zuul/t/ansible/buildset/130373d3bb284280b5560c6cde000c78

✔️ ansible-tox-linters SUCCESS in 5m 07s
✔️ build-ansible-collection SUCCESS in 6m 38s
✔️ ansible-galaxy-importer SUCCESS in 8m 23s (non-voting)

@centosinfra-prod-github-app

Copy link
Copy Markdown

Pull request merge failed: Resource not accessible by integration, You may need to manually rebase your PR and retry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug This issue/PR relates to a bug mergeit

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants