search

cloudbase / cloudbase-init

Cross-platform instance initialization
http://openstack.org
Apache License 2.0
421 stars 149 forks source link

add the vmware guestinfo service #20

Closed rgl closed 4 years ago

rgl commented 4 years ago

This is the initial draft for getting the userdata from the vmware guestinfo interface from the same guestinfo properties as https://github.com/vmware/cloud-init-vmware-guestinfo#configuration.

What do you think of including this in this repo? Is in an acceptable state?

rgl commented 4 years ago

I've added more details about the dependencies inside the VMwareGuestInfoService class documentation. Is that OK or should that also be included in the commit message?

bhoriuchi commented 4 years ago

@rgl This is great! One recommendation though. There is currently an official vmware cloud-init datasource https://github.com/vmware/cloud-init-vmware-guestinfo#configuration that reads guestinfo properties for how the meta and userdata are encoded. In quick overview of the example comment on your commit it looks like the input is expected to be base64+gzip but does not mention this in the info https://github.com/cloudbase/cloudbase-init/pull/20/files#diff-79a52948a1c2159908ab0bec485abdeaR39-R40

rgl commented 4 years ago

this is now ready, can you please review it?

rgl commented 4 years ago

@bhoriuchi I've updated the docstring to mention the gzip+base64

rgl commented 4 years ago

I'm the only user. Maybe others would start to use it if they known about it in the cloudbase-init documentation.

I will understand if you are not comfortable to include this upstream, I can always maintain a fork of it or have a way to easily include this over the upstream version

Will do the requested changes when possible. Thanks for the review!

bhoriuchi commented 4 years ago

I would also like to use this feature.

ader1990 commented 4 years ago

I think that for testing purposes we can use a precompiled binary from this source: https://github.com/vmware/open-vm-tools/blob/master/open-vm-tools/rpctool/rpctool.c

I need to investigate how much effort would be to actually translate the used features in native python code too.

codyja commented 4 years ago

Sorry for another +1, but me and several others are extremely excited to see this and try it out!

ader1990 commented 4 years ago

@bhoriuchi @codyja great to hear that :)

@rgl to include this feature, we need unit tests too :) There are some other code parts which can be optimized and use util methods.

rhockenbury commented 4 years ago

Any updates on this? @rgl

rgl commented 4 years ago

Not yet. Will look into it soonish.

ader1990 commented 4 years ago

@rgl @rhockenbury I will rewrite the implementation and post it on launchpad soon. Will notify when it's in a working state.

ader1990 commented 4 years ago

Here is a working implementation based, tested using a dummy rpctool.exe that returns basic info: https://review.opendev.org/#/c/701737/

Here is a functional test: https://github.com/ader1990/cloudbase-init-1/commit/4637946733da504e589f154f5deef798af8bc764/checks?check_suite_id=392549838 The functional test is based on this configuration and rpctool.exe dummy (built from python): https://github.com/ader1990/cloudbase-init-test-resources-1/tree/vmware_guest_info/vmwareguest

I need to add the unit tests and documentation, let me know if you have any comments.

rhockenbury commented 4 years ago

@ader1990 Is there a build available with the vmware service? Preferably with the runcmd and jinja templating work.

ader1990 commented 4 years ago

@rhockenbury you can use the github actions to build the cloudbase-init msi for you. You need to cherry-pick the desired cloudbase-init patches and push them on a branch on GitHub.

Then fork this repo: https://github.com/ader1990/cloudbase-init-installer-1, update these two lines with the first step repo/branch and then push the installer commit on GitHub: https://github.com/ader1990/cloudbase-init-installer-1/blob/master/.github/workflows/build_cbsinit.yml#L12

You ll then go to the actions tab on your installer repo on Github, then wait and after a while, the action will finish and you can download the zipped msi from the artifacts tab. example: https://github.com/ader1990/cloudbase-init-installer-1/commit/ba4e59a194f46376999501f2edd47773bb16f743/checks?check_suite_id=353485048

rhockenbury commented 4 years ago

@ader1990 I saw that the VMwareGuestInfoService made it to master - Do you know what's the timeline for cutting a new release?

ader1990 commented 4 years ago

@rhockenbury @rgl @codyja @bhoriuchi The feature has been merged to master: https://github.com/cloudbase/cloudbase-init/commit/a77477e16e89979684ec31ffd2b0d479cf18b245

Please check the docs for more information: https://cloudbase-init.readthedocs.io/en/latest/services.html#vmware-guestinfo-service

ader1990 commented 4 years ago

@ader1990 I saw that the VMwareGuestInfoService made it to master - Do you know what's the timeline for cutting a new release?

A new release should come soon. It would be very helpful If you can test the VMware guest service in your environment, to make sure your requirements are met.

rhockenbury commented 4 years ago

Thanks. I have been using the vmware service without issue, but that was an older build I had put together. I'll test out what's currently on master.

If others need a build - https://github.com/rhockenbury/cloudbase-init-installer-1/suites/429131013/artifacts/1450680

rhockenbury commented 4 years ago

No issues on my end with the vmware guest service.

rgl commented 4 years ago

@rhockenbury you can use the github actions to build the cloudbase-init msi for you. You need to cherry-pick the desired cloudbase-init patches and push them on a branch on GitHub.

Then fork this repo: https://github.com/ader1990/cloudbase-init-installer-1, update these two lines with the first step repo/branch and then push the installer commit on GitHub: https://github.com/ader1990/cloudbase-init-installer-1/blob/master/.github/workflows/build_cbsinit.yml#L12

I was trying to build the msi but the build https://github.com/rgl/cloudbase-init-installer/runs/423797759#step:4:809 is failed with:

 "D:\a\cloudbase-init-installer\cloudbase-init-installer\CloudbaseInitSetup\CloudbaseInitSetup.wixproj" (default target) (4) ->
       (BeforeBuild target) -> 
         heat.exe : error HEAT5052: The directory 'D:\a\cloudbase-init-installer-1\cloudbase-init-installer-1\BuildAutomation\Python_CloudbaseInit' could not be found. [D:\a\cloudbase-init-installer\cloudbase-init-installer\CloudbaseInitSetup\CloudbaseInitSetup.wixproj]

This is because of the hardcoded path at CloudbaseInitSetup/CloudbaseInitSetup.wixproj.

I've fixed that to use a relative path at https://github.com/rgl/cloudbase-init-installer/commit/8d0fa1c2927c22ec74f1b67b9a5f90f01d05b25d tomorrow I will try the generated msi in VMware.

rgl commented 4 years ago

Finally had a chance to try it out, and it works nicely! :-)

Just have one suggestion. In https://github.com/cloudbase/cloudbase-init/blob/a77477e16e89979684ec31ffd2b0d479cf18b245/cloudbaseinit/metadata/services/vmwareguestinfoservice.py#L47-L58 can we just use the YAML parser (because its a super-set of JSON)?

ader1990 commented 4 years ago

Finally had a chance to try it out, and it works nicely! :-)

Just have one suggestion. In

https://github.com/cloudbase/cloudbase-init/blob/a77477e16e89979684ec31ffd2b0d479cf18b245/cloudbaseinit/metadata/services/vmwareguestinfoservice.py#L47-L58

can we just use the YAML parser (because its a super-set of JSON)?

Great to hear that, @rgl

For the YAML parser, there is the need to be compatible with vmware extension of cloud-init's implementation: https://github.com/vmware/cloud-init-vmware-guestinfo/blob/master/DataSourceVMwareGuestInfo.py#L401

Thank you, Adrian Vladu

rgl commented 4 years ago

I was trying to say that when we are using the yaml parser we are being compatible with json and yaml.

ader1990 commented 4 years ago

I was trying to say that when we are using the yaml parser we are being compatible with json and yaml.

In theory, yes, but in the real world, this does not happen. A valid JSON is not a valid YAML if you use spaces / tabs or newlines in the "wrong" YAML way. And I cannot make any assumptions about the actual implementations in Python.

But to give you a small example that a valid JSON is not a valid YAML:

{
"test": 
"test"
}

If all is in order, can you please close this PR?

rgl commented 4 years ago

I was trying to say that when we are using the yaml parser we are being compatible with json and yaml.

In theory, yes, but in the real world, this does not happen. A valid JSON is not a valid YAML if you use spaces / tabs or newlines in the "wrong" YAML way. And I cannot make any assumptions about the actual implementations in Python.

I see. The used Python version does not have a yaml 1.2 compatible parser?

But to give you a small example that a valid JSON is not a valid YAML:

{
"test": 
"test"
}

That is valid yaml 1.2 (a version from 2009).

If all is in order, can you please close this PR?

Will do!

Thank You!

PS: For easier reference this was closed at https://github.com/cloudbase/cloudbase-init/commit/a77477e16e89979684ec31ffd2b0d479cf18b245

codyja commented 4 years ago

Does anyone have an MSI handy or have anymore ideas when that new release will come? Thanks

rgl commented 4 years ago

@codyja the version that I've tried is at https://github.com/rgl/cloudbase-init-installer/actions/runs/34335168

ader1990 commented 4 years ago

Does anyone have an MSI handy or have anymore ideas when that new release will come? Thanks

The beta installers have the vmwareguestinfo support: https://www.cloudbase.it/downloads/CloudbaseInitSetup_x64.msi