* 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`.
`snapd.mounts-pre.target` will be before any mount unit,
`snapd.mounts.target`. Now we can schedule before or after mounts
without needing to modify the mount units.
We also install those mounts to `snapd.mounts.target` so that we can
make snapd.service for example, "want" all mounts.
According to documentation `systemctl disable` removes all symlinks
include manually created ones rather than just the ones matching
the `[Install]` section.
When booting in OEM configuration mode, the systemd target is set to
`oem-config.target` instead of the usual `multi-user.target`; if our
mount units require solely the multi-user target, which is not reached
during OEM configuration mode, they won't be started.
So, let's add `default.target` to our mount units: this target is a
symbolic link to the system default target, which is usually
`multi-user.target` for a desktop session.
Most likely, there's no need to keep `multi-user.target` in the
`WantedBy` line, but since this is a hotfix, let's take a defensive
approach and keep it until we have the time to verify that removing it
does not cause any regression.
Fixes LP:#1983528
