Repository navigation
feat(install): install the snap package by default when supported - #4380
olivercalder wants to merge 2 commits into
Conversation
|
I pointed a robot at this and it had feedback which I reviewed, tossed some, and left this. Up to you if you want to make updates. Low/Medium:
Low:
Nit: Migration instructions in
Low: README quickstart prerequisites not updated
|
|
/ok to test 239405c |
- `install.sh` now selects snap when the `snap` command exists and `OPENSHELL_VERSION` is unset or `dev`. - Explicit `OPENSHELL_INSTALL_METHOD=snap|deb|rpm` still takes precedence. - Exact releases and prereleases continue to use native packages during automatic selection. - Docker-related snap errors now mention DEB/RPM overrides. - Installer selection tests cover default snap selection, native fallbacks, overrides, pinned releases, and prereleases. - Snap canary jobs no longer force `OPENSHELL_INSTALL_METHOD=snap`, so they test automatic selection. - Updated installation, VM driver, release canary, CI, and test guest documentation. - Fixed some leftover "Snap" language which should be "snap" Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
…election Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
239405c to
e15b8bf
Compare
|
Thanks for the review @krishicks ! I've pushed a commit to improve the things you pointed out. |
|
/ok to test e15b8bf |
Compatibility review (advisory)Unaddressed breaking changes The default Linux installer now selects snap on snapd hosts instead of the documented native package. This breaks native upgrades and Docker-free Podman or VM installations despite documented overrides. 1. cli — ACKED
Review details
Advisory review; publication gates are unchanged. |
Yes, this is the intention of the PR. The |
|
Due to a new rule you need to create an issue for this (see failed CI checks). I think this PR predates that change, which is unfortunate. |
elezar
left a comment
There was a problem hiding this comment.
Could we include Docker compatibility in the automatic Snap-selection check?
Currently, linux_package_method() selects Snap whenever snapd is available and the version is unset or dev, but install_linux_snap() subsequently rejects hosts with the Docker snap or without Docker. This makes the default installer fail on hosts where it previously selected a native package, including Podman or VM setups.
For fresh installations, I suggest selecting Snap automatically only when Docker is present and the Docker snap is absent, otherwise falling back to DEB/RPM. Explicit OPENSHELL_INSTALL_METHOD=snap should retain the current preflight errors, and existing OpenShell Snap installations should remain on the Snap path.
Could we add selection tests for these cases? The canary’s automatic-selection rejection cases would also need updating, while retaining rejection coverage for explicit Snap requests.
This fallback would preserve native installation options; it would not establish compatibility between native OpenShell and the Docker snap, given the AppArmor limitation documented in #3666.
I tinkered with implementing this but was left unsatisfied with the result... I don't think it's correct to fall back to native package installations for the deb (not the rpm) if there's not a non-snap Docker daemon installed: if the user doesn't have docker/podman/vm backends installed, then that just leaves them with a non-functional non-distro .deb package, instead of a non-functional snap. If I'm a new user who hears about OpenShell in the news and wants to give it a try, I probably don't have any backend already installed, and I expect to just run The short-mid goal IMO, once the The .rpm package is different though. The .deb doesn't pull in any compute backend as a dependency (though it ships the VM driver, while the .rpm does pull in podman as a dependency. So rpm users will get a functional system, while deb users will not at the moment without manual configuration to use the VM driver, which So where does that leave us? Here's the decision flow I would propose:
These are my initial thoughts on the topic, happy to discuss more or be overridden. I'll talk about this issue with others on my team to to get their advice. |
|
I've opened #4627 which should hopefully address much of the complexity in question by removing all the special caveats and docker checks required for the snap, since it should just work on its own using the VM driver. So I'll revisit this PR after that new PR (hopefully) lands. |
Summary
Make the
openshellsnap the default installation method again when snapd is installed. It was previously the default in #3656, then made opt-in again in #3726 due to issues with TLS certificates because the gateway was run as a system service instead of a user service. Those issues have since been addressed in #4099, so we can make the snap a default installation target again.Related Issue
Fixes #4525
Also follows a Slack conversation with @drew about making the snap the default installation method again.
Changes
install.shnow selects snap when thesnapcommand exists andOPENSHELL_VERSIONis unset ordev.OPENSHELL_INSTALL_METHOD=snap|deb|rpmstill takes precedence.OPENSHELL_INSTALL_METHOD=snap, so they test automatic selection.Testing
Checklist