cmd/snapd/cli: add snap routine user-service-precondition command by olivercalder · Pull Request #17379 · canonical/snapd

olivercalder · 2026-07-16T17:32:37Z

This PR is based on #17378. It's the second in a chain to implement ExecCondition checks to prevent user daemons from running within sessions with class greeter. Spec here: https://docs.google.com/document/d/1PeWQrLDExIb5I7HqDYm1dn9WzQcqksKBn5a6V5ASvoI/edit

Add a new snap routine subcommand designed to exit with an error code if invoked from within a systemd session with class which isgreeter. The intent is for this to be called from ExecCondition stanzas in systemd unit files. Thus, the subcommand is hidden.

Because this will be invoked by systemd via ExecCondition when deciding whether to start the service, the SuccessExitStatus also applies. Thus, we need to be sure not to exit with an error code which happens to match one of the success codes. To accomplish this, the new snap routine user-service-precondition command includes an optional argument --error-exit-code which takes an integer as an argument, defaulting to 1. The caller must ensure that the exit code passed in is not a success exit status.

CC @aleasto

This work is tracked internally by https://warthogs.atlassian.net/browse/SNAPDENG-37244

Add a new `logind` package similar to the existing `systemd` package. Add a private function to invoke `loginctl`, and expose a `SessionClass` method which calls `loginctl show-session auto -p Class` and parses the resulting `Class=foo` to return `foo`. Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

Copilot

Pull request overview

This PR introduces a hidden snap routine user-service-precondition subcommand intended for use in systemd ExecCondition= to prevent snap user daemons from starting in greeter sessions, by querying the current session class via loginctl.

Changes:

Add a new logind package that wraps loginctl show-session auto -p Class and parses the session class.
Add the hidden snap routine user-service-precondition command that exits non-zero for greeter (or when session class cannot be determined), with a configurable --error-exit-code.
Add unit tests for the new logind package and the new routine command.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
logind/logind.go	Adds `loginctl` wrapper and `SessionClass()` parser used to determine current session class.
logind/logind_test.go	Adds tests covering class parsing and error formatting behavior.
logind/export_test.go	Exposes setters on `logind.Error` for black-box tests.
cmd/snapd/cli/export_test.go	Adds a mock hook for the logind session class function used by the routine command.
cmd/snapd/cli/cmd_routine_user_service_precondition.go	Adds the hidden `snap routine user-service-precondition` command that enforces the greeter/session-class precondition.
cmd/snapd/cli/cmd_routine_user_service_precondition_test.go	Adds tests validating exit-status behavior and stderr output for greeter/no-session cases.

github-actions · 2026-07-16T17:55:41Z

Fri Jul 24 13:27:10 UTC 2026
The following results are from: https://github.com/canonical/snapd/actions/runs/29927412155

Failures:

Preparing:

openstack:ubuntu-26.04-64:tests/main/interfaces-posix-mq
openstack:ubuntu-24.04-64:tests/main/nvidia-files:535
openstack:ubuntu-24.04-64:tests/main/nvidia-files:535_server

Executing:

openstack:ubuntu-26.04-64:tests/main/broken-seeding
openstack:ubuntu-26.04-64:tests/main/snap-user-service
openstack:ubuntu-26.04-64:tests/main/auto-refresh-pre-download:restart
openstack:ubuntu-26.04-64:tests/main/auto-refresh-pre-download:ignore
openstack:ubuntu-26.04-64:tests/main/auto-refresh-pre-download:close
openstack:ubuntu-26.04-64:tests/main/auto-refresh-pre-download:close_mid_restart
openstack:ubuntu-26.04-64:tests/main/bad-interfaces-warn

Skipped tests from snapd-testing-skip

If you wish to have any of the below tests run in your PR, in your PR description, add 'unskip:' followed by a copy-and-pasted list of the below tests you wish to run (unskip plus test list must be valid yaml)

openstack:ubuntu-24.04-64:tests/main/i18n
openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-flag-restart
openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-integration-tests
openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-prompt-restoration
openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-smoke
openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-snapd-startup
openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-support
openstack:ubuntu-26.04-64:tests/main/i18n
openstack:ubuntu-26.04-64:tests/main/interfaces-requests-activates-handlers

codecov · 2026-07-16T17:55:56Z

Codecov Report

❌ Patch coverage is 74.24242% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.92%. Comparing base (91bc64b) to head (a5f73fc).
⚠️ Report is 5 commits behind head on master.

Files with missing lines	Patch %	Lines
systemd/logind/logind.go	65.85%	13 Missing and 1 partial ⚠️
...snapd/cli/cmd_routine_user_service_precondition.go	86.36%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #17379      +/-   ##
==========================================
- Coverage   78.92%   78.92%   -0.01%     
==========================================
  Files        1395     1395              
  Lines      195977   196064      +87     
  Branches     2466     2466              
==========================================
+ Hits       154680   154743      +63     
- Misses      32018    32035      +17     
- Partials     9279     9286       +7

Flag	Coverage Δ
unittests	`78.92% <74.24%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…Class

pedronis

the panic shouldn't be used

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

Add a new `snap routine` subcommand designed to exit with an error code if invoked from within a systemd session with class which is`greeter`. The intent is for this to be called from `ExecCondition` stanzas in systemd unit files. Thus, the subcommand is hidden. Because this will be invoked by `systemd` via `ExecCondition` when deciding whether to start the service, the `SuccessExitStatus` also applies. Thus, we need to be sure not to exit with an error code which happens to match one of the success codes. To accomplish this, the new `snap routine user-service-precondition` command includes an optional argument `--error-exit-code` which takes an integer as an argument, defaulting to 1. The caller must ensure that the exit code passed in is not a success exit status. Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

In order to exit with a particular error code, a panic was used, which was then caught and the code used to exit cleanly. However, this is unnecessary for `snap routine user-service-precondition`, we can instead simply add a new error type and handle it like other special error cases in `exitCodeFromError`. Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

pedronis

small wondering

pedronis · 2026-07-22T11:45:30Z

+		return ErrExtraArgs
+	}
+	if x.ErrorExitCode < 1 || x.ErrorExitCode > 255 {
+		return &userSessionPreconditionError{code: 1, msg: "invalid --error-exit-code: must be in range 1-255"}


maybe code: 2 for this?

Yeah that makes more sense, thanks

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

pedronis

high level +1 but get closer reviews once the prereq are merged

zyga

Lgtm with two small suggestions inline.

zyga · 2026-07-24T09:22:05Z

 	switch {
 	case err == nil:
 		return 0
+	case errors.As(err, &userSessionPreconditionErr):


This could have been a more generic error type that can be used by any CLI.

In my go-cmdr library I use a pair of interfaces: https://pkg.go.dev/gitlab.com/zygoon/go-cmdr#ExitCoder and https://pkg.go.dev/gitlab.com/zygoon/go-cmdr#ExitMessenger to allow commands to influence the exit code and the (custom) error message a specific error should convey.

zyga · 2026-07-24T09:23:59Z

+	// strip the "Class=" prefix from the output
+	orig := strings.TrimSpace(string(out))
+	before, after, ok := strings.Cut(orig, "=")
+	if !ok || before != "Class" || after == "" || strings.Contains(after, "=") {


Nitpick: propName, propValue?

olivercalder added this to the 2.78 milestone Jul 16, 2026

olivercalder requested review from andrewphelpsj, Copilot, natibek, pedronis and zyga July 16, 2026 17:32

Copilot started reviewing on behalf of olivercalder July 16, 2026 17:33 View session

Copilot AI reviewed Jul 16, 2026

View reviewed changes

Comment thread logind/logind.go Outdated

Comment thread cmd/snapd/cli/cmd_routine_user_service_precondition.go Outdated

olivercalder mentioned this pull request Jul 16, 2026

wrappers,tests: add ExecCondition=snap routine user-service-precondition #17380

Open

fixup! logind: add logind package with loginctl show-session auto -p …

f21c9cd

…Class

olivercalder force-pushed the snap-routine-user-service-precondition branch from 10aeb6b to c78b2f4 Compare July 16, 2026 23:56

pedronis reviewed Jul 17, 2026

View reviewed changes

Comment thread cmd/snapd/cli/cmd_routine_user_service_precondition.go Outdated

olivercalder added 5 commits July 20, 2026 21:14

logind: add context to loginctlCmd and improve implementation

5daa051

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

systemd/logind: move logind package to systemd/logind

0777383

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

fixup! cmd/snapd/cli: add snap routine user-service-precondition command

c99ccbc

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

olivercalder force-pushed the snap-routine-user-service-precondition branch from c78b2f4 to a5f73fc Compare July 21, 2026 03:17

olivercalder requested a review from pedronis July 21, 2026 03:33

pedronis reviewed Jul 22, 2026

View reviewed changes

cmd/snapd/cli: exit with 2 instead of 1 if given invalid error exit code

4da75a1

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

olivercalder requested a review from pedronis July 22, 2026 15:37

pedronis approved these changes Jul 22, 2026

View reviewed changes

zyga approved these changes Jul 24, 2026

View reviewed changes

github-actions Bot added the Auto rerun spread Auto reruns spread up to 4 times in non-draft PRs w/ >=1 approval and <20 fails in any fund. system label Jul 24, 2026

Uh oh!

Conversation

olivercalder commented Jul 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Failures:

Preparing:

Executing:

Skipped tests from snapd-testing-skip

Uh oh!

codecov Bot commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

pedronis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pedronis left a comment

Choose a reason for hiding this comment

Uh oh!

pedronis Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

olivercalder Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

pedronis left a comment

Choose a reason for hiding this comment

Uh oh!

zyga left a comment

Choose a reason for hiding this comment

Uh oh!

zyga Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

zyga Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions Bot commented Jul 16, 2026 •

edited

Loading

codecov Bot commented Jul 16, 2026 •

edited

Loading