Skip to content

Summary: Add 'network-config' support in ovf-env.xml through VMware g… #947

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.

Sign up for GitHub

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

Merged
TheRealFalcon merged 2 commits into from
Jul 20, 2021
Merged

Summary: Add 'network-config' support in ovf-env.xml through VMware g… #947

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7a9749f
Summary: Add 'network-config' support in ovf-env.xml through VMware g…
PengpengSun Jul 16, 2021
5e7599f
Merge branch 'main' into PengpengSun-OVF-read-network-config-from-vmw…
TheRealFalcon Jul 20, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions cloudinit/sources/DataSourceOVF.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,8 +358,11 @@ def _get_data(self):
if contents:
break
if contents:
(md, ud, cfg) = read_ovf_environment(contents)
read_network = ('com.vmware.guestinfo' == name)
Copy link
Copy Markdown
Contributor

@t-8ch t-8ch Sep 13, 2021

Choose a reason for hiding this comment

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

Should this not have been com.vmware.guestInfo (different capitalization) the same as in line 353 above?

Copy link
Copy Markdown
Contributor

@t-8ch t-8ch Sep 13, 2021

Choose a reason for hiding this comment

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

Also why is this not supposed to work with the ISO transport?

Copy link
Copy Markdown
Contributor Author

@PengpengSun PengpengSun Sep 13, 2021

Choose a reason for hiding this comment

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

@t-8ch Thanks for catching this. I should add more tests on this change. Will file a PR to fix it and add tests.

Copy link
Copy Markdown
Contributor Author

@PengpengSun PengpengSun Sep 13, 2021

Choose a reason for hiding this comment

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

@t-8ch For ISO transport, It can be supported, but user need create ISO for each instance with specified network configuration, I don't see it's a common user case when I opened this PR.
And com.vmware.guestInfo is VMware platform specified, I get this request from inside of VMware, the guestinfo.ovfEnv can be updated easily from outside of instance(vm).

Copy link
Copy Markdown
Contributor

@t-8ch t-8ch Sep 13, 2021

Choose a reason for hiding this comment

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

Would not VMware itself be able to generate the ISO on demand, at least when specifying the ISO transport in VMware?

Copy link
Copy Markdown
Contributor Author

@PengpengSun PengpengSun Sep 14, 2021

Choose a reason for hiding this comment

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

Yes, AFAIK, ISO can be generated on demand on VMware platform, till now, network config is never included in such ISO. The ISO transport is not VMware specified, if there is indeed such requirement, I can make the change and let cloud-init team review it.

Copy link
Copy Markdown
Contributor

@t-8ch t-8ch Oct 6, 2021

Choose a reason for hiding this comment

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

@PengpengSun Thanks for your patch!

Personally I would like to see the network-config also be supported via the ISO.
It should not necessarily change more oftern than the user-data .

Also the ISO transport is defined directly by the upstream DMTF standard.
(See https://www.dmtf.org/sites/default/files/standards/documents/DSP0243_2.1.1.pdf , 11.2 Transport media type)

If you prefer I can also send a patch.

Copy link
Copy Markdown
Contributor Author

@PengpengSun PengpengSun Oct 6, 2021

Choose a reason for hiding this comment

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

@t-8ch Please fell free to send the patch, I was hesitating to add ISO transport because it's not specified to VMware platform. Let's see if cloud-init team is ok with it too.

(md, ud, cfg) = read_ovf_environment(contents, read_network)
self.environment = contents
if 'network-config' in md and md['network-config']:
self._network_config = md['network-config']
found.append(name)

# There was no OVF transports found
Expand Down Expand Up @@ -507,20 +510,27 @@ def read_vmware_imc(config):

# This will return a dict with some content
# meta-data, user-data, some config
def read_ovf_environment(contents):
def read_ovf_environment(contents, read_network=False):
props = get_properties(contents)
md = {}
cfg = {}
ud = None
cfg_props = ['password']
md_props = ['seedfrom', 'local-hostname', 'public-keys', 'instance-id']
network_props = ['network-config']
for (prop, val) in props.items():
if prop == 'hostname':
prop = "local-hostname"
if prop in md_props:
md[prop] = val
elif prop in cfg_props:
cfg[prop] = val
elif prop in network_props and read_network:
try:
network_config = base64.b64decode(val.encode())
md[prop] = safeload_yaml_or_dict(network_config).get('network')
except Exception:
LOG.debug("Ignore network-config in wrong format")
elif prop == "user-data":
try:
ud = base64.b64decode(val.encode())
Expand Down
8 changes: 8 additions & 0 deletions doc/sources/ovf/example/ovf-env.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,14 @@
-->
<Property oe:key="user-data" oe:value="IyEvYmluL3NoCmVjaG8gImhpIHdvcmxkIgo="/>
<Property oe:key="password" oe:value="passw0rd"/>
<!--
network-config is optional, it can only be read from VMware guestinfo.ovfEnv
The value for network-config is to be base64 encoded.
It will be decoded, and then processed normally as network-config.
Set ovf-env.xml to VMware guestinfo.ovfEnv by below command:
'vmware-rpctool "info-set guestinfo.ovfEnv `cat ./ovf-env.xml`"'
-->
<Property oe:key="network-config" oe:value="bmV0d29yazoKICB2ZXJzaW9uOiAyCiAgZXRoZXJuZXRzOgogICAgbmljczoKICAgICAgbWF0Y2g6CiAgICAgICAgbmFtZTogZXRoKgogICAgICBkaGNwNDogeWVz"/>
</PropertySection>

</Environment>
97 changes: 97 additions & 0 deletions tests/unittests/test_datasource/test_ovf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,103 @@ def test_with_no_userdata(self):
self.assertEqual({'password': "passw0rd"}, cfg)
self.assertIsNone(ud)

def test_with_b64_network_config_enable_read_network(self):
network_config = dedent("""\
network:
version: 2
ethernets:
nics:
nameservers:
addresses:
- 127.0.0.53
search:
- eng.vmware.com
- vmware.com
match:
name: eth*
gateway4: 10.10.10.253
dhcp4: false
addresses:
- 10.10.10.1/24
""")
network_config_b64 = base64.b64encode(network_config.encode()).decode()
props = {"network-config": network_config_b64,
"password": "passw0rd",
"instance-id": "inst-001"}
env = fill_properties(props)
md, ud, cfg = dsovf.read_ovf_environment(env, True)
self.assertEqual("inst-001", md["instance-id"])
self.assertEqual({'password': "passw0rd"}, cfg)
self.assertEqual(
{'version': 2, 'ethernets':
{'nics':
{'nameservers':
{'addresses': ['127.0.0.53'],
'search': ['eng.vmware.com', 'vmware.com']},
'match': {'name': 'eth*'},
'gateway4': '10.10.10.253',
'dhcp4': False,
'addresses': ['10.10.10.1/24']}}},
md["network-config"])
self.assertIsNone(ud)

def test_with_non_b64_network_config_enable_read_network(self):
network_config = dedent("""\
network:
version: 2
ethernets:
nics:
nameservers:
addresses:
- 127.0.0.53
search:
- eng.vmware.com
- vmware.com
match:
name: eth*
gateway4: 10.10.10.253
dhcp4: false
addresses:
- 10.10.10.1/24
""")
props = {"network-config": network_config,
"password": "passw0rd",
"instance-id": "inst-001"}
env = fill_properties(props)
md, ud, cfg = dsovf.read_ovf_environment(env, True)
self.assertEqual({"instance-id": "inst-001"}, md)
self.assertEqual({'password': "passw0rd"}, cfg)
self.assertIsNone(ud)

def test_with_b64_network_config_disable_read_network(self):
network_config = dedent("""\
network:
version: 2
ethernets:
nics:
nameservers:
addresses:
- 127.0.0.53
search:
- eng.vmware.com
- vmware.com
match:
name: eth*
gateway4: 10.10.10.253
dhcp4: false
addresses:
- 10.10.10.1/24
""")
network_config_b64 = base64.b64encode(network_config.encode()).decode()
props = {"network-config": network_config_b64,
"password": "passw0rd",
"instance-id": "inst-001"}
env = fill_properties(props)
md, ud, cfg = dsovf.read_ovf_environment(env)
self.assertEqual({"instance-id": "inst-001"}, md)
self.assertEqual({'password': "passw0rd"}, cfg)
self.assertIsNone(ud)


class TestMarkerFiles(CiTestCase):

Expand Down