add the vmware guestinfo service #20

rgl · 2019-10-16T20:24:44Z

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?

ader1990

Thanks for the contribution: a small nit to be taken care of -- the hardcoded rpctool path.

In the commit message, can you put more information on this service and what are the requirements to be installed on the image?

ader1990 · 2019-10-18T10:31:01Z

cloudbaseinit/metadata/services/vmwareguestinfoservice.py

+from cloudbaseinit.metadata.services import base
+from cloudbaseinit.osutils import factory as osutils_factory
+
+RPCTOOL_PATH = 'C:/Program Files/VMware/VMware Tools/rpctool.exe'


this path needs to be made configurable, you can set its default value to this path

rgl · 2019-10-19T05:23:22Z

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 · 2019-10-21T22:11:31Z

@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 · 2019-10-22T00:02:03Z

this is now ready, can you please review it?

rgl · 2019-10-22T08:36:11Z

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

rhockenbury · 2019-11-13T03:08:06Z

cloudbaseinit/metadata/services/vmwareguestinfoservice.py

+    |                    | scripts as described at doc/source/userdata.rst      |
+    +--------------------+------------------------------------------------------+
+
+    Each property value should be gzip compressed and must be base64 encoded.


Just to clarify - the should here is intentional, correct? In that https://github.com/cloudbase/cloudbase-init/pull/20/files#diff-79a52948a1c2159908ab0bec485abdeaR130-R132 suggest that gzipping is not required.

Yes, correct.

rhockenbury · 2019-11-13T03:12:45Z

cloudbaseinit/conf/vmwareguestinfo.py

+        self._options = [
+            cfg.StrOpt(
+                'rpctool_path',
+                default='C:/Program Files/VMware/VMware Tools/rpctool.exe',


Does this tool get installed when the vmware tools is installed on windows?

ader1990

@rgl I have updated my review with the main points being: do not add this service as a default, move docs to correct place and remove the username patch from this patch).

On the actual usability of this code, are you actively using this feature? Or do you know people using / require it? the main problem with this feature is testing / maintaining it, as it requires proprietary software (rpctool) and proprietary underlying cloud service.

ader1990 · 2019-11-25T14:37:47Z

cloudbaseinit/conf/default.py

                    'cloudbaseinit.metadata.services.httpservice.HttpService',
                    'cloudbaseinit.metadata.services'
+                    '.vmwareguestinfoservice.VMwareGuestInfoService',
+                    'cloudbaseinit.metadata.services'


this metadata service should not be put as a default service, as it will degrade the performance (cloudbase-init runs mostly on openstack).

ader1990 · 2019-11-25T14:38:26Z

cloudbaseinit/metadata/services/vmwareguestinfoservice.py

+
+class VMwareGuestInfoService(base.BaseMetadataService):
+    """
+    This uses the VMware Guest Info interface to obtain the cloud init data


All this information should go into the docs, in the services page.

ader1990 · 2019-11-25T14:39:05Z

cloudbaseinit/plugins/common/sshpublickeys.py

            LOG.debug('Public keys not found in metadata')
            return base.PLUGIN_EXECUTION_DONE, False

-        username = CONF.username


this change should be removed from this patch, I will add another patch to have this fix merged asap.

rgl · 2019-11-25T15:10:00Z

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 · 2019-11-25T15:33:33Z

I would also like to use this feature.

ader1990 · 2019-11-25T15:48:36Z

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 · 2019-11-25T15:51:25Z

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

ader1990 · 2019-11-25T15:59:56Z

@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 · 2020-01-09T04:06:32Z

Any updates on this? @rgl

rgl · 2020-01-09T06:40:04Z

Not yet. Will look into it soonish.

ader1990 · 2020-01-09T13:41:31Z

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

ader1990 · 2020-01-09T17:43:59Z

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 · 2020-01-10T16:03:26Z

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

ader1990 · 2020-01-10T16:10:15Z

@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 · 2020-01-30T00:52:52Z

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

ader1990 · 2020-01-30T11:46:48Z

@rhockenbury @rgl @codyja @bhoriuchi The feature has been merged to master:
a77477e

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

ader1990 · 2020-01-30T11:50:03Z

@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 · 2020-01-30T16:02:41Z

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 · 2020-02-03T15:55:08Z

No issues on my end with the vmware guest service.

rgl · 2020-02-03T18:58:05Z

@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 rgl/cloudbase-init-installer@8d0fa1c tomorrow I will try the generated msi in VMware.

rgl · 2020-02-07T22:41:01Z

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

Just have one suggestion. In

cloudbase-init/cloudbaseinit/metadata/services/vmwareguestinfoservice.py

Lines 47 to 58 in a77477e

    
           @staticmethod 
        
           def _parse_data(raw_data): 
        
               """Parse data as json. Fallback to yaml if json parsing fails""" 
        
               try: 
        
                   return json.loads(raw_data) 
        
               except (TypeError, ValueError, AttributeError): 
        
                   loader = getattr(yaml, 'CLoader', yaml.Loader) 
        
                   try: 
        
                       return yaml.load(raw_data, Loader=loader) 
        
                   except (TypeError, ValueError, AttributeError): 
        
                       raise YamlParserConfigError("Invalid yaml data provided.")

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

ader1990 · 2020-02-10T17:55:27Z

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

Just have one suggestion. In

cloudbase-init/cloudbaseinit/metadata/services/vmwareguestinfoservice.py

Lines 47 to 58 in a77477e

@staticmethod

def _parse_data(raw_data):

"""Parse data as json. Fallback to yaml if json parsing fails"""

try:

return json.loads(raw_data)

except (TypeError, ValueError, AttributeError):

loader = getattr(yaml, 'CLoader', yaml.Loader)

try:

return yaml.load(raw_data, Loader=loader)

except (TypeError, ValueError, AttributeError):

raise YamlParserConfigError("Invalid yaml data provided.")

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 · 2020-02-10T23:16:07Z

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

ader1990 · 2020-02-14T14:55:17Z

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 · 2020-02-14T17:26:03Z

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 a77477e

codyja · 2020-02-14T19:47:09Z

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

rgl · 2020-02-15T06:59:32Z

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

ader1990 · 2020-02-15T15:20:10Z

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

ader1990 requested changes Oct 18, 2019

View reviewed changes

rgl force-pushed the add-vmware-guestinfo-service branch from 18d4a60 to cd926ca Compare October 19, 2019 05:20

rgl force-pushed the add-vmware-guestinfo-service branch 2 times, most recently from 395053f to 6cbbf52 Compare October 21, 2019 22:03

rgl force-pushed the add-vmware-guestinfo-service branch 4 times, most recently from 5388c51 to e7b6f40 Compare October 21, 2019 23:58

rgl force-pushed the add-vmware-guestinfo-service branch from e7b6f40 to bceee0a Compare October 22, 2019 00:03

add the vmware guestinfo service

9155c75

rgl force-pushed the add-vmware-guestinfo-service branch from bceee0a to 9155c75 Compare October 22, 2019 08:34

rhockenbury reviewed Nov 13, 2019

View reviewed changes

ader1990 requested changes Nov 25, 2019

View reviewed changes

rhockenbury mentioned this pull request Jan 30, 2020

Windows Support kubernetes-sigs/cluster-api#2218

Closed

4 tasks

rgl closed this Feb 14, 2020

add the vmware guestinfo service #20

add the vmware guestinfo service #20

Uh oh!

Conversation

rgl commented Oct 16, 2019

Uh oh!

ader1990 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rgl commented Oct 19, 2019

Uh oh!

bhoriuchi commented Oct 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rgl commented Oct 22, 2019

Uh oh!

rgl commented Oct 22, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ader1990 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rgl commented Nov 25, 2019

Uh oh!

bhoriuchi commented Nov 25, 2019

Uh oh!

ader1990 commented Nov 25, 2019

Uh oh!

codyja commented Nov 25, 2019

Uh oh!

ader1990 commented Nov 25, 2019

Uh oh!

rhockenbury commented Jan 9, 2020

Uh oh!

rgl commented Jan 9, 2020

Uh oh!

ader1990 commented Jan 9, 2020

Uh oh!

ader1990 commented Jan 9, 2020

Uh oh!

rhockenbury commented Jan 10, 2020

Uh oh!

ader1990 commented Jan 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rhockenbury commented Jan 30, 2020

Uh oh!

ader1990 commented Jan 30, 2020

Uh oh!

ader1990 commented Jan 30, 2020

Uh oh!

rhockenbury commented Jan 30, 2020

Uh oh!

rhockenbury commented Feb 3, 2020

Uh oh!

rgl commented Feb 3, 2020

Uh oh!

rgl commented Feb 7, 2020

Uh oh!

ader1990 commented Feb 10, 2020

Uh oh!

rgl commented Feb 10, 2020

bhoriuchi commented Oct 21, 2019 •

edited

Loading

ader1990 left a comment •

edited

Loading

ader1990 commented Jan 10, 2020 •

edited

Loading

rgl commented Feb 14, 2020 •

edited

Loading