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_vm_info: Fix AttributeError #1919

Merged
merged 2 commits into from
Nov 15, 2023

Conversation

mariolenz
Copy link
Collaborator

@mariolenz mariolenz commented Nov 13, 2023

SUMMARY

According to the documentation ipConfig in GuestNicInfo(vim.vm.GuestInfo.NicInfo) might be unset. This can lead to an AttributeError: 'NoneType' object has no attribute 'ipAddress'.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME
ADDITIONAL INFORMATION

I wasn't really able to reproduce this with:

  • powered off VM
  • powered on VM without VMware Tools installed
  • powered on VM without VMware Tools installed and running
  • powered on VM without VMware Tools installed and running, not all NICs configured

But it looks like some users run into this issue.

Copy link

codecov bot commented Nov 13, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (22aa241) 20.29% compared to head (7c53e93) 20.29%.
Report is 5 commits behind head on main.

❗ Current head 7c53e93 differs from pull request most recent head f036763. Consider uploading reports for the commit f036763 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1919      +/-   ##
==========================================
- Coverage   20.29%   20.29%   -0.01%     
==========================================
  Files         189      189              
  Lines       24582    24583       +1     
  Branches     5568     5569       +1     
==========================================
  Hits         4988     4988              
- Misses      19565    19566       +1     
  Partials       29       29              
Flag Coverage Δ
sanity 19.02% <ø> (-0.01%) ⬇️
units 18.60% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mariolenz
Copy link
Collaborator Author

recheck

@mariolenz mariolenz closed this Nov 13, 2023
@mariolenz mariolenz reopened this Nov 13, 2023
@mariolenz
Copy link
Collaborator Author

@AndrewSav Could you please test if this PR fixes your issue?

@AndrewSav
Copy link

AndrewSav commented Nov 13, 2023

@mariolenz Thank you for your work!

@AndrewSav Could you please test if this PR fixes your issue?

Is there a ansible-galaxy collection install I can execute to get the PR installed? We are running our playbooks on AWX in a custom execution environment image, and this command is how collections are getting installed there. I do not have another practical way of testing in other than building another EE image, and running it but I'm not sure how do I get this change into that image, can you advise?

@mariolenz
Copy link
Collaborator Author

Is there a ansible-galaxy collection install I can execute to get the PR installed?

No, there isn't. Since I couldn't reproduce your issue (and therefor don't know if this really fixes you issue without introducing another one) the idea is that you test the code before I merge and release it on galaxy.

One way to get this PR so you can test it might look like this:

mkdir -p  ~/.ansible/collections/ansible_collections/community
git clone https://github.com/ansible-collections/community.vmware ~/.ansible/collections/ansible_collections/community/vmware
cd ~/.ansible/collections/ansible_collections/community/vmware
git fetch origin pull/1919/head:20231113-vmware_vm_info
git checkout 20231113-vmware_vm_info

We are running our playbooks on AWX in a custom execution environment image, and this command is how collections are getting installed there. I do not have another practical way of testing in other than building another EE image, and running it but I'm not sure how do I get this change into that image, can you advise?

I don't know how to get the changes into AWX / an EE. I think AWX and testing upstream code changes are no friends... but I might be wrong there.

@AndrewSav
Copy link

the idea is that you test the code before I merge and release it on galaxy.

Thank you, a realize that. You can ran galaxy with as in ansible-galaxy install 'git+https:// - with git prefix for source, which would not require you to release in on galaxy, I was referring to this. Obviously if you have not tested this approach on this repo before we cannot be sure it would work, hence was my question.

I don't know how to get the changes into AWX / an EE. I think AWX and testing upstream code changes are no friends... but I might be wrong there.

You may well be right, but this is a practical issue, AWX provides a lot of infrastructure, such a secret and inventory management (inventory probably is not relevant in our context), so It can take significant amount of time even reproducing it outside it, let alone testing the patch. I really appreciate your work and I'm eager to test the fix, but I need to figure out how to do that. It can take awhile.

@AndrewSav
Copy link

@mariolenz we have tested this now and it does fix our issue. Thank you again!

@mariolenz
Copy link
Collaborator Author

@AndrewSav Thanks for testing!

You might have to wait two weeks or so for the next release, though.

Copy link

Build succeeded (gate pipeline).
https://ansible.softwarefactory-project.io/zuul/buildset/c24ccb219fd94429b52456088e53f576

✔️ ansible-tox-linters SUCCESS in 9m 59s
✔️ build-ansible-collection SUCCESS in 8m 14s
✔️ ansible-galaxy-importer SUCCESS in 5m 00s

Copy link

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

@mariolenz mariolenz closed this Nov 15, 2023
@mariolenz mariolenz reopened this Nov 15, 2023
Copy link

Build succeeded (gate pipeline).
https://ansible.softwarefactory-project.io/zuul/buildset/8d5d4ba139154e37afb798a6b6a879c4

✔️ ansible-tox-linters SUCCESS in 9m 49s
✔️ build-ansible-collection SUCCESS in 8m 20s
✔️ ansible-galaxy-importer SUCCESS in 4m 19s

Copy link

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

@mariolenz mariolenz merged commit fa98edf into ansible-collections:main Nov 15, 2023
27 checks passed
@mariolenz mariolenz deleted the 20231113-vmware_vm_info branch November 15, 2023 21:07
@aseefahmed
Copy link

@mariolenz is there any ETA when its going to publish into galaxy?

@mariolenz
Copy link
Collaborator Author

There's no ETA. One or two weeks, I guess.

@sapanp007
Copy link

@mariolenz is it published into galaxy?

@mariolenz
Copy link
Collaborator Author

I would have liked to fix some more bugs and maybe implement some new features, but I'm afraid I didn't find the time.

Therefore, I've just released 4.0.1 to fix this at least. And yes @sapanp007, it's available on galaxy. Since a couple of minutes...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants