Merge ~ruansx/cloud-init:add-zstack-datasource into cloud-init:master

Proposed by Steve Ruan on 2019-09-07
Status: Merged
Approved by: Scott Moser on 2019-09-18
Approved revision: faac76e6adbfbab9e1e3ea59089611e7f95a1532
Merge reported by: Server Team CI bot
Merged at revision: not available
Proposed branch: ~ruansx/cloud-init:add-zstack-datasource
Merge into: cloud-init:master
Diff against target: 198 lines (+94/-2)
7 files modified
cloudinit/apport.py (+1/-0)
cloudinit/sources/DataSourceEc2.py (+15/-1)
doc/rtd/topics/datasources.rst (+1/-0)
doc/rtd/topics/datasources/zstack.rst (+36/-0)
tests/unittests/test_datasource/test_ec2.py (+28/-0)
tests/unittests/test_ds_identify.py (+8/-1)
tools/ds-identify (+5/-0)
Reviewer Review Type Date Requested Status
Server Team CI bot continuous-integration 2019-09-07 Approve on 2019-09-18
Scott Moser Approve on 2019-09-17
Review via email: mp+372445@code.launchpad.net

Commit message

Add datasource for ZStack platform.

Zstack platform provides a AWS Ec2 metadata service, and
identifies their platform to the guest by setting the 'chassis asset tag'
to a string that ends with '.zstack.io'.

LP: #1841181

To post a comment you must log in.
Scott Moser (smoser) wrote :

Quick review things:
 * make a better commit message (the 'Commit message` above will be be used when this is ultimately squashed and merged)
 * add 'LP: #1841181' to your commit message. See git log for examples. Thats how MPs get associated with bugs.
 * Tests needed.
    * tests/unittests/test_ds_identify.py should be fairly to understand and update for Zstack
    * Probably add a tests/unittests/test_datasource/test_zstack.py
 * Add doc/rtd/topics/datasources/zstack.rst and update doc/rtd/topics/datasources.rst to reference it. See Exoscale for a recently added datasource example.

  * Did you check to see if simply adding Zstack in the same way as BrightBox is added would work? That could be even less code.

Steve Ruan (ruansx) wrote :

Thanks.
It's more clean to add ZStack in same way as BrightBox´╝îand I changed it.

Scott Moser (smoser) wrote :

Some comments inline. I also updated your commit message a bit.

Scott Moser (smoser) wrote :

Also, this request is still relevant.

 * Add doc/rtd/topics/datasources/zstack.rst and update doc/rtd/topics/datasources.rst to reference it. See Exoscale for a recently added datasource example.

Scott Moser (smoser) wrote :
Steve Ruan (ruansx) wrote :

Thanks, Scott.
All comments are fixed.

Scott Moser (smoser) wrote :

This looks good to me now, thanks.

Steve, one style comment in-line though.

I'll get someone to point the c-i bot at this too.

PASSED: Continuous integration, rev:c8d47870a851f3a5b7a96fabb1442ab0224b9388
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1138/
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/1138//rebuild

review: Approve (continuous-integration)
Scott Moser (smoser) wrote :

@Steve,

Just for your ease, I put together a change that updates the Zstack/ZSTACK and adds a test case also.

http://paste.ubuntu.com/p/KZWYKZCmcd/

Please grab that and update.

Ryan Harper (raharper) wrote :

Suggest a different word in the documentation, see inline comment.

Steve Ruan (ruansx) wrote :

Sorry for the late response because there is a short vacation.

Thanks for the comment. @Scott and @Ryan.

Scott Moser (smoser) wrote :

I'm +1 now. But you do have to fix a merge issue (look for <<<<).

Steve Ruan (ruansx) wrote :

doc/rtd/topics/datasources.rst is changed in master branch. Someone move the section "datasource/xx" from the buttom to the middle, and my commit also changed this part.

The conflict is fixed now.

Scott Moser (smoser) :
review: Approve

Autolanding: FAILED
More details in the following jenkins job:
https://jenkins.ubuntu.com/server/job/cloud-init-autoland-test/333/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

review: Needs Fixing (continuous-integration)

Autolanding: FAILED
More details in the following jenkins job:
https://jenkins.ubuntu.com/server/job/cloud-init-autoland-test/334/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

review: Needs Fixing (continuous-integration)

Autolanding: FAILED
More details in the following jenkins job:
https://jenkins.ubuntu.com/server/job/cloud-init-autoland-test/335/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

review: Needs Fixing (continuous-integration)
Scott Moser (smoser) wrote :

Steve,
I'm sorry for not having run pycodestyle on my suggested changes..
the Lander rejected this due to some code formatting.
Fix at http://paste.ubuntu.com/p/9st9YCstxf/

you can run those tests locally with:
 tox
or specifically the pycodestyle with
  tox -e pycodestyle

Fix that and we can land. Sorry for the loss of another day.

Steve Ruan (ruansx) wrote :

Thanks Scott. It's my fault, I should run tox before pushing to the branch.

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/cloudinit/apport.py b/cloudinit/apport.py
2index 003ff1f..fde1f75 100644
3--- a/cloudinit/apport.py
4+++ b/cloudinit/apport.py
5@@ -37,6 +37,7 @@ KNOWN_CLOUD_NAMES = [
6 'Scaleway',
7 'SmartOS',
8 'VMware',
9+ 'ZStack',
10 'Other']
11
12 # Potentially clear text collected logs
13diff --git a/cloudinit/sources/DataSourceEc2.py b/cloudinit/sources/DataSourceEc2.py
14index 1010745..6c72ace 100644
15--- a/cloudinit/sources/DataSourceEc2.py
16+++ b/cloudinit/sources/DataSourceEc2.py
17@@ -33,6 +33,7 @@ class CloudNames(object):
18 ALIYUN = "aliyun"
19 AWS = "aws"
20 BRIGHTBOX = "brightbox"
21+ ZSTACK = "zstack"
22 # UNKNOWN indicates no positive id. If strict_id is 'warn' or 'false',
23 # then an attempt at the Ec2 Metadata service will be made.
24 UNKNOWN = "unknown"
25@@ -477,10 +478,16 @@ def identify_brightbox(data):
26 return CloudNames.BRIGHTBOX
27
28
29+def identify_zstack(data):
30+ if data['asset_tag'].endswith('.zstack.io'):
31+ return CloudNames.ZSTACK
32+
33+
34 def identify_platform():
35 # identify the platform and return an entry in CloudNames.
36 data = _collect_platform_data()
37- checks = (identify_aws, identify_brightbox, lambda x: CloudNames.UNKNOWN)
38+ checks = (identify_aws, identify_brightbox, identify_zstack,
39+ lambda x: CloudNames.UNKNOWN)
40 for checker in checks:
41 try:
42 result = checker(data)
43@@ -498,6 +505,7 @@ def _collect_platform_data():
44 uuid: system-uuid from dmi or /sys/hypervisor
45 uuid_source: 'hypervisor' (/sys/hypervisor/uuid) or 'dmi'
46 serial: dmi 'system-serial-number' (/sys/.../product_serial)
47+ asset_tag: 'dmidecode -s chassis-asset-tag'
48
49 On Ec2 instances experimentation is that product_serial is upper case,
50 and product_uuid is lower case. This returns lower case values for both.
51@@ -520,6 +528,12 @@ def _collect_platform_data():
52
53 data['serial'] = serial.lower()
54
55+ asset_tag = util.read_dmi_data('chassis-asset-tag')
56+ if asset_tag is None:
57+ asset_tag = ''
58+
59+ data['asset_tag'] = asset_tag.lower()
60+
61 return data
62
63
64diff --git a/doc/rtd/topics/datasources.rst b/doc/rtd/topics/datasources.rst
65index 8e58be9..a337c08 100644
66--- a/doc/rtd/topics/datasources.rst
67+++ b/doc/rtd/topics/datasources.rst
68@@ -45,6 +45,7 @@ The following is a list of documents for each supported datasource:
69 datasources/oracle.rst
70 datasources/ovf.rst
71 datasources/smartos.rst
72+ datasources/zstack.rst
73
74
75 Creation
76diff --git a/doc/rtd/topics/datasources/zstack.rst b/doc/rtd/topics/datasources/zstack.rst
77new file mode 100644
78index 0000000..36e60ff
79--- /dev/null
80+++ b/doc/rtd/topics/datasources/zstack.rst
81@@ -0,0 +1,36 @@
82+.. _datasource_zstack:
83+
84+ZStack
85+======
86+ZStack platform provides a AWS Ec2 metadata service, but with different datasource identity.
87+More information about ZStack can be found at `ZStack <https://www.zstack.io>`__.
88+
89+Discovery
90+---------
91+To determine whether a vm running on ZStack platform, cloud-init checks DMI information
92+by 'dmidecode -s chassis-asset-tag', if the output ends with '.zstack.io', it's running
93+on ZStack platform:
94+
95+
96+Metadata
97+^^^^^^^^
98+Same as EC2, instance metadata can be queried at
99+
100+::
101+
102+ GET http://169.254.169.254/2009-04-04/meta-data/
103+ instance-id
104+ local-hostname
105+
106+Userdata
107+^^^^^^^^
108+Same as EC2, instance userdata can be queried at
109+
110+::
111+
112+ GET http://169.254.169.254/2009-04-04/user-data/
113+ meta_data.json
114+ user_data
115+ password
116+
117+.. vi: textwidth=78
118diff --git a/tests/unittests/test_datasource/test_ec2.py b/tests/unittests/test_datasource/test_ec2.py
119index 1ec8e00..6fabf25 100644
120--- a/tests/unittests/test_datasource/test_ec2.py
121+++ b/tests/unittests/test_datasource/test_ec2.py
122@@ -662,4 +662,32 @@ class TestConvertEc2MetadataNetworkConfig(test_helpers.CiTestCase):
123 expected,
124 ec2.convert_ec2_metadata_network_config(self.network_metadata))
125
126+
127+class TesIdentifyPlatform(test_helpers.CiTestCase):
128+
129+ def collmock(self, **kwargs):
130+ """return non-special _collect_platform_data updated with changes."""
131+ unspecial = {
132+ 'asset_tag': '3857-0037-2746-7462-1818-3997-77',
133+ 'serial': 'H23-C4J3JV-R6',
134+ 'uuid': '81c7e555-6471-4833-9551-1ab366c4cfd2',
135+ 'uuid_source': 'dmi',
136+ }
137+ unspecial.update(**kwargs)
138+ return unspecial
139+
140+ @mock.patch('cloudinit.sources.DataSourceEc2._collect_platform_data')
141+ def test_identify_zstack(self, m_collect):
142+ """zstack should be identified if cassis-asset-tag ends in .zstack.io
143+ """
144+ m_collect.return_value = self.collmock(asset_tag='123456.zstack.io')
145+ self.assertEqual(ec2.CloudNames.ZSTACK, ec2.identify_platform())
146+
147+ @mock.patch('cloudinit.sources.DataSourceEc2._collect_platform_data')
148+ def test_identify_zstack_full_domain_only(self, m_collect):
149+ """zstack asset-tag matching should match only on full domain boundary.
150+ """
151+ m_collect.return_value = self.collmock(asset_tag='123456.buzzstack.io')
152+ self.assertEqual(ec2.CloudNames.UNKNOWN, ec2.identify_platform())
153+
154 # vi: ts=4 expandtab
155diff --git a/tests/unittests/test_ds_identify.py b/tests/unittests/test_ds_identify.py
156index de87be2..7aeeb91 100644
157--- a/tests/unittests/test_ds_identify.py
158+++ b/tests/unittests/test_ds_identify.py
159@@ -609,6 +609,10 @@ class TestDsIdentify(DsIdentifyBase):
160 self.assertEqual(expected, [p for p in expected if p in toks],
161 "path did not have expected tokens")
162
163+ def test_zstack_is_ec2(self):
164+ """EC2: chassis asset tag ends with 'zstack.io'"""
165+ self._test_ds_found('Ec2-ZStack')
166+
167
168 class TestIsIBMProvisioning(DsIdentifyBase):
169 """Test the is_ibm_provisioning method in ds-identify."""
170@@ -971,8 +975,11 @@ VALID_CFG = {
171 {'name': 'blkid', 'ret': 2, 'out': ''},
172 ],
173 'files': {ds_smartos.METADATA_SOCKFILE: 'would be a socket\n'},
174+ },
175+ 'Ec2-ZStack': {
176+ 'ds': 'Ec2',
177+ 'files': {P_CHASSIS_ASSET_TAG: '123456.zstack.io\n'},
178 }
179-
180 }
181
182 # vi: ts=4 expandtab
183diff --git a/tools/ds-identify b/tools/ds-identify
184index 2447d14..f76f2a6 100755
185--- a/tools/ds-identify
186+++ b/tools/ds-identify
187@@ -895,6 +895,11 @@ ec2_identify_platform() {
188 *.brightbox.com) _RET="Brightbox"; return 0;;
189 esac
190
191+ local asset_tag="${DI_DMI_CHASSIS_ASSET_TAG}"
192+ case "$asset_tag" in
193+ *.zstack.io) _RET="ZStack"; return 0;;
194+ esac
195+
196 # AWS http://docs.aws.amazon.com/AWSEC2/
197 # latest/UserGuide/identify_ec2_instances.html
198 local uuid="" hvuuid="${PATH_SYS_HYPERVISOR}/uuid"

Subscribers

People subscribed via source and target branches