Merge ~smoser/cloud-init:bug/1683038-ec2-no-warn-on-explicit into cloud-init:master
- Git
- lp:~smoser/cloud-init
- bug/1683038-ec2-no-warn-on-explicit
- Merge into master
Status: | Work in progress |
---|---|
Proposed branch: | ~smoser/cloud-init:bug/1683038-ec2-no-warn-on-explicit |
Merge into: | cloud-init:master |
Diff against target: |
373 lines (+318/-10) 2 files modified
cloudinit/sources/DataSourceEc2.py (+44/-10) tests/unittests/test_datasource/test_ec2.py (+274/-0) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Server Team CI bot | continuous-integration | Approve | |
Chad Smith | Approve | ||
cloud-init Commiters | Pending | ||
Review via email: mp+324274@code.launchpad.net |
Commit message
Ec2: do not warn user of strict if Ec2 is explicitly set.
If the system is configured with datasource_list of either just
'Ec2' or 'Ec2' and 'None', then consider this as if they had set
strict_id = true. A change is made to the signature of
'warn_if_necessary' to make unit testing easier.
Additionally, fix a bug in ds-identify. If the user configured:
datasource_list: ["Ec2", "None"]
then ds-identify would write
datasource_list: ["Ec2", "None", "None"]
which would break the logic to avoid warning.
LP: #1683038
Description of the change
Server Team CI bot (server-team-bot) wrote : | # |
Ryan Harper (raharper) wrote : | # |
Generally looks fine, one question w.r.t confirming explict_dslist check
Chad Smith (chad.smith) : | # |
Chad Smith (chad.smith) : | # |
Chad Smith (chad.smith) : | # |
Scott Moser (smoser) wrote : | # |
On Mon, 22 May 2017, Ryan Harper wrote:
> Generally looks fine, one question w.r.t confirming explict_dslist check
>
> Diff comments:
>
> > diff --git a/cloudinit/
> > index 2f9c7ed..818a639 100644
> > --- a/cloudinit/
> > +++ b/cloudinit/
> > @@ -266,6 +267,12 @@ def parse_strict_
> > return mode, sleep
> >
> >
> > +def _is_explicit_
> > + if 'Ec2' not in dslist:
> > + return False
> > + return (len(dslist) == 1 or (len(dslist) == 2 and 'None' in dslist))
>
> if we know dslist is len == 2, then We could ensure it looks like:
> ['Ec2', None]
>
> Right? That's exactly what we're looking for? Verus this [None, 'Ec2']
> which would pass this test but not be what we're looking for IIUC
Yeah, I avoided checking for 'Ec2' because if this datasoruce is running,
then Ec2 is in the list. But I think it makes more sense to be explicit
Will cahnge as you sigguested.
if dslist == ['Ec2', 'None'] or dslist == ['Ec2']
sm
Scott Moser (smoser) wrote : | # |
some responses in line. i'll fix some things too. thanks for review.
Scott Moser (smoser) wrote : | # |
I've pushed some new commits, please see and review.
thanks.
Server Team CI bot (server-team-bot) wrote : | # |
PASSED: Continuous integration, rev:d0e1e415ef2
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
Click here to trigger a rebuild:
https:/
Server Team CI bot (server-team-bot) wrote : | # |
PASSED: Continuous integration, rev:194e15a6785
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
Click here to trigger a rebuild:
https:/
Chad Smith (chad.smith) wrote : | # |
pre-approved on assumption of a new unit test for behavior, if the following branch[1] lands before this one you could create a CiTestCase setting with_logs = True and self.assertIn(" not warning", self.logs.
References:
[1] https:/
Scott Moser (smoser) wrote : | # |
I split the ds-identify portion out into
https:/
that way i can get it in now, and fix the ec2 warn portion itself with a nice test.
Server Team CI bot (server-team-bot) wrote : | # |
PASSED: Continuous integration, rev:4556902992e
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
Click here to trigger a rebuild:
https:/
- e8fcbfd... by Scott Moser
-
Add a docstring for _collect_
platform_ data. - c4d0d40... by Scott Moser
-
initial test_ec2.py
Server Team CI bot (server-team-bot) wrote : | # |
FAILED: Continuous integration, rev:0c5f1e428c0
https:/
Executed test runs:
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
- 092ea40... by Scott Moser
-
Add test for warn_if_necessary.
Change here also changes function signature of warn_if_necessary
to allow for more direct testing without using 'activate.'
Scott Moser (smoser) wrote : | # |
I have to think some more on the 'FIXME' bits here.
But i'd like feedback on teh added unit tests.
Server Team CI bot (server-team-bot) wrote : | # |
FAILED: Continuous integration, rev:4999cd8ef76
https:/
Executed test runs:
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
Server Team CI bot (server-team-bot) wrote : | # |
PASSED: Continuous integration, rev:092ea4027f2
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
Click here to trigger a rebuild:
https:/
- a247285... by Scott Moser
-
add warning on unexpected situation strict_id=true and platform unknown.
Server Team CI bot (server-team-bot) wrote : | # |
PASSED: Continuous integration, rev:a2472853a66
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
Click here to trigger a rebuild:
https:/
Scott Moser (smoser) wrote : | # |
I think that the signature change is fine.
the other fixme I think is best fixed by just having ds-identify write an entry in the config that says that it wrote a datasource_list, and how it did that.
Ie, it could declare that it essentially copied the datasource list like this:
di_datasourc
or something to that affect. Then cloud-init could read that. A 'copy' there would be different than 'search'. The copy would show no warning, but the search would.
I'm not sure stil..
We *should* grab the other parts of this merge proposal and get them into main.
Scott Moser (smoser) wrote : | # |
I grabbed the initial test portions of this merge proposal and put them into another
at https:/
note, those do not have the tests for WarnIfNecessary
Unmerged commits
- a247285... by Scott Moser
-
add warning on unexpected situation strict_id=true and platform unknown.
- 092ea40... by Scott Moser
-
Add test for warn_if_necessary.
Change here also changes function signature of warn_if_necessary
to allow for more direct testing without using 'activate.' - c4d0d40... by Scott Moser
-
initial test_ec2.py
- e8fcbfd... by Scott Moser
-
Add a docstring for _collect_
platform_ data. - e358e2f... by Scott Moser
-
Ec2: do not warn user of strict if Ec2 is explicitly set.
If the system is configured with datasource_list of either just
'Ec2' or 'Ec2' and 'None', then consider this as if they had set
strict_id = true.LP: #1683038
Preview Diff
1 | diff --git a/cloudinit/sources/DataSourceEc2.py b/cloudinit/sources/DataSourceEc2.py | |||
2 | index 9e2fdc0..d0a2f8e 100644 | |||
3 | --- a/cloudinit/sources/DataSourceEc2.py | |||
4 | +++ b/cloudinit/sources/DataSourceEc2.py | |||
5 | @@ -69,7 +69,8 @@ class DataSourceEc2(sources.DataSource): | |||
6 | 69 | LOG.debug("strict_mode: %s, cloud_platform=%s", | 69 | LOG.debug("strict_mode: %s, cloud_platform=%s", |
7 | 70 | strict_mode, self.cloud_platform) | 70 | strict_mode, self.cloud_platform) |
8 | 71 | if strict_mode == "true" and self.cloud_platform == Platforms.UNKNOWN: | 71 | if strict_mode == "true" and self.cloud_platform == Platforms.UNKNOWN: |
10 | 72 | return False | 72 | if not _is_explicit_dslist(self.sys_cfg.get('datasource_list')): |
11 | 73 | return False | ||
12 | 73 | elif self.cloud_platform == Platforms.NO_EC2_METADATA: | 74 | elif self.cloud_platform == Platforms.NO_EC2_METADATA: |
13 | 74 | return False | 75 | return False |
14 | 75 | 76 | ||
15 | @@ -229,10 +230,15 @@ class DataSourceEc2(sources.DataSource): | |||
16 | 229 | def activate(self, cfg, is_new_instance): | 230 | def activate(self, cfg, is_new_instance): |
17 | 230 | if not is_new_instance: | 231 | if not is_new_instance: |
18 | 231 | return | 232 | return |
23 | 232 | if self.cloud_platform == Platforms.UNKNOWN: | 233 | # FIXME: Think about this function signature change. |
24 | 233 | warn_if_necessary( | 234 | # This change allows 'warn_if_necessary' unit test to do more |
25 | 234 | util.get_cfg_by_path(cfg, STRICT_ID_PATH, STRICT_ID_DEFAULT), | 235 | # without testing 'activate'. I need to test though the upgrade |
26 | 235 | cfg) | 236 | # adn reboot path where an old obj.pkl file has 'activate' called |
27 | 237 | # on it. I think it is fine... as the new code will have a | ||
28 | 238 | # cloud_platform on it already. | ||
29 | 239 | warn_if_necessary( | ||
30 | 240 | util.get_cfg_by_path(cfg, STRICT_ID_PATH, STRICT_ID_DEFAULT), | ||
31 | 241 | cfg, platform=self.cloud_platform) | ||
32 | 236 | 242 | ||
33 | 237 | 243 | ||
34 | 238 | def read_strict_mode(cfgval, default): | 244 | def read_strict_mode(cfgval, default): |
35 | @@ -273,7 +279,17 @@ def parse_strict_mode(cfgval): | |||
36 | 273 | return mode, sleep | 279 | return mode, sleep |
37 | 274 | 280 | ||
38 | 275 | 281 | ||
40 | 276 | def warn_if_necessary(cfgval, cfg): | 282 | def _is_explicit_dslist(dslist): |
41 | 283 | """Is dslist explicitly configured to search Ec2 datasource only?""" | ||
42 | 284 | if not dslist: | ||
43 | 285 | return False | ||
44 | 286 | return dslist in (['Ec2'], ['Ec2', 'None']) | ||
45 | 287 | |||
46 | 288 | |||
47 | 289 | def warn_if_necessary(cfgval, cfg, platform): | ||
48 | 290 | if platform != Platforms.UNKNOWN: | ||
49 | 291 | return | ||
50 | 292 | |||
51 | 277 | try: | 293 | try: |
52 | 278 | mode, sleep = parse_strict_mode(cfgval) | 294 | mode, sleep = parse_strict_mode(cfgval) |
53 | 279 | except ValueError as e: | 295 | except ValueError as e: |
54 | @@ -283,6 +299,18 @@ def warn_if_necessary(cfgval, cfg): | |||
55 | 283 | if mode == "false": | 299 | if mode == "false": |
56 | 284 | return | 300 | return |
57 | 285 | 301 | ||
58 | 302 | dslist = cfg.get('datasource_list') | ||
59 | 303 | if _is_explicit_dslist(cfg.get('datasource_list')): | ||
60 | 304 | LOG.debug("mode=%s but explicit dslist (%s), not warning.", | ||
61 | 305 | mode, dslist) | ||
62 | 306 | return | ||
63 | 307 | |||
64 | 308 | if mode == "true": | ||
65 | 309 | # Unknown platform and strict is true. get_data should have | ||
66 | 310 | # returned false. | ||
67 | 311 | LOG.warning("Unexpected error. strict_mode=%s platform=%s (%s)", | ||
68 | 312 | mode, platform, cfgval) | ||
69 | 313 | |||
70 | 286 | warnings.show_warning('non_ec2_md', cfg, mode=True, sleep=sleep) | 314 | warnings.show_warning('non_ec2_md', cfg, mode=True, sleep=sleep) |
71 | 287 | 315 | ||
72 | 288 | 316 | ||
73 | @@ -316,10 +344,16 @@ def identify_platform(): | |||
74 | 316 | 344 | ||
75 | 317 | 345 | ||
76 | 318 | def _collect_platform_data(): | 346 | def _collect_platform_data(): |
81 | 319 | # returns a dictionary with all lower case values: | 347 | """Returns a dictionary of platform info from dmi or /sys/hypervisor. |
82 | 320 | # uuid: system-uuid from dmi or /sys/hypervisor | 348 | |
83 | 321 | # uuid_source: 'hypervisor' (/sys/hypervisor/uuid) or 'dmi' | 349 | Keys in the dictionary are as follows: |
84 | 322 | # serial: dmi 'system-serial-number' (/sys/.../product_serial) | 350 | uuid: system-uuid from dmi or /sys/hypervisor |
85 | 351 | uuid_source: 'hypervisor' (/sys/hypervisor/uuid) or 'dmi' | ||
86 | 352 | serial: dmi 'system-serial-number' (/sys/.../product_serial) | ||
87 | 353 | |||
88 | 354 | On Ec2 instances experimentation is that product_serial is upper case, | ||
89 | 355 | and product_uuid is lower case. This returns lower case values for both. | ||
90 | 356 | """ | ||
91 | 323 | data = {} | 357 | data = {} |
92 | 324 | try: | 358 | try: |
93 | 325 | uuid = util.load_file("/sys/hypervisor/uuid").strip() | 359 | uuid = util.load_file("/sys/hypervisor/uuid").strip() |
94 | diff --git a/tests/unittests/test_datasource/test_ec2.py b/tests/unittests/test_datasource/test_ec2.py | |||
95 | 326 | new file mode 100644 | 360 | new file mode 100644 |
96 | index 0000000..443eda9 | |||
97 | --- /dev/null | |||
98 | +++ b/tests/unittests/test_datasource/test_ec2.py | |||
99 | @@ -0,0 +1,274 @@ | |||
100 | 1 | # This file is part of cloud-init. See LICENSE file for license information. | ||
101 | 2 | |||
102 | 3 | import httpretty | ||
103 | 4 | import mock | ||
104 | 5 | |||
105 | 6 | from .. import helpers as test_helpers | ||
106 | 7 | from cloudinit import helpers | ||
107 | 8 | from cloudinit.sources import DataSourceEc2 as ec2 | ||
108 | 9 | |||
109 | 10 | |||
110 | 11 | # collected from api version 2009-04-04/ with | ||
111 | 12 | # python3 -c 'import json | ||
112 | 13 | # from cloudinit.ec2_utils import get_instance_metadata as gm | ||
113 | 14 | # print(json.dumps(gm("2009-04-04"), indent=1, sort_keys=True))' | ||
114 | 15 | DEFAULT_METADATA = { | ||
115 | 16 | "ami-id": "ami-80861296", | ||
116 | 17 | "ami-launch-index": "0", | ||
117 | 18 | "ami-manifest-path": "(unknown)", | ||
118 | 19 | "block-device-mapping": {"ami": "/dev/sda1", "root": "/dev/sda1"}, | ||
119 | 20 | "hostname": "ip-10-0-0-149", | ||
120 | 21 | "instance-action": "none", | ||
121 | 22 | "instance-id": "i-0052913950685138c", | ||
122 | 23 | "instance-type": "t2.micro", | ||
123 | 24 | "local-hostname": "ip-10-0-0-149", | ||
124 | 25 | "local-ipv4": "10.0.0.149", | ||
125 | 26 | "placement": {"availability-zone": "us-east-1b"}, | ||
126 | 27 | "profile": "default-hvm", | ||
127 | 28 | "public-hostname": "", | ||
128 | 29 | "public-ipv4": "107.23.188.247", | ||
129 | 30 | "public-keys": {"brickies": ["ssh-rsa AAAAB3Nz....w== brickies"]}, | ||
130 | 31 | "reservation-id": "r-00a2c173fb5782a08", | ||
131 | 32 | "security-groups": "wide-open" | ||
132 | 33 | } | ||
133 | 34 | |||
134 | 35 | |||
135 | 36 | def register_ssh_keys(rfunc, base_url, keys_data): | ||
136 | 37 | """handle ssh key inconsistencies. | ||
137 | 38 | |||
138 | 39 | public-keys in the ec2 metadata is inconsistently formated compared | ||
139 | 40 | to other entries. | ||
140 | 41 | Given keys_data of {name1: pubkey1, name2: pubkey2} | ||
141 | 42 | |||
142 | 43 | This registers the following urls: | ||
143 | 44 | base_url 0={name1}\n1={name2} # (for each name) | ||
144 | 45 | base_url/ 0={name1}\n1={name2} # (for each name) | ||
145 | 46 | base_url/0 openssh-key | ||
146 | 47 | base_url/0/ openssh-key | ||
147 | 48 | base_url/0/openssh-key {pubkey1} | ||
148 | 49 | base_url/0/openssh-key/ {pubkey1} | ||
149 | 50 | ... | ||
150 | 51 | """ | ||
151 | 52 | |||
152 | 53 | base_url = base_url.rstrip("/") | ||
153 | 54 | odd_index = '\n'.join( | ||
154 | 55 | ["{0}={1}".format(n, name) | ||
155 | 56 | for n, name in enumerate(sorted(keys_data))]) | ||
156 | 57 | |||
157 | 58 | rfunc(base_url, odd_index) | ||
158 | 59 | rfunc(base_url + "/", odd_index) | ||
159 | 60 | |||
160 | 61 | for n, name in enumerate(sorted(keys_data)): | ||
161 | 62 | val = keys_data[name] | ||
162 | 63 | if isinstance(val, list): | ||
163 | 64 | val = '\n'.join(val) | ||
164 | 65 | burl = base_url + "/%s" % n | ||
165 | 66 | rfunc(burl, "openssh-key") | ||
166 | 67 | rfunc(burl + "/", "openssh-key") | ||
167 | 68 | rfunc(burl + "/%s/openssh-key" % name, val) | ||
168 | 69 | rfunc(burl + "/%s/openssh-key/" % name, val) | ||
169 | 70 | |||
170 | 71 | |||
171 | 72 | def register_mock_metaserver(base_url, data): | ||
172 | 73 | """Register with httpretty a ec2 metadata like service serving 'data'. | ||
173 | 74 | |||
174 | 75 | If given a dictionary, it will populate urls under base_url for | ||
175 | 76 | that dictionary. For example, input of | ||
176 | 77 | {"instance-id": "i-abc", "mac": "00:16:3e:00:00:00"} | ||
177 | 78 | populates | ||
178 | 79 | base_url with 'instance-id\nmac' | ||
179 | 80 | base_url/ with 'instance-id\nmac' | ||
180 | 81 | base_url/instance-id with i-abc | ||
181 | 82 | base_url/mac with 00:16:3e:00:00:00 | ||
182 | 83 | """ | ||
183 | 84 | def register_helper(register, base_url, body): | ||
184 | 85 | if isinstance(body, str): | ||
185 | 86 | register(base_url, body) | ||
186 | 87 | elif isinstance(body, list): | ||
187 | 88 | register(base_url.rstrip('/'), '\n'.join(body) + '\n') | ||
188 | 89 | register(base_url.rstrip('/') + '/', '\n'.join(body) + '\n') | ||
189 | 90 | elif isinstance(body, dict): | ||
190 | 91 | vals = [] | ||
191 | 92 | for k, v in body.items(): | ||
192 | 93 | if k == 'public-keys': | ||
193 | 94 | register_ssh_keys( | ||
194 | 95 | register, base_url.rstrip('/') + '/public-keys/', v) | ||
195 | 96 | continue | ||
196 | 97 | if isinstance(v, (str, list)): | ||
197 | 98 | suffix = k.rstrip('/') | ||
198 | 99 | else: | ||
199 | 100 | suffix = k.rstrip('/') + '/' | ||
200 | 101 | vals.append(suffix) | ||
201 | 102 | url = base_url.rstrip('/') + '/' + suffix | ||
202 | 103 | register_helper(register, url, v) | ||
203 | 104 | register(base_url.rstrip('/'), '\n'.join(vals) + '\n') | ||
204 | 105 | register(base_url.rstrip('/') + '/', '\n'.join(vals) + '\n') | ||
205 | 106 | elif body is None: | ||
206 | 107 | register(base_url.rstrip('/'), 'not found', status_code=404) | ||
207 | 108 | |||
208 | 109 | def myreg(*argc, **kwargs): | ||
209 | 110 | # print("register_url(%s, %s)" % (argc, kwargs)) | ||
210 | 111 | return httpretty.register_uri(httpretty.GET, *argc, **kwargs) | ||
211 | 112 | |||
212 | 113 | register_helper(myreg, base_url, data) | ||
213 | 114 | |||
214 | 115 | |||
215 | 116 | class TestEc2(test_helpers.HttprettyTestCase): | ||
216 | 117 | valid_platform_data = { | ||
217 | 118 | 'uuid': 'ec212f79-87d1-2f1d-588f-d86dc0fd5412', | ||
218 | 119 | 'uuid_source': 'dmi', | ||
219 | 120 | 'serial': 'ec212f79-87d1-2f1d-588f-d86dc0fd5412', | ||
220 | 121 | } | ||
221 | 122 | |||
222 | 123 | def setUp(self): | ||
223 | 124 | super(TestEc2, self).setUp() | ||
224 | 125 | self.metadata_addr = ec2.DataSourceEc2.metadata_urls[0] | ||
225 | 126 | self.api_ver = '2009-04-04' | ||
226 | 127 | |||
227 | 128 | @property | ||
228 | 129 | def metadata_url(self): | ||
229 | 130 | return '/'.join([self.metadata_addr, self.api_ver, 'meta-data', '']) | ||
230 | 131 | |||
231 | 132 | @property | ||
232 | 133 | def userdata_url(self): | ||
233 | 134 | return '/'.join([self.metadata_addr, self.api_ver, 'user-data']) | ||
234 | 135 | |||
235 | 136 | def _patch_add_cleanup(self, mpath, *args, **kwargs): | ||
236 | 137 | p = mock.patch(mpath, *args, **kwargs) | ||
237 | 138 | p.start() | ||
238 | 139 | self.addCleanup(p.stop) | ||
239 | 140 | |||
240 | 141 | def _setup_ds(self, sys_cfg=None, platform_data=None, md=None, ud=None): | ||
241 | 142 | distro = {} | ||
242 | 143 | paths = helpers.Paths({}) | ||
243 | 144 | if sys_cfg is None: | ||
244 | 145 | sys_cfg = {} | ||
245 | 146 | ds = ec2.DataSourceEc2(sys_cfg=sys_cfg, distro=distro, paths=paths) | ||
246 | 147 | if platform_data is not None: | ||
247 | 148 | self._patch_add_cleanup( | ||
248 | 149 | "cloudinit.sources.DataSourceEc2._collect_platform_data", | ||
249 | 150 | return_value=platform_data) | ||
250 | 151 | |||
251 | 152 | if md: | ||
252 | 153 | register_mock_metaserver(self.metadata_url, md) | ||
253 | 154 | register_mock_metaserver(self.userdata_url, ud) | ||
254 | 155 | |||
255 | 156 | return ds | ||
256 | 157 | |||
257 | 158 | @httpretty.activate | ||
258 | 159 | def test_valid_platform_with_strict_true(self): | ||
259 | 160 | """Valid platform data should return true with strict_id true.""" | ||
260 | 161 | ds = self._setup_ds( | ||
261 | 162 | platform_data=self.valid_platform_data, | ||
262 | 163 | sys_cfg={'datasource': {'Ec2': {'strict_id': True}}}, | ||
263 | 164 | md=DEFAULT_METADATA) | ||
264 | 165 | ret = ds.get_data() | ||
265 | 166 | self.assertEqual(True, ret) | ||
266 | 167 | |||
267 | 168 | @httpretty.activate | ||
268 | 169 | def test_valid_platform_with_strict_false(self): | ||
269 | 170 | """Valid platform data should return true with strict_id false.""" | ||
270 | 171 | ds = self._setup_ds( | ||
271 | 172 | platform_data=self.valid_platform_data, | ||
272 | 173 | sys_cfg={'datasource': {'Ec2': {'strict_id': False}}}, | ||
273 | 174 | md=DEFAULT_METADATA) | ||
274 | 175 | ret = ds.get_data() | ||
275 | 176 | self.assertEqual(True, ret) | ||
276 | 177 | |||
277 | 178 | @httpretty.activate | ||
278 | 179 | def test_unknown_platform_with_strict_true(self): | ||
279 | 180 | """Unknown platform data with strict_id true should return False.""" | ||
280 | 181 | uuid = 'ab439480-72bf-11d3-91fc-b8aded755F9a' | ||
281 | 182 | ds = self._setup_ds( | ||
282 | 183 | platform_data={'uuid': uuid, 'uuid_source': 'dmi', 'serial': ''}, | ||
283 | 184 | sys_cfg={'datasource': {'Ec2': {'strict_id': True}}}, | ||
284 | 185 | md=DEFAULT_METADATA) | ||
285 | 186 | ret = ds.get_data() | ||
286 | 187 | self.assertEqual(False, ret) | ||
287 | 188 | |||
288 | 189 | @httpretty.activate | ||
289 | 190 | def test_unknown_platform_with_strict_false(self): | ||
290 | 191 | """Unknown platform data with strict_id false should return True.""" | ||
291 | 192 | uuid = 'ab439480-72bf-11d3-91fc-b8aded755F9a' | ||
292 | 193 | ds = self._setup_ds( | ||
293 | 194 | platform_data={'uuid': uuid, 'uuid_source': 'dmi', 'serial': ''}, | ||
294 | 195 | sys_cfg={'datasource': {'Ec2': {'strict_id': False}}}, | ||
295 | 196 | md=DEFAULT_METADATA) | ||
296 | 197 | ret = ds.get_data() | ||
297 | 198 | self.assertEqual(True, ret) | ||
298 | 199 | |||
299 | 200 | |||
300 | 201 | class TestWarnIfNecessary(test_helpers.CiTestCase): | ||
301 | 202 | """Test warn_if_necessary behaves as expected. | ||
302 | 203 | |||
303 | 204 | warn_if_necessary is called with the value of the 'strict_id' setting. | ||
304 | 205 | It should call show_warning if a warning should be shown.""" | ||
305 | 206 | |||
306 | 207 | with_logs = True | ||
307 | 208 | |||
308 | 209 | @mock.patch("cloudinit.sources.DataSourceEc2.warnings.show_warning") | ||
309 | 210 | def test_no_warnings_if_platform_identified(self, m_show_warning): | ||
310 | 211 | """If platform is something known, then no warninings expected.""" | ||
311 | 212 | ec2.warn_if_necessary("warn", cfg={}, platform=ec2.Platforms.AWS) | ||
312 | 213 | ec2.warn_if_necessary("warn", cfg={}, platform=ec2.Platforms.BRIGHTBOX) | ||
313 | 214 | ec2.warn_if_necessary("warn", cfg={}, platform=ec2.Platforms.SEEDED) | ||
314 | 215 | ec2.warn_if_necessary("warn", cfg={}, platform=ec2.Platforms.ALIYUN) | ||
315 | 216 | self.assertEqual(0, m_show_warning.call_count) | ||
316 | 217 | |||
317 | 218 | @mock.patch("cloudinit.sources.DataSourceEc2.warnings.show_warning") | ||
318 | 219 | def test_warn_if_platform_unknown_and_strict_true(self, m_show_warning): | ||
319 | 220 | """If platform is unknown and strict is warn, warnings expected.""" | ||
320 | 221 | ec2.warn_if_necessary("warn", cfg={}, platform=ec2.Platforms.UNKNOWN) | ||
321 | 222 | self.assertEqual(1, m_show_warning.call_count) | ||
322 | 223 | |||
323 | 224 | @mock.patch("cloudinit.sources.DataSourceEc2.warnings.show_warning") | ||
324 | 225 | def test_no_warn_if_platform_unknown_and_not_strict(self, m_show_warning): | ||
325 | 226 | """If platform is unknown but strict is false, no warnings.""" | ||
326 | 227 | ec2.warn_if_necessary("false", cfg={}, platform=ec2.Platforms.UNKNOWN) | ||
327 | 228 | self.assertEqual(0, m_show_warning.call_count) | ||
328 | 229 | |||
329 | 230 | @mock.patch("cloudinit.sources.DataSourceEc2.warnings.show_warning") | ||
330 | 231 | def test_no_warn_if_unknown_strict_true_explicit_ds(self, m_show_warning): | ||
331 | 232 | """No warnings on strict_id=warn, platform=unknown, ec2 only.""" | ||
332 | 233 | ec2.warn_if_necessary("false", cfg={'datasource_list': ['Ec2']}, | ||
333 | 234 | platform=ec2.Platforms.UNKNOWN) | ||
334 | 235 | self.assertEqual(0, m_show_warning.call_count) | ||
335 | 236 | |||
336 | 237 | @mock.patch("cloudinit.sources.DataSourceEc2.warnings.show_warning") | ||
337 | 238 | def test_no_warn_if_unknown_strict_true_exp_and_none(self, m_show_warning): | ||
338 | 239 | """No warnings on strict_id=warn, platform=unknown, Ec2, None. | ||
339 | 240 | |||
340 | 241 | FIXME: If ds-identify writes datasource_list due to | ||
341 | 242 | strict_id = warn and DS_MAYBE, will this code path | ||
342 | 243 | be taken? That *should* get a warning. | ||
343 | 244 | |||
344 | 245 | If datasource_list contains ['Ec2', 'None'], then no warning | ||
345 | 246 | should be seen as this is explicit configuration of Ec2.""" | ||
346 | 247 | ec2.warn_if_necessary( | ||
347 | 248 | "warn", cfg={'datasource_list': ['Ec2', 'None']}, | ||
348 | 249 | platform=ec2.Platforms.UNKNOWN) | ||
349 | 250 | self.assertEqual(0, m_show_warning.call_count) | ||
350 | 251 | |||
351 | 252 | @mock.patch("cloudinit.sources.DataSourceEc2.warnings.show_warning") | ||
352 | 253 | def test_warn_if_unknown_strict_true_noexp_and_none(self, m_show_warning): | ||
353 | 254 | """Warn if strict_id=warn, platform=unknown, but not explicit list. | ||
354 | 255 | |||
355 | 256 | This is not an 'explicit' configuration because it has extra | ||
356 | 257 | entry in its datasource_list. | ||
357 | 258 | """ | ||
358 | 259 | ec2.warn_if_necessary( | ||
359 | 260 | "warn", cfg={'datasource_list': ['OpenStack', 'Ec2', 'None']}, | ||
360 | 261 | platform=ec2.Platforms.UNKNOWN) | ||
361 | 262 | self.assertNotIn('Unexpected error', self.logs.getvalue()) | ||
362 | 263 | self.assertEqual(1, m_show_warning.call_count) | ||
363 | 264 | |||
364 | 265 | @mock.patch("cloudinit.sources.DataSourceEc2.warnings.show_warning") | ||
365 | 266 | def test_additional_warn_on_strict_and_unknown(self, m_show_warning): | ||
366 | 267 | """If strict_id is true and platform is unknown, unexpected error.""" | ||
367 | 268 | ec2.warn_if_necessary( | ||
368 | 269 | "true", cfg={}, platform=ec2.Platforms.UNKNOWN) | ||
369 | 270 | self.assertIn('Unexpected error', self.logs.getvalue()) | ||
370 | 271 | self.assertEqual(1, m_show_warning.call_count) | ||
371 | 272 | |||
372 | 273 | |||
373 | 274 | # vi: ts=4 expandtab |
PASSED: Continuous integration, rev:11c0967458c be80e36f64ee53c 47d0a448a3e137 /jenkins. ubuntu. com/server/ job/cloud- init-ci/ 351/ /jenkins. ubuntu. com/server/ job/cloud- init-ci/ nodes=metal- amd64/351 /jenkins. ubuntu. com/server/ job/cloud- init-ci/ nodes=metal- arm64/351 /jenkins. ubuntu. com/server/ job/cloud- init-ci/ nodes=metal- ppc64el/ 351 /jenkins. ubuntu. com/server/ job/cloud- init-ci/ nodes=vm- i386/351
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
Click here to trigger a rebuild: /jenkins. ubuntu. com/server/ job/cloud- init-ci/ 351/rebuild
https:/