Merge ~smoser/cloud-init:bug/1701325-no-dmi-data-in-container into cloud-init:master

Proposed by Scott Moser on 2017-06-29
Status: Merged
Merged at revision: 4d9f24f5c385cb7fa21d87a097ccd9a297613a75
Proposed branch: ~smoser/cloud-init:bug/1701325-no-dmi-data-in-container
Merge into: cloud-init:master
Diff against target: 65 lines (+30/-0)
2 files modified
cloudinit/util.py (+7/-0)
tests/unittests/test_util.py (+23/-0)
Reviewer Review Type Date Requested Status
Server Team CI bot continuous-integration Approve on 2017-06-29
Joshua Powers (community) Approve on 2017-06-29
Ryan Harper 2017-06-29 Approve on 2017-06-29
Review via email: mp+326546@code.launchpad.net

Commit Message

read_dmi_data: always return None when inside a container.

This fixes stacktrace and warning message that would be printed
to the log if running inside a container and read_dmi_data tried
to access a key that was not present.

In a container, the /sys/class/dmi/id data is not relevant to the
but to the host. Additionally an unpriviledged container might see
strange behavior:
   # cd /sys/class/dmi/id/
   # id -u
   0
   # ls -l chassis_serial
   -r-------- 1 nobody nogroup 4096 Jun 29 16:49 chassis_serial
   # cat chassis_serial
   cat: /sys/class/dmi/id/chassis_serial: Permission denied

The solution here is to just always return None when running in a
container.

LP: #1701325

To post a comment you must log in.
Ryan Harper (raharper) wrote :

This is a good fix.

review: Approve

FAILED: Continuous integration, rev:2a60190bbc4b23d2fd907307cd41f7e04329b2b3
https://jenkins.ubuntu.com/server/job/cloud-init-ci/25/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

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

review: Needs Fixing (continuous-integration)
Joshua Powers (powersj) wrote :

+1 once this is fixed:
tests/unittests/test_util.py:468:1: E303 too many blank lines (3)

Create artful container, upgraded to built version of deb, rebooted. Cleaned out /var/lib/cloud (kept seed) and /var/log/cloud-init* and rebooted. All looks good and stack traces are gone.

review: Approve

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

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

review: Approve (continuous-integration)

PASSED: Continuous integration, rev:b308922a729e63d1c5acd8414a206124034d51e5
https://jenkins.ubuntu.com/server/job/cloud-init-ci/26/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: CentOS 6 & 7: Build & Test
    IN_PROGRESS: Declarative: Post Actions

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

review: Approve (continuous-integration)

PASSED: Continuous integration, rev:5e35beb88381e44010b1c9fbff3f6cfecc920642
https://jenkins.ubuntu.com/server/job/cloud-init-ci/28/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: CentOS 6 & 7: Build & Test
    IN_PROGRESS: Declarative: Post Actions

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

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/util.py b/cloudinit/util.py
2index c93b6d7..b486e18 100644
3--- a/cloudinit/util.py
4+++ b/cloudinit/util.py
5@@ -2397,6 +2397,10 @@ def read_dmi_data(key):
6 """
7 Wrapper for reading DMI data.
8
9+ If running in a container return None. This is because DMI data is
10+ assumed to be not useful in a container as it does not represent the
11+ container but rather the host.
12+
13 This will do the following (returning the first that produces a
14 result):
15 1) Use a mapping to translate `key` from dmidecode naming to
16@@ -2407,6 +2411,9 @@ def read_dmi_data(key):
17 If all of the above fail to find a value, None will be returned.
18 """
19
20+ if is_container():
21+ return None
22+
23 syspath_value = _read_dmi_syspath(key)
24 if syspath_value is not None:
25 return syspath_value
26diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py
27index a73fd26..65035be 100644
28--- a/tests/unittests/test_util.py
29+++ b/tests/unittests/test_util.py
30@@ -365,6 +365,9 @@ class TestReadDMIData(helpers.FilesystemMockingTestCase):
31 self.addCleanup(shutil.rmtree, self.new_root)
32 self.patchOS(self.new_root)
33 self.patchUtils(self.new_root)
34+ p = mock.patch("cloudinit.util.is_container", return_value=False)
35+ self.addCleanup(p.stop)
36+ self._m_is_container = p.start()
37
38 def _create_sysfs_parent_directory(self):
39 util.ensure_dir(os.path.join('sys', 'class', 'dmi', 'id'))
40@@ -453,6 +456,26 @@ class TestReadDMIData(helpers.FilesystemMockingTestCase):
41 self._create_sysfs_file(sysfs_key, dmi_value)
42 self.assertEqual(expected, util.read_dmi_data(dmi_key))
43
44+ def test_container_returns_none(self):
45+ """In a container read_dmi_data should always return None."""
46+
47+ # first verify we get the value if not in container
48+ self._m_is_container.return_value = False
49+ key, val = ("system-product-name", "my_product")
50+ self._create_sysfs_file('product_name', val)
51+ self.assertEqual(val, util.read_dmi_data(key))
52+
53+ # then verify in container returns None
54+ self._m_is_container.return_value = True
55+ self.assertIsNone(util.read_dmi_data(key))
56+
57+ def test_container_returns_none_on_unknown(self):
58+ """In a container even bogus keys return None."""
59+ self._m_is_container.return_value = True
60+ self._create_sysfs_file('product_name', "should-be-ignored")
61+ self.assertIsNone(util.read_dmi_data("bogus"))
62+ self.assertIsNone(util.read_dmi_data("system-product-name"))
63+
64
65 class TestMultiLog(helpers.FilesystemMockingTestCase):
66

Subscribers

People subscribed via source and target branches

to all changes: