Merge ~oddbloke/cloud-init/+git/cloud-init:oci-vnic into cloud-init:master
- Git
- lp:~oddbloke/cloud-init/+git/cloud-init
- oci-vnic
- Merge into master
Status: | Merged |
---|---|
Approved by: | Dan Watkins |
Approved revision: | 63e2ed22336e8a8838cf10c586b07d751c811dc2 |
Merge reported by: | Server Team CI bot |
Merged at revision: | not available |
Proposed branch: | ~oddbloke/cloud-init/+git/cloud-init:oci-vnic |
Merge into: | cloud-init:master |
Diff against target: |
431 lines (+324/-4) 3 files modified
cloudinit/sources/DataSourceOracle.py (+88/-1) cloudinit/sources/tests/test_oracle.py (+212/-2) doc/rtd/topics/datasources/oracle.rst (+24/-1) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Server Team CI bot | continuous-integration | Approve | |
Chad Smith | Approve | ||
Dan Watkins | Abstain | ||
Review via email: mp+371053@code.launchpad.net |
Commit message
DataSourceOracle: configure secondary NICs on Virtual Machines
Oracle Cloud Infrastructure's Instance Metadata Service provides network
configuration information for non-primary NICs. This commit introduces
support, on Virtual Machines[0], for fetching that network metadata,
converting it to v1 network-config[1] and combining it into the network
configuration generated for the primary interface.
By default, this behaviour is not enabled. Configuring the Oracle
datasource to `configure_
datasource:
Oracle:
Failures to fetch and generate secondary NIC configuration will log a
warning, but otherwise will not affect boot.
[0] The expected use of the IMDS-provided network configuration is
substantially different on Bare Metal Machines, so support for that
will be addressed separately.
[1] This is v1 config, because cloudinit.
and we need to integrate the secondary NICs into that configuration.
Description of the change
Server Team CI bot (server-team-bot) wrote : | # |
PASSED: Continuous integration, rev:c5a8c2ff40c
https:/
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:/
Dan Watkins (oddbloke) wrote : | # |
Docs are now present, so switching my review to Abstain.
Server Team CI bot (server-team-bot) wrote : | # |
PASSED: Continuous integration, rev:bb32cd9fa45
https:/
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:/
Chad Smith (chad.smith) : | # |
Ryan Harper (raharper) wrote : | # |
Looks good. A few in-line questions.
Dan Watkins (oddbloke) wrote : | # |
Thanks for the reviews! Responses inline.
Dan Watkins (oddbloke) : | # |
- eb5dfc5... by Dan Watkins
-
DataSourceOracle: add explanatory comment about MTU 9000
And move it to a constant so the comment isn't clogging up other logic.
Dan Watkins (oddbloke) wrote : | # |
Made some changes locally, still thinking about one other. (Not pushing them up until all comments are addressed, so I don't have to go digging to find the current set of comments.)
Chad Smith (chad.smith) wrote : | # |
minor logging nit and a folowup question on _network_
- 8898a51... by Dan Watkins
-
DataSourceOracle: drop log level down to debug
Dan Watkins (oddbloke) : | # |
- f7209d5... by Dan Watkins
-
DataSourceOracle: don't return _and_ mutate network_config
Also raise log level to WARNING on Bare Metal Machines.
Server Team CI bot (server-team-bot) wrote : | # |
PASSED: Continuous integration, rev:7e0f6891b37
https:/
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:/
Ryan Harper (raharper) : | # |
Chad Smith (chad.smith) wrote : | # |
Again another volley of trivial comments. Otherwise looks good to me!
Chad Smith (chad.smith) : | # |
Dan Watkins (oddbloke) wrote : | # |
Really good feedback, Chad, thanks! Will address tomorrow.
- b950373... by Dan Watkins
-
DataSourceOracle: add explanatory comment per review
- 63e2ed2... by Dan Watkins
-
test_oracle: use a verbatim dump of OPC Bare Metal network data
Dan Watkins (oddbloke) wrote : | # |
Both comments requiring action addressed.
Server Team CI bot (server-team-bot) wrote : | # |
FAILED: Continuous integration, rev:246c359e480
https:/
Executed test runs:
SUCCESS: Checkout
FAILED: Unit & Style Tests
Click here to trigger a rebuild:
https:/
Chad Smith (chad.smith) wrote : | # |
+1 on this branch once CI is fixed for pycodestyle
cloudinit/
cloudinit/
Server Team CI bot (server-team-bot) wrote : | # |
PASSED: Continuous integration, rev:63e2ed22336
https:/
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:/
Preview Diff
1 | diff --git a/cloudinit/sources/DataSourceOracle.py b/cloudinit/sources/DataSourceOracle.py | |||
2 | index 76cfa38..086af79 100644 | |||
3 | --- a/cloudinit/sources/DataSourceOracle.py | |||
4 | +++ b/cloudinit/sources/DataSourceOracle.py | |||
5 | @@ -16,7 +16,7 @@ Notes: | |||
6 | 16 | """ | 16 | """ |
7 | 17 | 17 | ||
8 | 18 | from cloudinit.url_helper import combine_url, readurl, UrlError | 18 | from cloudinit.url_helper import combine_url, readurl, UrlError |
10 | 19 | from cloudinit.net import dhcp | 19 | from cloudinit.net import dhcp, get_interfaces_by_mac |
11 | 20 | from cloudinit import net | 20 | from cloudinit import net |
12 | 21 | from cloudinit import sources | 21 | from cloudinit import sources |
13 | 22 | from cloudinit import util | 22 | from cloudinit import util |
14 | @@ -28,8 +28,80 @@ import re | |||
15 | 28 | 28 | ||
16 | 29 | LOG = logging.getLogger(__name__) | 29 | LOG = logging.getLogger(__name__) |
17 | 30 | 30 | ||
18 | 31 | BUILTIN_DS_CONFIG = { | ||
19 | 32 | # Don't use IMDS to configure secondary NICs by default | ||
20 | 33 | 'configure_secondary_nics': False, | ||
21 | 34 | } | ||
22 | 31 | CHASSIS_ASSET_TAG = "OracleCloud.com" | 35 | CHASSIS_ASSET_TAG = "OracleCloud.com" |
23 | 32 | METADATA_ENDPOINT = "http://169.254.169.254/openstack/" | 36 | METADATA_ENDPOINT = "http://169.254.169.254/openstack/" |
24 | 37 | VNIC_METADATA_URL = 'http://169.254.169.254/opc/v1/vnics/' | ||
25 | 38 | # https://docs.cloud.oracle.com/iaas/Content/Network/Troubleshoot/connectionhang.htm#Overview, | ||
26 | 39 | # indicates that an MTU of 9000 is used within OCI | ||
27 | 40 | MTU = 9000 | ||
28 | 41 | |||
29 | 42 | |||
30 | 43 | def _add_network_config_from_opc_imds(network_config): | ||
31 | 44 | """ | ||
32 | 45 | Fetch data from Oracle's IMDS, generate secondary NIC config, merge it. | ||
33 | 46 | |||
34 | 47 | The primary NIC configuration should not be modified based on the IMDS | ||
35 | 48 | values, as it should continue to be configured for DHCP. As such, this | ||
36 | 49 | takes an existing network_config dict which is expected to have the primary | ||
37 | 50 | NIC configuration already present. It will mutate the given dict to | ||
38 | 51 | include the secondary VNICs. | ||
39 | 52 | |||
40 | 53 | :param network_config: | ||
41 | 54 | A v1 network config dict with the primary NIC already configured. This | ||
42 | 55 | dict will be mutated. | ||
43 | 56 | |||
44 | 57 | :raises: | ||
45 | 58 | Exceptions are not handled within this function. Likely exceptions are | ||
46 | 59 | those raised by url_helper.readurl (if communicating with the IMDS | ||
47 | 60 | fails), ValueError/JSONDecodeError (if the IMDS returns invalid JSON), | ||
48 | 61 | and KeyError/IndexError (if the IMDS returns valid JSON with unexpected | ||
49 | 62 | contents). | ||
50 | 63 | """ | ||
51 | 64 | resp = readurl(VNIC_METADATA_URL) | ||
52 | 65 | vnics = json.loads(str(resp)) | ||
53 | 66 | |||
54 | 67 | if 'nicIndex' in vnics[0]: | ||
55 | 68 | # TODO: Once configure_secondary_nics defaults to True, lower the level | ||
56 | 69 | # of this log message. (Currently, if we're running this code at all, | ||
57 | 70 | # someone has explicitly opted-in to secondary VNIC configuration, so | ||
58 | 71 | # we should warn them that it didn't happen. Once it's default, this | ||
59 | 72 | # would be emitted on every Bare Metal Machine launch, which means INFO | ||
60 | 73 | # or DEBUG would be more appropriate.) | ||
61 | 74 | LOG.warning( | ||
62 | 75 | 'VNIC metadata indicates this is a bare metal machine; skipping' | ||
63 | 76 | ' secondary VNIC configuration.' | ||
64 | 77 | ) | ||
65 | 78 | return | ||
66 | 79 | |||
67 | 80 | interfaces_by_mac = get_interfaces_by_mac() | ||
68 | 81 | |||
69 | 82 | for vnic_dict in vnics[1:]: | ||
70 | 83 | # We skip the first entry in the response because the primary interface | ||
71 | 84 | # is already configured by iSCSI boot; applying configuration from the | ||
72 | 85 | # IMDS is not required. | ||
73 | 86 | mac_address = vnic_dict['macAddr'].lower() | ||
74 | 87 | if mac_address not in interfaces_by_mac: | ||
75 | 88 | LOG.debug('Interface with MAC %s not found; skipping', mac_address) | ||
76 | 89 | continue | ||
77 | 90 | name = interfaces_by_mac[mac_address] | ||
78 | 91 | subnet = { | ||
79 | 92 | 'type': 'static', | ||
80 | 93 | 'address': vnic_dict['privateIp'], | ||
81 | 94 | 'netmask': vnic_dict['subnetCidrBlock'].split('/')[1], | ||
82 | 95 | 'gateway': vnic_dict['virtualRouterIp'], | ||
83 | 96 | 'control': 'manual', | ||
84 | 97 | } | ||
85 | 98 | network_config['config'].append({ | ||
86 | 99 | 'name': name, | ||
87 | 100 | 'type': 'physical', | ||
88 | 101 | 'mac_address': mac_address, | ||
89 | 102 | 'mtu': MTU, | ||
90 | 103 | 'subnets': [subnet], | ||
91 | 104 | }) | ||
92 | 33 | 105 | ||
93 | 34 | 106 | ||
94 | 35 | class DataSourceOracle(sources.DataSource): | 107 | class DataSourceOracle(sources.DataSource): |
95 | @@ -39,6 +111,13 @@ class DataSourceOracle(sources.DataSource): | |||
96 | 39 | vendordata_pure = None | 111 | vendordata_pure = None |
97 | 40 | _network_config = sources.UNSET | 112 | _network_config = sources.UNSET |
98 | 41 | 113 | ||
99 | 114 | def __init__(self, sys_cfg, *args, **kwargs): | ||
100 | 115 | super(DataSourceOracle, self).__init__(sys_cfg, *args, **kwargs) | ||
101 | 116 | |||
102 | 117 | self.ds_cfg = util.mergemanydict([ | ||
103 | 118 | util.get_cfg_by_path(sys_cfg, ['datasource', self.dsname], {}), | ||
104 | 119 | BUILTIN_DS_CONFIG]) | ||
105 | 120 | |||
106 | 42 | def _is_platform_viable(self): | 121 | def _is_platform_viable(self): |
107 | 43 | """Check platform environment to report if this datasource may run.""" | 122 | """Check platform environment to report if this datasource may run.""" |
108 | 44 | return _is_platform_viable() | 123 | return _is_platform_viable() |
109 | @@ -121,6 +200,14 @@ class DataSourceOracle(sources.DataSource): | |||
110 | 121 | self._network_config = cmdline.read_initramfs_config() | 200 | self._network_config = cmdline.read_initramfs_config() |
111 | 122 | if not self._network_config: | 201 | if not self._network_config: |
112 | 123 | self._network_config = self.distro.generate_fallback_config() | 202 | self._network_config = self.distro.generate_fallback_config() |
113 | 203 | if self.ds_cfg.get('configure_secondary_nics'): | ||
114 | 204 | try: | ||
115 | 205 | # Mutate self._network_config to include secondary VNICs | ||
116 | 206 | _add_network_config_from_opc_imds(self._network_config) | ||
117 | 207 | except Exception: | ||
118 | 208 | util.logexc( | ||
119 | 209 | LOG, | ||
120 | 210 | "Failed to fetch secondary network configuration!") | ||
121 | 124 | return self._network_config | 211 | return self._network_config |
122 | 125 | 212 | ||
123 | 126 | 213 | ||
124 | diff --git a/cloudinit/sources/tests/test_oracle.py b/cloudinit/sources/tests/test_oracle.py | |||
125 | index 282382c..3e14677 100644 | |||
126 | --- a/cloudinit/sources/tests/test_oracle.py | |||
127 | +++ b/cloudinit/sources/tests/test_oracle.py | |||
128 | @@ -18,10 +18,52 @@ import uuid | |||
129 | 18 | DS_PATH = "cloudinit.sources.DataSourceOracle" | 18 | DS_PATH = "cloudinit.sources.DataSourceOracle" |
130 | 19 | MD_VER = "2013-10-17" | 19 | MD_VER = "2013-10-17" |
131 | 20 | 20 | ||
132 | 21 | # `curl -L http://169.254.169.254/opc/v1/vnics/` on a Oracle Bare Metal Machine | ||
133 | 22 | # with a secondary VNIC attached (vnicId truncated for Python line length) | ||
134 | 23 | OPC_BM_SECONDARY_VNIC_RESPONSE = """\ | ||
135 | 24 | [ { | ||
136 | 25 | "vnicId" : "ocid1.vnic.oc1.phx.abyhqljtyvcucqkhdqmgjszebxe4hrb!!TRUNCATED||", | ||
137 | 26 | "privateIp" : "10.0.0.8", | ||
138 | 27 | "vlanTag" : 0, | ||
139 | 28 | "macAddr" : "90:e2:ba:d4:f1:68", | ||
140 | 29 | "virtualRouterIp" : "10.0.0.1", | ||
141 | 30 | "subnetCidrBlock" : "10.0.0.0/24", | ||
142 | 31 | "nicIndex" : 0 | ||
143 | 32 | }, { | ||
144 | 33 | "vnicId" : "ocid1.vnic.oc1.phx.abyhqljtfmkxjdy2sqidndiwrsg63zf!!TRUNCATED||", | ||
145 | 34 | "privateIp" : "10.0.4.5", | ||
146 | 35 | "vlanTag" : 1, | ||
147 | 36 | "macAddr" : "02:00:17:05:CF:51", | ||
148 | 37 | "virtualRouterIp" : "10.0.4.1", | ||
149 | 38 | "subnetCidrBlock" : "10.0.4.0/24", | ||
150 | 39 | "nicIndex" : 0 | ||
151 | 40 | } ]""" | ||
152 | 41 | |||
153 | 42 | # `curl -L http://169.254.169.254/opc/v1/vnics/` on a Oracle Virtual Machine | ||
154 | 43 | # with a secondary VNIC attached | ||
155 | 44 | OPC_VM_SECONDARY_VNIC_RESPONSE = """\ | ||
156 | 45 | [ { | ||
157 | 46 | "vnicId" : "ocid1.vnic.oc1.phx.abyhqljtch72z5pd76cc2636qeqh7z_truncated", | ||
158 | 47 | "privateIp" : "10.0.0.230", | ||
159 | 48 | "vlanTag" : 1039, | ||
160 | 49 | "macAddr" : "02:00:17:05:D1:DB", | ||
161 | 50 | "virtualRouterIp" : "10.0.0.1", | ||
162 | 51 | "subnetCidrBlock" : "10.0.0.0/24" | ||
163 | 52 | }, { | ||
164 | 53 | "vnicId" : "ocid1.vnic.oc1.phx.abyhqljt4iew3gwmvrwrhhf3bp5drj_truncated", | ||
165 | 54 | "privateIp" : "10.0.0.231", | ||
166 | 55 | "vlanTag" : 1041, | ||
167 | 56 | "macAddr" : "00:00:17:02:2B:B1", | ||
168 | 57 | "virtualRouterIp" : "10.0.0.1", | ||
169 | 58 | "subnetCidrBlock" : "10.0.0.0/24" | ||
170 | 59 | } ]""" | ||
171 | 60 | |||
172 | 21 | 61 | ||
173 | 22 | class TestDataSourceOracle(test_helpers.CiTestCase): | 62 | class TestDataSourceOracle(test_helpers.CiTestCase): |
174 | 23 | """Test datasource DataSourceOracle.""" | 63 | """Test datasource DataSourceOracle.""" |
175 | 24 | 64 | ||
176 | 65 | with_logs = True | ||
177 | 66 | |||
178 | 25 | ds_class = oracle.DataSourceOracle | 67 | ds_class = oracle.DataSourceOracle |
179 | 26 | 68 | ||
180 | 27 | my_uuid = str(uuid.uuid4()) | 69 | my_uuid = str(uuid.uuid4()) |
181 | @@ -79,6 +121,16 @@ class TestDataSourceOracle(test_helpers.CiTestCase): | |||
182 | 79 | self.assertEqual( | 121 | self.assertEqual( |
183 | 80 | 'metadata (http://169.254.169.254/openstack/)', ds.subplatform) | 122 | 'metadata (http://169.254.169.254/openstack/)', ds.subplatform) |
184 | 81 | 123 | ||
185 | 124 | def test_sys_cfg_can_enable_configure_secondary_nics(self): | ||
186 | 125 | # Confirm that behaviour is toggled by sys_cfg | ||
187 | 126 | ds, _mocks = self._get_ds() | ||
188 | 127 | self.assertFalse(ds.ds_cfg['configure_secondary_nics']) | ||
189 | 128 | |||
190 | 129 | sys_cfg = { | ||
191 | 130 | 'datasource': {'Oracle': {'configure_secondary_nics': True}}} | ||
192 | 131 | ds, _mocks = self._get_ds(sys_cfg=sys_cfg) | ||
193 | 132 | self.assertTrue(ds.ds_cfg['configure_secondary_nics']) | ||
194 | 133 | |||
195 | 82 | @mock.patch(DS_PATH + "._is_iscsi_root", return_value=True) | 134 | @mock.patch(DS_PATH + "._is_iscsi_root", return_value=True) |
196 | 83 | def test_without_userdata(self, m_is_iscsi_root): | 135 | def test_without_userdata(self, m_is_iscsi_root): |
197 | 84 | """If no user-data is provided, it should not be in return dict.""" | 136 | """If no user-data is provided, it should not be in return dict.""" |
198 | @@ -133,9 +185,12 @@ class TestDataSourceOracle(test_helpers.CiTestCase): | |||
199 | 133 | self.assertEqual(self.my_md['uuid'], ds.get_instance_id()) | 185 | self.assertEqual(self.my_md['uuid'], ds.get_instance_id()) |
200 | 134 | self.assertEqual(my_userdata, ds.userdata_raw) | 186 | self.assertEqual(my_userdata, ds.userdata_raw) |
201 | 135 | 187 | ||
202 | 188 | @mock.patch(DS_PATH + "._add_network_config_from_opc_imds", | ||
203 | 189 | side_effect=lambda network_config: network_config) | ||
204 | 136 | @mock.patch(DS_PATH + ".cmdline.read_initramfs_config") | 190 | @mock.patch(DS_PATH + ".cmdline.read_initramfs_config") |
205 | 137 | @mock.patch(DS_PATH + "._is_iscsi_root", return_value=True) | 191 | @mock.patch(DS_PATH + "._is_iscsi_root", return_value=True) |
207 | 138 | def test_network_cmdline(self, m_is_iscsi_root, m_initramfs_config): | 192 | def test_network_cmdline(self, m_is_iscsi_root, m_initramfs_config, |
208 | 193 | _m_add_network_config_from_opc_imds): | ||
209 | 139 | """network_config should read kernel cmdline.""" | 194 | """network_config should read kernel cmdline.""" |
210 | 140 | distro = mock.MagicMock() | 195 | distro = mock.MagicMock() |
211 | 141 | ds, _ = self._get_ds(distro=distro, patches={ | 196 | ds, _ = self._get_ds(distro=distro, patches={ |
212 | @@ -151,9 +206,12 @@ class TestDataSourceOracle(test_helpers.CiTestCase): | |||
213 | 151 | self.assertEqual([mock.call()], m_initramfs_config.call_args_list) | 206 | self.assertEqual([mock.call()], m_initramfs_config.call_args_list) |
214 | 152 | self.assertFalse(distro.generate_fallback_config.called) | 207 | self.assertFalse(distro.generate_fallback_config.called) |
215 | 153 | 208 | ||
216 | 209 | @mock.patch(DS_PATH + "._add_network_config_from_opc_imds", | ||
217 | 210 | side_effect=lambda network_config: network_config) | ||
218 | 154 | @mock.patch(DS_PATH + ".cmdline.read_initramfs_config") | 211 | @mock.patch(DS_PATH + ".cmdline.read_initramfs_config") |
219 | 155 | @mock.patch(DS_PATH + "._is_iscsi_root", return_value=True) | 212 | @mock.patch(DS_PATH + "._is_iscsi_root", return_value=True) |
221 | 156 | def test_network_fallback(self, m_is_iscsi_root, m_initramfs_config): | 213 | def test_network_fallback(self, m_is_iscsi_root, m_initramfs_config, |
222 | 214 | _m_add_network_config_from_opc_imds): | ||
223 | 157 | """test that fallback network is generated if no kernel cmdline.""" | 215 | """test that fallback network is generated if no kernel cmdline.""" |
224 | 158 | distro = mock.MagicMock() | 216 | distro = mock.MagicMock() |
225 | 159 | ds, _ = self._get_ds(distro=distro, patches={ | 217 | ds, _ = self._get_ds(distro=distro, patches={ |
226 | @@ -175,6 +233,76 @@ class TestDataSourceOracle(test_helpers.CiTestCase): | |||
227 | 175 | self.assertEqual(ncfg, ds.network_config) | 233 | self.assertEqual(ncfg, ds.network_config) |
228 | 176 | self.assertEqual(1, m_initramfs_config.call_count) | 234 | self.assertEqual(1, m_initramfs_config.call_count) |
229 | 177 | 235 | ||
230 | 236 | @mock.patch(DS_PATH + "._add_network_config_from_opc_imds") | ||
231 | 237 | @mock.patch(DS_PATH + ".cmdline.read_initramfs_config", | ||
232 | 238 | return_value={'some': 'config'}) | ||
233 | 239 | @mock.patch(DS_PATH + "._is_iscsi_root", return_value=True) | ||
234 | 240 | def test_secondary_nics_added_to_network_config_if_enabled( | ||
235 | 241 | self, _m_is_iscsi_root, _m_initramfs_config, | ||
236 | 242 | m_add_network_config_from_opc_imds): | ||
237 | 243 | |||
238 | 244 | needle = object() | ||
239 | 245 | |||
240 | 246 | def network_config_side_effect(network_config): | ||
241 | 247 | network_config['secondary_added'] = needle | ||
242 | 248 | |||
243 | 249 | m_add_network_config_from_opc_imds.side_effect = ( | ||
244 | 250 | network_config_side_effect) | ||
245 | 251 | |||
246 | 252 | distro = mock.MagicMock() | ||
247 | 253 | ds, _ = self._get_ds(distro=distro, patches={ | ||
248 | 254 | '_is_platform_viable': {'return_value': True}, | ||
249 | 255 | 'crawl_metadata': { | ||
250 | 256 | 'return_value': { | ||
251 | 257 | MD_VER: {'system_uuid': self.my_uuid, | ||
252 | 258 | 'meta_data': self.my_md}}}}) | ||
253 | 259 | ds.ds_cfg['configure_secondary_nics'] = True | ||
254 | 260 | self.assertEqual(needle, ds.network_config['secondary_added']) | ||
255 | 261 | |||
256 | 262 | @mock.patch(DS_PATH + "._add_network_config_from_opc_imds") | ||
257 | 263 | @mock.patch(DS_PATH + ".cmdline.read_initramfs_config", | ||
258 | 264 | return_value={'some': 'config'}) | ||
259 | 265 | @mock.patch(DS_PATH + "._is_iscsi_root", return_value=True) | ||
260 | 266 | def test_secondary_nics_not_added_to_network_config_by_default( | ||
261 | 267 | self, _m_is_iscsi_root, _m_initramfs_config, | ||
262 | 268 | m_add_network_config_from_opc_imds): | ||
263 | 269 | |||
264 | 270 | def network_config_side_effect(network_config): | ||
265 | 271 | network_config['secondary_added'] = True | ||
266 | 272 | |||
267 | 273 | m_add_network_config_from_opc_imds.side_effect = ( | ||
268 | 274 | network_config_side_effect) | ||
269 | 275 | |||
270 | 276 | distro = mock.MagicMock() | ||
271 | 277 | ds, _ = self._get_ds(distro=distro, patches={ | ||
272 | 278 | '_is_platform_viable': {'return_value': True}, | ||
273 | 279 | 'crawl_metadata': { | ||
274 | 280 | 'return_value': { | ||
275 | 281 | MD_VER: {'system_uuid': self.my_uuid, | ||
276 | 282 | 'meta_data': self.my_md}}}}) | ||
277 | 283 | self.assertNotIn('secondary_added', ds.network_config) | ||
278 | 284 | |||
279 | 285 | @mock.patch(DS_PATH + "._add_network_config_from_opc_imds") | ||
280 | 286 | @mock.patch(DS_PATH + ".cmdline.read_initramfs_config") | ||
281 | 287 | @mock.patch(DS_PATH + "._is_iscsi_root", return_value=True) | ||
282 | 288 | def test_secondary_nic_failure_isnt_blocking( | ||
283 | 289 | self, _m_is_iscsi_root, m_initramfs_config, | ||
284 | 290 | m_add_network_config_from_opc_imds): | ||
285 | 291 | |||
286 | 292 | m_add_network_config_from_opc_imds.side_effect = Exception() | ||
287 | 293 | |||
288 | 294 | distro = mock.MagicMock() | ||
289 | 295 | ds, _ = self._get_ds(distro=distro, patches={ | ||
290 | 296 | '_is_platform_viable': {'return_value': True}, | ||
291 | 297 | 'crawl_metadata': { | ||
292 | 298 | 'return_value': { | ||
293 | 299 | MD_VER: {'system_uuid': self.my_uuid, | ||
294 | 300 | 'meta_data': self.my_md}}}}) | ||
295 | 301 | ds.ds_cfg['configure_secondary_nics'] = True | ||
296 | 302 | self.assertEqual(ds.network_config, m_initramfs_config.return_value) | ||
297 | 303 | self.assertIn('Failed to fetch secondary network configuration', | ||
298 | 304 | self.logs.getvalue()) | ||
299 | 305 | |||
300 | 178 | 306 | ||
301 | 179 | @mock.patch(DS_PATH + "._read_system_uuid", return_value=str(uuid.uuid4())) | 307 | @mock.patch(DS_PATH + "._read_system_uuid", return_value=str(uuid.uuid4())) |
302 | 180 | class TestReadMetaData(test_helpers.HttprettyTestCase): | 308 | class TestReadMetaData(test_helpers.HttprettyTestCase): |
303 | @@ -335,4 +463,86 @@ class TestLoadIndex(test_helpers.CiTestCase): | |||
304 | 335 | oracle._load_index("\n".join(["meta_data.json", "user_data"]))) | 463 | oracle._load_index("\n".join(["meta_data.json", "user_data"]))) |
305 | 336 | 464 | ||
306 | 337 | 465 | ||
307 | 466 | class TestNetworkConfigFromOpcImds(test_helpers.CiTestCase): | ||
308 | 467 | |||
309 | 468 | with_logs = True | ||
310 | 469 | |||
311 | 470 | def setUp(self): | ||
312 | 471 | super(TestNetworkConfigFromOpcImds, self).setUp() | ||
313 | 472 | self.add_patch(DS_PATH + '.readurl', 'm_readurl') | ||
314 | 473 | self.add_patch(DS_PATH + '.get_interfaces_by_mac', | ||
315 | 474 | 'm_get_interfaces_by_mac') | ||
316 | 475 | |||
317 | 476 | def test_failure_to_readurl(self): | ||
318 | 477 | # readurl failures should just bubble out to the caller | ||
319 | 478 | self.m_readurl.side_effect = Exception('oh no') | ||
320 | 479 | with self.assertRaises(Exception) as excinfo: | ||
321 | 480 | oracle._add_network_config_from_opc_imds({}) | ||
322 | 481 | self.assertEqual(str(excinfo.exception), 'oh no') | ||
323 | 482 | |||
324 | 483 | def test_empty_response(self): | ||
325 | 484 | # empty response error should just bubble out to the caller | ||
326 | 485 | self.m_readurl.return_value = '' | ||
327 | 486 | with self.assertRaises(Exception): | ||
328 | 487 | oracle._add_network_config_from_opc_imds([]) | ||
329 | 488 | |||
330 | 489 | def test_invalid_json(self): | ||
331 | 490 | # invalid JSON error should just bubble out to the caller | ||
332 | 491 | self.m_readurl.return_value = '{' | ||
333 | 492 | with self.assertRaises(Exception): | ||
334 | 493 | oracle._add_network_config_from_opc_imds([]) | ||
335 | 494 | |||
336 | 495 | def test_no_secondary_nics_does_not_mutate_input(self): | ||
337 | 496 | self.m_readurl.return_value = json.dumps([{}]) | ||
338 | 497 | # We test this by passing in a non-dict to ensure that no dict | ||
339 | 498 | # operations are used; failure would be seen as exceptions | ||
340 | 499 | oracle._add_network_config_from_opc_imds(object()) | ||
341 | 500 | |||
342 | 501 | def test_bare_metal_machine_skipped(self): | ||
343 | 502 | # nicIndex in the first entry indicates a bare metal machine | ||
344 | 503 | self.m_readurl.return_value = OPC_BM_SECONDARY_VNIC_RESPONSE | ||
345 | 504 | # We test this by passing in a non-dict to ensure that no dict | ||
346 | 505 | # operations are used | ||
347 | 506 | self.assertFalse(oracle._add_network_config_from_opc_imds(object())) | ||
348 | 507 | self.assertIn('bare metal machine', self.logs.getvalue()) | ||
349 | 508 | |||
350 | 509 | def test_missing_mac_skipped(self): | ||
351 | 510 | self.m_readurl.return_value = OPC_VM_SECONDARY_VNIC_RESPONSE | ||
352 | 511 | self.m_get_interfaces_by_mac.return_value = {} | ||
353 | 512 | |||
354 | 513 | network_config = {'version': 1, 'config': [{'primary': 'nic'}]} | ||
355 | 514 | oracle._add_network_config_from_opc_imds(network_config) | ||
356 | 515 | |||
357 | 516 | self.assertEqual(1, len(network_config['config'])) | ||
358 | 517 | self.assertIn( | ||
359 | 518 | 'Interface with MAC 00:00:17:02:2b:b1 not found; skipping', | ||
360 | 519 | self.logs.getvalue()) | ||
361 | 520 | |||
362 | 521 | def test_secondary_nic(self): | ||
363 | 522 | self.m_readurl.return_value = OPC_VM_SECONDARY_VNIC_RESPONSE | ||
364 | 523 | mac_addr, nic_name = '00:00:17:02:2b:b1', 'ens3' | ||
365 | 524 | self.m_get_interfaces_by_mac.return_value = { | ||
366 | 525 | mac_addr: nic_name, | ||
367 | 526 | } | ||
368 | 527 | |||
369 | 528 | network_config = {'version': 1, 'config': [{'primary': 'nic'}]} | ||
370 | 529 | oracle._add_network_config_from_opc_imds(network_config) | ||
371 | 530 | |||
372 | 531 | # The input is mutated | ||
373 | 532 | self.assertEqual(2, len(network_config['config'])) | ||
374 | 533 | |||
375 | 534 | secondary_nic_cfg = network_config['config'][1] | ||
376 | 535 | self.assertEqual(nic_name, secondary_nic_cfg['name']) | ||
377 | 536 | self.assertEqual('physical', secondary_nic_cfg['type']) | ||
378 | 537 | self.assertEqual(mac_addr, secondary_nic_cfg['mac_address']) | ||
379 | 538 | self.assertEqual(9000, secondary_nic_cfg['mtu']) | ||
380 | 539 | |||
381 | 540 | self.assertEqual(1, len(secondary_nic_cfg['subnets'])) | ||
382 | 541 | subnet_cfg = secondary_nic_cfg['subnets'][0] | ||
383 | 542 | # These values are hard-coded in OPC_VM_SECONDARY_VNIC_RESPONSE | ||
384 | 543 | self.assertEqual('10.0.0.231', subnet_cfg['address']) | ||
385 | 544 | self.assertEqual('24', subnet_cfg['netmask']) | ||
386 | 545 | self.assertEqual('10.0.0.1', subnet_cfg['gateway']) | ||
387 | 546 | self.assertEqual('manual', subnet_cfg['control']) | ||
388 | 547 | |||
389 | 338 | # vi: ts=4 expandtab | 548 | # vi: ts=4 expandtab |
390 | diff --git a/doc/rtd/topics/datasources/oracle.rst b/doc/rtd/topics/datasources/oracle.rst | |||
391 | index f2383ce..98c4657 100644 | |||
392 | --- a/doc/rtd/topics/datasources/oracle.rst | |||
393 | +++ b/doc/rtd/topics/datasources/oracle.rst | |||
394 | @@ -8,7 +8,7 @@ This datasource reads metadata, vendor-data and user-data from | |||
395 | 8 | 8 | ||
396 | 9 | Oracle Platform | 9 | Oracle Platform |
397 | 10 | --------------- | 10 | --------------- |
399 | 11 | OCI provides bare metal and virtual machines. In both cases, | 11 | OCI provides bare metal and virtual machines. In both cases, |
400 | 12 | the platform identifies itself via DMI data in the chassis asset tag | 12 | the platform identifies itself via DMI data in the chassis asset tag |
401 | 13 | with the string 'OracleCloud.com'. | 13 | with the string 'OracleCloud.com'. |
402 | 14 | 14 | ||
403 | @@ -22,5 +22,28 @@ Cloud-init has a specific datasource for Oracle in order to: | |||
404 | 22 | implementation. | 22 | implementation. |
405 | 23 | 23 | ||
406 | 24 | 24 | ||
407 | 25 | Configuration | ||
408 | 26 | ------------- | ||
409 | 27 | |||
410 | 28 | The following configuration can be set for the datasource in system | ||
411 | 29 | configuration (in ``/etc/cloud/cloud.cfg`` or ``/etc/cloud/cloud.cfg.d/``). | ||
412 | 30 | |||
413 | 31 | The settings that may be configured are: | ||
414 | 32 | |||
415 | 33 | * **configure_secondary_nics**: A boolean, defaulting to False. If set | ||
416 | 34 | to True on an OCI Virtual Machine, cloud-init will fetch networking | ||
417 | 35 | metadata from Oracle's IMDS and use it to configure the non-primary | ||
418 | 36 | network interface controllers in the system. If set to True on an | ||
419 | 37 | OCI Bare Metal Machine, it will have no effect (though this may | ||
420 | 38 | change in the future). | ||
421 | 39 | |||
422 | 40 | An example configuration with the default values is provided below: | ||
423 | 41 | |||
424 | 42 | .. sourcecode:: yaml | ||
425 | 43 | |||
426 | 44 | datasource: | ||
427 | 45 | Oracle: | ||
428 | 46 | configure_secondary_nics: false | ||
429 | 47 | |||
430 | 25 | .. _Oracle Compute Infrastructure: https://cloud.oracle.com/ | 48 | .. _Oracle Compute Infrastructure: https://cloud.oracle.com/ |
431 | 26 | .. vi: textwidth=78 | 49 | .. vi: textwidth=78 |
Marking myself as Needs Fixing because I need to write docs before this lands (but that can happen while code review is ongoing).