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

o, o/snapstate, tests: on classic auto-install snapd as prereq for non-essential snap install #14173

Conversation

ernestl
Copy link
Collaborator

@ernestl ernestl commented Jul 12, 2024

On classic systems, auto install snapd as prerequisite for any snap that is not os, base, kernel, gadget or snapd.

Previously it was only installed on systems without core, leaving a small amount of systems with snapd deb + core - inconsistent with the majority of systems in terms of re-execution and upgrade path.

The change triggered an explosion of unit test failures. The most efficient approach for addressing this was to install snapd as part of the base test (prevents many breakages) and remove it where required (only a few places).
This mirrors the spread change: #14294

Jira: SNAPDENG-24011

This change overlaps with existing experimental feature: https://github.com/canonical/snapd/blob/master/overlord/snapstate/snapmgr.go#L1167
We should consider deprecating this in a followup.

@ernestl ernestl added this to the 2.65 milestone Jul 12, 2024
@ernestl ernestl force-pushed the SNAPDENG-24011_enable_classic_with_core_to_install_snapd branch from 342c5aa to 65fdec5 Compare July 12, 2024 10:04
@ernestl ernestl force-pushed the SNAPDENG-24011_enable_classic_with_core_to_install_snapd branch from 65fdec5 to 4d79b09 Compare July 12, 2024 10:56
@ernestl ernestl added the Run nested The PR also runs tests inluded in nested suite label Jul 17, 2024
@ernestl ernestl closed this Jul 17, 2024
@ernestl ernestl reopened this Jul 17, 2024
@ernestl ernestl force-pushed the SNAPDENG-24011_enable_classic_with_core_to_install_snapd branch 4 times, most recently from 96918bd to 0612244 Compare July 24, 2024 11:07
@ernestl ernestl force-pushed the SNAPDENG-24011_enable_classic_with_core_to_install_snapd branch from 0612244 to 1515b80 Compare July 31, 2024 21:26
@ernestl ernestl modified the milestones: 2.65, 2.66 Aug 15, 2024
@ernestl ernestl force-pushed the SNAPDENG-24011_enable_classic_with_core_to_install_snapd branch 5 times, most recently from a6badc6 to f2e055e Compare September 13, 2024 22:23
@ernestl ernestl added the Needs Samuele review Needs a review from Samuele before it can land label Sep 14, 2024
@ernestl ernestl changed the title overlord, overlord/snapstate: install snapd on classic systems when m… overlord, overlord/snapstate, tests: auto-install snapd on classic systems as prereq for non os,base,base,snapd Sep 14, 2024
@ernestl ernestl changed the title overlord, overlord/snapstate, tests: auto-install snapd on classic systems as prereq for non os,base,base,snapd overlord, overlord/snapstate, tests: auto-install snapd on classic as prereq for non os,base,kernel,gadget or snapd install Sep 14, 2024
@ernestl ernestl changed the title overlord, overlord/snapstate, tests: auto-install snapd on classic as prereq for non os,base,kernel,gadget or snapd install overlord, overlord/snapstate, tests: auto-install snapd on classic as prereq for non-essential snap install Sep 14, 2024
@ernestl ernestl changed the title overlord, overlord/snapstate, tests: auto-install snapd on classic as prereq for non-essential snap install overlord, overlord/snapstate, tests: auto-install snapd on classic as prereq for standard snap install Sep 14, 2024
@ernestl ernestl changed the title overlord, overlord/snapstate, tests: auto-install snapd on classic as prereq for standard snap install overlord, overlord/snapstate, tests: auto-install snapd on classic as prereq for non-essential snap install Sep 14, 2024
@ernestl ernestl marked this pull request as ready for review September 14, 2024 07:53
@ernestl ernestl changed the title overlord, overlord/snapstate, tests: auto-install snapd on classic as prereq for non-essential snap install o, o/snapstate, tests: auto-install snapd on classic as prereq for non-essential snap install Sep 14, 2024
@ernestl ernestl changed the title o, o/snapstate, tests: auto-install snapd on classic as prereq for non-essential snap install o, o/snapstate, tests: on classic auto-install snapd as prereq for non-essential snap install Sep 14, 2024
# Install snaps with snaps with complex interface requirements.
# Use specific revisions to pin expected behaviour.
install_snaps() {
snap install firefox --revision=4848 # 130.0-2
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe drop firefox? it's large, has many interfaces and for sure will pull in additional content snap dependencies

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Can do, if its ok, will wait a bit to gather more input on what constitutes a solid check for interface "migration".

Copy link
Collaborator

@pedronis pedronis left a comment

Choose a reason for hiding this comment

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

remark

overlord/snapstate/handlers.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@pedronis pedronis left a comment

Choose a reason for hiding this comment

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

thanks

Copy link
Collaborator

@sergiocazzolato sergiocazzolato left a comment

Choose a reason for hiding this comment

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

some questions inline

Copy link
Member

@andrewphelpsj andrewphelpsj left a comment

Choose a reason for hiding this comment

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

Thanks, looks good to me! Removing the experimental feature is a good idea, yes.

@ernestl ernestl force-pushed the SNAPDENG-24011_enable_classic_with_core_to_install_snapd branch from 5bb362b to b13af6b Compare September 19, 2024 07:13
@ernestl
Copy link
Collaborator Author

ernestl commented Sep 19, 2024

Failures:

fedora-os |

  • openstack:fedora-40-64:tests/main/selinux-clean | known, unrelated

nested-ubuntu-24.04 |

  • google-nested:ubuntu-24.04-64:tests/nested/manual/hybrid-remodel | known, not related

@ernestl ernestl merged commit 14df48a into canonical:master Sep 19, 2024
52 of 54 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Samuele review Needs a review from Samuele before it can land Run nested The PR also runs tests inluded in nested suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants