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

Add VM.GetEnvironment() to retrieve OVF Environment #528

Merged
merged 12 commits into from
Mar 10, 2023

Conversation

odraghi
Copy link
Contributor

@odraghi odraghi commented Nov 29, 2022

Signed-off-by: Olivier DRAGHI [email protected]

Description

I need a way for a service running inside a VM to query cloud director as an OrgAdmin and find itself.
I think about checking the VM moref that is available with vmtools :
vmtoolsd --cmd "info-get guestinfo.ovfenv" | grep "ve:vCenterId" | cut -d'"' -f2

Unfortunately getting ovfenv ( Environment ) was not implemeted in govdc.

Related issue: (#526)

@dataclouder
Copy link
Contributor

Thanks for this contribution.
Please, expand the description of this PR with the following:

  1. What is the problem that you are trying to solve and the current code doesn't solve? In other words, define what is the purpose of this proposal.
  2. Add at least one test that proves this change is doing what it is supposed to do. If you are unable to write such test (the gocheck environment requires some learning) please suggest what such test should do to prove that the change you propose is effective.

Signed-off-by: Olivier DRAGHI <[email protected]>
@odraghi odraghi marked this pull request as draft January 10, 2023 20:37
@odraghi odraghi marked this pull request as ready for review January 10, 2023 20:40
Copy link
Collaborator

@adambarreiro adambarreiro left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!
The new test passed for me in 10.4.1 and 10.3.0.
Just a couple of comments/suggestions/changes:

@odraghi odraghi requested review from dataclouder and removed request for lvirbalas and Didainius February 23, 2023 07:51
Copy link
Collaborator

@lvirbalas lvirbalas left a comment

Choose a reason for hiding this comment

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

(Tidying error messages).

Copy link
Collaborator

@adambarreiro adambarreiro left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

@Didainius
Copy link
Collaborator

@odraghi ,
Thanks for all fixes. I am testing the SDK itself as well as will run our Terraform provider for VCD tests because the OVF properties belong to VM struct and it can have side effects

@odraghi odraghi requested review from lvirbalas and removed request for dataclouder, Didainius and adezxc March 9, 2023 16:03
@odraghi
Copy link
Contributor Author

odraghi commented Mar 9, 2023

@Didainius you're welcome, let me know if you need me to change something

@Didainius
Copy link
Collaborator

I have tried to run tests in such combination:

Copy link
Collaborator

@lvirbalas lvirbalas left a comment

Choose a reason for hiding this comment

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

Thank you for contributing!

@Didainius Didainius merged commit 25a0629 into vmware:main Mar 10, 2023
@Didainius
Copy link
Collaborator

Thanks @odraghi , this is in main now

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.

5 participants