From 132440451dc467d4b700bc3e728ddc869f05ff4a Mon Sep 17 00:00:00 2001 From: Michael Hudson-Doyle Date: Wed, 31 Mar 2021 12:19:55 +1300 Subject: [PATCH] more reliably restart client when server restarts Splitting subiquity into server and client means that in general old versions of the client can still be running when the server is updated (the client running on tty1 will be restarted by snapd/systemd when the snap is updated but clients running via e.g. ssh will not). I implemented a way for the client to detect this and restart itself: the server sets a header in all responses that indicates if it has been updated. So far so good. But the way it knows that it has been updated is to check the presence of a file that is only created when subiquity itself triggers the refresh, so it's not there in the case of manual refresh, and as reported in https://bugs.launchpad.net/bugs/1921820 this can lead to the client crashing because it cannot parse the new server's response. This simply changes to creating the marker file in the snap post-refresh hook, which will be executed for manual snap refreshes as well. While I'm at it, remove the rest of the post-install hook that restarted subiquity clients running on the serial line as the generic machinery will work for these too. --- snap/hooks/post-refresh | 7 ++----- subiquity/server/controllers/refresh.py | 1 - subiquitycore/snapd.py | 5 +++++ 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/snap/hooks/post-refresh b/snap/hooks/post-refresh index e712809c..73ca5425 100755 --- a/snap/hooks/post-refresh +++ b/snap/hooks/post-refresh @@ -1,6 +1,3 @@ #!/bin/sh -# snapd will restart snap.subiquity.subiquity-service.service for us, -# but any processes running on the serial lines are created via -# systemd overrides in the installer layer of the squashfs and so need -# restarting separately. -systemctl restart 'serial-subiquity@*.service' +mkdir -p /run/subiquity +touch /run/subiquity/updating diff --git a/subiquity/server/controllers/refresh.py b/subiquity/server/controllers/refresh.py index b5f15cce..69cb6256 100644 --- a/subiquity/server/controllers/refresh.py +++ b/subiquity/server/controllers/refresh.py @@ -191,7 +191,6 @@ class RefreshController(SubiquityController): @with_context() async def start_update(self, context): - open(self.app.state_path('updating'), 'w').close() change = await self.app.snapd.post( 'v2/snaps/{}'.format(self.snap_name), {'action': 'refresh'}) diff --git a/subiquitycore/snapd.py b/subiquitycore/snapd.py index 2977dadc..38538a5c 100644 --- a/subiquitycore/snapd.py +++ b/subiquitycore/snapd.py @@ -118,6 +118,9 @@ class ResponseSet: return _FakeFileResponse(f) +update_marker_file = '.subiquity/run/subiquity/updating' + + class FakeSnapdConnection: def __init__(self, snap_data_dir, scale_factor): self.snap_data_dir = snap_data_dir @@ -130,6 +133,8 @@ class FakeSnapdConnection: def post(self, path, body, **args): if path == "v2/snaps/subiquity" and body['action'] == 'refresh': + # The post-refresh hook does this in the real world. + open(update_marker_file, 'w').close() return _FakeMemoryResponse({ "type": "async", "change": "7",