StealthOS/subiquity
StealthOS
/
subiquity
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

network: use pyroute2 to manage default routes 

The existing event based method of watching for has_network has a flaw.

The incoming route_change events from probert do not distinguish routes
on the same interface but a different metric, so if 2 routes on one
interface appear, we only get one event.  Then if one of those routes is
removed, we will inappropriately remove this route from the
default_routes list.

Aside from the code watching the event stream, the set of default routes
is an elaborate boolean value.

Simplify the code by passing around a boolean, and when we get a
route_change event, use that to go looking again at the list of default
routes.

LP: #2004659
This commit is contained in:
Dan Bungert 2023-02-14 18:17:33 -07:00
parent a6f6c655e2
commit e095d5040f
9 changed files with 158 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
apt-deps.txt
View File

@ -32,6 +32,7 @@ python3-nose
python3-parameterized
python3-pip
python3-pyflakes
python3-pyroute2
python3-pytest
python3-pytest-xdist
python3-pyudev

1
snapcraft.yaml
View File

@ -110,6 +110,7 @@ parts:
- python3-minimal
- python3-oauthlib
- python3-pkg-resources
- python3-pyroute2
- python3-pyrsistent
- python3-pyudev
- python3-requests

6
subiquity/client/controllers/network.py
View File

@ -17,7 +17,7 @@ import logging
import os
import shutil
import tempfile
from typing import List, Optional
from typing import Optional
from aiohttp import web
@ -74,9 +74,9 @@ class NetworkController(SubiquityTuiController, NetworkAnswersMixin):
if act == LinkAction.DEL:
self.view.del_link(info)
async def route_watch_POST(self, routes: List[int]) -> None:
async def route_watch_POST(self, has_default_route: bool) -> None:
if self.view is not None:
self.view.update_default_routes(routes)
self.view.update_has_default_route(has_default_route)
async def apply_starting_POST(self) -> None:
if self.view is not None:

2
subiquity/common/apidef.py
View File

@ -450,7 +450,7 @@ class NetEventAPI:
def POST(act: LinkAction, info: Payload[NetDevInfo]) -> None: ...
class route_watch:
def POST(routes: List[int]) -> None: ...
def POST(has_default_route: bool) -> None: ...
class apply_starting:
def POST() -> None: ...

14
subiquity/server/controllers/network.py
View File

@ -246,8 +246,7 @@ class NetworkController(BaseNetworkController, SubiquityController):
self.apply_config(context)
with context.child("wait_for_apply"):
await self.apply_config_task.wait()
self.model.has_network = bool(
self.network_event_receiver.default_routes)
self.model.has_network = self.network_event_receiver.has_default_route
async def _apply_config(self, *, context=None, silent=False):
try:
@ -284,8 +283,7 @@ class NetworkController(BaseNetworkController, SubiquityController):
wlan_support_install_state=self.wlan_support_install_state())
async def configured(self):
self.model.has_network = bool(
self.network_event_receiver.default_routes)
self.model.has_network = self.network_event_receiver.has_default_route
self.model.needs_wpasupplicant = (
self.wlan_support_install_state() == WLANSupportInstallState.DONE)
await super().configured()
@ -308,7 +306,7 @@ class NetworkController(BaseNetworkController, SubiquityController):
run_bg_task(
self._call_client(
client, conn, lock, "route_watch",
self.network_event_receiver.default_routes))
self.network_event_receiver.has_default_route))
async def subscription_DELETE(self, socket_path: str) -> None:
if socket_path not in self.clients:
@ -347,9 +345,9 @@ class NetworkController(BaseNetworkController, SubiquityController):
super().apply_error(stage)
self._call_clients("apply_error", stage)
def update_default_routes(self, routes):
super().update_default_routes(routes)
self._call_clients("route_watch", routes)
def update_has_default_route(self, has_default_route):
super().update_has_default_route(has_default_route)
self._call_clients("route_watch", has_default_route)
def _send_update(self, act, dev):
with self.context.child(

66
subiquitycore/controllers/network.py
View File

@ -21,6 +21,7 @@ import os
import subprocess
from typing import Optional
import pyroute2
import yaml
from probert.network import IFF_UP, NetworkEventReceiver
@ -56,7 +57,7 @@ class SubiquityNetworkEventReceiver(NetworkEventReceiver):
def __init__(self, controller):
self.controller = controller
self.model = controller.model
self.default_routes = set()
self.has_default_route = False
def new_link(self, ifindex, link):
netdev = self.model.new_link(ifindex, link)
@ -65,9 +66,8 @@ class SubiquityNetworkEventReceiver(NetworkEventReceiver):
def del_link(self, ifindex):
netdev = self.model.del_link(ifindex)
if ifindex in self.default_routes:
self.default_routes.remove(ifindex)
self.controller.update_default_routes(self.default_routes)
self.probe_default_routes()
self.controller.update_has_default_route(self.has_default_route)
if netdev is not None:
self.controller.del_link(netdev)
@ -76,9 +76,9 @@ class SubiquityNetworkEventReceiver(NetworkEventReceiver):
if netdev is None:
return
flags = getattr(netdev.info, "flags", 0)
if not (flags & IFF_UP) and ifindex in self.default_routes:
self.default_routes.remove(ifindex)
self.controller.update_default_routes(self.default_routes)
if not (flags & IFF_UP):
self.probe_default_routes()
self.controller.update_has_default_route(self.has_default_route)
self.controller.update_link(netdev)
def route_change(self, action, data):
@ -87,13 +87,31 @@ class SubiquityNetworkEventReceiver(NetworkEventReceiver):
return
if data['table'] != 254:
return
ifindex = data['ifindex']
if action == "NEW" or action == "CHANGE":
self.default_routes.add(ifindex)
elif action == "DEL" and ifindex in self.default_routes:
self.default_routes.remove(ifindex)
log.debug('default routes %s', self.default_routes)
self.controller.update_default_routes(self.default_routes)
self.probe_default_routes()
self.controller.update_has_default_route(self.has_default_route)
def _default_route_exists(self, routes):
return any(route['table'] == 254 and not route['dst']
for route in routes)
def probe_default_routes(self):
with pyroute2.NDB() as ndb:
self.has_default_route = self._default_route_exists(ndb.routes)
log.debug('default routes %s', self.has_default_route)
@staticmethod
def create(controller: BaseController, dry_run: bool) \
-> "SubiquityNetworkEventReceiver":
if dry_run:
return DryRunSubiquityNetworkEventReceiver(controller)
else:
return SubiquityNetworkEventReceiver(controller)
class DryRunSubiquityNetworkEventReceiver(SubiquityNetworkEventReceiver):
def probe_default_routes(self):
self.has_default_route = True
log.debug('dryrun default routes %s', self.has_default_route)
default_netplan = '''
@ -149,7 +167,8 @@ class BaseNetworkController(BaseController):
self.parse_netplan_configs()
self._watching = False
self.network_event_receiver = SubiquityNetworkEventReceiver(self)
self.network_event_receiver = \
SubiquityNetworkEventReceiver.create(self, self.opts.dry_run)
def parse_netplan_configs(self):
self.model.parse_netplan_configs(self.root)
@ -485,8 +504,8 @@ class BaseNetworkController(BaseController):
pass
@abc.abstractmethod
def update_default_routes(self, routes):
if routes:
def update_has_default_route(self, has_default_route):
if has_default_route:
self.app.hub.broadcast(CoreChannels.NETWORK_UP)
@abc.abstractmethod
@ -612,8 +631,8 @@ class NetworkController(BaseNetworkController, TuiController,
if not self.view_shown:
self.apply_config(silent=True)
self.view_shown = True
self.view.update_default_routes(
self.network_event_receiver.default_routes)
self.view.update_has_default_route(
self.network_event_receiver.has_default_route)
return self.view
def end_ui(self):
@ -621,8 +640,7 @@ class NetworkController(BaseNetworkController, TuiController,
def done(self):
log.debug("NetworkController.done next_screen")
self.model.has_network = bool(
self.network_event_receiver.default_routes)
self.model.has_network = self.network_event_receiver.has_default_route
self.app.next_screen()
def cancel(self):
@ -643,10 +661,10 @@ class NetworkController(BaseNetworkController, TuiController,
if self.view is not None:
self.view.show_network_error(stage)
def update_default_routes(self, routes):
super().update_default_routes(routes)
def update_has_default_route(self, has_default_route):
super().update_has_default_route(has_default_route)
if self.view:
self.view.update_default_routes(routes)
self.view.update_has_default_route(has_default_route)
def new_link(self, netdev):
super().new_link(netdev)

14
subiquitycore/controllers/tests/__init__.py Normal file
View File

@ -0,0 +1,14 @@
# Copyright 2015 Canonical, Ltd.
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU Affero General Public License as
# published by the Free Software Foundation, either version 3 of the
# License, or (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU Affero General Public License for more details.
#
# You should have received a copy of the GNU Affero General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.

87
subiquitycore/controllers/tests/test_network.py Normal file
View File

@ -0,0 +1,87 @@
# Copyright 2023 Canonical, Ltd.
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU Affero General Public License as
# published by the Free Software Foundation, either version 3 of the
# License, or (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU Affero General Public License for more details.
#
# You should have received a copy of the GNU Affero General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
import unittest
from unittest.mock import Mock
from subiquitycore.controllers.network import SubiquityNetworkEventReceiver
class TestRoutes(unittest.IsolatedAsyncioTestCase):
def setUp(self):
self.er = SubiquityNetworkEventReceiver(Mock())
def test_empty(self):
self.assertFalse(self.er._default_route_exists([]))
def test_one_good(self):
routes = [{
"target": "localhost",
"tflags": 0,
"table": 254,
"ifname": "ens3",
"dst": "",
"dst_len": 0,
"gateway": "10.0.2.2"
}]
self.assertTrue(self.er._default_route_exists(routes))
def test_one_good_one_other(self):
routes = [{
"target": "localhost",
"tflags": 0,
"table": 254,
"ifname": "ens3",
"dst": "",
"dst_len": 0,
"gateway": "10.0.2.2"
}, {
"target": "localhost",
"tflags": 0,
"table": 254,
"ifname": "ens3",
"dst": "10.0.2.0",
"dst_len": 24,
"gateway": None
}]
self.assertTrue(self.er._default_route_exists(routes))
def test_one_other(self):
routes = [{
"target": "localhost",
"tflags": 0,
"table": 254,
"ifname": "ens3",
"dst": "10.0.2.0",
"dst_len": 24,
"gateway": None
}]
self.assertFalse(self.er._default_route_exists(routes))
def test_wrong_table(self):
routes = [{
"target": "localhost",
"tflags": 0,
"table": 255,
"ifname": "ens3",
"dst": "",
"dst_len": 0,
"gateway": "10.0.2.2"
}]
self.assertFalse(self.er._default_route_exists(routes))

6
subiquitycore/ui/views/network.py
View File

@ -324,9 +324,9 @@ class NetworkView(BaseView):
dev_info = netdev_table.dev_info
meth("{}/{}".format(dev_info.name, action.name), dev_info)
def update_default_routes(self, routes):
log.debug('view route_watcher %s', routes)
if routes:
def update_has_default_route(self, has_default_route):
log.debug('view route_watcher %s', has_default_route)
if has_default_route:
label = _("Done")
else:
label = _("Continue without network")