Merge lp:~blake-rouse/maas/fix-interface-container-discovery into lp:maas/trunk

Proposed by Blake Rouse on 2016-03-17
Status: Merged
Approved by: Blake Rouse on 2016-03-17
Approved revision: 4802
Merged at revision: 4800
Proposed branch: lp:~blake-rouse/maas/fix-interface-container-discovery
Merge into: lp:maas/trunk
Diff against target: 221 lines (+37/-82)
4 files modified
src/provisioningserver/utils/network.py (+1/-17)
src/provisioningserver/utils/ps.py (+16/-3)
src/provisioningserver/utils/tests/test_network.py (+2/-39)
src/provisioningserver/utils/tests/test_ps.py (+18/-23)
To merge this branch: bzr merge lp:~blake-rouse/maas/fix-interface-container-discovery
Reviewer Review Type Date Requested Status
Gavin Panella (community) 2016-03-17 Approve on 2016-03-17
Review via email: mp+289371@code.launchpad.net

Commit message

Revert r4795 as it was doing the wrong thing. Fix the running_in_container function to correctly detect that process is running in a container. Uses systemd-detect-virt which includes all the logic to determine this information.

To post a comment you must log in.
Gavin Panella (allenap) wrote :

I don't understand how this fixes the linked bug, but it looks fine.

review: Approve
Blake Rouse (blake-rouse) wrote :

Thanks for the review. I added the lru_cache decorator and fixes the tests to clear it.

MAAS Lander (maas-lander) wrote :
Download full text (1016.5 KiB)

The attempt to merge lp:~blake-rouse/maas/fix-interface-container-discovery into lp:maas failed. Below is the output from the failed tests.

Hit:1 http://security.ubuntu.com/ubuntu xenial-security InRelease
Get:2 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial InRelease [116 kB]
Hit:3 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-updates InRelease
Hit:4 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-backports InRelease
Get:5 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial/main Sources [1,105 kB]
Get:6 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial/universe Sources [7,509 kB]
Get:7 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial/main amd64 Packages [1,435 kB]
Get:8 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial/universe amd64 Packages [7,241 kB]
Fetched 17.4 MB in 3s (5,297 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
    --no-install-recommends install apache2 archdetect-deb authbind bind9 bind9utils build-essential bzr-builddeb chromium-browser chromium-chromedriver curl daemontools debhelper dh-apport dh-systemd distro-info dnsutils firefox freeipmi-tools git gjs ipython isc-dhcp-common libjs-angularjs libjs-jquery libjs-jquery-hotkeys libjs-yui3-full libjs-yui3-min libpq-dev make nodejs-legacy npm postgresql pxelinux python3-all python3-apt python3-bson python3-convoy python3-coverage python3-crochet python3-cssselect python3-curtin python3-dev python3-distro-info python3-django python3-django-nose python3-django-piston3 python3-dnspython python3-docutils python3-formencode python3-hivex python3-httplib2 python3-jinja2 python3-jsonschema python3-lxml python3-mock python3-netaddr python3-netifaces python3-oauth python3-oauthlib python3-openssl python3-paramiko python3-petname python3-pexpect python3-psycopg2 python3-pyinotify python3-pyparsing python3-pyvmomi python3-requests python3-seamicroclient python3-setuptools python3-simplestreams python3-sphinx python3-tempita python3-twisted python3-txtftp python3-tz python3-yaml python3-zope.interface python-bson python-crochet python-django python-django-piston python-djorm-ext-pgarray python-formencode python-lxml python-netaddr python-netifaces python-pocket-lint python-psycopg2 python-tempita python-twisted python-yaml socat syslinux-common tgt ubuntu-cloudimage-keyring wget xvfb
Reading package lists...
Building dependency tree...
Reading state information...
apache2 is already the newest version (2.4.18-1ubuntu1).
archdetect-deb is already the newest version (1.114ubuntu3).
authbind is already the newest version (2.1.1+nmu1).
bind9 is already the newest version (1:9.10.3.dfsg.P2-5).
bind9utils is already the newest version (1:9.10.3.dfsg.P2-5).
build-essential is already the newest version (12.1ubuntu2).
curl is already the newest version (7.47.0-1ubuntu2).
debhelper is already the newest version (9.20160115ubuntu2).
dh-apport is already the newest version (2.20-0ubuntu3).
dh-systemd is already the newest version (1.29ubuntu1).
distro-info is already the newest version (0.14build1).
dnsutils is already the newest version (1:9.10.3.dfsg.P2-5)...

4802. By Blake Rouse on 2016-03-17

Fix formatting.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/provisioningserver/utils/network.py'
2--- src/provisioningserver/utils/network.py 2016-03-16 19:27:17 +0000
3+++ src/provisioningserver/utils/network.py 2016-03-17 18:23:46 +0000
4@@ -625,28 +625,12 @@
5 exclude_types = ["loopback", "ipip"]
6 if not running_in_container():
7 exclude_types.append("ethernet")
8- original_ipaddr_info = get_ip_addr()
9 ipaddr_info = {
10 name: ipaddr
11- for name, ipaddr in original_ipaddr_info.items()
12+ for name, ipaddr in get_ip_addr().items()
13 if (ipaddr["type"] not in exclude_types and
14 not ipaddr["type"].startswith("unknown-"))
15 }
16-
17- # It's not always 100% that we can determine that this machine is running
18- # in a container. To handle this case we include "ethernet" interfaces
19- # when no other interfaces could be identified. See lp:1554999.
20- if len(ipaddr_info) == 0 and "ethernet" in exclude_types:
21- # No interfaces have been discovered from "ip addr" excluding
22- # "ethernet". Re-filter including ethernet.
23- exclude_types.remove("ethernet")
24- ipaddr_info = {
25- name: ipaddr
26- for name, ipaddr in original_ipaddr_info.items()
27- if (ipaddr["type"] not in exclude_types and
28- not ipaddr["type"].startswith("unknown-"))
29- }
30-
31 for name, ipaddr in ipaddr_info.items():
32 iface_type = "physical"
33 parents = []
34
35=== modified file 'src/provisioningserver/utils/ps.py'
36--- src/provisioningserver/utils/ps.py 2016-03-07 23:20:52 +0000
37+++ src/provisioningserver/utils/ps.py 2016-03-17 18:23:46 +0000
38@@ -7,9 +7,14 @@
39 'running_in_container',
40 ]
41
42+from functools import lru_cache
43 import os
44
45 from provisioningserver.utils.fs import read_text_file
46+from provisioningserver.utils.shell import (
47+ call_and_check,
48+ ExternalProcessError,
49+)
50
51
52 def is_pid_in_container(pid, proc_path="/proc"):
53@@ -28,9 +33,17 @@
54 return False
55
56
57-def running_in_container(proc_path="/proc"):
58+@lru_cache(maxsize=1)
59+def running_in_container():
60 """Return True if running in an LXC or Docker container."""
61- return is_pid_in_container(1, proc_path=proc_path)
62+ try:
63+ call_and_check(["systemd-detect-virt", "-c"])
64+ except ExternalProcessError:
65+ # Exited non-zero so not in a container.
66+ return False
67+ else:
68+ # Exited zero so inside a container.
69+ return True
70
71
72 def get_running_pids_with_command(
73@@ -56,7 +69,7 @@
74 pids.append(int(pid))
75
76 if (exclude_container_processes and
77- not running_in_container(proc_path=proc_path)):
78+ not running_in_container()):
79 return [
80 pid
81 for pid in pids
82
83=== modified file 'src/provisioningserver/utils/tests/test_network.py'
84--- src/provisioningserver/utils/tests/test_network.py 2016-03-16 19:27:17 +0000
85+++ src/provisioningserver/utils/tests/test_network.py 2016-03-17 18:23:46 +0000
86@@ -837,14 +837,8 @@
87 }
88 self.assertInterfacesResult(ip_addr, {}, {}, MatchesDict({}))
89
90- def test__ignores_ethernet_when_physical(self):
91+ def test__ignores_ethernet(self):
92 ip_addr = {
93- "eth0": {
94- "type": "ethernet.physical",
95- "mac": factory.make_mac_address(),
96- "flags": ["UP"],
97- "inet": [],
98- },
99 "vnet": {
100 "type": "ethernet",
101 "mac": factory.make_mac_address(),
102@@ -852,38 +846,7 @@
103 "inet": ["192.168.122.2/24"],
104 },
105 }
106- expected_result = MatchesDict({
107- "eth0": MatchesDict({
108- "type": Equals("physical"),
109- "mac_address": Equals(ip_addr["eth0"]["mac"]),
110- "enabled": Is(True),
111- "parents": Equals([]),
112- "links": Equals([]),
113- "source": Equals("ipaddr"),
114- }),
115- })
116- self.assertInterfacesResult(ip_addr, {}, {}, expected_result)
117-
118- def test__includes_ethernet_when_only_ethernet(self):
119- ip_addr = {
120- "vnet": {
121- "type": "ethernet",
122- "mac": factory.make_mac_address(),
123- "flags": ["UP"],
124- "inet": [],
125- },
126- }
127- expected_result = MatchesDict({
128- "vnet": MatchesDict({
129- "type": Equals("physical"),
130- "mac_address": Equals(ip_addr["vnet"]["mac"]),
131- "enabled": Is(True),
132- "parents": Equals([]),
133- "links": Equals([]),
134- "source": Equals("ipaddr"),
135- }),
136- })
137- self.assertInterfacesResult(ip_addr, {}, {}, expected_result)
138+ self.assertInterfacesResult(ip_addr, {}, {}, MatchesDict({}))
139
140 def test__ignores_ipip(self):
141 ip_addr = {
142
143=== modified file 'src/provisioningserver/utils/tests/test_ps.py'
144--- src/provisioningserver/utils/tests/test_ps.py 2016-03-07 23:20:52 +0000
145+++ src/provisioningserver/utils/tests/test_ps.py 2016-03-17 18:23:46 +0000
146@@ -11,12 +11,14 @@
147
148 from maastesting.factory import factory
149 from maastesting.testcase import MAASTestCase
150+from provisioningserver.utils import ps as ps_module
151 from provisioningserver.utils.fs import atomic_write
152 from provisioningserver.utils.ps import (
153 get_running_pids_with_command,
154 is_pid_in_container,
155 running_in_container,
156 )
157+from provisioningserver.utils.shell import ExternalProcessError
158
159
160 NOT_IN_CONTAINER = dedent("""\
161@@ -92,30 +94,17 @@
162
163 class TestRunningInContainer(MAASTestCase):
164
165- scenarios = (
166- ("not_in_container", {
167- "result": False,
168- "cgroup": NOT_IN_CONTAINER,
169- }),
170- ("in_docker_container", {
171- "result": True,
172- "cgroup": IN_DOCKER_CONTAINER,
173- }),
174- ("in_lxc_container", {
175- "result": True,
176- "cgroup": IN_LXC_CONTAINER,
177- }),
178- )
179+ def test__returns_False_when_ExternalProcessError(self):
180+ mock_call = self.patch(ps_module, "call_and_check")
181+ mock_call.side_effect = ExternalProcessError(
182+ 1, ["systemd-detect-virt", "-c"], output="none")
183+ running_in_container.cache_clear()
184+ self.assertFalse(running_in_container())
185
186- def test__result(self):
187- proc_path = self.make_dir()
188- pid_path = os.path.join(proc_path, "1")
189- os.mkdir(pid_path)
190- atomic_write(
191- self.cgroup.encode("ascii"),
192- os.path.join(pid_path, "cgroup"))
193- self.assertEqual(
194- self.result, running_in_container(proc_path=proc_path))
195+ def test__returns_True_when_not_ExternalProcessError(self):
196+ self.patch(ps_module, "call_and_check")
197+ running_in_container.cache_clear()
198+ self.assertTrue(running_in_container())
199
200
201 class TestGetRunningPIDsWithCommand(MAASTestCase):
202@@ -162,6 +151,9 @@
203 self.make_process(
204 proc_path, pid,
205 in_container=True, command=command)
206+ mock_running_in_container = self.patch(
207+ ps_module, "running_in_container")
208+ mock_running_in_container.return_value = False
209 self.assertItemsEqual(
210 pids_running_command,
211 get_running_pids_with_command(command, proc_path=proc_path))
212@@ -201,6 +193,9 @@
213 self.make_process(
214 proc_path, pid,
215 in_container=True, command=factory.make_name("command"))
216+ mock_running_in_container = self.patch(
217+ ps_module, "running_in_container")
218+ mock_running_in_container.return_value = True
219 self.assertItemsEqual(
220 pids_running_command,
221 get_running_pids_with_command(command, proc_path=proc_path))