test(conformance): smoke Snap artifacts in Ubuntu VM - #2869
Conversation
elezar
added
the
test:snap
|
/ok-to-test d96c61a |
7d50815 to
3bbf47e
Compare
a66ebe5 to
3f771b4
Compare
3f771b4 to
4a7c134
Compare
4a7c134 to
4dfef75
Compare
4dfef75 to
e68f04a
Compare
e68f04a to
d8d5072
Compare
d8d5072 to
8e73cd6
Compare
8e73cd6 to
0843e09
Compare
johntmyers
left a comment
johntmyers
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
This project-valid Snap CI refactor has one blocking revision-coupling defect in Release Canary. The current-head Branch Checks, Helm Lint, and Snap Smoke are green, but the supported older-branch Release Dev path can fail before testing its artifact.
Action required: keep the executing canary workflow's repository revision checked out when loading the new local action, then push an update.
Blocking findings:
GATOR-0843e094-01: Release Canary can load the local smoke action from an artifact-producing revision that predates the action.
Carried findings:
- None
Non-blocking suggestions:
- Add
test:snapto the opt-in test-label enumeration inCONTRIBUTING.md.
Gator metadata
- Validation: Maintainer-authored CI/release improvement tied to the Snap gateway lifecycle work in #2055
- Docs:
CI.mdand the release-canary skill are updated;CONTRIBUTING.mdlabel discovery has a non-blocking omission - Checks: Current-head required checks are green
- E2E:
test:snapis applied and Snap Smoke passed for the current head - Head SHA:
0843e09412d63b951104f0562c99891db51df7f4 - Base SHA:
0fce474c127057164e477111256fd2ceaa866a08 - Merge base SHA:
0fce474c127057164e477111256fd2ceaa866a08 - Patch ID:
c48874d272e4b1f04374369f6990d77488e1e320 - Gator payload:
7 - Review mode:
initial - Previous reviewed SHA: none
- Review budget exhausted: no
- Maintainer decision required: no
- Next state:
gator:in-review
johntmyers
added
the
gator:in-review
0843e09 to
2d6b888
Compare
johntmyers
left a comment
johntmyers
left a comment
There was a problem hiding this comment.
gator-agent
Re-check After Author Update
Thanks @elezar. I reviewed the new author-only delta: the Release Canary checkout now stays on the executing workflow revision while the existing run ID continues to select the Snap artifact, so GATOR-0843e094-01 is resolved. I also confirmed the test:snap documentation link was added to CONTRIBUTING.md. No blocking findings remain.
Current-head Branch Checks and the test:snap Branch E2E workflow are running, so Gator is moving to pipeline monitoring.
Blocking findings:
- None
Carried findings:
- None
Gator metadata
- Validation: Maintainer-authored CI/release improvement tied to the Snap gateway lifecycle work in #2055
- Docs:
CI.md, the release-canary skill, andCONTRIBUTING.mdcover the new Snap test path - Checks: Current-head required checks are queued or running; DCO and Helm Lint are green
- E2E:
test:snapis applied; current-head Branch E2E run 32970119053 is in progress - Head SHA:
2d6b888d9bcf2ed43676a22be72dff5bee9c8dbe - Base SHA:
d3a417e422ba9830bf43402b2f008473be1da2b4 - Merge base SHA:
d3a417e422ba9830bf43402b2f008473be1da2b4 - Patch ID:
b541e60bd66472f2d7e0fda03d36443e4a79a1e5 - Gator payload:
7 - Review mode:
follow_up - Previous reviewed SHA:
0843e09412d63b951104f0562c99891db51df7f4 - Review budget exhausted: no
- Maintainer decision required: no
- Next state:
gator:watch-pipeline
johntmyers
added
gator:watch-pipeline
johntmyers
added
the
gator:approval-needed
johntmyers
added
gator:blocked
8df9f30 to
40099d6
Compare
|
/ok to test 40099d6 |
johntmyers
left a comment
johntmyers
left a comment
There was a problem hiding this comment.
gator-agent
Re-check After Author Update
Thanks @elezar. The feedback ledger confirms this head is rebase-equivalent to the previously reviewed patch, so the resolved Release Canary finding remains closed and no repeat code review is needed. No blocking findings remain.
Current-head Branch E2E is running, and Branch Checks and Helm Lint are queued after maintainer runner validation, so Gator is returning to pipeline monitoring.
Blocking findings:
- None
Carried findings:
- None
Gator metadata
- Validation: Maintainer-authored CI/release improvement tied to the Snap gateway lifecycle work in #2055
- Docs:
CI.md, the release-canary skill, andCONTRIBUTING.mdcover the Snap test path - Checks: Current-head Branch Checks and Helm Lint are queued; Branch E2E is in progress
- E2E:
test:e2eandtest:snapare applied; current-head run33734388410is in progress - Head SHA:
40099d696ad85b55b2e2fd3b1192a3348e3949d0 - Base SHA:
8d7db254032ef2d0ccb5d39b56a0f18d79eb87af - Merge base SHA:
8d7db254032ef2d0ccb5d39b56a0f18d79eb87af - Patch ID:
69ccf8ec62538d87736169d546c3b3583188fc01 - Gator payload:
8 - Review mode:
already_reviewed(rebase-equivalent) - Previous reviewed SHA:
8df9f30cd35cb67cf6434a50069b3d253aff5aef - Review budget exhausted: yes
- Maintainer decision required: no
- Next state:
gator:watch-pipeline
johntmyers
added
gator:watch-pipeline
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
BlockedGator is blocked because this pull request is currently a draft. The previous Next action: @elezar, mark the pull request ready for review when this revision is ready. Gator will then run the required current-head critical-only review and dispatch or monitor the authorized tests. Gator metadata
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
BlockedGator remains blocked on current head Next action: @elezar, update the branch to resolve the merge conflicts and mark the pull request ready for review when this revision is ready. Gator will then run the required critical-only review and continue monitoring the authorized tests. Gator metadata
|
BlockedGator remains blocked on current head Next action: @elezar, update the branch to resolve the merge conflicts and mark the pull request ready for review when this revision is ready. Gator will then run the required current-head critical-only review and dispatch or monitor the authorized tests. Gator metadata
|
BlockedGator remains blocked on current head Next action: @elezar, update the branch to resolve the merge conflicts, investigate the failing Ubuntu Snap smoke job, and mark the pull request ready for review when this revision is ready. Gator will then run the required current-head critical-only review and dispatch or monitor the authorized tests. Gator metadata
|
BlockedGator remains blocked on current head Next action: @elezar, update the branch to resolve the merge conflicts and mark the pull request ready for review when this revision is ready. Gator will then run the required critical-only review and continue monitoring the authorized tests. Gator metadata
|
BlockedThanks @elezar, this pull request is now ready for review, so the draft blocker is cleared. Gator remains blocked on current head Next action: @elezar, update the branch to resolve the merge conflicts. Gator will then run the required critical-only review and continue monitoring the authorized tests. Gator metadata
|
olivercalder
left a comment
olivercalder
left a comment
There was a problem hiding this comment.
Hi there, I'm part of the snapd team and have done a bit of work on the openshell snap in the past. Thanks so much for working on this! I had parked #2250 until the build/package/test tooling had migrated to bazel, but that work has since been dropped. I was hoping to improve testing of the openshell snap before picking up #2250 again, since it will enable the snap by default on supported systems.
This PR seems to be doing exactly that! So thanks.
One note: the openshell snap already has a snap store assertion which allows it to auto-connect its openshell:docker interface plug to any docker interface slot, including the system :docker slot provided by snapd. Thus, the openshell snap should be compatible with either a traditional Docker daemon install or the docker snap. Additionally, the docker snap should behave just like any other Docker daemon would, so it should also be possible for the openshell snap to talk to it even it has connected to the system :docker slot instead. But we want to test this.
Can we extend this smoke test to test the openshell snap with all three configurations:
dockersnap with direct connection betweenopenshellanddockersnaps, sosudo snap connect openshell:docker docker:docker-daemonwith thedockersnap is already installeddockersnap with system:dockerplug, sosudo snap connect openshell:docker :dockerif thedockersnap is already installed- system Docker daemon, which will be auto-connected if the
dockersnap is not present, otherwise we can manually connect to viasudo snap connect openshell:docker
Somewhat amusingly, if the docker snap is installed, then there are two viable slot candidates for auto-connection of the openshell:docker plug (docker:docker-daemon and the system :docker slot), so the auto-connection doesn't occur when openshell is installed. So in this case one must manually connect to either the system docker slot (sudo snap connect openshell:docker) or the docker snap's slot (sudo snap connect openshell:docker docker:docker-daemon). Connecting to the system :docker slot should allow openshell to interact with the docker snap as well as traditional Docker daemon installs, while connecting to docker:docker-daemon would only let it talk to the docker snap. The latter is more restrictive, so perhaps desirable. But we could also adjust the store assertions so the openshell snap would always auto-connect to the system :docker slot, and never the docker snap's docker:docker-daemon, for a more streamlined install but a bit less secure.
| shell: bash | ||
|
|
||
| jobs: | ||
| build-snap: |
There was a problem hiding this comment.
What is the relationship between this job and the build-snap: job in snap-ackage.yml? Seems both do many of the same things
There was a problem hiding this comment.
This might be a rebase hiccup. The intent was to split this in a similar way as what we do for RPM packages. The one is a callable unit that can be called from various points with different parameters (e.g. architecture).
| checkout-ref: ${{ github.sha }} | ||
| upload-channel: latest/edge | ||
| github-environment: latest/edge | ||
| publish: true |
There was a problem hiding this comment.
I'm probably mistaken, but will this mean that every time build-snap runs, the result will be published to the latest/edge channel? If this workflow runs on PRs and not just main, that would mean arbitrary PR changes would be published to latest/edge. But it also seems like publishing was implicitly true before and now this is adding a way to turn it off, so I may be wrong.
There was a problem hiding this comment.
The release-dev workflow SHOULD only run on main. I flipped the default in the snap-package job to ensure that this is not the case on PRs.
| - name: Connect log-observe interface | ||
| become: true | ||
| ansible.builtin.command: | ||
| cmd: snap connect openshell:log-observe | ||
|
|
||
| - name: Connect system-observe interface | ||
| become: true | ||
| ansible.builtin.command: | ||
| cmd: snap connect openshell:system-observe |
There was a problem hiding this comment.
These should both be autoconnected via a store assertion on the openshell snap, since 25 June, so this shouldn't be necessary. It's idempotent though, so it won't hurt. Existing docs in openshell around this are out of date, unfortunately.
There was a problem hiding this comment.
That's good to know. I was just going by what was being done in the existing "canary" job and trying to replicate that in Ansible. (and have something that's somewhat reproducible). Feel free to make suggestions.
There was a problem hiding this comment.
Hmm actually, if you're testing artifacts from the build pipeline with --dangerous which haven't been uploaded to the store, then they probably don't have store assertions, though I'm not sure. And that would also complicate testing with any non-snap docker since connecting to the system :docker slot without a store assertion requires snapd 2.77, which is currently in beta.
There was a problem hiding this comment.
The point is to test the snaps BEFORE publishing them to the store. Happy to iterate on a way to do this.
There was a problem hiding this comment.
I'm checking now whether assertions are required. But if so, I think really there are two different things we want to check:
- Does the
openshellsnap work when all the connections are in place?- That's what this test is for
- Does the
openshellsnap get the expected auto-connections when installed?- That sounds to me like it should be part of the
install.shtest, not here
- That sounds to me like it should be part of the
Edit: indeed, we do need manual connections always when testing a .snap file from outside the store:
ubuntu@clean-slate:~$ ls
openshell_0.0.116_amd64.snap
ubuntu@clean-slate:~$ sudo snap install ./openshell_0.0.116_amd64.snap
error: cannot find signatures with metadata for snap/component "./openshell_0.0.116_amd64.snap"
ubuntu@clean-slate:~$ sudo snap install ./openshell_0.0.116_amd64.snap --dangerous
2026-09-03T21:33:26Z INFO Waiting for automatic snapd restart...
2026-09-03T21:33:27Z INFO Waiting for automatic snapd restart...
2026-09-03T21:33:28Z INFO Waiting for automatic snapd restart...
2026-09-03T21:33:29Z INFO Waiting for automatic snapd restart...
openshell 0.0.116 installed
ubuntu@clean-slate:~$ snap connections openshell
Interface Plug Slot Notes
docker openshell:docker - -
home openshell:home :home -
log-observe openshell:log-observe - -
network openshell:network :network -
network-bind openshell:network-bind :network-bind -
system-observe openshell:system-observe - -
So everything here is correct. It's just not required in other places, like install.sh or tests of that. Which is something I should separately update...
| | Distro | Docker package | Docker Snap | Rootless Podman | SELinux | Package format | | ||
| | --- | --- | --- | --- | --- | --- | | ||
| | Ubuntu 24.04 | Yes | Yes | No | No | `.deb` | | ||
| | Ubuntu 26.04 | Yes | Yes | Yes | No | `.deb` | | ||
| | CentOS Stream 10 | No | No | No | Yes | `.rpm` | | ||
| | Fedora 44 | No | No | Yes | Yes | `.rpm` | | ||
| | Rocky Linux 9 | Yes | No | No | Yes | `.rpm` | |
There was a problem hiding this comment.
The docker snap should act like any other docker install, so is there a reason the docker snap is not supported on other systems? All the listed systems are supported by snapd, though it is not pre-installed. Also I would think the docker snap should certainly work fine on 24.04.
There was a problem hiding this comment.
The only reason is "not explicitly tested". Happy to expand the matrix once we have the basics in place.
| - name: Connect Docker Snap interface | ||
| become: true | ||
| ansible.builtin.command: | ||
| cmd: snap connect openshell:docker docker:docker-daemon |
There was a problem hiding this comment.
This is correct and is actually required (I initially thought it wasn't). Here's the reason:
The openshell snap has a store assertion:
type: snap-declaration
revision: 5
snap-id: ltw2m6EZ9UVOiglLDFP4blLwLO92hNhu
snap-name: openshell
plugs:
docker:
allow-auto-connection: trueThis means that it will auto-connect the docker interface if there's one connection slot candidate. However, since snapd provides an implicit :docker system slot, if the docker snap is installed before installing the openshell snap, then there will be two candidates for the openshell:docker plug to auto-connect to: the snapd-provided :docker slot, and the docker snap's docker:docker-daemon. Because there are two auto-connection candidates, snapd does not auto-connect to either, leaving it up to the user to choose which they'd like to connect. So here, this connection is required.
Because the docker snap acts just like any other system Docker daemon, we could actually connect the openshell:docker plug to the system :docker slot instead of docker:docker-daemon, and everything should still work as expected, regardless of whether Docker is a traditional or snap install. Really, we want to test all three configurations:
openshell:docker docker:docker-daemonopenshell:docker :dockerwithdockersnap installedopenshell:docker :dockerwithoutdockersnap installed, instead a non-snap Docker daemon
There was a problem hiding this comment.
I suppose that's the idea behind these VM configs is that we can define the ansible roles (and combinations) to test all of these. Is this something you have time / capacity to look at once the basics are in place?
There was a problem hiding this comment.
It's not something I think I should commit to driving, no, as my focus needs to be on $dayjob commitments. I'm not in the weeds enough about how all the workflows are designed in OpenShell to do it quickly/cleanly.
That said, I would expect that it should be fairly simple: add a few parameters, something like docker: (system|snap) and slot: (":docker"|"docker:docker-daemon"), then have three jobs which share almost everything:
docker: "system", slot: ":docker"docker: "snap", slot: ":docker"docker: "snap", slot: "docker:docker-daemon"
Then the relevant workflow task(s) which is responsible for installing docker before the test would have:
case "$docker" in
snap)
sudo snap install docker
;;
system)
# <however you install system docker, I'm not sure>
;;
*)
echo "invalid parameter for docker install type: $docker"
exit 1
;;
esacAnd the relevant workflow task(s) which handle setting up the snap connections for the openshell snap would just have:
sudo snap connect openshell:docker "$slot"rather than hard-coding docker:docker-daemon as is done now.
Apologies, as I don't usually write GitHub-style workflows, I'm used to other systems.
Signed-off-by: Evan Lezar <elezar@nvidia.com>
Signed-off-by: Evan Lezar <elezar@nvidia.com>
Signed-off-by: Evan Lezar <elezar@nvidia.com>
Signed-off-by: Evan Lezar <elezar@nvidia.com>
Signed-off-by: Evan Lezar <elezar@nvidia.com>
Signed-off-by: Evan Lezar <elezar@nvidia.com>
Signed-off-by: Evan Lezar <elezar@nvidia.com>
|
🌿 Preview your docs: https://nvidia-preview-pr-2869.docs.buildwithfern.com/openshell |
4 participants
Summary
Add a VM-based Ubuntu Snap conformance smoke test for branch-built artifacts. It exercises the Snap and Docker-Snap lifecycle before a Release Dev publish, while documenting the temporary JWT bootstrap required by the current Snap package.
Related Issue
Related to #2055.
Changes
Testing
mise run pre-commitpassesChecklist