Fix lookups return empty string when looking up resources inside folders #445

ThSchrad · 2023-11-14T18:35:00Z

SUMMARY

When lookups search for resources inside folders, they return an empty string even if the resource is available in this path. For example, a vm_moid lookup with the term /mydc/vm/myfolder1/myvm2 returns an empty string while a lookup for /mydc/vm/myvm1 returns the expected moid.

One could've fixed this issue with no major overhaul of the file. But by looking at the code, I expect that more issues would arise such as finding the wrong folder if multiple folders have the same name, e. g. /mydc/vm/myfolder1/foo and /mydc/vm/myfolder2/foo.

So I rewrote large parts of the file to strictly follow the path top-down. On some occasions the lookups now make multiple requests against the api to ensure we find the correct resource (e. g. finding the root resource pool on a standalone host). This will certainly decrease performance at the expense of correct results.

Also I think the code is no easier to read and reason about as there is no complex recursive function anymore.

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

cluster_moid
datacenter_moid
datastore_moid
folder_moid
host_moid
network_moid
resource_pool_moid
vm_moid

ADDITIONAL INFORMATION

I used an ansible playbook to reproduce the issue. Please note that parts of this playbook (path, moid) must be adjusted to the environment in which the tests are executed. The credentials were set via environment variables.

- name: Test
  hosts: localhost
  gather_facts: false
  tasks:
    - name: Test
      ansible.builtin.debug:
        msg: "{{ lookup('vmware.vmware_rest.' + item['type'] + '_moid', item['path']) == item['moid'] }}"
      loop:
        - type: datacenter
          path: /mydc
          moid: datacenter-xxxxxx
        - type: network
          path: /mydc/network/mynet1
          moid: network-xxxxxx
        - type: datastore
          path: /mydc/datastore/myds1
          moid: datastore-xxxxxx
        - type: folder
          path: /mydc/vm
          moid: group-xxxxxx
        - type: folder
          path: /mydc/vm/myfolder1
          moid: group-xxxxxx
        - type: vm
          path: /mydc/vm/myvm1
          moid: vm-xxxxxx
        - type: vm
          path: /mydc/vm/myfolder1/myvm2
          moid: vm-xxxxxx
        - type: cluster
          path: /mydc/host/mycluster1
          moid: domain-xxxxxx
        - type: host
          path: /mydc/host/mycluster1/myhost1
          moid: host-xxxxxx
        - type: resource_pool
          path: /mydc/host/myhost1/Resources
          moid: resgroup-xxxxxx
        - type: resource_pool
          path: /mydc/host/myhost1/Resources/myrp1
          moid: resgroup-xxxxxx

The output before the changes was:

ok: [localhost] => (item={'type': 'datacenter', 'path': '/mydc', 'moid': 'datacenter-xxxxxx'}) => {
    "msg": true
}
ok: [localhost] => (item={'type': 'network', 'path': '/mydc/network/mynet1', 'moid': 'network-xxxxxx'}) => {
    "msg": true
}
ok: [localhost] => (item={'type': 'datastore', 'path': '/mydc/datastore/myds1', 'moid': 'datastore-xxxxxx'}) => {
    "msg": true
}
ok: [localhost] => (item={'type': 'folder', 'path': '/mydc/vm', 'moid': 'group-xxxxxx'}) => {
    "msg": true
}
ok: [localhost] => (item={'type': 'folder', 'path': '/mydc/vm/myfolder1', 'moid': 'group-xxxxxx'}) => {
    "msg": false
}
ok: [localhost] => (item={'type': 'vm', 'path': '/mydc/vm/myvm1', 'moid': 'vm-xxxxxx'}) => {
    "msg": true
}
ok: [localhost] => (item={'type': 'vm', 'path': '/mydc/vm/myfolder1/myvm2', 'moid': 'vm-xxxxxx'}) => {
    "msg": false
}
ok: [localhost] => (item={'type': 'cluster', 'path': '/mydc/host/mycluster1', 'moid': 'domain-xxxxxx'}) => {
    "msg": true
}
ok: [localhost] => (item={'type': 'host', 'path': '/mydc/host/mycluster1/myhost1', 'moid': 'host-xxxxxx'}) => {
    "msg": true
}
ok: [localhost] => (item={'type': 'resource_pool', 'path': '/mydc/host/myhost1/Resources', 'moid': 'resgroup-xxxxxx'}) => {
    "msg": false
}
ok: [localhost] => (item={'type': 'resource_pool', 'path': '/mydc/host/myhost1/Resources/myrp1', 'moid': 'resgroup-xxxxxx'}) => {
    "msg": false
}

The output after the changes was:

ok: [localhost] => (item={'type': 'datacenter', 'path': '/mydc', 'moid': 'datacenter-xxxxxx'}) => {
    "msg": true
}
ok: [localhost] => (item={'type': 'network', 'path': '/mydc/network/mynet1', 'moid': 'network-xxxxxx'}) => {
    "msg": true
}
ok: [localhost] => (item={'type': 'datastore', 'path': '/mydc/datastore/myds1', 'moid': 'datastore-xxxxxx'}) => {
    "msg": true
}
ok: [localhost] => (item={'type': 'folder', 'path': '/mydc/vm', 'moid': 'group-xxxxxx'}) => {
    "msg": true
}
ok: [localhost] => (item={'type': 'folder', 'path': '/mydc/vm/myfolder1', 'moid': 'group-xxxxxx'}) => {
    "msg": true
}
ok: [localhost] => (item={'type': 'vm', 'path': '/mydc/vm/myvm1', 'moid': 'vm-xxxxxx'}) => {
    "msg": true
}
ok: [localhost] => (item={'type': 'vm', 'path': '/mydc/vm/myfolder1/myvm2', 'moid': 'vm-xxxxxx'}) => {
    "msg": true
}
ok: [localhost] => (item={'type': 'cluster', 'path': '/mydc/host/mycluster1', 'moid': 'domain-xxxxxx'}) => {
    "msg": true
}
ok: [localhost] => (item={'type': 'host', 'path': '/mydc/host/mycluster1/myhost1', 'moid': 'host-xxxxxx'}) => {
    "msg": true
}
ok: [localhost] => (item={'type': 'resource_pool', 'path': '/mydc/host/myhost1/Resources', 'moid': 'resgroup-xxxxxx'}) => {
    "msg": true
}
ok: [localhost] => (item={'type': 'resource_pool', 'path': '/mydc/host/myhost1/Resources/myrp1', 'moid': 'resgroup-xxxxxx'}) => {
    "msg": true
}

ThSchrad · 2023-11-14T18:44:40Z

Fixes #359 and #324.

softwarefactory-project-zuul · 2023-11-14T19:11:49Z

Build failed.
https://ansible.softwarefactory-project.io/zuul/buildset/63e29f1c41584dd39f3805078a3d26c2

❌ ansible-test-cloud-integration-vmware-rest FAILURE in 23m 02s
✔️ build-ansible-collection SUCCESS in 11m 20s
✔️ tox-cloud-refresh-examples-vmware SUCCESS in 11m 26s
✔️ ansible-galaxy-importer SUCCESS in 4m 42s

ThSchrad · 2023-11-22T12:09:27Z

Build failed. https://ansible.softwarefactory-project.io/zuul/buildset/63e29f1c41584dd39f3805078a3d26c2

❌ ansible-test-cloud-integration-vmware-rest FAILURE in 23m 02s ✔️ build-ansible-collection SUCCESS in 11m 20s ✔️ tox-cloud-refresh-examples-vmware SUCCESS in 11m 26s ✔️ ansible-galaxy-importer SUCCESS in 4m 42s

Is this something I can fix? To me it seems that the CI pipeline can't connect to the vcenter.

alinabuzachis · 2023-11-22T12:55:29Z

recheck

softwarefactory-project-zuul · 2023-11-22T13:50:33Z

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

✔️ ansible-test-cloud-integration-vmware-rest SUCCESS in 15m 44s
✔️ build-ansible-collection SUCCESS in 10m 16s
✔️ tox-cloud-refresh-examples-vmware SUCCESS in 10m 51s
✔️ ansible-galaxy-importer SUCCESS in 3m 29s

mikemorency · 2024-07-12T13:28:14Z

recheck

softwarefactory-project-zuul · 2024-07-12T13:29:19Z

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.
Warning:
Unable to update github.com/ansible-collections/vmware.vmware_rest

SUMMARY This change rewrites the lookup plugin logic to use a top down approach to finding objects. The plugins will now step through an objects path and try to lookup each intermediate object until it gets to the final object. The old method has a few issues: The existing plugins use an incorrect search method which yield inconsistent results (as described in #500) They also cannot find resources with the same name, even if the paths given are unique. For example, if /datacenter/vms/my-folder and /datacenter/hosts/my-folder both exist, you can never find either one Items with the same name will cause conflicts, even if those items have slightly different paths. For example: /datacenter/vms/foo and /datacenter/hosts/foo existing at the same time makes it difficult to search for either folder Fixes: #500 #445 #324 ISSUE TYPE Bugfix Pull Request COMPONENT NAME all lookup plugins ADDITIONAL INFORMATION The original top down approach is described in #445. I tried to get that version working but ran into a few issues. Reviewed-by: Danielle Barda Reviewed-by: Ondra Machacek <machacek.ondra@gmail.com> Reviewed-by: mikemorency

mikemorency · 2024-09-09T16:42:31Z

Closing as no longer needed. PR mentioned in #500 fixes this. Thank you @ThSchrad for your contribution, it was very helpful!

ThSchrad · 2024-09-09T17:53:10Z

You're welcome and thanks to all contributors for providing us with such a helpful collection.

Fix lookups not finding nested resources

a29b19d

ThSchrad added 2 commits November 22, 2023 14:12

Fix linting errors

76d51ae

Fix isort linting errors

d40111c

alinabuzachis added the bug This issue/PR relates to a bug label Feb 28, 2024

dbk-rabel mentioned this pull request Jul 2, 2024

recursive_folder_or_rp_moid_search awaits objects_moid to be in order (but it isn't necessarily) #500

Closed

mikemorency mentioned this pull request Aug 27, 2024

refactor lookup plugins using top down method #523

Merged

mikemorency closed this Sep 9, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix lookups return empty string when looking up resources inside folders #445

Fix lookups return empty string when looking up resources inside folders #445

ThSchrad commented Nov 14, 2023

ThSchrad commented Nov 14, 2023

softwarefactory-project-zuul bot commented Nov 14, 2023

ThSchrad commented Nov 22, 2023

alinabuzachis commented Nov 22, 2023

softwarefactory-project-zuul bot commented Nov 22, 2023

mikemorency commented Jul 12, 2024

softwarefactory-project-zuul bot commented Jul 12, 2024

mikemorency commented Sep 9, 2024 •

edited

Loading

ThSchrad commented Sep 9, 2024

Fix lookups return empty string when looking up resources inside folders #445

Fix lookups return empty string when looking up resources inside folders #445

Conversation

ThSchrad commented Nov 14, 2023

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

ThSchrad commented Nov 14, 2023

softwarefactory-project-zuul bot commented Nov 14, 2023

ThSchrad commented Nov 22, 2023

alinabuzachis commented Nov 22, 2023

softwarefactory-project-zuul bot commented Nov 22, 2023

mikemorency commented Jul 12, 2024

softwarefactory-project-zuul bot commented Jul 12, 2024

mikemorency commented Sep 9, 2024 • edited Loading

ThSchrad commented Sep 9, 2024

mikemorency commented Sep 9, 2024 •

edited

Loading