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.

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

vmware_guest: Fix vApp property handling #2220

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ppmathis
Copy link

SUMMARY

The vmware_guest module did not properly handle setting vApp properties if the VmConfigSpec never contained an active vAppConfig. This resulted in an exception due to incorrectly trying to access property on None.

This PR changes the behaviour to default the current list of vApp properties to an empty list if there has never been any vAppConfig.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

vmware_guest

ADDITIONAL INFORMATION

A simple reproducer would be a to attempt to use community.vmware.vmware_guest like this:

- name: Configure vApp options
  community.vmware.vmware_guest:
    state: present
    hostname: xxx
    username: xxx
    password: xxx
    name: reproducer-vm
    vapp_properties:
      - id: name
        type: string
        label: This is a test
        category: Test
  delegate_to: localhost
  become: false
  run_once: true

Doing so on any VM within vSphere that has its vApp options disabled, meaning that no current vApp properties exist, leads to this exception:

fatal: [test-host -> localhost]: FAILED! =>
changed: false
module_stderr: |-
    Traceback (most recent call last):
      File "/Users/xxx/.ansible/tmp/ansible-tmp-1729754011.0982752-47065-171137552843633/AnsiballZ_vmware_guest.py", line 107, in <module>
        _ansiballz_main()
      File "/Users/xxx/.ansible/tmp/ansible-tmp-1729754011.0982752-47065-171137552843633/AnsiballZ_vmware_guest.py", line 99, in _ansiballz_main
        invoke_module(zipped_mod, temp_path, ANSIBALLZ_PARAMS)
      File "/Users/xxx/.ansible/tmp/ansible-tmp-1729754011.0982752-47065-171137552843633/AnsiballZ_vmware_guest.py", line 47, in invoke_module
        runpy.run_module(mod_name='ansible_collections.community.vmware.plugins.modules.vmware_guest', init_globals=dict(_module_fqn='ansible_collections.community.vmware.plugins.modules.vmware_guest', _modlib_path=modlib_path),
      File "<frozen runpy>", line 226, in run_module
      File "<frozen runpy>", line 98, in _run_module_code
      File "<frozen runpy>", line 88, in _run_code
      File "/var/folders/lb/q308qwkx1gggdlfxx_xqj9n00000gq/T/ansible_community.vmware.vmware_guest_payload_zwjpusr8/ansible_community.vmware.vmware_guest_payload.zip/ansible_collections/community/vmware/plugins/modules/vmware_guest.py", line 3640, in <module>
      File "/var/folders/lb/q308qwkx1gggdlfxx_xqj9n00000gq/T/ansible_community.vmware.vmware_guest_payload_zwjpusr8/ansible_community.vmware.vmware_guest_payload.zip/ansible_collections/community/vmware/plugins/modules/vmware_guest.py", line 3579, in main
      File "/var/folders/lb/q308qwkx1gggdlfxx_xqj9n00000gq/T/ansible_community.vmware.vmware_guest_payload_zwjpusr8/ansible_community.vmware.vmware_guest_payload.zip/ansible_collections/community/vmware/plugins/modules/vmware_guest.py", line 3195, in reconfigure_vm
      File "/var/folders/lb/q308qwkx1gggdlfxx_xqj9n00000gq/T/ansible_community.vmware.vmware_guest_payload_zwjpusr8/ansible_community.vmware.vmware_guest_payload.zip/ansible_collections/community/vmware/plugins/modules/vmware_guest.py", line 2071, in configure_vapp_properties
    AttributeError: 'NoneType' object has no attribute 'property'
module_stdout: ''
msg: |-
    MODULE FAILURE
    See stdout/stderr for the exact error
rc: 1

With the fix from this commit applied, the situation where vAppConfig equals None is gracefully handled and the vApp properties can be successfully set.

@ppmathis
Copy link
Author

I've just checked out the CI logs, but to the best of my knowledge they are not related to this MR and rather a form of flaky tests, as they do not mess with vApp properties and the configure_vapp_properties method bails out for vapp_properties: [] before any changed code runs.

@mariolenz
Copy link
Collaborator

recheck

@mariolenz
Copy link
Collaborator

@ppmathis FYI You, as the author of this PR, should also be able to re-run the CI again by simply commenting recheck like I did.

Sorry, I didn't have the time to look at this. Although what I can say at the spur of the moment, you have to add a changelog fragment.

The vmware_guest module did not properly handle setting vApp properties if the
VmConfigSpec never contained an active vAppConfig. This resulted in an exception
due to trying to access `property` on `None`. This commit changes the behavior
to default the current list of vApp properties to an empty list if there has
never been any vAppConfig.
@ppmathis
Copy link
Author

@mariolenz Thanks a lot for your guidance! I've just added a bugfix changelog fragment as asked.

@mariolenz
Copy link
Collaborator

There have been some problems in the CI and also some changes to vmware_guest meanwhile, so I'll close and re-open this PR to trigger a new clean CI run. I'm not sure if a recheck would be really enough.

@mariolenz mariolenz closed this Nov 23, 2024
@mariolenz mariolenz reopened this Nov 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants