When a HTTP client sends a query but closes the socket before an answer
is received, aiohttp signals it on the server end by raising an
asyncio.CancelledError in the associated query handler.
By default, when a task is cancelled with asyncio, the task(s) that it
is currently awaiting on are cancelled as well.
The GET handler for /drivers?wait=true awaits on the "list drivers"
task. Therefore, if the GET handler gets cancelled, so will be the "list
drivers" task.
When that happens, any subsequent call to GET /drivers?wait=true will
make the server raise an asyncio.CancelledError because the "list
drivers" task has already been cancelled. This results in:
* the socket being closed from the server end
* an aiohttp.client_exceptions.ServerDisconnectedError exception raised
on the client end. This type of exception is unhandled and makes the
client crash.
Fixed by preventing the "list drivers" task from being cancelled when
the GET /drivers query handler gets cancelled.
https://bugs.launchpad.net/subiquity/+bug/1968729
Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>
The following commit added an unconditional return statement in the try
block of _move_screen, effectively making the associated else block
unreachable.
a7bcc7fa add a way to wait for something with notification after 0.1s
Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>
When we reach the storage screens on the installer, if the devices
probing operation has not finished, we:
* display a temporary "Probing" screen.
* create an asynchronous task (a.k.a., probing task) that will
eventually show the "Guide Storage" screen when the probing operation
finishes.
The probing task checks, when it finishes, that the screen currently
visible is the "Probing" screen. This is the expectation and is true in
most scenarios. But in case a different screen is visible, we skip
refreshing the display.
Unfortunately, sometimes, a "Progress" screen is shown for some time
before the "Probing" screen appears. Consequently, we do not refresh the
screen if the probing operation finishes whilst the Progress screen is
visible.
In order to keep the view returned by make_ui() up-to-date and make sure
that the right screen is shown even if the probing operation finishes
early, we use the level indirection that was implemented in make_ui.
https://bugs.launchpad.net/subiquity/+bug/1968161
Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>
The make_ui() function / coroutine returns a BaseView (i.e., a
screen to display to the user).
That being said, when the application calls, make_ui(), it does not come
with a guarantee that the view returned will be displayed to the user
immediately.
One of the reason is that there are multiple await statements before the
we call the ui.set_body function. Therefore, tasks running concurrently
cannot reliably expect that they execute after the display is refreshed.
Perhaps more importantly, when the make_ui() function takes more than .1
second to execute, we display a "Progress" screen that stays visible for
at least one second. This can effectively delay a lot the moment when
the view returned by make_ui() is shown to the user. A lot can happen in
the meantime.
As the result, the view returned by make_ui can be outdated by the time
we show it on the screen.
One way to work around this problem is to store in the controller a
reference to the view that it returns in make_ui(). This way, the
controller can modify the view and keep it up-to-date until it gets
shown to the user.
Unfortunately, some controllers (e.g., the storage controller) do not
modify / mutate the existing view object when a modification is needed ;
but instead instantiate a new view object.
This patch introduces a level of indirection that can be used by these
controllers. Instead of returning a view object from make_ui(), the
controllers are now allowed to return a callback ; which in turn will
return a view object.
https://bugs.launchpad.net/subiquity/+bug/1968161
Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>
We used to only accept lists of strings for commands. We now accept
sequences of strings instead ; which are lists of strings or tuple of
strings.
Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>
Having a variable named arun_command in tests interferes with tags
generated with ctags: the default tag does not point to the actual
function definition; but to the variable in test_ubuntu_advantage.py.
Excluding the tests when generating the tags would be an option but for
now we'll just rename the variable.
Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>
After calling .communicate() on an asyncio.subprocess.Process object,
the attribute returncode gets set to a non-None value. Type checkers are
not able to figure this out.
Fixed by adding an assert to help type checkers out.
Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>
Early commands is currently documented as follows:
> The autoinstall config is available at /autoinstall.yaml
> (irrespective of how it was provided) and the file will be re-read
> after the early-commands have run to allow them to alter the config
> if necessary.
The previous change to let cloud take precedence over iso location broke
this functionality. Fix that by copying to the iso location the file of
choice.
Per LP: #1968160, with 2 or more disks, go to guided storage config, hit
done. At file system summary, hit back, and choose the other disk.
While this screen does say so, one might not notice that the first disk
is still setup to be formatted.
Instead, when going back to guided storage, a reset is also done.
We won't ship systemd officially in 22.04, hence remove it from the UI.
However it's still there and distributed and can be enabled manually.
Co-authored-by: Didier Roche <didrocks@ubuntu.com>
update requires root access to the machine and there is no --simulate
option so skip it in dry-run mode.
Co-authored-by: Didier Roche <didrocks@ubuntu.com>
UDI sets the SNAP env var to '.' for development purposes.
See: https://github.com/canonical/ubuntu-desktop-installer/commit/9eb6f04
It is unlikely that under test or production that env var will
ever by just '.'. On the other hand in dry-run we want this controller
to interpret it as '/' if not properly set, thus discarding the '.'.
When refreshing the filesystem GUI, we used to set the status (i.e.,
enabled or disabled) of the "Create LVM volume group" and "Create
software RAID" buttons multiple times ; once for each disk found.
When multiple disks are listed, this creates intermediate status changes
that are not wanted, e.g.:
* "Create software RAID" button gets set to disabled after finding a first
unpartitioned disk ;
* "Create software RAID" button is set again to disabled after finding
a partitioned disk (with no unmounted partitions) ;
* "Create software RAID" button is set to enabled after finding a
second unpartitioned disk.
Fixed by evaluating the status of the buttons only once after looping
through all disks.
Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>
Dan Bungert