Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crash when switching sources having different variations #1832

Merged
merged 2 commits into from
Oct 10, 2023
Merged

Fix crash when switching sources having different variations #1832

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
65612ed
examples: add desktop source with different variations
ogayot Oct 10, 2023
d482049
filesystem: do not keep the variation info when the source changes
ogayot Oct 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/answers/desktop.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#source-catalog: examples/sources/desktop.yaml
#source-catalog: examples/sources/desktop-standard.yaml
Source:
source: ubuntu-desktop
Welcome:
Expand Down
29 changes: 29 additions & 0 deletions examples/sources/desktop-standard.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
- default: true
description:
en: A full featured Ubuntu Desktop.
id: ubuntu-desktop
locale_support: langpack
name:
en: Ubuntu Desktop
path: standard.squashfs
preinstalled_langs:
- de
- en
- es
- fr
- it
- pt
- ru
- zh
- ''
size: 4033826816
type: fsimage-layered
variant: desktop
variations:
enhanced-secureboot:
path: standard.enhanced-secureboot.squashfs
size: 4336062464
snapd_system_label: enhanced-secureboot-desktop
standard:
path: standard.squashfs
size: 4033826816
40 changes: 34 additions & 6 deletions examples/sources/desktop.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,39 @@
- default: true
description:
en: A minimal but usable Ubuntu Desktop.
id: ubuntu-desktop-minimal
locale_support: langpack
name:
en: Ubuntu Desktop (minimized)
path: minimal.squashfs
preinstalled_langs:
- de
- en
- es
- fr
- it
- pt
- ru
- zh
- ''
size: 4015337472
type: fsimage-layered
variant: desktop
variations:
minimal:
path: minimal.squashfs
size: 4015337472
minimal-enhanced-secureboot:
path: minimal.enhanced-secureboot.squashfs
size: 4319010816
snapd_system_label: enhanced-secureboot-desktop
- description:
en: A full featured Ubuntu Desktop.
id: ubuntu-desktop
locale_support: langpack
name:
en: Ubuntu Desktop
path: standard.squashfs
path: minimal.standard.squashfs
preinstalled_langs:
- de
- en
Expand All @@ -16,14 +44,14 @@
- ru
- zh
- ''
size: 4033826816
size: 5735194624
type: fsimage-layered
variant: desktop
variations:
enhanced-secureboot:
path: standard.enhanced-secureboot.squashfs
size: 4336062464
path: minimal.standard.enhanced-secureboot.squashfs
size: 6006235136
snapd_system_label: enhanced-secureboot-desktop
standard:
path: standard.squashfs
size: 4033826816
path: minimal.standard.squashfs
size: 5735194624
1 change: 1 addition & 0 deletions subiquity/server/controllers/filesystem.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,7 @@ def disallowed_encryption(msg):
return info

async def _examine_systems(self):
self._variation_info.clear()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we also need to reset self._info?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a fair point. self._info is set through a call to set_info_for_capability - which only happens after a POST to /storage/guided (or /storage). I would need a build a test-case for that as well.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, not blocking for this

catalog_entry = self.app.base_model.source.current
for name, variation in catalog_entry.variations.items():
system = None
Expand Down
30 changes: 30 additions & 0 deletions subiquity/server/controllers/tests/test_filesystem.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,36 @@ async def fake_run(cmd, **kwargs):
await self.fsc._pre_shutdown()
mock_run.assert_called_once_with(["mountpoint", "/target"])

async def test_examine_systems(self):
# In LP: #2037723 and other similar reports, the user selects the
# source 'ubuntu-desktop-minimal' first and then switches to
# 'ubuntu-desktop'. The variations of those two sources are different.
# Upon switching to the new source, we forgot to discard the old
# variations. This lead to a crash further in the install.
self.fsc.model = model = make_model(Bootloader.UEFI)
make_disk(model)
self.app.base_model.source.current.type = "fsimage"
self.app.base_model.source.current.variations = {
"minimal": CatalogEntryVariation(path="", size=1),
}

self.app.dr_cfg = DRConfig()
self.app.dr_cfg.systems_dir_exists = True

await self.fsc._examine_systems()

self.assertEqual(len(self.fsc._variation_info), 1)
self.assertEqual(self.fsc._variation_info["minimal"].name, "minimal")

self.app.base_model.source.current.variations = {
"default": CatalogEntryVariation(path="", size=1),
}

await self.fsc._examine_systems()

self.assertEqual(len(self.fsc._variation_info), 1)
self.assertEqual(self.fsc._variation_info["default"].name, "default")


class TestGuided(IsolatedAsyncioTestCase):
boot_expectations = [
Expand Down