Some tests were failing on debian-sid because we weren't coping with
systemd versions that contained a release candidate suffix. This deals
with it simply by ignoring which should be enough given that we only
compare them roughly to check if systemd is not too old.
```
error: cannot get logs: cannot get systemd version: cannot convert systemd
version to number: 256~rc3
```
Signed-off-by: Miguel Pires <miguel.pires@canonical.com>
- debug.snapd.log to enable debug traces for snapd service. This is
done by creating an environment file used by the snapd service.
- debug.systemd.log-level to be able to set systemd log level, by
adding a configuration file to /etc/systemd/system.conf.d/.
* systemd: add function that implements "systemd-escape" in addition to already existing "systemd-escape --path"
* s/cgroup: escape created unit name in CreateTransientScopeForTracking
With the addition of component hooks, we'll have unit names that include
a '+', like 'snap.snapname+comp.hook.install'. This causes systemd to
complain that the unit isn't properly escaped. On the command line,
systemd-run will properly escape this for you (with a warning), but the
dbus API doesn't do that.
* s/naming: teach ParseSecurityTag to handle tags from component hooks
* Revert "systemd: add function that implements "systemd-escape" in addition to already existing "systemd-escape --path""
This reverts commit 0521600ec8fa785b69d2b7a85fa8da9be4938a5a.
* systemd: add functions for escaping security tags to valid systemd unit names
We must at least partially escape unit names that are created from
security tags, since they may potentially contain '+' characters from
snap components.
Since we already use unit names with '-' in them, we cannot simply use a
reimplementation of systemd-escape. This is because '-' is escaped by
systemd-escape. Note that '-' is a valid character is a unit name, since
it is used as the replacement for the '/' character by systemd-escapes.
Thus, we have our own functions for converting a security tag to a unit
name, and the inverse. These functions only escape the '+' character
that appears in security tags.
* s/cgroup: use new conversions from security tags to unit names, and the inverse
* systemd: update doc comment on UnitNameFromSecurityTag
Co-authored-by: Maciej Borzecki <maciek.borzecki@gmail.com>
* s/naming: add ComponentName method to HookSecurityTag interface
* systemd: split tests for UnitNameFromSecurityTag and SecurityTagFromUnitName
* s/naming: add test for invalid snap instance that is a part of a component
* s/naming: refactor ParseSecurityTag to clarify that components cannot have apps yet
* systemd, s/cgroup: rename security tag and unit name conversion functions for clarity
---------
Co-authored-by: Maciej Borzecki <maciek.borzecki@gmail.com>
* kernel,overlord: use function from kernel pkg to get kernel early
mount dir.
* o/snapstate/sequence: add ancillary methods to retrieve components
for a given snap revision in the sequence. One of the methods can be
given a component type for filtering.
* o/snapstate/backend: add methods to set-up kernel-modules components
* o/snapstate: add handlers to set-up kernel-modules components
* o/snapstate: add specific setup task for kernel-modules components
* o/{devicestate,snapstate}: change name of kernel set-up tasks
* systemd: add before dependency to early mounts for local-fs.target
So dependencies are the same as when mounting in /snap, as that is
where we will do also the early mounts now.
* kernel: use normal mount points when building drivers tree
* overlord/devicestate: no need to remove early mount anymore
* overlord/snapstate: consider kernel type when ensuring the mounts
* kernel: put modules back in "updates" folder
As that gives priority to the kernel modules under that directory (see
depmod.d(5)).
Do not restart the mount units when starting snapd even if the file
itself has been modified. This is to prevent systemd stopping services
when the mount unit gets restarted, as there are required dependencies
in the mount path for services defined in snaps.
We were doing a reload-or-restart, but we actually need a restart if a
mount unit is modified, as reload is relevant only to ask services to
reload their (internal) configuration. This was hitting us when
updating a component, as they get mounted in the same place as the
previous component revision (for a given owner snap revision), and
therefore we are rewriting a mount unit that does not change its name.
The old component was still showing in that location.
* wrappers: fix test to not use mocked systemctl command
Fix the test to not use a mocked systemctl command so that we can get more
reliable execution even when running in a slow environment. The issue is that
parts of the 'stop' chain run in a goroutine, so the order of `systemctl show`
and `systemctl stop` calls isn't guaranteed. No top of this, since this can run
in parallel now, the code would need to use `testutilg.MockLockedCommand` to
prevent corruption of the call log.
Fixes the following issue seen in unit tests or OBS builds:
```
----------------------------------------------------------------------
FAIL: services_test.go:3371: servicesTestSuite.TestStartServicesStopsServicesIncludingActivation
using shellcheck: "/usr/bin/shellcheck"
services_test.go:3433:
c.Check(r.Calls(), DeepEquals, [][]string{
// Enable phase for the service activation units, we have one set of system daemon and one set of user daemon
{"systemctl", "daemon-reload"},
{"systemctl", "--user", "daemon-reload"},
{"systemctl", "--no-reload", "enable", "snap.hello-snap.svc1.sock1.socket", "snap.hello-snap.svc1.sock2.socket"},
{"systemctl", "daemon-reload"},
{"systemctl", "--user", "--global", "--no-reload", "enable", "snap.hello-snap.svc2.sock1.socket", "snap.hello-snap.svc2.sock2.socket"},
// Start phase for service activation units, we have rigged the game by making sure this stage fails,
// so only one of the services will attempt to start
{"systemctl", "start", "snap.hello-snap.svc1.sock1.socket"},
// Stop phase, where we attempt to stop the activation units and the primary services
// We first attempt to stop the user services, then the system services
{"systemctl", "--user", "stop", "snap.hello-snap.svc2.sock1.socket"},
{"systemctl", "--user", "show", "--property=ActiveState", "snap.hello-snap.svc2.sock1.socket"},
{"systemctl", "--user", "stop", "snap.hello-snap.svc2.sock2.socket"},
{"systemctl", "--user", "show", "--property=ActiveState", "snap.hello-snap.svc2.sock2.socket"},
{"systemctl", "--user", "stop", "snap.hello-snap.svc2.service"},
{"systemctl", "--user", "show", "--property=ActiveState", "snap.hello-snap.svc2.service"},
{"systemctl", "stop", "snap.hello-snap.svc1.sock1.socket", "snap.hello-snap.svc1.sock2.socket", "snap.hello-snap.svc1.service"},
{"systemctl", "show", "--property=ActiveState", "snap.hello-snap.svc1.sock1.socket"},
// Disable phase, where the activation units are being disabled
{"systemctl", "--no-reload", "disable", "snap.hello-snap.svc1.sock1.socket", "snap.hello-snap.svc1.sock2.socket"},
{"systemctl", "daemon-reload"},
{"systemctl", "--user", "--global", "--no-reload", "disable", "snap.hello-snap.svc2.sock1.socket", "snap.hello-snap.svc2.sock2.socket"},
})
... obtained [][]string = [][]string{[]string{"systemctl", "daemon-reload"}, []string{"systemctl", "--user", "daemon-reload"}, []string{"systemctl", "--no-reload", "enable", "snap.hello-snap.svc1.sock1.socket", "snap.hello-snap.svc1.sock2.socket"}, []string{"systemctl", "daemon-reload"}, []string{"systemctl", "--user", "--global", "--no-reload", "enable", "snap.hello-snap.svc2.soc
k1.socket", "snap.hello-snap.svc2.sock2.socket"}, []string{"systemctl", "start", "snap.hello-snap.svc1.sock1.socket"}, []string{"systemctl", "--user", "stop", "snap.hello-snap.svc2.sock1.socket"}, []string{"systemctl", "--user", "show", "--property=ActiveState", "snap.hello-snap.svc2.sock1.socket"}, []string{"systemctl", "--user", "show", "--property=ActiveState", "snap.hello-snap
.svc2.sock2.socket"}, []string{"systemctl", "--user", "stop", "snap.hello-snap.svc2.sock2.socket"}, []string{"systemctl", "--user", "stop", "snap.hello-snap.svc2.service"}, []string{"systemctl", "--user", "show", "--property=ActiveState", "snap.hello-snap.svc2.service"}, []string{"systemctl", "stop", "snap.hello-snap.svc1.sock1.socket", "snap.hello-snap.svc1.sock2.socket", "snap.h
ello-snap.svc1.service"}, []string{"systemctl", "show", "--property=ActiveState", "snap.hello-snap.svc1.sock1.socket"}, []string{"systemctl", "--no-reload", "disable", "snap.hello-snap.svc1.sock1.socket", "snap.hello-snap.svc1.sock2.socket"}, []string{"systemctl", "daemon-reload"}, []string{"systemctl", "--user", "--global", "--no-reload", "disable", "snap.hello-snap.svc2.sock1.so
cket", "snap.hello-snap.svc2.sock2.socket"}}
... expected [][]string = [][]string{[]string{"systemctl", "daemon-reload"}, []string{"systemctl", "--user", "daemon-reload"}, []string{"systemctl", "--no-reload", "enable", "snap.hello-snap.svc1.sock1.socket", "snap.hello-snap.svc1.sock2.socket"}, []string{"systemctl", "daemon-reload"}, []string{"systemctl", "--user", "--global", "--no-reload", "enable", "snap.hello-snap.svc2.soc
k1.socket", "snap.hello-snap.svc2.sock2.socket"}, []string{"systemctl", "start", "snap.hello-snap.svc1.sock1.socket"}, []string{"systemctl", "--user", "stop", "snap.hello-snap.svc2.sock1.socket"}, []string{"systemctl", "--user", "show", "--property=ActiveState", "snap.hello-snap.svc2.sock1.socket"}, []string{"systemctl", "--user", "stop", "snap.hello-snap.svc2.sock2.socket"}, []st
ring{"systemctl", "--user", "show", "--property=ActiveState", "snap.hello-snap.svc2.sock2.socket"}, []string{"systemctl", "--user", "stop", "snap.hello-snap.svc2.service"}, []string{"systemctl", "--user", "show", "--property=ActiveState", "snap.hello-snap.svc2.service"}, []string{"systemctl", "stop", "snap.hello-snap.svc1.sock1.socket", "snap.hello-snap.svc1.sock2.socket", "snap.h
ello-snap.svc1.service"}, []string{"systemctl", "show", "--property=ActiveState", "snap.hello-snap.svc1.sock1.socket"}, []string{"systemctl", "--no-reload", "disable", "snap.hello-snap.svc1.sock1.socket", "snap.hello-snap.svc1.sock2.socket"}, []string{"systemctl", "daemon-reload"}, []string{"systemctl", "--user", "--global", "--no-reload", "disable", "snap.hello-snap.svc2.sock1.so
cket", "snap.hello-snap.svc2.sock2.socket"}}
... Difference:
... [8]: []string[5] != []string[4]
... [9]: []string[4] != []string[5]
```
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* wrappers: fix more unreliable unit tests
The following tests were found to be unreliable on very slow systems:
```
----------------------------------------------------------------------
FAIL: services_test.go:3043: servicesTestSuite.TestAddSnapServicesWithDisabledServices
services_test.go:3102:
c.Assert(r.Calls(), DeepEquals, [][]string{
{"systemctl", "daemon-reload"},
})
... obtained [][]string = [][]string(nil)
... expected [][]string = [][]string{[]string{"systemctl", "daemon-reload"}}
... Difference:
... [][]string[0] != [][]string[1]
----------------------------------------------------------------------
FAIL: services_test.go:3500: servicesTestSuite.TestNoStartDisabledServices
services_test.go:3545:
c.Assert(r.Calls(), DeepEquals, [][]string{
{"systemctl", "--no-reload", "enable", svc2Name},
{"systemctl", "daemon-reload"},
{"systemctl", "start", svc2Name},
})
... obtained [][]string = [][]string(nil)
... expected [][]string = [][]string{[]string{"systemctl", "--no-reload", "enable", "snap.hello-snap.svc2.service"}, []string{"systemctl", "daemon-reload"}, []string{"systemctl", "start", "snap.hello-snap.svc2.service"}}
... Difference:
... [][]string[0] != [][]string[3]
----------------------------------------------------------------------
FAIL: services_test.go:2867: servicesTestSuite.TestQueryDisabledServices
services_test.go:2924:
c.Assert(err, IsNil)
... value *errors.errorString = &errors.errorString{s:"cannot get unit \"snap.hello-snap.svc2.service\" status: missing Id, UnitFileState, Type, Names, NeedDaemonReload in ‘systemctl show’ output"} ("cannot get unit \"snap.hello-snap.svc2.service\" status: missing Id, UnitFileState, Type, Names, NeedDaemonReload in ‘systemctl show’ output")
----------------------------------------------------------------------
FAIL: services_test.go:2943: servicesTestSuite.TestQueryDisabledServicesActivatedServices
services_test.go:3023:
c.Assert(err, IsNil)
... value *errors.errorString = &errors.errorString{s:"cannot get unit \"snap.hello-snap.svc1.service\" status: missing Id, UnitFileState, Type, Names, NeedDaemonReload in ‘systemctl show’ output"} ("cannot get unit \"snap.hello-snap.svc1.service\" status: missing Id, UnitFileState, Type, Names, NeedDaemonReload in ‘systemctl show’ output")
OOPS: 188 passed, 4 FAILED
--- FAIL: TestWrappers (3.50s)
```
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* systemd/systemdtest: handle non service units when mocking outputs
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* wrappers: simplify mocks by using systemdtest helpers
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
---------
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
We define specific mount unit templates for kernel-modules components,
as we need to mount them before any service that loads kernel modules.
These units are of two types:
1. Units to mount the kernel-modules component containers.
2. Units that mount bits of these containers under
/usr/lib/{modules,firmware}.
Remove SnapName from systemd.MountUnitOptions, as it was not used
anymore (instead, the mount unit description already has the container
name). Remove it from the EnsureMountUnitFile method as well.
* overlord,snap: rename MinimalContainerInfo to a more sensible name
* snap: add ComponentInfo methods to build mount/files dirs
* snap,overlord: create and use ancillary build components functions
* o/snapstate/backend: refactor addMountUnit so it accepts components
Change addMountUnit arguments so we can use it for both snaps and
components.
* snap: split ReadComponentInfoFromContainer so yaml can be read
independently of reading the full container.
* o/snapstate/backend: add function to read component from path
* o/snapstate/backend: implement {,Undo}SetupComponent
Implement methods to set-up components and undo this set-up, which
consists on creating mount units and copying the component container
to the right place.
* many: address review comments
* overlord,snap: component file includes the snap instance name
* snap: add MountDescription to ContainerPlaceInfo interface
* overlord,systemd: pass description instead of revision when creating
mount files. As we do not need to set it explicitly.
* overlord,snap: unexport snap.ComponentPlaceInfo
We only need to usde the snap.ContainerPlaceInfo interface in the
backend now.
* overlord,systemd: address some review comments
* overlord,snap: address review comments
Some invocations to external programs used exec.CombinedOutput, that
combines stdout and strerr into a single byte array. This can be an
issue if this output is parsed, as many programs print debug output or
warnings to stderr and that data is unexpected by the parsers. This
patch changes to using osutil.RunSplitOutput or osutil.RunCmd (that
return separately stdout and stderr) when we need to parse stdout, and
also in some other cases when printing separately both streams could
be helpful. Fixes LP #1885597.
Replace ioutil.WriteFile with os.WriteFile since the former has been
deprecated since go1.16 and simply calls the latter.
Signed-off-by: Miguel Pires <miguel.pires@canonical.com>
* osutil: add support for symlinks to EnsureFileState
This is part of the preparation for using Ensure* functions instead
of Add* functions in backend.LinkSnap and backend.UnlinkSnap.
Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com>
* osutil: remove FileState.Type() and use file mode instead
Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com>
* osutil: add more tests and doc comment for SymlinkFileState
Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com>
* osutil: adjust doc comment for SymlinkFileState
Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com>
* osutil: make unit tests and EnsureFileState more readable
Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com>
* osutil: only allow regular files in FileState types
- FileReference
- FileReferencePlusMode
- MemoryFileState
Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com>
* osutil: make file type check more explicit in FileState types
- FileReference
- FileReferencePlusMode
- MemoryFileState
Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com>
* osutil: remove unused checkFileType function
Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com>
---------
Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com>
Go 1.19 includes some changes to gofmt which intend to make lists and
heading clearer when rendered (https://go.dev/doc/go1.19). This commit
is the result of running the new gofmt and manually fixing some of it.
This was necessary because the new gofmt assumed lines beginning w/ tabs
to start lists or examples. While this is often true in our codebase,
we occasionally also use tabs to indent the lines after a TODO or FIXME
prefix or in yaml (e.g., excerpts of a snap.yaml). This meant that a lot of the
reformatted comments were broken and had to be fixed manually.
Signed-off-by: Miguel Pires <miguel.pires@canonical.com>
There are scenarios where not changing the target does not work:
* preseeding an image with an old version of snapd
* reverting to an older version snapd
* reverting to an older version of core on UC16
* having and old package of snapd with reexecution of the snap
Also we use `Before=local-fs.target` which is a bit better more correct
than the older `Before=snapd.service`.
