From 680da7d2991cb44a4e0b9b965165aae043fb3aa6 Mon Sep 17 00:00:00 2001
From: Michael Hudson-Doyle <michael.hudson@canonical.com>
Date: Mon, 25 Mar 2019 10:23:33 +1300
Subject: [PATCH 01/11] rename/redefine default route waiter to watcher

the callbacks now get called whenever the set of default routes changes,
not just when one appears
---
 subiquitycore/controllers/network.py | 36 +++++++++++++++-------------
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/subiquitycore/controllers/network.py b/subiquitycore/controllers/network.py
index 8dfbfde5..bc2b8d1b 100644
--- a/subiquitycore/controllers/network.py
+++ b/subiquitycore/controllers/network.py
@@ -91,14 +91,15 @@ class WaitForDefaultRouteTask(CancelableTask):
     def __repr__(self):
         return 'WaitForDefaultRouteTask(%r)' % (self.timeout,)
 
-    def got_route(self):
-        self.event_receiver.remove_default_route_waiter(self.got_route)
-        os.write(self.success_w, b'x')
+    def route_update(self, routes):
+        if routes:
+            self.event_receiver.remove_default_route_watcher(self.route_update)
+            os.write(self.success_w, b'x')
 
     def start(self):
         self.fail_r, self.fail_w = os.pipe()
         self.success_r, self.success_w = os.pipe()
-        self.event_receiver.add_default_route_waiter(self.got_route)
+        self.event_receiver.add_default_route_watcher(self.route_update)
 
     def _bg_run(self):
         try:
@@ -125,7 +126,7 @@ class SubiquityNetworkEventReceiver(NetworkEventReceiver):
     def __init__(self, model):
         self.model = model
         self.view = None
-        self.default_route_waiters = []
+        self.default_route_watchers = []
         self.default_routes = set()
 
     def new_link(self, ifindex, link):
@@ -154,20 +155,19 @@ class SubiquityNetworkEventReceiver(NetworkEventReceiver):
         ifindex = data['ifindex']
         if action == "NEW" or action == "CHANGE":
             self.default_routes.add(ifindex)
-            for waiter in self.default_route_waiters:
-                waiter()
         elif action == "DEL" and ifindex in self.default_routes:
             self.default_routes.remove(ifindex)
+        for watcher in self.default_route_watchers:
+            watcher(self.default_routes)
         log.debug('default routes %s', self.default_routes)
 
-    def add_default_route_waiter(self, waiter):
-        self.default_route_waiters.append(waiter)
-        if self.default_routes:
-            waiter()
+    def add_default_route_watcher(self, watcher):
+        self.default_route_watchers.append(watcher)
+        watcher(self.default_routes)
 
-    def remove_default_route_waiter(self, waiter):
-        if waiter in self.default_route_waiters:
-            self.default_route_waiters.remove(waiter)
+    def remove_default_route_watcher(self, watcher):
+        if watcher in self.default_route_watchers:
+            self.default_route_watchers.remove(watcher)
 
 
 default_netplan = '''
@@ -218,11 +218,13 @@ class NetworkController(BaseController, TaskWatcher):
         self.model.parse_netplan_configs(self.root)
 
         self.network_event_receiver = SubiquityNetworkEventReceiver(self.model)
-        self.network_event_receiver.add_default_route_waiter(self.got_route)
+        self.network_event_receiver.add_default_route_watcher(
+            self.route_watcher)
         self._done_by_action = False
 
-    def got_route(self):
-        self.signal.emit_signal('network-change')
+    def route_watcher(self, routes):
+        if routes:
+            self.signal.emit_signal('network-change')
 
     def start(self):
         self._observer_handles = []

From ff59de4a9cec3e00421a8ac87dc0e5e767ff8193 Mon Sep 17 00:00:00 2001
From: Michael Hudson-Doyle <michael.hudson@canonical.com>
Date: Mon, 25 Mar 2019 10:32:44 +1300
Subject: [PATCH 02/11] change "Done" button to "Continue without network" if
 there is no default route

---
 subiquitycore/ui/views/network.py | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/subiquitycore/ui/views/network.py b/subiquitycore/ui/views/network.py
index 973d31f9..60b899f6 100644
--- a/subiquitycore/ui/views/network.py
+++ b/subiquitycore/ui/views/network.py
@@ -51,6 +51,7 @@ from subiquitycore.ui.utils import (
     make_action_menu_row,
     screen,
     )
+from subiquitycore.ui.width import widget_width
 from .network_configure_manual_interface import (
     AddVlanStretchy,
     BondStretchy,
@@ -125,14 +126,17 @@ class NetworkView(BaseView):
             bp,
         ]
 
-        buttons = button_pile([
-                    done_btn(_("Done"), on_press=self.done),
+        self.buttons = button_pile([
+                    done_btn("TBD", on_press=self.done),  # See _route_watcher
                     back_btn(_("Back"), on_press=self.cancel),
                     ])
         self.bottom = Pile([
-            ('pack', buttons),
+            ('pack', self.buttons),
         ])
 
+        self.controller.network_event_receiver.add_default_route_watcher(
+            self._route_watcher)
+
         self.error_showing = False
 
         super().__init__(screen(
@@ -163,6 +167,19 @@ class NetworkView(BaseView):
         log.debug("_action %s %s", action.name, device.name)
         meth(device)
 
+    def _route_watcher(self, routes):
+        log.debug('view route_watcher %s', routes)
+        if routes:
+            label = _("Done")
+        else:
+            label = _("Continue without network")
+        self.buttons.base_widget[0].set_label(label)
+        self.buttons.width = max(
+            14,
+            widget_width(self.buttons.base_widget[0]),
+            widget_width(self.buttons.base_widget[1]),
+            )
+
     def _notes_for_device(self, dev):
         notes = []
         if dev.type == "eth" and not dev.info.is_connected:
@@ -378,7 +395,11 @@ class NetworkView(BaseView):
     def done(self, result=None):
         if self.error_showing:
             self.bottom.contents[0:2] = []
+        self.controller.network_event_receiver.remove_default_route_watcher(
+            self._route_watcher)
         self.controller.network_finish(self.model.render())
 
     def cancel(self, button=None):
+        self.controller.network_event_receiver.remove_default_route_watcher(
+            self._route_watcher)
         self.controller.cancel()

From 594db86618609363314006152f8144d13e15c035 Mon Sep 17 00:00:00 2001
From: Michael Hudson-Doyle <michael.hudson@canonical.com>
Date: Mon, 25 Mar 2019 10:34:05 +1300
Subject: [PATCH 03/11] have DownNetworkDevices do its work in a background
 thread

not that it should block, but no reason not to
---
 subiquitycore/controllers/network.py | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/subiquitycore/controllers/network.py b/subiquitycore/controllers/network.py
index bc2b8d1b..931bded4 100644
--- a/subiquitycore/controllers/network.py
+++ b/subiquitycore/controllers/network.py
@@ -52,10 +52,15 @@ class DownNetworkDevices(BackgroundTask):
         self.devs_to_delete = devs_to_delete
 
     def __repr__(self):
-        return 'DownNetworkDevices(%s)' % ([dev.name for dev in
-                                            self.devs_to_down],)
+        return 'DownNetworkDevices({}, {})'.format(
+            [dev.name for dev in self.devs_to_down],
+            [dev.name for dev in self.devs_to_delete],
+            )
 
     def start(self):
+        pass
+
+    def _bg_run(self):
         for dev in self.devs_to_down:
             try:
                 log.debug('downing %s', dev.name)
@@ -72,14 +77,8 @@ class DownNetworkDevices(BackgroundTask):
             except subprocess.CalledProcessError as cp:
                 log.info("deleting %s failed with %r", dev.name, cp.stderr)
 
-    def _bg_run(self):
-        return True
-
     def end(self, observer, fut):
-        if fut.result():
-            observer.task_succeeded()
-        else:
-            observer.task_failed()
+        observer.task_succeeded()
 
 
 class WaitForDefaultRouteTask(CancelableTask):

From 3bb9268974ef37e8dbd632db25ede1c4ee93d91c Mon Sep 17 00:00:00 2001
From: Michael Hudson-Doyle <michael.hudson@canonical.com>
Date: Mon, 25 Mar 2019 11:03:42 +1300
Subject: [PATCH 04/11] apply network changes immediately, rather than when the
 "Done" is pressed

diff makes this change look more involved than really necessary :(
---
 subiquitycore/controllers/network.py          | 211 +++++++-----------
 subiquitycore/models/network.py               |   1 +
 subiquitycore/ui/views/network.py             |  45 ++--
 .../network_configure_manual_interface.py     |   3 +
 ...test_network_configure_manual_interface.py |   2 +
 5 files changed, 109 insertions(+), 153 deletions(-)

diff --git a/subiquitycore/controllers/network.py b/subiquitycore/controllers/network.py
index 931bded4..e01e79c7 100644
--- a/subiquitycore/controllers/network.py
+++ b/subiquitycore/controllers/network.py
@@ -16,7 +16,6 @@
 from functools import partial
 import logging
 import os
-import select
 import subprocess
 
 import yaml
@@ -27,13 +26,11 @@ from subiquitycore.models.network import BondParameters, sanitize_config
 from subiquitycore.tasksequence import (
     BackgroundTask,
     BackgroundProcess,
-    CancelableTask,
     PythonSleep,
     TaskSequence,
     TaskWatcher,
     )
 from subiquitycore.ui.views.network import (
-    ApplyingConfigWidget,
     NetworkView,
     )
 from subiquitycore.controller import BaseController
@@ -81,44 +78,18 @@ class DownNetworkDevices(BackgroundTask):
         observer.task_succeeded()
 
 
-class WaitForDefaultRouteTask(CancelableTask):
+class ApplyWatcher(TaskWatcher):
+    def __init__(self, view):
+        self.view = view
 
-    def __init__(self, timeout, event_receiver):
-        self.timeout = timeout
-        self.event_receiver = event_receiver
+    def task_complete(self, stage):
+        pass
 
-    def __repr__(self):
-        return 'WaitForDefaultRouteTask(%r)' % (self.timeout,)
+    def tasks_finished(self):
+        self.view.hide_apply_spinner()
 
-    def route_update(self, routes):
-        if routes:
-            self.event_receiver.remove_default_route_watcher(self.route_update)
-            os.write(self.success_w, b'x')
-
-    def start(self):
-        self.fail_r, self.fail_w = os.pipe()
-        self.success_r, self.success_w = os.pipe()
-        self.event_receiver.add_default_route_watcher(self.route_update)
-
-    def _bg_run(self):
-        try:
-            r, _, _ = select.select([self.fail_r, self.success_r], [], [],
-                                    self.timeout)
-            return self.success_r in r
-        finally:
-            os.close(self.fail_r)
-            os.close(self.fail_w)
-            os.close(self.success_r)
-            os.close(self.success_w)
-
-    def end(self, observer, fut):
-        if fut.result():
-            observer.task_succeeded()
-        else:
-            observer.task_failed('timeout')
-
-    def cancel(self):
-        os.write(self.fail_w, b'x')
+    def task_error(self, stage, info):
+        self.view.show_network_error(stage, info)
 
 
 class SubiquityNetworkEventReceiver(NetworkEventReceiver):
@@ -195,7 +166,7 @@ network:
 '''
 
 
-class NetworkController(BaseController, TaskWatcher):
+class NetworkController(BaseController):
 
     root = "/"
 
@@ -203,9 +174,9 @@ class NetworkController(BaseController, TaskWatcher):
         super().__init__(common)
         self.model = self.base_model.network
         self.answers = self.all_answers.get("Network", {})
+        self.view = None
         if self.opts.dry_run:
             self.root = os.path.abspath(".subiquity")
-            self.tried_once = False
             netplan_path = self.netplan_path
             netplan_dir = os.path.dirname(netplan_path)
             if os.path.exists(netplan_dir):
@@ -258,7 +229,14 @@ class NetworkController(BaseController, TaskWatcher):
     def start_scan(self, dev):
         self.observer.trigger_scan(dev.ifindex)
 
+    def done(self):
+        self.view = None
+        self.model.has_network = bool(
+            self.network_event_receiver.default_routes)
+        self.signal.emit_signal('next-screen')
+
     def cancel(self):
+        self.view = None
         self.signal.emit_signal('prev-screen')
 
     def _action_get(self, id):
@@ -323,11 +301,11 @@ class NetworkController(BaseController, TaskWatcher):
             raise Exception("could not process action {}".format(action))
 
     def default(self):
-        view = NetworkView(self.model, self)
-        self.network_event_receiver.view = view
-        self.ui.set_body(view)
+        self.view = NetworkView(self.model, self)
+        self.network_event_receiver.view = self.view
+        self.ui.set_body(self.view)
         if self.answers.get('accept-default', False):
-            self.network_finish(self.model.render())
+            self.done()
         elif self.answers.get('actions', False):
             self._run_iterator(self._run_actions(self.answers['actions']))
 
@@ -339,6 +317,67 @@ class NetworkController(BaseController, TaskWatcher):
             netplan_config_file_name = '00-snapd-config.yaml'
         return os.path.join(self.root, 'etc/netplan', netplan_config_file_name)
 
+    def apply_config(self):
+        config = self.model.render()
+
+        devs_to_delete = []
+        devs_to_down = []
+        for dev in self.model.get_all_netdevs(include_deleted=True):
+            if dev.info is None:
+                continue
+            if dev.is_virtual:
+                devs_to_delete.append(dev)
+                continue
+            if dev.config != self.model.config.config_for_device(dev.info):
+                devs_to_down.append(dev)
+
+        log.debug("network config: \n%s",
+                  yaml.dump(sanitize_config(config), default_flow_style=False))
+
+        for p in netplan.configs_in_root(self.root, masked=True):
+            if p == self.netplan_path:
+                continue
+            os.rename(p, p + ".dist-" + self.opts.project)
+
+        write_file(self.netplan_path, '\n'.join((
+            ("# This is the network config written by '%s'" %
+             self.opts.project),
+            yaml.dump(config, default_flow_style=False))), omode="w")
+
+        self.model.parse_netplan_configs(self.root)
+
+        if self.opts.dry_run:
+            delay = 0.1/self.scale_factor
+            tasks = [
+                ('one', BackgroundProcess(['sleep', str(delay)])),
+                ('two', PythonSleep(delay)),
+                ('three', BackgroundProcess(['sleep', str(delay)])),
+                ]
+            if os.path.exists('/lib/netplan/generate'):
+                # If netplan appears to be installed, run generate to at
+                # least test that what we wrote is acceptable to netplan.
+                tasks.append(('generate',
+                              BackgroundProcess(['netplan', 'generate',
+                                                 '--root', self.root])))
+        else:
+            tasks = []
+            if devs_to_down or devs_to_delete:
+                tasks.extend([
+                    ('stop-networkd',
+                     BackgroundProcess(['systemctl',
+                                        'stop', 'systemd-networkd.service'])),
+                    ('down',
+                     DownNetworkDevices(self.observer.rtlistener,
+                                        devs_to_down, devs_to_delete)),
+                    ])
+            tasks.extend([
+                ('apply', BackgroundProcess(['netplan', 'apply'])),
+                ])
+
+        self.view.show_apply_spinner()
+        ts = TaskSequence(self.run_in_bg, tasks, ApplyWatcher(self.view))
+        ts.run()
+
     def add_vlan(self, device, vlan):
         return self.model.new_vlan(device, vlan)
 
@@ -361,87 +400,3 @@ class NetworkController(BaseController, TaskWatcher):
             existing.config['parameters'] = params
             existing.name = result['name']
             return existing
-
-    def network_finish(self, config):
-        log.debug("network config: \n%s",
-                  yaml.dump(sanitize_config(config), default_flow_style=False))
-
-        for p in netplan.configs_in_root(self.root, masked=True):
-            if p == self.netplan_path:
-                continue
-            os.rename(p, p + ".dist-" + self.opts.project)
-
-        write_file(self.netplan_path, '\n'.join((
-            ("# This is the network config written by '%s'" %
-             self.opts.project),
-            yaml.dump(config, default_flow_style=False))), omode="w")
-
-        self.model.parse_netplan_configs(self.root)
-        if self.opts.dry_run:
-            delay = 0.1/self.scale_factor
-            tasks = [
-                ('one', BackgroundProcess(['sleep', str(delay)])),
-                ('two', PythonSleep(delay)),
-                ('three', BackgroundProcess(['sleep', str(delay)])),
-                ]
-            if os.path.exists('/lib/netplan/generate'):
-                # If netplan appears to be installed, run generate to at
-                # least test that what we wrote is acceptable to netplan.
-                tasks.append(('generate',
-                              BackgroundProcess(['netplan', 'generate',
-                                                 '--root', self.root])))
-            if not self.tried_once:
-                tasks.append(
-                    ('timeout',
-                     WaitForDefaultRouteTask(3, self.network_event_receiver))
-                )
-                tasks.append(('fail', BackgroundProcess(['false'])))
-                self.tried_once = True
-        else:
-            devs_to_delete = []
-            devs_to_down = []
-            for dev in self.model.get_all_netdevs(include_deleted=True):
-                if dev.info is None:
-                    continue
-                devcfg = self.model.config.config_for_device(dev.info)
-                if dev.is_virtual:
-                    devs_to_delete.append(dev)
-                elif dev.config != devcfg:
-                    devs_to_down.append(dev)
-            tasks = []
-            if devs_to_down or devs_to_delete:
-                tasks.extend([
-                    ('stop-networkd',
-                     BackgroundProcess(['systemctl',
-                                        'stop', 'systemd-networkd.service'])),
-                    ('down',
-                     DownNetworkDevices(self.observer.rtlistener,
-                                        devs_to_down, devs_to_delete)),
-                    ])
-            tasks.extend([
-                ('apply', BackgroundProcess(['netplan', 'apply'])),
-                ('timeout',
-                 WaitForDefaultRouteTask(30, self.network_event_receiver)),
-                ])
-
-        def cancel():
-            self.cs.cancel()
-            self.task_error('canceled')
-        self.acw = ApplyingConfigWidget(len(tasks), cancel)
-        self.ui.frame.body.show_overlay(self.acw, min_width=60)
-
-        self.cs = TaskSequence(self.run_in_bg, tasks, self)
-        self.cs.run()
-
-    def task_complete(self, stage):
-        self.acw.advance()
-
-    def task_error(self, stage, info=None):
-        self.ui.frame.body.remove_overlay()
-        self.ui.frame.body.show_network_error(stage, info)
-        if self.answers.get('accept-default', False) or self._done_by_action:
-            self.network_finish(self.model.render())
-
-    def tasks_finished(self):
-        self.loop.set_alarm_in(
-            0.0, lambda loop, ud: self.signal.emit_signal('next-screen'))
diff --git a/subiquitycore/models/network.py b/subiquitycore/models/network.py
index e7bff54d..506b94cb 100644
--- a/subiquitycore/models/network.py
+++ b/subiquitycore/models/network.py
@@ -234,6 +234,7 @@ class NetworkModel(object):
     def __init__(self, support_wlan=True):
         self.support_wlan = support_wlan
         self.devices_by_name = {}  # Maps interface names to NetworkDev
+        self.has_network = False
 
     def parse_netplan_configs(self, netplan_root):
         self.config = netplan.Config()
diff --git a/subiquitycore/ui/views/network.py b/subiquitycore/ui/views/network.py
index 60b899f6..30f17cc2 100644
--- a/subiquitycore/ui/views/network.py
+++ b/subiquitycore/ui/views/network.py
@@ -23,8 +23,6 @@ import logging
 
 from urwid import (
     connect_signal,
-    LineBox,
-    ProgressBar,
     Text,
     )
 
@@ -35,14 +33,13 @@ from subiquitycore.models.network import (
 from subiquitycore.ui.actionmenu import ActionMenu
 from subiquitycore.ui.buttons import (
     back_btn,
-    cancel_btn,
     done_btn,
     menu_btn,
     )
 from subiquitycore.ui.container import (
     Pile,
-    WidgetWrap,
     )
+from subiquitycore.ui.spinner import Spinner
 from subiquitycore.ui.stretchy import StretchyOverlay
 from subiquitycore.ui.table import ColSpec, TablePile, TableRow
 from subiquitycore.ui.utils import (
@@ -66,26 +63,6 @@ from subiquitycore.view import BaseView
 log = logging.getLogger('subiquitycore.views.network')
 
 
-class ApplyingConfigWidget(WidgetWrap):
-
-    def __init__(self, step_count, cancel_func):
-        self.cancel_func = cancel_func
-        button = cancel_btn(_("Cancel"), on_press=self.do_cancel)
-        self.bar = ProgressBar(normal='progress_incomplete',
-                               complete='progress_complete',
-                               current=0, done=step_count)
-        box = LineBox(Pile([self.bar,
-                            button_pile([button])]),
-                      title=_("Applying network config"))
-        super().__init__(box)
-
-    def advance(self):
-        self.bar.current += 1
-
-    def do_cancel(self, sender):
-        self.cancel_func()
-
-
 def _stretchy_shower(cls, *args):
     def impl(self, device):
         self.show_stretchy_overlay(cls(self, device, *args))
@@ -180,6 +157,24 @@ class NetworkView(BaseView):
             widget_width(self.buttons.base_widget[1]),
             )
 
+    def show_apply_spinner(self):
+        s = Spinner(self.controller.loop)
+        s.start()
+        c = TablePile([
+            TableRow([
+                Text(_("Applying changes")),
+                s,
+                ]),
+            ], align='center')
+        self.bottom.contents[0:0] = [
+            (c, self.bottom.options()),
+            (Text(""), self.bottom.options()),
+            ]
+
+    def hide_apply_spinner(self):
+        if len(self.bottom.contents) > 2:
+            self.bottom.contents[0:2] = []
+
     def _notes_for_device(self, dev):
         notes = []
         if dev.type == "eth" and not dev.info.is_connected:
@@ -397,7 +392,7 @@ class NetworkView(BaseView):
             self.bottom.contents[0:2] = []
         self.controller.network_event_receiver.remove_default_route_watcher(
             self._route_watcher)
-        self.controller.network_finish(self.model.render())
+        self.controller.done()
 
     def cancel(self, button=None):
         self.controller.network_event_receiver.remove_default_route_watcher(
diff --git a/subiquitycore/ui/views/network_configure_manual_interface.py b/subiquitycore/ui/views/network_configure_manual_interface.py
index 20cb6aff..ed582918 100644
--- a/subiquitycore/ui/views/network_configure_manual_interface.py
+++ b/subiquitycore/ui/views/network_configure_manual_interface.py
@@ -247,6 +247,7 @@ class EditNetworkStretchy(Stretchy):
             self.device.config['dhcp{v}'.format(v=self.ip_version)] = True
         else:
             pass
+        self.parent.controller.apply_config()
         self.parent.update_link(self.device)
         self.parent.remove_overlay()
 
@@ -300,6 +301,7 @@ class AddVlanStretchy(Stretchy):
         dev = self.parent.controller.add_vlan(
             self.device, self.form.vlan.value)
         self.parent.new_link(dev)
+        self.parent.controller.apply_config()
 
     def cancel(self, sender=None):
         self.parent.remove_overlay()
@@ -479,6 +481,7 @@ class BondStretchy(Stretchy):
         for dev in touched_devices:
             self.parent.update_link(dev)
         self.parent.remove_overlay()
+        self.parent.controller.apply_config()
 
     def cancel(self, sender=None):
         self.parent.remove_overlay()
diff --git a/subiquitycore/ui/views/tests/test_network_configure_manual_interface.py b/subiquitycore/ui/views/tests/test_network_configure_manual_interface.py
index 037f1fe7..de0f18f2 100644
--- a/subiquitycore/ui/views/tests/test_network_configure_manual_interface.py
+++ b/subiquitycore/ui/views/tests/test_network_configure_manual_interface.py
@@ -3,6 +3,7 @@ from unittest import mock
 
 import urwid
 
+from subiquitycore.controllers.network import NetworkController
 from subiquitycore.models.network import NetworkDev
 from subiquitycore.testing import view_helpers
 from subiquitycore.ui.views.network_configure_manual_interface import (
@@ -28,6 +29,7 @@ class TestNetworkConfigureIPv4InterfaceView(unittest.TestCase):
         device.config = {}
         base_view = BaseView(urwid.Text(""))
         base_view.update_link = lambda device: None
+        base_view.controller = mock.create_autospec(spec=NetworkController)
         stretchy = EditNetworkStretchy(base_view, device, 4)
         base_view.show_stretchy_overlay(stretchy)
         stretchy.method_form.method.value = "manual"

From eae2497d41673e2a0207ffe37ee695cd71b817f9 Mon Sep 17 00:00:00 2001
From: Michael Hudson-Doyle <michael.hudson@canonical.com>
Date: Mon, 25 Mar 2019 11:30:26 +1300
Subject: [PATCH 05/11] auto disable inactive NICs

This disables any NIC that does not have a global address by the first
time the network screen is shown
---
 subiquitycore/controllers/network.py | 29 ++++++++++++++++++++++++++--
 subiquitycore/models/network.py      |  1 +
 subiquitycore/ui/views/network.py    |  5 ++++-
 3 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/subiquitycore/controllers/network.py b/subiquitycore/controllers/network.py
index e01e79c7..821fbbea 100644
--- a/subiquitycore/controllers/network.py
+++ b/subiquitycore/controllers/network.py
@@ -175,6 +175,7 @@ class NetworkController(BaseController):
         self.model = self.base_model.network
         self.answers = self.all_answers.get("Network", {})
         self.view = None
+        self.view_shown = False
         if self.opts.dry_run:
             self.root = os.path.abspath(".subiquity")
             netplan_path = self.netplan_path
@@ -300,8 +301,31 @@ class NetworkController(BaseController):
         else:
             raise Exception("could not process action {}".format(action))
 
+    def update_initial_configs(self):
+        # Any device that does not have a (global) address by the time
+        # we get to the network screen is marked as disabled, with an
+        # explanation.
+        for dev in self.model.get_all_netdevs():
+            has_global_address = False
+            if dev.info is None:
+                continue
+            for a in dev.info.addresses.values():
+                if a.scope == "global":
+                    has_global_address = True
+                    break
+            if not has_global_address:
+                dev.remove_ip_networks_for_version(4)
+                dev.remove_ip_networks_for_version(6)
+                log.debug("disabling %s", dev.name)
+                dev.disabled_reason = _("autoconfiguration failed")
+
     def default(self):
+        if not self.view_shown:
+            self.update_initial_configs()
         self.view = NetworkView(self.model, self)
+        if not self.view_shown:
+            self.apply_config(silent=True)
+            self.view_shown = True
         self.network_event_receiver.view = self.view
         self.ui.set_body(self.view)
         if self.answers.get('accept-default', False):
@@ -317,7 +341,7 @@ class NetworkController(BaseController):
             netplan_config_file_name = '00-snapd-config.yaml'
         return os.path.join(self.root, 'etc/netplan', netplan_config_file_name)
 
-    def apply_config(self):
+    def apply_config(self, silent=False):
         config = self.model.render()
 
         devs_to_delete = []
@@ -374,7 +398,8 @@ class NetworkController(BaseController):
                 ('apply', BackgroundProcess(['netplan', 'apply'])),
                 ])
 
-        self.view.show_apply_spinner()
+        if not silent:
+            self.view.show_apply_spinner()
         ts = TaskSequence(self.run_in_bg, tasks, ApplyWatcher(self.view))
         ts.run()
 
diff --git a/subiquitycore/models/network.py b/subiquitycore/models/network.py
index 506b94cb..faf7cc1d 100644
--- a/subiquitycore/models/network.py
+++ b/subiquitycore/models/network.py
@@ -110,6 +110,7 @@ class NetworkDev(object):
         self.type = typ
         self.config = {}
         self.info = None
+        self.disabled_reason = None
 
     def dhcp_addresses(self):
         r = {4: [], 6: []}
diff --git a/subiquitycore/ui/views/network.py b/subiquitycore/ui/views/network.py
index 30f17cc2..219938b5 100644
--- a/subiquitycore/ui/views/network.py
+++ b/subiquitycore/ui/views/network.py
@@ -224,7 +224,10 @@ class NetworkView(BaseView):
                 if dev2.type == "vlan" and dev.name == dev2.config.get('link'):
                     break
             else:
-                address_info.append((Text(_("disabled")), Text("")))
+                reason = dev.disabled_reason
+                if reason is None:
+                    reason = ""
+                address_info.append((Text(_("disabled")), Text(reason)))
         rows = []
         for label, value in address_info:
             rows.append(TableRow([Text(""), label, (2, value)]))

From b571b3d1a9ebc6e90f653b91731bfde39139c26b Mon Sep 17 00:00:00 2001
From: Michael Hudson-Doyle <michael.hudson@canonical.com>
Date: Mon, 25 Mar 2019 11:33:07 +1300
Subject: [PATCH 06/11] track and display DHCP status

Mark all devices with DHCP enabled as pending when the config is
applied, and mark them as timed out ten seconds later if they have not
received any addresses.
---
 subiquitycore/controllers/network.py | 24 ++++++++++++++++++++++++
 subiquitycore/models/network.py      | 12 ++++++++++++
 subiquitycore/ui/views/network.py    |  9 ++++++++-
 3 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/subiquitycore/controllers/network.py b/subiquitycore/controllers/network.py
index 821fbbea..f6f61c10 100644
--- a/subiquitycore/controllers/network.py
+++ b/subiquitycore/controllers/network.py
@@ -176,6 +176,7 @@ class NetworkController(BaseController):
         self.answers = self.all_answers.get("Network", {})
         self.view = None
         self.view_shown = False
+        self.dhcp_check_handle = None
         if self.opts.dry_run:
             self.root = os.path.abspath(".subiquity")
             netplan_path = self.netplan_path
@@ -319,6 +320,13 @@ class NetworkController(BaseController):
                 log.debug("disabling %s", dev.name)
                 dev.disabled_reason = _("autoconfiguration failed")
 
+    def check_dchp_results(self, device_versions):
+        log.debug('check_dchp_results for %s', device_versions)
+        for dev, v in device_versions:
+            if not dev.dhcp_addresses()[v]:
+                dev.set_dhcp_state(v, "TIMEDOUT")
+                self.network_event_receiver.update_link(dev.ifindex)
+
     def default(self):
         if not self.view_shown:
             self.update_initial_configs()
@@ -342,11 +350,22 @@ class NetworkController(BaseController):
         return os.path.join(self.root, 'etc/netplan', netplan_config_file_name)
 
     def apply_config(self, silent=False):
+        if self.dhcp_check_handle is not None:
+            self.loop.remove_alarm(self.dhcp_check_handle)
+            self.dhcp_check_handle = None
+
         config = self.model.render()
 
         devs_to_delete = []
         devs_to_down = []
+        dhcp_device_versions = []
         for dev in self.model.get_all_netdevs(include_deleted=True):
+            for v in 4, 6:
+                if dev.dhcp_enabled(v):
+                    if not silent:
+                        dev.set_dhcp_state(v, "PENDING")
+                        self.network_event_receiver.update_link(dev.ifindex)
+                    dhcp_device_versions.append((dev, v))
             if dev.info is None:
                 continue
             if dev.is_virtual:
@@ -402,6 +421,11 @@ class NetworkController(BaseController):
             self.view.show_apply_spinner()
         ts = TaskSequence(self.run_in_bg, tasks, ApplyWatcher(self.view))
         ts.run()
+        if dhcp_device_versions:
+            self.dhcp_check_handle = self.loop.set_alarm_in(
+                10,
+                lambda loop, ud: self.check_dchp_results(ud),
+                dhcp_device_versions)
 
     def add_vlan(self, device, vlan):
         return self.model.new_vlan(device, vlan)
diff --git a/subiquitycore/models/network.py b/subiquitycore/models/network.py
index faf7cc1d..185116b0 100644
--- a/subiquitycore/models/network.py
+++ b/subiquitycore/models/network.py
@@ -111,6 +111,10 @@ class NetworkDev(object):
         self.config = {}
         self.info = None
         self.disabled_reason = None
+        self._dhcp_state = {
+            4: None,
+            6: None,
+            }
 
     def dhcp_addresses(self):
         r = {4: [], 6: []}
@@ -129,6 +133,14 @@ class NetworkDev(object):
     def dhcp_enabled(self, version):
         return self.config.get('dhcp{v}'.format(v=version), False)
 
+    def dhcp_state(self, version):
+        if not self.config.get('dhcp{v}'.format(v=version), False):
+            return None
+        return self._dhcp_state[version]
+
+    def set_dhcp_state(self, version, state):
+        self._dhcp_state[version] = state
+
     @property
     def name(self):
         return self._name
diff --git a/subiquitycore/ui/views/network.py b/subiquitycore/ui/views/network.py
index 219938b5..0dfada48 100644
--- a/subiquitycore/ui/views/network.py
+++ b/subiquitycore/ui/views/network.py
@@ -201,10 +201,17 @@ class NetworkView(BaseView):
                 if addrs:
                     address_info.extend(
                         [(label, Text(addr)) for addr in addrs])
+                elif dev.dhcp_state(v) == "PENDING":
+                    s = Spinner(self.controller.loop, align='left')
+                    s.rate = 0.3
+                    s.start()
+                    address_info.append((label, s))
+                elif dev.dhcp_state(v) == "TIMEDOUT":
+                    address_info.append((label, Text(_("timed out"))))
                 else:
                     address_info.append((
                         label,
-                        Text(_("no address")),
+                        Text(_("unknown state {}".format(dev.dhcp_state(v))))
                         ))
             else:
                 addrs = []

From 307155a7fb1f703c86f056a8fa23136480915fd2 Mon Sep 17 00:00:00 2001
From: Michael Hudson-Doyle <michael.hudson@canonical.com>
Date: Mon, 25 Mar 2019 11:33:35 +1300
Subject: [PATCH 07/11] bump probert commit

---
 snapcraft.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/snapcraft.yaml b/snapcraft.yaml
index 773a81df..3f092f4b 100644
--- a/snapcraft.yaml
+++ b/snapcraft.yaml
@@ -96,5 +96,5 @@ parts:
     stage-packages: [libc6, libnl-3-dev, libnl-genl-3-dev, libnl-route-3-dev]
     source: https://github.com/CanonicalLtd/probert.git
     source-type: git
-    source-commit: 8b56d73068ec1f293d3db3b0b44966ede9ed1c94
+    source-commit: d4276ab044f4cc8311fb66a9050f6674726cbbf2
     requirements: requirements.txt

From 23488a6f2eee0825251c9b34b9ae596e16c68d8d Mon Sep 17 00:00:00 2001
From: Michael Hudson-Doyle <michael.hudson@canonical.com>
Date: Mon, 25 Mar 2019 15:06:59 +1300
Subject: [PATCH 08/11] some fixes found in testing

---
 subiquitycore/models/network.py   |  5 ++++-
 subiquitycore/ui/views/network.py | 20 ++++++++++++--------
 2 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/subiquitycore/models/network.py b/subiquitycore/models/network.py
index 185116b0..832cf417 100644
--- a/subiquitycore/models/network.py
+++ b/subiquitycore/models/network.py
@@ -131,7 +131,10 @@ class NetworkDev(object):
         return r
 
     def dhcp_enabled(self, version):
-        return self.config.get('dhcp{v}'.format(v=version), False)
+        if self.config is None:
+            return False
+        else:
+            return self.config.get('dhcp{v}'.format(v=version), False)
 
     def dhcp_state(self, version):
         if not self.config.get('dhcp{v}'.format(v=version), False):
diff --git a/subiquitycore/ui/views/network.py b/subiquitycore/ui/views/network.py
index 0dfada48..13a52758 100644
--- a/subiquitycore/ui/views/network.py
+++ b/subiquitycore/ui/views/network.py
@@ -138,6 +138,7 @@ class NetworkView(BaseView):
         self.del_link(device)
         for dev in touched_devs:
             self.update_link(dev)
+        self.controller.apply_config()
 
     def _action(self, sender, action, device):
         action, meth = action
@@ -224,13 +225,7 @@ class NetworkView(BaseView):
         if len(address_info) == 0:
             # Do not show an interface as disabled if it is part of a bond or
             # has a vlan on it.
-            for dev2 in self.model.get_all_netdevs():
-                if dev2.type == "bond" and \
-                  dev.name in dev2.config.get('interfaces', []):
-                    break
-                if dev2.type == "vlan" and dev.name == dev2.config.get('link'):
-                    break
-            else:
+            if not dev.is_used:
                 reason = dev.disabled_reason
                 if reason is None:
                     reason = ""
@@ -241,6 +236,9 @@ class NetworkView(BaseView):
         return rows
 
     def new_link(self, new_dev):
+        log.debug(
+            "new_link %s %s %s",
+            new_dev.name, new_dev.ifindex, (new_dev in self.cur_netdevs))
         if new_dev in self.dev_to_table:
             self.update_link(new_dev)
             return
@@ -255,6 +253,9 @@ class NetworkView(BaseView):
             (w, self.device_pile.options('pack'))]
 
     def update_link(self, dev):
+        log.debug(
+            "update_link %s %s %s",
+            dev.name, dev.ifindex, (dev in self.cur_netdevs))
         # Update the display of dev to represent the current state.
         #
         # The easiest way of doing this would be to just create a new table
@@ -288,11 +289,14 @@ class NetworkView(BaseView):
             self.device_pile.focus._select_first_selectable()
 
     def del_link(self, dev):
-        log.debug("del_link %s", (dev in self.cur_netdevs))
+        log.debug(
+            "del_link %s %s %s",
+            dev.name, dev.ifindex, (dev in self.cur_netdevs))
         if dev in self.cur_netdevs:
             netdev_i = self.cur_netdevs.index(dev)
             self._remove_row(netdev_i+1)
             del self.cur_netdevs[netdev_i]
+            del self.dev_to_table[dev]
         if isinstance(self._w, StretchyOverlay):
             stretchy = self._w.stretchy
             if getattr(stretchy, 'device', None) is dev:

From 099e6586706ecac8a1d1a7829f96d418d1206926 Mon Sep 17 00:00:00 2001
From: Michael Hudson-Doyle <michael.hudson@canonical.com>
Date: Tue, 26 Mar 2019 10:13:09 +1300
Subject: [PATCH 09/11] make applying changes less jarring for virtual devices

---
 subiquitycore/ui/views/network.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/subiquitycore/ui/views/network.py b/subiquitycore/ui/views/network.py
index 13a52758..a4945336 100644
--- a/subiquitycore/ui/views/network.py
+++ b/subiquitycore/ui/views/network.py
@@ -292,6 +292,10 @@ class NetworkView(BaseView):
         log.debug(
             "del_link %s %s %s",
             dev.name, dev.ifindex, (dev in self.cur_netdevs))
+        # If a virtual device disappears while we still have config
+        # for it, we assume it will be back soon.
+        if dev.is_virtual and dev.config is not None:
+            return
         if dev in self.cur_netdevs:
             netdev_i = self.cur_netdevs.index(dev)
             self._remove_row(netdev_i+1)

From b485cf487bcd0ad4defc40027fe414930af13099 Mon Sep 17 00:00:00 2001
From: Michael Hudson-Doyle <michael.hudson@canonical.com>
Date: Tue, 26 Mar 2019 10:18:32 +1300
Subject: [PATCH 10/11] make the re-dhcp on entry to the network screen a
 little more discreet

---
 subiquitycore/controllers/network.py | 2 ++
 subiquitycore/ui/views/network.py    | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/subiquitycore/controllers/network.py b/subiquitycore/controllers/network.py
index f6f61c10..494a2820 100644
--- a/subiquitycore/controllers/network.py
+++ b/subiquitycore/controllers/network.py
@@ -365,6 +365,8 @@ class NetworkController(BaseController):
                     if not silent:
                         dev.set_dhcp_state(v, "PENDING")
                         self.network_event_receiver.update_link(dev.ifindex)
+                    else:
+                        dev.set_dhcp_state(v, "RECONFIGURE")
                     dhcp_device_versions.append((dev, v))
             if dev.info is None:
                 continue
diff --git a/subiquitycore/ui/views/network.py b/subiquitycore/ui/views/network.py
index a4945336..f48c9cfc 100644
--- a/subiquitycore/ui/views/network.py
+++ b/subiquitycore/ui/views/network.py
@@ -209,6 +209,8 @@ class NetworkView(BaseView):
                     address_info.append((label, s))
                 elif dev.dhcp_state(v) == "TIMEDOUT":
                     address_info.append((label, Text(_("timed out"))))
+                elif dev.dhcp_state(v) == "RECONFIGURE":
+                    address_info.append((label, Text(_("-"))))
                 else:
                     address_info.append((
                         label,

From 3384eb5c84bee38e8d640e41ca18dcdbf3feb80c Mon Sep 17 00:00:00 2001
From: Michael Hudson-Doyle <michael.hudson@canonical.com>
Date: Tue, 26 Mar 2019 11:32:02 +1300
Subject: [PATCH 11/11] small fixes

---
 subiquitycore/controllers/network.py | 2 +-
 subiquitycore/ui/views/network.py    | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/subiquitycore/controllers/network.py b/subiquitycore/controllers/network.py
index 494a2820..80d82434 100644
--- a/subiquitycore/controllers/network.py
+++ b/subiquitycore/controllers/network.py
@@ -308,7 +308,7 @@ class NetworkController(BaseController):
         # explanation.
         for dev in self.model.get_all_netdevs():
             has_global_address = False
-            if dev.info is None:
+            if dev.info is None or not dev.config:
                 continue
             for a in dev.info.addresses.values():
                 if a.scope == "global":
diff --git a/subiquitycore/ui/views/network.py b/subiquitycore/ui/views/network.py
index f48c9cfc..7b3845a3 100644
--- a/subiquitycore/ui/views/network.py
+++ b/subiquitycore/ui/views/network.py
@@ -258,6 +258,8 @@ class NetworkView(BaseView):
         log.debug(
             "update_link %s %s %s",
             dev.name, dev.ifindex, (dev in self.cur_netdevs))
+        if dev not in self.cur_netdevs:
+            return
         # Update the display of dev to represent the current state.
         #
         # The easiest way of doing this would be to just create a new table