From 3bb9268974ef37e8dbd632db25ede1c4ee93d91c Mon Sep 17 00:00:00 2001 From: Michael Hudson-Doyle Date: Mon, 25 Mar 2019 11:03:42 +1300 Subject: [PATCH] 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"