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
=== modified file 'src/provisioningserver/utils/network.py'
--- src/provisioningserver/utils/network.py 2016-03-16 19:27:17 +0000
+++ src/provisioningserver/utils/network.py 2016-03-17 18:23:46 +0000
@@ -625,28 +625,12 @@
625 exclude_types = ["loopback", "ipip"]625 exclude_types = ["loopback", "ipip"]
626 if not running_in_container():626 if not running_in_container():
627 exclude_types.append("ethernet")627 exclude_types.append("ethernet")
628 original_ipaddr_info = get_ip_addr()
629 ipaddr_info = {628 ipaddr_info = {
630 name: ipaddr629 name: ipaddr
631 for name, ipaddr in original_ipaddr_info.items()630 for name, ipaddr in get_ip_addr().items()
632 if (ipaddr["type"] not in exclude_types and631 if (ipaddr["type"] not in exclude_types and
633 not ipaddr["type"].startswith("unknown-"))632 not ipaddr["type"].startswith("unknown-"))
634 }633 }
635
636 # It's not always 100% that we can determine that this machine is running
637 # in a container. To handle this case we include "ethernet" interfaces
638 # when no other interfaces could be identified. See lp:1554999.
639 if len(ipaddr_info) == 0 and "ethernet" in exclude_types:
640 # No interfaces have been discovered from "ip addr" excluding
641 # "ethernet". Re-filter including ethernet.
642 exclude_types.remove("ethernet")
643 ipaddr_info = {
644 name: ipaddr
645 for name, ipaddr in original_ipaddr_info.items()
646 if (ipaddr["type"] not in exclude_types and
647 not ipaddr["type"].startswith("unknown-"))
648 }
649
650 for name, ipaddr in ipaddr_info.items():634 for name, ipaddr in ipaddr_info.items():
651 iface_type = "physical"635 iface_type = "physical"
652 parents = []636 parents = []
653637
=== modified file 'src/provisioningserver/utils/ps.py'
--- src/provisioningserver/utils/ps.py 2016-03-07 23:20:52 +0000
+++ src/provisioningserver/utils/ps.py 2016-03-17 18:23:46 +0000
@@ -7,9 +7,14 @@
7 'running_in_container',7 'running_in_container',
8 ]8 ]
99
10from functools import lru_cache
10import os11import os
1112
12from provisioningserver.utils.fs import read_text_file13from provisioningserver.utils.fs import read_text_file
14from provisioningserver.utils.shell import (
15 call_and_check,
16 ExternalProcessError,
17)
1318
1419
15def is_pid_in_container(pid, proc_path="/proc"):20def is_pid_in_container(pid, proc_path="/proc"):
@@ -28,9 +33,17 @@
28 return False33 return False
2934
3035
31def running_in_container(proc_path="/proc"):36@lru_cache(maxsize=1)
37def running_in_container():
32 """Return True if running in an LXC or Docker container."""38 """Return True if running in an LXC or Docker container."""
33 return is_pid_in_container(1, proc_path=proc_path)39 try:
40 call_and_check(["systemd-detect-virt", "-c"])
41 except ExternalProcessError:
42 # Exited non-zero so not in a container.
43 return False
44 else:
45 # Exited zero so inside a container.
46 return True
3447
3548
36def get_running_pids_with_command(49def get_running_pids_with_command(
@@ -56,7 +69,7 @@
56 pids.append(int(pid))69 pids.append(int(pid))
5770
58 if (exclude_container_processes and71 if (exclude_container_processes and
59 not running_in_container(proc_path=proc_path)):72 not running_in_container()):
60 return [73 return [
61 pid74 pid
62 for pid in pids75 for pid in pids
6376
=== modified file 'src/provisioningserver/utils/tests/test_network.py'
--- src/provisioningserver/utils/tests/test_network.py 2016-03-16 19:27:17 +0000
+++ src/provisioningserver/utils/tests/test_network.py 2016-03-17 18:23:46 +0000
@@ -837,14 +837,8 @@
837 }837 }
838 self.assertInterfacesResult(ip_addr, {}, {}, MatchesDict({}))838 self.assertInterfacesResult(ip_addr, {}, {}, MatchesDict({}))
839839
840 def test__ignores_ethernet_when_physical(self):840 def test__ignores_ethernet(self):
841 ip_addr = {841 ip_addr = {
842 "eth0": {
843 "type": "ethernet.physical",
844 "mac": factory.make_mac_address(),
845 "flags": ["UP"],
846 "inet": [],
847 },
848 "vnet": {842 "vnet": {
849 "type": "ethernet",843 "type": "ethernet",
850 "mac": factory.make_mac_address(),844 "mac": factory.make_mac_address(),
@@ -852,38 +846,7 @@
852 "inet": ["192.168.122.2/24"],846 "inet": ["192.168.122.2/24"],
853 },847 },
854 }848 }
855 expected_result = MatchesDict({849 self.assertInterfacesResult(ip_addr, {}, {}, MatchesDict({}))
856 "eth0": MatchesDict({
857 "type": Equals("physical"),
858 "mac_address": Equals(ip_addr["eth0"]["mac"]),
859 "enabled": Is(True),
860 "parents": Equals([]),
861 "links": Equals([]),
862 "source": Equals("ipaddr"),
863 }),
864 })
865 self.assertInterfacesResult(ip_addr, {}, {}, expected_result)
866
867 def test__includes_ethernet_when_only_ethernet(self):
868 ip_addr = {
869 "vnet": {
870 "type": "ethernet",
871 "mac": factory.make_mac_address(),
872 "flags": ["UP"],
873 "inet": [],
874 },
875 }
876 expected_result = MatchesDict({
877 "vnet": MatchesDict({
878 "type": Equals("physical"),
879 "mac_address": Equals(ip_addr["vnet"]["mac"]),
880 "enabled": Is(True),
881 "parents": Equals([]),
882 "links": Equals([]),
883 "source": Equals("ipaddr"),
884 }),
885 })
886 self.assertInterfacesResult(ip_addr, {}, {}, expected_result)
887850
888 def test__ignores_ipip(self):851 def test__ignores_ipip(self):
889 ip_addr = {852 ip_addr = {
890853
=== modified file 'src/provisioningserver/utils/tests/test_ps.py'
--- src/provisioningserver/utils/tests/test_ps.py 2016-03-07 23:20:52 +0000
+++ src/provisioningserver/utils/tests/test_ps.py 2016-03-17 18:23:46 +0000
@@ -11,12 +11,14 @@
1111
12from maastesting.factory import factory12from maastesting.factory import factory
13from maastesting.testcase import MAASTestCase13from maastesting.testcase import MAASTestCase
14from provisioningserver.utils import ps as ps_module
14from provisioningserver.utils.fs import atomic_write15from provisioningserver.utils.fs import atomic_write
15from provisioningserver.utils.ps import (16from provisioningserver.utils.ps import (
16 get_running_pids_with_command,17 get_running_pids_with_command,
17 is_pid_in_container,18 is_pid_in_container,
18 running_in_container,19 running_in_container,
19)20)
21from provisioningserver.utils.shell import ExternalProcessError
2022
2123
22NOT_IN_CONTAINER = dedent("""\24NOT_IN_CONTAINER = dedent("""\
@@ -92,30 +94,17 @@
9294
93class TestRunningInContainer(MAASTestCase):95class TestRunningInContainer(MAASTestCase):
9496
95 scenarios = (97 def test__returns_False_when_ExternalProcessError(self):
96 ("not_in_container", {98 mock_call = self.patch(ps_module, "call_and_check")
97 "result": False,99 mock_call.side_effect = ExternalProcessError(
98 "cgroup": NOT_IN_CONTAINER,100 1, ["systemd-detect-virt", "-c"], output="none")
99 }),101 running_in_container.cache_clear()
100 ("in_docker_container", {102 self.assertFalse(running_in_container())
101 "result": True,
102 "cgroup": IN_DOCKER_CONTAINER,
103 }),
104 ("in_lxc_container", {
105 "result": True,
106 "cgroup": IN_LXC_CONTAINER,
107 }),
108 )
109103
110 def test__result(self):104 def test__returns_True_when_not_ExternalProcessError(self):
111 proc_path = self.make_dir()105 self.patch(ps_module, "call_and_check")
112 pid_path = os.path.join(proc_path, "1")106 running_in_container.cache_clear()
113 os.mkdir(pid_path)107 self.assertTrue(running_in_container())
114 atomic_write(
115 self.cgroup.encode("ascii"),
116 os.path.join(pid_path, "cgroup"))
117 self.assertEqual(
118 self.result, running_in_container(proc_path=proc_path))
119108
120109
121class TestGetRunningPIDsWithCommand(MAASTestCase):110class TestGetRunningPIDsWithCommand(MAASTestCase):
@@ -162,6 +151,9 @@
162 self.make_process(151 self.make_process(
163 proc_path, pid,152 proc_path, pid,
164 in_container=True, command=command)153 in_container=True, command=command)
154 mock_running_in_container = self.patch(
155 ps_module, "running_in_container")
156 mock_running_in_container.return_value = False
165 self.assertItemsEqual(157 self.assertItemsEqual(
166 pids_running_command,158 pids_running_command,
167 get_running_pids_with_command(command, proc_path=proc_path))159 get_running_pids_with_command(command, proc_path=proc_path))
@@ -201,6 +193,9 @@
201 self.make_process(193 self.make_process(
202 proc_path, pid,194 proc_path, pid,
203 in_container=True, command=factory.make_name("command"))195 in_container=True, command=factory.make_name("command"))
196 mock_running_in_container = self.patch(
197 ps_module, "running_in_container")
198 mock_running_in_container.return_value = True
204 self.assertItemsEqual(199 self.assertItemsEqual(
205 pids_running_command,200 pids_running_command,
206 get_running_pids_with_command(command, proc_path=proc_path))201 get_running_pids_with_command(command, proc_path=proc_path))