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.

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

Conversation

ogayot
Copy link
Member

@ogayot ogayot commented Oct 10, 2023

When the source changes, the available variations should change as well. If we keep the old variations in the FilesystemController._variations_info dictionary, we end up with a crash later in the install, showing something like:

2023-09-29 12:10:36,383 INFO subiquity.common.errorreport:415 saving crash report 'install failed crashed with KeyError' to /var/crash/1695989436.283925295.install_fail.crash
2023-09-29 12:10:36,383 ERROR root:30 finish: subiquity/Install/install: FAIL: 'minimal'

Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>
When the source changes, the available variations should change as well.
If we keep the old variations in the
FilesystemController._variations_info dictionary, we end up with a crash
later in the install.

Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>
@ogayot
Copy link
Member Author

ogayot commented Oct 10, 2023

I just tested the fix in the desktop installer - and I confirm the crash does not occur anymore

@@ -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

@ogayot ogayot merged commit 7549af4 into canonical:main Oct 10, 2023
11 checks passed
@mwhudson
Copy link
Collaborator

Oops. I'm sorry all this ended up so complicated :(

@dbungert dbungert mentioned this pull request Oct 10, 2023
@ogayot ogayot deleted the variation-bug branch October 13, 2023 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants