Merge ~xiaofengw/cloud-init:xiaofengw-add-custom-script-option into cloud-init:master

Proposed by Xiaofeng Wang
Status: Merged
Approved by: Ryan Harper
Approved revision: 68f7158757001d0ecfb676109b72342c818ec59f
Merge reported by: Server Team CI bot
Merged at revision: not available
Proposed branch: ~xiaofengw/cloud-init:xiaofengw-add-custom-script-option
Merge into: cloud-init:master
Diff against target: 248 lines (+169/-10)
5 files modified
cloudinit/sources/DataSourceOVF.py (+20/-1)
cloudinit/sources/helpers/vmware/imc/guestcust_error.py (+1/-0)
cloudinit/sources/helpers/vmware/imc/guestcust_util.py (+37/-0)
tests/unittests/test_datasource/test_ovf.py (+46/-9)
tests/unittests/test_vmware/test_guestcust_util.py (+65/-0)
Reviewer Review Type Date Requested Status
Ryan Harper Approve
Server Team CI bot continuous-integration Approve
Review via email: mp+372469@code.launchpad.net

Commit message

VMWware: add option into VMTools config to enable/disable custom script.

VMWware customization already has support to run a custom script during
the VM customization. Adding this option allows a VM administrator to
disable the execution of customization scripts. If set the script
will not execute and the customization status is set to
GUESTCUST_ERROR_SCRIPT_DISABLED.

To post a comment you must log in.
c516af5... by Xiaofeng Wang

VMWware: add option into VMTools configuration to enable/disable custom script.

VMWware customization has already support to run a custom script during the VM
customization. We add this option so that VM administrator could prevent custom
 script to run, then the customization will fail and customization status is
set to GUESTCUST_ERROR_SCRIPT_DISABLED.

Revision history for this message
Ryan Harper (raharper) wrote :

Thanks for resubmitting. This looks much better. A couple of inline comments below.

review: Needs Fixing
68f7158... by Xiaofeng Wang

VMWware: add option into VMTools configuration to enable/disable custom script.

Fix the comments from Ryan.

Revision history for this message
Xiaofeng Wang (xiaofengw) wrote :

> Thanks for resubmitting. This looks much better. A couple of inline comments
> below.

Thanks, all comments are fixed, and also add a new test case to cover "key=Bar=Wark" case. Please review. Thanks.

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want jenkins to rebuild you need to trigger it yourself):
https://code.launchpad.net/~xiaofengw/cloud-init/+git/cloud-init/+merge/372469/+edit-commit-message
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1117/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1117//rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:68f7158757001d0ecfb676109b72342c818ec59f
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1119/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1119//rebuild

review: Approve (continuous-integration)
Revision history for this message
Ryan Harper (raharper) wrote :

I've updated the commit message. Everything looks good now.

Thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/cloudinit/sources/DataSourceOVF.py b/cloudinit/sources/DataSourceOVF.py
index dd941d2..b156189 100644
--- a/cloudinit/sources/DataSourceOVF.py
+++ b/cloudinit/sources/DataSourceOVF.py
@@ -40,11 +40,15 @@ from cloudinit.sources.helpers.vmware.imc.guestcust_state \
40from cloudinit.sources.helpers.vmware.imc.guestcust_util import (40from cloudinit.sources.helpers.vmware.imc.guestcust_util import (
41 enable_nics,41 enable_nics,
42 get_nics_to_enable,42 get_nics_to_enable,
43 set_customization_status43 set_customization_status,
44 get_tools_config
44)45)
4546
46LOG = logging.getLogger(__name__)47LOG = logging.getLogger(__name__)
4748
49CONFGROUPNAME_GUESTCUSTOMIZATION = "deployPkg"
50GUESTCUSTOMIZATION_ENABLE_CUST_SCRIPTS = "enable-custom-scripts"
51
4852
49class DataSourceOVF(sources.DataSource):53class DataSourceOVF(sources.DataSource):
5054
@@ -148,6 +152,21 @@ class DataSourceOVF(sources.DataSource):
148 product_marker, os.path.join(self.paths.cloud_dir, 'data'))152 product_marker, os.path.join(self.paths.cloud_dir, 'data'))
149 special_customization = product_marker and not hasmarkerfile153 special_customization = product_marker and not hasmarkerfile
150 customscript = self._vmware_cust_conf.custom_script_name154 customscript = self._vmware_cust_conf.custom_script_name
155 custScriptConfig = get_tools_config(
156 CONFGROUPNAME_GUESTCUSTOMIZATION,
157 GUESTCUSTOMIZATION_ENABLE_CUST_SCRIPTS,
158 "true")
159 if custScriptConfig.lower() == "false":
160 # Update the customization status if there is a
161 # custom script is disabled
162 if special_customization and customscript:
163 msg = "Custom script is disabled by VM Administrator"
164 LOG.debug(msg)
165 set_customization_status(
166 GuestCustStateEnum.GUESTCUST_STATE_RUNNING,
167 GuestCustErrorEnum.GUESTCUST_ERROR_SCRIPT_DISABLED)
168 raise RuntimeError(msg)
169
151 ccScriptsDir = os.path.join(170 ccScriptsDir = os.path.join(
152 self.paths.get_cpath("scripts"),171 self.paths.get_cpath("scripts"),
153 "per-instance")172 "per-instance")
diff --git a/cloudinit/sources/helpers/vmware/imc/guestcust_error.py b/cloudinit/sources/helpers/vmware/imc/guestcust_error.py
index db5a00d..65ae739 100644
--- a/cloudinit/sources/helpers/vmware/imc/guestcust_error.py
+++ b/cloudinit/sources/helpers/vmware/imc/guestcust_error.py
@@ -10,5 +10,6 @@ class GuestCustErrorEnum(object):
10 """Specifies different errors of Guest Customization engine"""10 """Specifies different errors of Guest Customization engine"""
1111
12 GUESTCUST_ERROR_SUCCESS = 012 GUESTCUST_ERROR_SUCCESS = 0
13 GUESTCUST_ERROR_SCRIPT_DISABLED = 6
1314
14# vi: ts=4 expandtab15# vi: ts=4 expandtab
diff --git a/cloudinit/sources/helpers/vmware/imc/guestcust_util.py b/cloudinit/sources/helpers/vmware/imc/guestcust_util.py
index a590f32..eb78172 100644
--- a/cloudinit/sources/helpers/vmware/imc/guestcust_util.py
+++ b/cloudinit/sources/helpers/vmware/imc/guestcust_util.py
@@ -7,6 +7,7 @@
77
8import logging8import logging
9import os9import os
10import re
10import time11import time
1112
12from cloudinit import util13from cloudinit import util
@@ -117,4 +118,40 @@ def enable_nics(nics):
117 logger.warning("Can't connect network interfaces after %d attempts",118 logger.warning("Can't connect network interfaces after %d attempts",
118 enableNicsWaitRetries)119 enableNicsWaitRetries)
119120
121
122def get_tools_config(section, key, defaultVal):
123 """ Return the value of [section] key from VMTools configuration.
124
125 @param section: String of section to read from VMTools config
126 @returns: String value from key in [section] or defaultVal if
127 [section] is not present or vmware-toolbox-cmd is
128 not installed.
129 """
130
131 if not util.which('vmware-toolbox-cmd'):
132 logger.debug(
133 'vmware-toolbox-cmd not installed, returning default value')
134 return defaultVal
135
136 retValue = defaultVal
137 cmd = ['vmware-toolbox-cmd', 'config', 'get', section, key]
138
139 try:
140 (outText, _) = util.subp(cmd)
141 m = re.match(r'([a-zA-Z0-9 ]+)=(.*)', outText)
142 if m:
143 retValue = m.group(2).strip()
144 logger.debug("Get tools config: [%s] %s = %s",
145 section, key, retValue)
146 else:
147 logger.debug(
148 "Tools config: [%s] %s is not found, return default value: %s",
149 section, key, retValue)
150 except util.ProcessExecutionError as e:
151 logger.error("Failed running %s[%s]", cmd, e.exit_code)
152 logger.exception(e)
153
154 return retValue
155
156
120# vi: ts=4 expandtab157# vi: ts=4 expandtab
diff --git a/tests/unittests/test_datasource/test_ovf.py b/tests/unittests/test_datasource/test_ovf.py
index 349d54c..a615470 100644
--- a/tests/unittests/test_datasource/test_ovf.py
+++ b/tests/unittests/test_datasource/test_ovf.py
@@ -169,19 +169,56 @@ class TestDatasourceOVF(CiTestCase):
169 MARKER-ID = 12345345169 MARKER-ID = 12345345
170 """)170 """)
171 util.write_file(conf_file, conf_content)171 util.write_file(conf_file, conf_content)
172 with self.assertRaises(CustomScriptNotFound) as context:172 with mock.patch(MPATH + 'get_tools_config', return_value='true'):
173 wrap_and_call(173 with self.assertRaises(CustomScriptNotFound) as context:
174 'cloudinit.sources.DataSourceOVF',174 wrap_and_call(
175 {'util.read_dmi_data': 'vmware',175 'cloudinit.sources.DataSourceOVF',
176 'util.del_dir': True,176 {'util.read_dmi_data': 'vmware',
177 'search_file': self.tdir,177 'util.del_dir': True,
178 'wait_for_imc_cfg_file': conf_file,178 'search_file': self.tdir,
179 'get_nics_to_enable': ''},179 'wait_for_imc_cfg_file': conf_file,
180 ds.get_data)180 'get_nics_to_enable': ''},
181 ds.get_data)
181 customscript = self.tmp_path('test-script', self.tdir)182 customscript = self.tmp_path('test-script', self.tdir)
182 self.assertIn('Script %s not found!!' % customscript,183 self.assertIn('Script %s not found!!' % customscript,
183 str(context.exception))184 str(context.exception))
184185
186 def test_get_data_cust_script_disabled(self):
187 """If custom script is disabled by VMware tools configuration,
188 raise a RuntimeError.
189 """
190 paths = Paths({'cloud_dir': self.tdir})
191 ds = self.datasource(
192 sys_cfg={'disable_vmware_customization': False}, distro={},
193 paths=paths)
194 # Prepare the conf file
195 conf_file = self.tmp_path('test-cust', self.tdir)
196 conf_content = dedent("""\
197 [CUSTOM-SCRIPT]
198 SCRIPT-NAME = test-script
199 [MISC]
200 MARKER-ID = 12345346
201 """)
202 util.write_file(conf_file, conf_content)
203 # Prepare the custom sript
204 customscript = self.tmp_path('test-script', self.tdir)
205 util.write_file(customscript, "This is the post cust script")
206
207 with mock.patch(MPATH + 'get_tools_config', return_value='false'):
208 with mock.patch(MPATH + 'set_customization_status',
209 return_value=('msg', b'')):
210 with self.assertRaises(RuntimeError) as context:
211 wrap_and_call(
212 'cloudinit.sources.DataSourceOVF',
213 {'util.read_dmi_data': 'vmware',
214 'util.del_dir': True,
215 'search_file': self.tdir,
216 'wait_for_imc_cfg_file': conf_file,
217 'get_nics_to_enable': ''},
218 ds.get_data)
219 self.assertIn('Custom script is disabled by VM Administrator',
220 str(context.exception))
221
185 def test_get_data_non_vmware_seed_platform_info(self):222 def test_get_data_non_vmware_seed_platform_info(self):
186 """Platform info properly reports when on non-vmware platforms."""223 """Platform info properly reports when on non-vmware platforms."""
187 paths = Paths({'cloud_dir': self.tdir, 'run_dir': self.tdir})224 paths = Paths({'cloud_dir': self.tdir, 'run_dir': self.tdir})
diff --git a/tests/unittests/test_vmware/test_guestcust_util.py b/tests/unittests/test_vmware/test_guestcust_util.py
188new file mode 100644225new file mode 100644
index 0000000..b8fa994
--- /dev/null
+++ b/tests/unittests/test_vmware/test_guestcust_util.py
@@ -0,0 +1,65 @@
1# Copyright (C) 2019 Canonical Ltd.
2# Copyright (C) 2019 VMware INC.
3#
4# Author: Xiaofeng Wang <xiaofengw@vmware.com>
5#
6# This file is part of cloud-init. See LICENSE file for license information.
7
8from cloudinit import util
9from cloudinit.sources.helpers.vmware.imc.guestcust_util import (
10 get_tools_config,
11)
12from cloudinit.tests.helpers import CiTestCase, mock
13
14
15class TestGuestCustUtil(CiTestCase):
16 def test_get_tools_config_not_installed(self):
17 """
18 This test is designed to verify the behavior if vmware-toolbox-cmd
19 is not installed.
20 """
21 with mock.patch.object(util, 'which', return_value=None):
22 self.assertEqual(
23 get_tools_config('section', 'key', 'defaultVal'), 'defaultVal')
24
25 def test_get_tools_config_internal_exception(self):
26 """
27 This test is designed to verify the behavior if internal exception
28 is raised.
29 """
30 with mock.patch.object(util, 'which', return_value='/dummy/path'):
31 with mock.patch.object(util, 'subp',
32 return_value=('key=value', b''),
33 side_effect=util.ProcessExecutionError(
34 "subp failed", exit_code=99)):
35 # verify return value is 'defaultVal', not 'value'.
36 self.assertEqual(
37 get_tools_config('section', 'key', 'defaultVal'),
38 'defaultVal')
39
40 def test_get_tools_config_normal(self):
41 """
42 This test is designed to verify the value could be parsed from
43 key = value of the given [section]
44 """
45 with mock.patch.object(util, 'which', return_value='/dummy/path'):
46 # value is not blank
47 with mock.patch.object(util, 'subp',
48 return_value=('key = value ', b'')):
49 self.assertEqual(
50 get_tools_config('section', 'key', 'defaultVal'),
51 'value')
52 # value is blank
53 with mock.patch.object(util, 'subp',
54 return_value=('key = ', b'')):
55 self.assertEqual(
56 get_tools_config('section', 'key', 'defaultVal'),
57 '')
58 # value contains =
59 with mock.patch.object(util, 'subp',
60 return_value=('key=Bar=Wark', b'')):
61 self.assertEqual(
62 get_tools_config('section', 'key', 'defaultVal'),
63 'Bar=Wark')
64
65# vi: ts=4 expandtab

Subscribers

People subscribed via source and target branches