Merge "/bin/conman: Run ifplugd script for eth0."
diff --git a/conman/connection_manager.py b/conman/connection_manager.py
index 9bbfb78..c484d4a 100755
--- a/conman/connection_manager.py
+++ b/conman/connection_manager.py
@@ -214,11 +214,11 @@
# explicitly.
if os.path.exists(os.path.join(self._interface_status_dir,
self.ETHERNET_STATUS_FILE)):
- self._ethernet_file_exists = True
self._process_file(self._interface_status_dir, self.ETHERNET_STATUS_FILE)
else:
- self._ethernet_file_exists = False
- self.bridge.ethernet = self.is_ethernet_up()
+ ethernet_up = self.is_ethernet_up()
+ self.bridge.ethernet = ethernet_up
+ self.ifplugd_action('eth0', ethernet_up)
for path, prefix in ((self._moca_status_dir, self.MOCA_NODE_FILE_PREFIX),
(self._status_dir, self.COMMAND_FILE_PREFIX),
@@ -402,11 +402,6 @@
return self.bridge.internet() or any(wifi.internet() for wifi in self.wifi)
def _update_interfaces_and_routes(self):
- # If ifplugd has never written an ethernet status file, poll manually before
- # updating routes.
- if not self._ethernet_file_exists:
- self.bridge.ethernet = self.is_ethernet_up()
-
self.bridge.update_routes()
for wifi in self.wifi:
wifi.update_routes()
@@ -457,7 +452,6 @@
if filename == self.ETHERNET_STATUS_FILE:
try:
self.bridge.ethernet = bool(int(contents))
- self._ethernet_file_exists = True
except ValueError:
logging.error('Status file contents should be 0 or 1, not %s',
contents)
@@ -502,15 +496,16 @@
self.bridge.add_moca_station(node)
has_moca = self.bridge.moca
if had_moca != has_moca:
- self.tell_ifplugd_about_moca(has_moca)
+ self.ifplugd_action('moca0', has_moca)
def interface_by_name(self, interface_name):
for ifc in [self.bridge] + self.wifi:
if ifc.name == interface_name:
return ifc
- def tell_ifplugd_about_moca(self, up):
- subprocess.call(self.IFPLUGD_ACTION + ['moca0', 'up' if up else 'down'])
+ def ifplugd_action(self, interface_name, up):
+ subprocess.call(self.IFPLUGD_ACTION + [interface_name,
+ 'up' if up else 'down'])
def _wifi_scan(self, wifi):
"""Perform a wifi scan and update wifi.cycler."""
diff --git a/conman/connection_manager_test.py b/conman/connection_manager_test.py
index 5388b22..afc5c9a 100755
--- a/conman/connection_manager_test.py
+++ b/conman/connection_manager_test.py
@@ -178,6 +178,8 @@
WIFI_SETCLIENT = ['echo', 'setclient']
IFUP = ['echo', 'ifup']
IFPLUGD_ACTION = ['echo', 'ifplugd.action']
+ # This simulates the output of 'ip link' when eth0 is up.
+ IP_LINK = ['echo', 'eth0 LOWER_UP']
def __init__(self, *args, **kwargs):
super(ConnectionManager, self).__init__(*args, **kwargs)
@@ -216,8 +218,7 @@
else:
open(socket, 'w')
wifi.set_connection_check_result('restricted')
- self.write_interface_status_file(wifi.name, '1')
- self.write_gateway_file(wifi.name)
+ self.ifplugd_action(wifi.name, True)
return True
return False
@@ -251,10 +252,16 @@
super(ConnectionManager, self)._wifi_scan(wifi)
wifi.wifi_scan_counter += 1
- def tell_ifplugd_about_moca(self, up):
+ def ifplugd_action(self, interface_name, up):
# Typically, when moca comes up, conman calls ifplugd.action, which writes
- # this file.
- self.write_interface_status_file('moca0', '1' if up else '0')
+ # this file. Also, when conman starts, it calls ifplugd.action for eth0.
+ self.write_interface_status_file(interface_name, '1' if up else '0')
+
+ # ifplugd calls run-dhclient, which results in a gateway file if the link is
+ # up (and working).
+ if up:
+ self.write_gateway_file('br0' if interface_name in ('eth0', 'moca0')
+ else interface_name)
# Non-overrides
@@ -317,12 +324,7 @@
f.write(value)
def set_ethernet(self, up):
- self.write_interface_status_file('eth0', '1' if up else '0')
-
- if up:
- # On the real system, ifplugd would run dhclient, which would write a
- # gateway file.
- self.write_gateway_file('br0')
+ self.ifplugd_action('eth0', up)
def set_moca(self, up):
moca_node1_file = os.path.join(self._moca_status_dir,
@@ -331,11 +333,6 @@
f.write(FAKE_MOCA_NODE1_FILE if up else
FAKE_MOCA_NODE1_FILE_DISCONNECTED)
- if up:
- # On the real system, ifplugd would run dhclient, which would write a
- # gateway file.
- self.write_gateway_file('br0')
-
def run_until_interface_update(self):
while self._interface_update_counter == 0:
self.run_once()
@@ -407,15 +404,12 @@
c: A ConnectionManager set up by @connection_manager_test.
"""
- # Initially, no access.
- wvtest.WVFAIL(c.acs())
- wvtest.WVFAIL(c.internet())
+ # Initially, there is ethernet access (via explicit check of ethernet status,
+ # rather than the interface status file).
+ wvtest.WVPASS(c.acs())
+ wvtest.WVPASS(c.internet())
c.run_once()
-
- # Bring up ethernet, access.
- c.set_ethernet(True)
- c.run_once()
wvtest.WVPASS(c.acs())
wvtest.WVPASS(c.internet())
wvtest.WVPASS(c.bridge.current_route())
diff --git a/conman/interface.py b/conman/interface.py
index c2de318..d814950 100755
--- a/conman/interface.py
+++ b/conman/interface.py
@@ -48,10 +48,13 @@
Whether the connection is working.
"""
if not self.links:
+ logging.debug('Connection check for %s failed due to no links', self.name)
return False
- logging.debug('gateway ip for %s is %s', self.name, self._gateway_ip)
+ logging.debug('Gateway ip for %s is %s', self.name, self._gateway_ip)
if self._gateway_ip is None:
+ logging.debug('Connection check for %s failed due to no gateway IP',
+ self.name)
return False
# Temporarily add a route to make sure the connection check can be run.
@@ -65,6 +68,7 @@
'via', self._gateway_ip,
'dev', self.name,
'metric', str(METRIC_TEMPORARY_CONNECTION_CHECK))
+ added_temporary_route = True
cmd = [self.CONNECTION_CHECK, '-I', self.name]
if check_acs:
@@ -72,6 +76,10 @@
with open(os.devnull, 'w') as devnull:
result = subprocess.call(cmd, stdout=devnull, stderr=devnull) == 0
+ logging.debug('Connection check%s for %s %s',
+ ' (ACS)' if check_acs else '',
+ self.name,
+ 'passed' if result else 'failed')
# Delete the temporary route.
if added_temporary_route:
@@ -205,22 +213,27 @@
deciding whether to add or remove routes.
"""
logging.debug('Updating routes for %s', self.name)
- # We care about the distinction between None (unknown) and False (known
- # inaccessible) here.
- # pylint: disable=g-explicit-bool-comparison
- maybe_had_acs = self._has_acs != False
- maybe_had_internet = self._has_internet != False
+ maybe_had_acs = self._has_acs
+ maybe_had_internet = self._has_internet
if expire_cache:
self.expire_connection_status_cache()
has_acs = self.acs()
has_internet = self.internet()
- if ((not maybe_had_acs and has_acs) or
- (not maybe_had_internet and has_internet)):
+
+ # This is a little confusing: We want to try adding a route if we _may_
+ # have gone from no access to some access, and we want to try deleting the
+ # route if we _may_ have lost *all* access. So the first condition checks
+ # for truthiness but the elif checks for explicit Falsity (i.e. excluding
+ # the None/unknown case).
+ had_access = maybe_had_acs or maybe_had_internet
+ # pylint: disable=g-explicit-bool-comparison
+ maybe_had_access = maybe_had_acs != False or maybe_had_internet != False
+ has_access = has_acs or has_internet
+ if not had_access and has_access:
self.add_route()
- elif ((maybe_had_acs or maybe_had_internet) and not
- (has_acs or has_internet)):
+ elif maybe_had_access and not has_access:
self.delete_route()
diff --git a/conman/interface_test.py b/conman/interface_test.py
index edc3541..9905fea 100755
--- a/conman/interface_test.py
+++ b/conman/interface_test.py
@@ -54,6 +54,7 @@
if k[0] == key[0]:
logging.debug('Deleting route for %r (generalized from %s)', k, key)
del self.routing_table[k]
+ break
class Bridge(FakeInterfaceMixin, interface.Bridge):
