From 2926349ea86b12df6e5cb3428e531610318e4639 Mon Sep 17 00:00:00 2001 From: Jonathan Conder Date: Thu, 27 Aug 2026 19:13:46 +1200 Subject: [PATCH 01/15] Fix potential data race in projectFsRoot --- internal/workshop/lxd/lxd_backend_project.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/internal/workshop/lxd/lxd_backend_project.go b/internal/workshop/lxd/lxd_backend_project.go index 110eeba5b..f68ad76a8 100644 --- a/internal/workshop/lxd/lxd_backend_project.go +++ b/internal/workshop/lxd/lxd_backend_project.go @@ -18,6 +18,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "regexp" "slices" @@ -352,7 +353,13 @@ func (s *Backend) projectFsRoot(conn lxd.InstanceServer, ctx context.Context, pr continue } if err = meta.WaitExecution(ctx); err != nil { - logger.Debugf("cannot check %q bind-mounts: %v, findmnt output: %s", i.Name, err, errbuf.String()) + // It's unsafe to access errbuf before the DataDone channel is closed. + var details string + if _, ok := errors.AsType[*workshop.ErrExec](err); ok { + details = ", findmnt output: " + errbuf.String() + } + + logger.Debugf("cannot check %q bind-mounts: %v%s", i.Name, err, details) continue } From 476ad5aa3f78c375a739cc2af63b3c841fefedd4 Mon Sep 17 00:00:00 2001 From: Jonathan Conder Date: Tue, 28 Jul 2026 18:20:09 +1200 Subject: [PATCH 02/15] Expand LXD filters to include all instance types --- internal/workshop/lxd/lxd_backend.go | 2 +- internal/workshop/lxd/lxd_backend_project.go | 6 +++--- internal/workshop/lxd/tests/helper/helper.go | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/workshop/lxd/lxd_backend.go b/internal/workshop/lxd/lxd_backend.go index bf8a148d8..3ad2b46eb 100644 --- a/internal/workshop/lxd/lxd_backend.go +++ b/internal/workshop/lxd/lxd_backend.go @@ -1131,7 +1131,7 @@ func (s *Backend) ProjectWorkshops(ctx context.Context) ([]*workshop.Workshop, e // Get all the running workshops for this project. args := lxd.GetInstancesArgs{ - InstanceType: api.InstanceTypeContainer, + InstanceType: api.InstanceTypeAny, Filters: []string{"config.user.workshop.project-id=" + p.ProjectId}, } instances, err := conn.GetInstances(args) diff --git a/internal/workshop/lxd/lxd_backend_project.go b/internal/workshop/lxd/lxd_backend_project.go index f68ad76a8..612d57330 100644 --- a/internal/workshop/lxd/lxd_backend_project.go +++ b/internal/workshop/lxd/lxd_backend_project.go @@ -290,7 +290,7 @@ func (s *Backend) pruneProjects(client lxd.InstanceServer, ctx context.Context, // list of projects that we track (only if there are no remaining // workshops for this project) args := lxd.GetInstancesArgs{ - InstanceType: api.InstanceTypeContainer, + InstanceType: api.InstanceTypeAny, Filters: []string{"config.user.workshop.project-id=" + prj.ProjectId}, } workshops, err := client.GetInstances(args) @@ -314,7 +314,7 @@ func (s *Backend) pruneProjects(client lxd.InstanceServer, ctx context.Context, func (s *Backend) projectFsRoot(conn lxd.InstanceServer, ctx context.Context, projectId string) (path string, err error) { args := lxd.GetInstancesArgs{ - InstanceType: api.InstanceTypeContainer, + InstanceType: api.InstanceTypeAny, Filters: []string{"config.user.workshop.project-id=" + projectId}, } workshops, err := conn.GetInstances(args) @@ -410,7 +410,7 @@ func (s *Backend) updateProjectMounts(conn lxd.InstanceServer, ctx context.Conte projectCtx := context.WithValue(ctx, workshop.ContextProjectId, project.ProjectId) args := lxd.GetInstancesArgs{ - InstanceType: api.InstanceTypeContainer, + InstanceType: api.InstanceTypeAny, Filters: []string{"config.user.workshop.project-id=" + project.ProjectId}, } workshops, err := conn.GetInstances(args) diff --git a/internal/workshop/lxd/tests/helper/helper.go b/internal/workshop/lxd/tests/helper/helper.go index ddb51ca10..eb6659f23 100644 --- a/internal/workshop/lxd/tests/helper/helper.go +++ b/internal/workshop/lxd/tests/helper/helper.go @@ -58,7 +58,7 @@ func CleanupLxdProject(c *check.C, client lxd.InstanceServer, project string) { } } - args := lxd.GetInstancesArgs{InstanceType: api.InstanceTypeContainer} + args := lxd.GetInstancesArgs{InstanceType: api.InstanceTypeAny} instances, err := cli.GetInstances(args) c.Check(err, check.IsNil) for _, i := range instances { From 33b93ee55a38147dc3ab84fc8e72fcef469b6a24 Mon Sep 17 00:00:00 2001 From: Jonathan Conder Date: Tue, 28 Jul 2026 18:20:09 +1200 Subject: [PATCH 03/15] Replace start command shell script with LXD event listener --- .../tests/integration/backend_test.go | 31 ++-- internal/waitready/waitready.go | 2 +- internal/workshop/lxd/lxd_backend.go | 140 +++++++++++------- internal/workshop/lxd/start_command.sh | 12 -- internal/workshop/lxd/tests/helper/helper.go | 35 ++++- .../lxd/tests/integration/project_test.go | 4 + .../tests/integration/snapshot-format.yaml | 8 +- .../lxd/tests/integration/snapshot_test.go | 1 - .../tests/integration/workshop_exec_test.go | 14 +- .../lxd/tests/integration/workshop_test.go | 18 ++- tests/main/start/task.yaml | 24 --- 11 files changed, 162 insertions(+), 127 deletions(-) delete mode 100644 internal/workshop/lxd/start_command.sh diff --git a/internal/interfaces/lxd_device/tests/integration/backend_test.go b/internal/interfaces/lxd_device/tests/integration/backend_test.go index 62835b962..738ba65d6 100644 --- a/internal/interfaces/lxd_device/tests/integration/backend_test.go +++ b/internal/interfaces/lxd_device/tests/integration/backend_test.go @@ -41,6 +41,10 @@ import ( "github.com/canonical/workshop/internal/workshop/lxd/tests/helper" ) +func TestMain(m *testing.M) { + os.Exit(helper.RunTestsOrWorkshopCtl(m)) +} + type backendDeviceSuite struct { ctx context.Context be *lxdbackend.Backend @@ -81,17 +85,6 @@ func (f *backendDeviceSuite) readWorkshopFile(c *check.C, fname string) string { return string(buf) } -func defaultTestDevices(pid, w string) ([]workshop.Mount, []workshop.ProxyEntry) { - cwd, _ := os.Getwd() - mounts := []workshop.Mount{{ - Name: workshop.ConfigProjectPathDevice, - Type: workshop.HostWorkshop, - What: cwd, - Where: workshop.WorkshopProjectPath, - }} - return mounts, nil -} - func (f *backendDeviceSuite) SetUpTest(c *check.C) { dirs.SetRootDir(c.MkDir()) c.Assert(dirs.CreateDirs(), check.IsNil) @@ -125,8 +118,20 @@ func (f *backendDeviceSuite) SetUpTest(c *check.C) { f.setupRepo(c) - defer workshop.FakeDefaultDevices(defaultTestDevices)() - helper.LaunchTestWorkshop(c, f.ctx, f.be, c.MkDir()) + defer workshop.FakeDefaultDevices(helper.TestDevices)() + project := c.MkDir() + helper.LaunchTestWorkshop(c, f.ctx, f.be, project) + + // Required because WorkshopWorkshop mounts use + // x-systemd-requires=/project. + prjMount := workshop.Mount{ + Name: workshop.ConfigProjectPathDevice, + Type: workshop.HostWorkshop, + What: project, + Where: workshop.WorkshopProjectPath, + } + err = f.be.AddWorkshopMount(f.ctx, "test", prjMount) + c.Assert(err, check.IsNil) } func (f *backendDeviceSuite) TearDownTest(c *check.C) { diff --git a/internal/waitready/waitready.go b/internal/waitready/waitready.go index a3a4782dc..cf5d5380a 100644 --- a/internal/waitready/waitready.go +++ b/internal/waitready/waitready.go @@ -29,7 +29,7 @@ import ( "github.com/canonical/workshop/internal/systemd" ) -const Timeout = 5 * time.Minute +var Timeout = 5 * time.Minute // IsWaitreadyInvocation reports whether the process was invoked via a symlink // named waitready. This allows multiple logically unrelated commands to be diff --git a/internal/workshop/lxd/lxd_backend.go b/internal/workshop/lxd/lxd_backend.go index 3ad2b46eb..7ecc5bc9c 100644 --- a/internal/workshop/lxd/lxd_backend.go +++ b/internal/workshop/lxd/lxd_backend.go @@ -18,7 +18,7 @@ import ( "cmp" "context" "embed" - _ "embed" + "encoding/json" "errors" "fmt" "io" @@ -47,6 +47,7 @@ import ( "github.com/canonical/workshop/internal/revert" "github.com/canonical/workshop/internal/sdk" "github.com/canonical/workshop/internal/syscheck" + "github.com/canonical/workshop/internal/waitready" "github.com/canonical/workshop/internal/workshop" ) @@ -68,15 +69,11 @@ const ( ) var ( - startCommandTimeout = 1 * time.Minute - storagePoolDriver = "zfs" + storagePoolDriver = "zfs" workshopFormatsChecked = false ) -//go:embed start_command.sh -var startCommand string - func init() { if osutil.IsWSL() { storagePoolDriver = "btrfs" @@ -670,19 +667,8 @@ func (s *Backend) startWorkshop(conn lxd.InstanceServer, ctx context.Context, na rev := revert.New() defer rev.Fail() - // Enable autostart first so it doesn't race with workshop-waitready.service. - // See https://github.com/canonical/lxd/issues/18833. - if err := s.setAutoStart(conn, ctx, name, true); err != nil { - return err - } - cleanupCtx := context.WithoutCancel(ctx) rev.Add(func() { - // TODO: if this becomes a long-term thing, consider adding a timeout. - if e := s.setAutoStart(conn, cleanupCtx, name, false); e != nil { - logger.Noticef("On StartWorkshop: cannot reset %q workshop boot.autostart: %v", name, e) - } - // Stop workshop's timeout is handled by LXD API, so no need to have // a context with a timeout. if e := s.stopWorkshop(conn, cleanupCtx, name, true); e != nil { @@ -694,35 +680,12 @@ func (s *Backend) startWorkshop(conn lxd.InstanceServer, ctx context.Context, na return err } - var stderr strings.Builder - args := workshop.Execution{ - ExecArgs: workshop.ExecArgs{ - UserId: 0, - GroupId: 0, - Command: []string{ - "bash", "-euc", startCommand, - }, - WorkDir: "/", - Timeout: startCommandTimeout, - }, - ExecControls: workshop.ExecControls{ - Stderr: &stderr, - }, - } - - exectx, err := s.execCommand(conn, ctx, name, &args) - if err != nil { + if err := s.awaitReadyEvent(conn, ctx, name); err != nil { return err } - var errExec *workshop.ErrExec - if err := exectx.WaitExecution(ctx); errors.As(err, &errExec) { - message := strings.TrimSpace(stderr.String()) - if message == "" { - return err - } - return errors.New(message) - } else if err != nil { + // Workshop started, enable autostart. + if err := s.setAutoStart(conn, ctx, name, true); err != nil { return err } @@ -734,6 +697,83 @@ func (s *Backend) startWorkshop(conn lxd.InstanceServer, ctx context.Context, na return nil } +func (s *Backend) awaitReadyEvent(conn lxd.InstanceServer, ctx context.Context, name string) error { + projectId, ok := ctx.Value(workshop.ContextProjectId).(string) + if !ok { + return fmt.Errorf("context key project-id not found") + } + instance := InstanceName(name, projectId) + + ctx, cancel := context.WithTimeout(ctx, waitready.Timeout) + defer cancel() + + listener, err := conn.GetEvents() + if err != nil { + return err + } + defer listener.Disconnect() + + events := make(chan api.Event, 1) + defer close(events) + + target, err := listener.AddHandler([]string{"lifecycle"}, func(event api.Event) { + defer func() { _ = recover() }() + events <- event + }) + if err != nil { + return err + } + defer func() { _ = listener.RemoveHandler(target) }() + + ready, err := s.isInstanceReady(conn, instance) + for { + if err != nil { + return err + } + if ready { + return nil + } + select { + case event := <-events: + ready, err = s.isReadyEvent(event, instance) + case <-ctx.Done(): + ready, err := s.isInstanceReady(conn, instance) + if err == nil && ready { + return nil + } + return ctx.Err() + } + } +} + +func (s *Backend) isInstanceReady(conn lxd.InstanceServer, instance string) (bool, error) { + state, _, err := conn.GetInstanceState(instance) + if err != nil { + return false, err + } + return state.StatusCode == api.Ready, nil +} + +func (s *Backend) isReadyEvent(event api.Event, instance string) (bool, error) { + var lifecycle api.EventLifecycle + if err := json.Unmarshal(event.Metadata, &lifecycle); err != nil { + return false, err + } + + if lifecycle.Name != instance { + return false, nil + } + + switch lifecycle.Action { + case api.EventLifecycleInstanceReady: + return true, nil + case api.EventLifecycleInstanceShutdown, api.EventLifecycleInstanceStopped: + return false, fmt.Errorf("received %q event", lifecycle.Action) + default: + return false, nil + } +} + func (s *Backend) StopWorkshop(ctx context.Context, name string, force bool) error { conn, err := s.LxdClient(ctx) if err != nil { @@ -1339,6 +1379,8 @@ write_files: [Service] Type=notify ExecStart=/usr/local/lib/workshop/waitready + Restart=on-failure + RestartSec=2s [Install] WantedBy=multi-user.target @@ -1353,6 +1395,10 @@ runcmd: - ln -sf {{shquote .WorkshopCtlPath}} /usr/local/bin/workshopctl - ln -sf ../../bin/workshopctl /usr/local/lib/workshop/waitready - systemctl enable --now workshop-waitready.service + # Linger starts the user manager for the specified user on boot, which then creates /run/user/$UID, + # sets $XDG_RUNTIME_DIR and more. Interfaces such as desktop rely on both of these to be present. + # This does not introduce any additional modification beyond what a login session would normally create. + - loginctl enable-linger workshop `[1:] var cloudConfig strings.Builder @@ -1398,11 +1444,3 @@ runcmd: return cfg, nil } - -func FakeStartCommand(script string) func() { - old := startCommand - startCommand = script - return func() { - startCommand = old - } -} diff --git a/internal/workshop/lxd/start_command.sh b/internal/workshop/lxd/start_command.sh deleted file mode 100644 index 7997dafef..000000000 --- a/internal/workshop/lxd/start_command.sh +++ /dev/null @@ -1,12 +0,0 @@ -#!/bin/bash -# Wait until system is up an running before returning -# see: https://blog.simos.info/how-to-know-when-a-lxd-container-has-finished-starting-up/ -while [ "$(systemctl is-system-running 2>/dev/null)" != running ] \ -&& [ "$(systemctl is-system-running 2>/dev/null)" != degraded ] -do - : -done -# Linger starts the user manager for the specified user on boot, which then creates /run/user/$UID, -# sets $XDG_RUNTIME_DIR and more. Interfaces such as desktop rely on both of these to be present. -# This does not introduce any additional modification beyond what a login session would normally create. -loginctl enable-linger workshop diff --git a/internal/workshop/lxd/tests/helper/helper.go b/internal/workshop/lxd/tests/helper/helper.go index eb6659f23..d0a12d77f 100644 --- a/internal/workshop/lxd/tests/helper/helper.go +++ b/internal/workshop/lxd/tests/helper/helper.go @@ -22,13 +22,17 @@ import ( "os" "os/exec" "path/filepath" + "slices" "strings" + "testing" lxd "github.com/canonical/lxd/client" "github.com/canonical/lxd/shared/api" "gopkg.in/check.v1" + "github.com/canonical/workshop/internal/dirs" "github.com/canonical/workshop/internal/sdk" + "github.com/canonical/workshop/internal/waitready" "github.com/canonical/workshop/internal/workshop" ) @@ -43,8 +47,35 @@ actions: var MinimalImageServer = "simplestreams:https://cloud-images.ubuntu.com/minimal/releases" -func DefaultTestDevices(pid, w string) ([]workshop.Mount, []workshop.ProxyEntry) { - return nil, nil +var defaultDevices = workshop.DefaultDevices + +// RunTestsOrWorkshopCtl is intended to be called from TestMain; it allows test +// binaries to mount themselves into workshops in place of workshopctl. When +// run via a symlink with the right name, they behave the same as workshopctl. +func RunTestsOrWorkshopCtl(m *testing.M) int { + if waitready.IsWaitreadyInvocation() { + if err := waitready.WaitReady(); err != nil { + fmt.Fprintf(os.Stderr, "error: %s\n", err) + return 1 + } + return 0 + } + + executable, err := os.Executable() + if err != nil { + panic(fmt.Errorf("cannot get executable path: %w", err)) + } + dirs.WorkshopCtlPath = executable + + return m.Run() +} + +func TestDevices(pid, w string) ([]workshop.Mount, []workshop.ProxyEntry) { + mounts, _ := defaultDevices(pid, w) + mounts = slices.DeleteFunc(mounts, func(m workshop.Mount) bool { + return m.Name != "workshop.bin" + }) + return mounts, nil } func CleanupLxdProject(c *check.C, client lxd.InstanceServer, project string) { diff --git a/internal/workshop/lxd/tests/integration/project_test.go b/internal/workshop/lxd/tests/integration/project_test.go index 995b9887c..1eb989381 100644 --- a/internal/workshop/lxd/tests/integration/project_test.go +++ b/internal/workshop/lxd/tests/integration/project_test.go @@ -36,6 +36,10 @@ import ( "github.com/canonical/workshop/internal/workshop/lxd/tests/helper" ) +func TestMain(m *testing.M) { + os.Exit(helper.RunTestsOrWorkshopCtl(m)) +} + type wsProject struct { ctx context.Context client lxd.InstanceServer diff --git a/internal/workshop/lxd/tests/integration/snapshot-format.yaml b/internal/workshop/lxd/tests/integration/snapshot-format.yaml index 0ae5474e8..7b7c9d769 100644 --- a/internal/workshop/lxd/tests/integration/snapshot-format.yaml +++ b/internal/workshop/lxd/tests/integration/snapshot-format.yaml @@ -15,7 +15,7 @@ launched: gid 1000 1000 raw.lxc: lxc.mount.entry = tmpfs tmp tmpfs defaults security.nesting: 'true' - cloud-init.user-data: '17fd6f33c4c2802b0142eefe3d41d4f98b1c0ce338ce06da2b3e6d06776ac3c2204ed2cc7c6c367d40cdfc6e11d82e6a' + cloud-init.user-data: '8cb63e0464ae87dca0a4b43e73fa6420b8b81e758a313b166732c886113af229acbaf83af34ce8b5fc1006772aae84f4' user.workshop.name: test user.workshop.project-id: '42424242' description: '' @@ -55,7 +55,7 @@ started: gid 1000 1000 raw.lxc: lxc.mount.entry = tmpfs tmp tmpfs defaults security.nesting: 'true' - cloud-init.user-data: '17fd6f33c4c2802b0142eefe3d41d4f98b1c0ce338ce06da2b3e6d06776ac3c2204ed2cc7c6c367d40cdfc6e11d82e6a' + cloud-init.user-data: '8cb63e0464ae87dca0a4b43e73fa6420b8b81e758a313b166732c886113af229acbaf83af34ce8b5fc1006772aae84f4' user.workshop.name: test user.workshop.project-id: '42424242' description: '' @@ -95,7 +95,7 @@ sdk-attached: gid 1000 1000 raw.lxc: lxc.mount.entry = tmpfs tmp tmpfs defaults security.nesting: 'true' - cloud-init.user-data: '17fd6f33c4c2802b0142eefe3d41d4f98b1c0ce338ce06da2b3e6d06776ac3c2204ed2cc7c6c367d40cdfc6e11d82e6a' + cloud-init.user-data: '8cb63e0464ae87dca0a4b43e73fa6420b8b81e758a313b166732c886113af229acbaf83af34ce8b5fc1006772aae84f4' user.workshop.name: test user.workshop.project-id: '42424242' description: '' @@ -147,7 +147,7 @@ sdk-mounted: gid 1000 1000 raw.lxc: lxc.mount.entry = tmpfs tmp tmpfs defaults security.nesting: 'true' - cloud-init.user-data: '17fd6f33c4c2802b0142eefe3d41d4f98b1c0ce338ce06da2b3e6d06776ac3c2204ed2cc7c6c367d40cdfc6e11d82e6a' + cloud-init.user-data: '8cb63e0464ae87dca0a4b43e73fa6420b8b81e758a313b166732c886113af229acbaf83af34ce8b5fc1006772aae84f4' user.workshop.name: test user.workshop.project-id: '42424242' description: '' diff --git a/internal/workshop/lxd/tests/integration/snapshot_test.go b/internal/workshop/lxd/tests/integration/snapshot_test.go index aac04a3aa..6268ffae4 100644 --- a/internal/workshop/lxd/tests/integration/snapshot_test.go +++ b/internal/workshop/lxd/tests/integration/snapshot_test.go @@ -77,7 +77,6 @@ func (s *snapshotSuite) SetUpSuite(c *check.C) { s.restoreImageServer = lxdbackend.FakeImageServer(helper.MinimalImageServer) dirs.SetRootDir(c.MkDir()) - dirs.WorkshopCtlPath = filepath.Join(c.MkDir(), "workshopctl") dirs.SocketPath = filepath.Join(dirs.DataDir, "workshop.socket") c.Assert(dirs.CreateDirs(), check.IsNil) diff --git a/internal/workshop/lxd/tests/integration/workshop_exec_test.go b/internal/workshop/lxd/tests/integration/workshop_exec_test.go index 5eacaaf38..de9b9b1bf 100644 --- a/internal/workshop/lxd/tests/integration/workshop_exec_test.go +++ b/internal/workshop/lxd/tests/integration/workshop_exec_test.go @@ -56,18 +56,6 @@ type wsExec struct { var _ = check.Suite(&wsExec{}) -func execTestDevices(projectDir string) func(pid, w string) ([]workshop.Mount, []workshop.ProxyEntry) { - mounts := []workshop.Mount{{ - Name: workshop.ConfigProjectPathDevice, - Type: workshop.HostWorkshop, - What: projectDir, - Where: workshop.WorkshopProjectPath, - }} - return func(pid, w string) ([]workshop.Mount, []workshop.ProxyEntry) { - return mounts, nil - } -} - func (f *wsExec) SetUpSuite(c *check.C) { dirs.SetRootDir(c.MkDir()) c.Assert(dirs.CreateDirs(), check.IsNil) @@ -120,7 +108,7 @@ func (f *wsExec) SetUpSuite(c *check.C) { f.lxdClient, err = f.be.(*lxdbackend.Backend).LxdClient(f.ctx) c.Check(err, check.IsNil) - f.restoreDevices = workshop.FakeDefaultDevices(execTestDevices(c.MkDir())) + f.restoreDevices = workshop.FakeDefaultDevices(helper.TestDevices) f.newProjectidRestore = testutil.FakeFunc(func() (string, error) { return f.project.ProjectId, nil diff --git a/internal/workshop/lxd/tests/integration/workshop_test.go b/internal/workshop/lxd/tests/integration/workshop_test.go index 23b0be473..0a1787f3c 100644 --- a/internal/workshop/lxd/tests/integration/workshop_test.go +++ b/internal/workshop/lxd/tests/integration/workshop_test.go @@ -42,6 +42,7 @@ import ( "github.com/canonical/workshop/internal/sdk" "github.com/canonical/workshop/internal/syscheck" "github.com/canonical/workshop/internal/testutil" + "github.com/canonical/workshop/internal/waitready" "github.com/canonical/workshop/internal/workshop" lxdbackend "github.com/canonical/workshop/internal/workshop/lxd" "github.com/canonical/workshop/internal/workshop/lxd/tests/helper" @@ -77,7 +78,7 @@ func (f *wsOps) SetUpSuite(c *check.C) { c.Assert(os.Mkdir(f.project.Path, os.ModePerm), check.IsNil) f.ctx = helper.CreateTestContext(f.usr.Username, "42424242") - f.restoreDevices = workshop.FakeDefaultDevices(helper.DefaultTestDevices) + f.restoreDevices = workshop.FakeDefaultDevices(helper.TestDevices) f.restoreImageServer = lxdbackend.FakeImageServer(helper.MinimalImageServer) f.restoreUserLookup = osutil.FakeUserLookup(func(name string) (*user.User, error) { return f.usr, nil @@ -791,13 +792,18 @@ func (f *wsOps) TestLxdBackendWorkshopStartFailed(c *check.C) { helper.LaunchTestWorkshop(c, f.ctx, f.bd, f.project.Path) defer helper.RemoveTestWorkshop(c, f.ctx, f.bd) - err := f.bd.StopWorkshop(f.ctx, "test", true) + // Disable the waitready service, so start will time out. + fs, err := f.bd.WorkshopFs(f.ctx, "test") + c.Assert(err, check.IsNil) + err = fs.Remove("/etc/systemd/system/multi-user.target.wants/workshop-waitready.service") + c.Assert(fs.Close(), check.IsNil) + c.Assert(err, check.IsNil) + + err = f.bd.StopWorkshop(f.ctx, "test", true) c.Check(err, check.IsNil) - // Leaves the workshop instance in a started state with a failed start - // command. The StartWorkshop API must clean up its previous progress, i.e. - // set the workshop to the Stopped state. - defer lxdbackend.FakeStartCommand("exit 1")() + // Speed up the test. + defer testutil.FakeFunc(time.Second, &waitready.Timeout)() err = f.bd.StartWorkshop(f.ctx, "test") c.Check(err, check.NotNil) diff --git a/tests/main/start/task.yaml b/tests/main/start/task.yaml index d85a2f58a..131da03a7 100644 --- a/tests/main/start/task.yaml +++ b/tests/main/start/task.yaml @@ -14,28 +14,6 @@ execute: | workshop_exec exec -- test -d /run/user/1000 } - # TODO: remove this when `workshop start` begins to rely on LXD events. - function check_lxd_ready() { - echo "Check LXD itself reports the instance as Ready" - filter="name=ws-start-$(< .workshop.lock)" - - for _ in {1..50}; do - if lxc list --format csv --columns s --project workshop.ubuntu "$filter" | grep -Fqx READY; then - return 0 - fi - sleep 0.1 - done - - echo "Timed out waiting for LXD to report instance as READY" >&2 - workshop_exec exec -- systemctl status workshop-waitready.service || true - workshop_exec exec -- journalctl -u workshop-waitready.service || true - workshop_exec exec -- systemctl || true - workshop_exec exec -- systemctl status || true - workshop_exec exec -- journalctl -b || true - lxc info --show-log --project workshop.ubuntu "ws-start-$(< .workshop.lock)" - exit 1 - } - check_linger workshop_exec stop @@ -48,7 +26,6 @@ execute: | echo "Check the workshop is in Ready state" workshop_exec list | MATCH "^ws-start[[:space:]]+Ready[[:space:]]+-$" - check_lxd_ready check_linger echo "Check the workshop also becomes Ready when systemd boots degraded" @@ -57,5 +34,4 @@ execute: | workshop_exec stop workshop_exec start workshop_exec list | MATCH "^ws-start[[:space:]]+Ready[[:space:]]+-$" - check_lxd_ready workshop_exec exec -- systemctl is-system-running | MATCH '^degraded$' From 13f7b753696ec8190c0c80ad80a3c6ebd5bbb48f Mon Sep 17 00:00:00 2001 From: Jonathan Conder Date: Fri, 28 Aug 2026 13:16:10 +1200 Subject: [PATCH 04/15] Add temporary support for starting pre-0.9.4 workshops --- internal/daemon/api_connections.go | 6 +- internal/overlord/workshopstate/manager.go | 10 ++++ internal/overlord/workshopstate/request.go | 4 +- internal/workshop/lxd/lxd_backend.go | 64 +++++++++++++++++++++- 4 files changed, 80 insertions(+), 4 deletions(-) diff --git a/internal/daemon/api_connections.go b/internal/daemon/api_connections.go index 85fadc4d9..92d7b871a 100644 --- a/internal/daemon/api_connections.go +++ b/internal/daemon/api_connections.go @@ -256,7 +256,11 @@ func v1GetConnections(c *Command, r *http.Request, _ *userState) Response { onlyConnected := qselect == "" if workshop != "" { - if err := checkWorkshopExists(r.Context(), c.d.overlord.WorkshopManager(), projectId, workshop); err != nil { + st := c.d.overlord.State() + st.Lock() + err := checkWorkshopExists(r.Context(), c.d.overlord.WorkshopManager(), projectId, workshop) + st.Unlock() + if err != nil { return statusNotFound("cannot access %q workshop: %w", workshop, err) } } diff --git a/internal/overlord/workshopstate/manager.go b/internal/overlord/workshopstate/manager.go index 73a32a111..45e359761 100644 --- a/internal/overlord/workshopstate/manager.go +++ b/internal/overlord/workshopstate/manager.go @@ -123,6 +123,10 @@ func (w *WorkshopManager) Workshop(ctx context.Context, name, pId string) (*work return nil, err } + if w.FormatRevision().N >= 11 && workshop.Format.N < 11 { + w.state.Warnf("%q workshop needs to be refreshed; a future Workshop release will drop support for starting it", name) + } + return workshop, nil } @@ -176,6 +180,12 @@ func (w *WorkshopManager) Workshops(ctx context.Context, pId string) ([]*worksho return nil, err } + for _, workshop := range workshops { + if w.FormatRevision().N >= 11 && workshop.Format.N < 11 { + w.state.Warnf("%q workshop needs to be refreshed; a future Workshop release will drop support for starting it", workshop.Name) + } + } + return workshops, nil } diff --git a/internal/overlord/workshopstate/request.go b/internal/overlord/workshopstate/request.go index c649dac40..7d68a86ea 100644 --- a/internal/overlord/workshopstate/request.go +++ b/internal/overlord/workshopstate/request.go @@ -661,8 +661,7 @@ func (w *WorkshopManager) Exec(ctx context.Context, name, projectId string, args return nil, err } - ctx = context.WithValue(ctx, workshop.ContextProjectId, project.ProjectId) - wp, err := w.backend.Workshop(ctx, name) + wp, err := w.Workshop(ctx, name, projectId) if err != nil { return nil, err } @@ -671,6 +670,7 @@ func (w *WorkshopManager) Exec(ctx context.Context, name, projectId string, args return nil, err } + ctx = context.WithValue(ctx, workshop.ContextProjectId, project.ProjectId) wrkspc, err := w.backend.WorkshopFs(ctx, name) if err != nil { return nil, err diff --git a/internal/workshop/lxd/lxd_backend.go b/internal/workshop/lxd/lxd_backend.go index 7ecc5bc9c..aad709d7a 100644 --- a/internal/workshop/lxd/lxd_backend.go +++ b/internal/workshop/lxd/lxd_backend.go @@ -680,9 +680,15 @@ func (s *Backend) startWorkshop(conn lxd.InstanceServer, ctx context.Context, na return err } - if err := s.awaitReadyEvent(conn, ctx, name); err != nil { + waited, err := s.legacyWaitready(conn, ctx, name) + if err != nil { return err } + if !waited { + if err := s.awaitReadyEvent(conn, ctx, name); err != nil { + return err + } + } // Workshop started, enable autostart. if err := s.setAutoStart(conn, ctx, name, true); err != nil { @@ -697,6 +703,62 @@ func (s *Backend) startWorkshop(conn lxd.InstanceServer, ctx context.Context, na return nil } +func (s *Backend) legacyWaitready(conn lxd.InstanceServer, ctx context.Context, name string) (bool, error) { + projectId, ok := ctx.Value(workshop.ContextProjectId).(string) + if !ok { + return false, fmt.Errorf("context key project-id not found") + } + + inst, _, err := conn.GetInstance(InstanceName(name, projectId)) + if err != nil { + return false, err + } + + if inst.Config["user.workshop.format-revision"] != "" { + format, err := sdk.ParseRevision(inst.Config["user.workshop.format-revision"]) + if err != nil { + return false, err + } + if format.N >= 11 { + return false, nil + } + } + + var stdout strings.Builder + var stderr strings.Builder + args := workshop.Execution{ + ExecArgs: workshop.ExecArgs{ + Command: []string{"systemctl", "is-system-running", "--wait"}, + WorkDir: "/", + Timeout: waitready.Timeout, + }, + ExecControls: workshop.ExecControls{ + Stdout: &stdout, + Stderr: &stderr, + }, + } + + exectx, err := s.execCommand(conn, ctx, name, &args) + if err != nil { + return false, err + } + + if err := exectx.WaitExecution(ctx); err != nil { + if _, ok := errors.AsType[*workshop.ErrExec](err); ok { + if strings.TrimSpace(stdout.String()) == "degraded" { + return true, nil + } + message := strings.TrimSpace(stderr.String()) + if message == "" { + return false, err + } + return false, errors.New(message) + } + return false, err + } + return true, nil +} + func (s *Backend) awaitReadyEvent(conn lxd.InstanceServer, ctx context.Context, name string) error { projectId, ok := ctx.Value(workshop.ContextProjectId).(string) if !ok { From 04b1d0f5ccea493bc17db0058a15e0a62ef056c3 Mon Sep 17 00:00:00 2001 From: Jonathan Conder Date: Thu, 27 Aug 2026 18:59:49 +1200 Subject: [PATCH 05/15] Add support for freezing filesystems before taking snapshots --- cmd/workshopctl/main.go | 9 + internal/dirs/dirs.go | 3 + internal/fsfreeze/export_test.go | 51 +++ internal/fsfreeze/fsfreeze.go | 283 +++++++++++++ internal/fsfreeze/fsfreeze_test.go | 387 ++++++++++++++++++ internal/fsfreeze/sys/generate.sh | 26 ++ internal/fsfreeze/sys/syscall_linux.go | 37 ++ internal/fsfreeze/sys/sysnum_linux.go | 29 ++ internal/fsfreeze/sys/zsysnum_linux.go | 11 + .../workshop/lxd/lxd_backend_snapshots.go | 115 +++++- internal/workshop/lxd/tests/helper/helper.go | 9 + 11 files changed, 959 insertions(+), 1 deletion(-) create mode 100644 internal/fsfreeze/export_test.go create mode 100644 internal/fsfreeze/fsfreeze.go create mode 100644 internal/fsfreeze/fsfreeze_test.go create mode 100755 internal/fsfreeze/sys/generate.sh create mode 100644 internal/fsfreeze/sys/syscall_linux.go create mode 100644 internal/fsfreeze/sys/sysnum_linux.go create mode 100644 internal/fsfreeze/sys/zsysnum_linux.go diff --git a/cmd/workshopctl/main.go b/cmd/workshopctl/main.go index 242ace426..701da9623 100644 --- a/cmd/workshopctl/main.go +++ b/cmd/workshopctl/main.go @@ -27,6 +27,7 @@ import ( "github.com/canonical/workshop/client" "github.com/canonical/workshop/internal/dirs" + "github.com/canonical/workshop/internal/fsfreeze" "github.com/canonical/workshop/internal/waitready" ) @@ -45,6 +46,14 @@ func main() { return } + if fsfreeze.IsFsfreezeInvocation() { + if err := fsfreeze.FreezeLocalFilesystems(os.Stdin, os.Stdout); err != nil { + fmt.Fprintf(os.Stderr, "error: %s\n", err) + os.Exit(1) + } + return + } + // Set the user and group IDs to the workshop user uid := uint32(1000) // Change this to the workshop UID diff --git a/internal/dirs/dirs.go b/internal/dirs/dirs.go index d15f6490f..a394e7703 100644 --- a/internal/dirs/dirs.go +++ b/internal/dirs/dirs.go @@ -59,6 +59,9 @@ var ( // Cache directory for deb packages AptCacheDir = "/var/cache/apt/archives" + + // Symlink to workshopctl that freezes VM filesystems. + FsFreezePath = "/usr/local/lib/workshop/fsfreeze" ) // Variables for workshopd (host paths) diff --git a/internal/fsfreeze/export_test.go b/internal/fsfreeze/export_test.go new file mode 100644 index 000000000..548e1966a --- /dev/null +++ b/internal/fsfreeze/export_test.go @@ -0,0 +1,51 @@ +// Copyright (c) 2026 Canonical Ltd +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License version 3 as +// published by the Free Software Foundation. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +package fsfreeze + +import ( + "os" + "time" + + "github.com/canonical/workshop/internal/osutil" + "github.com/canonical/workshop/internal/testutil" +) + +func MockFilesystems(path string) func() { + return testutil.FakeFunc(path, &filesystemsPath) +} + +func MockMountinfo(path string) func() { + return testutil.FakeFunc(path, &mountinfoPath) +} + +func MockTimeout(t time.Duration) func() { + return testutil.FakeFunc(t, &timeout) +} + +func MockFsFreeze(f func(*os.File) error) func() { + return testutil.FakeFunc(f, &fsFreeze) +} + +func MockFsThaw(f func(*os.File) error) func() { + return testutil.FakeFunc(f, &fsThaw) +} + +func LocalMounts() ([]*osutil.MountInfoEntry, string, error) { + mounts, rootFS, err := localMounts(mountinfoPath, filesystemsPath) + if err != nil { + return nil, "", err + } + return mounts, rootFS.String(), nil +} diff --git a/internal/fsfreeze/fsfreeze.go b/internal/fsfreeze/fsfreeze.go new file mode 100644 index 000000000..64300e3ca --- /dev/null +++ b/internal/fsfreeze/fsfreeze.go @@ -0,0 +1,283 @@ +// Copyright (c) 2026 Canonical Ltd +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License version 3 as +// published by the Free Software Foundation. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +package fsfreeze + +import ( + "bufio" + "bytes" + "cmp" + "context" + "errors" + "fmt" + "io" + "os" + "os/signal" + "path/filepath" + "slices" + "strings" + "sync" + "syscall" + "time" + + "golang.org/x/sys/unix" + + "github.com/canonical/workshop/internal/fsfreeze/sys" + "github.com/canonical/workshop/internal/osutil" + "github.com/canonical/workshop/internal/revert" +) + +var ( + timeout = 2 * time.Minute + + mountinfoPath = "/proc/self/mountinfo" + filesystemsPath = "/proc/filesystems" + + fsFreeze = sys.FsFreeze + fsThaw = sys.FsThaw +) + +type superblock struct { + major int + minor int +} + +func (d superblock) String() string { + return fmt.Sprintf("%d:%d", d.major, d.minor) +} + +// IsFsfreezeInvocation reports whether the process was invoked via a symlink +// named fsfreeze. This allows multiple logically unrelated commands to be +// embedded in a single multi-call binary (even in tests). +func IsFsfreezeInvocation() bool { + return len(os.Args) > 0 && filepath.Base(os.Args[0]) == "fsfreeze" +} + +// FreezeLocalFilesystems freezes every filesystem associated with a block +// device. It uses the sd_notify protocol to signal readiness on stdout. When +// stdin is closed, the filesystems are thawed. It also thaws them on error, +// which can be a signal (HUP, INT, PIPE, or TERM) or a timeout. In this case, +// stdin should still be closed in order to unblock stdin.Read(). +func FreezeLocalFilesystems(stdin io.Reader, stdout io.Writer) (err error) { + ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGHUP, syscall.SIGINT, syscall.SIGPIPE, syscall.SIGTERM) + defer stop() + + mounts, root, err := localMounts(mountinfoPath, filesystemsPath) + if err != nil { + return err + } + + frozen, err := freeze(ctx, mounts, root) + if err != nil { + return err + } + defer func() { + err = cmp.Or(err, thaw(frozen)) + }() + + if _, err := fmt.Fprintln(stdout, "READY=1"); err != nil { + return fmt.Errorf("cannot report readiness: %w", err) + } + + ctx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + + ctx, cancelCause := context.WithCancelCause(ctx) + defer cancelCause(nil) + go func() { + defer cancelCause(io.EOF) + if _, err1 := io.Copy(io.Discard, stdin); err1 != nil { + cancelCause(err1) + } + }() + + <-ctx.Done() + if err := context.Cause(ctx); errors.Is(err, io.EOF) { + return nil + } else { + return err + } +} + +// localMounts lists mounts involving block devices. The list is in reverse +// mount order, to guarantee that loop mounts are frozen before the filesystem +// holding the backing file. Only one representative of each superblock is +// listed, to avoid freezing a frozen filesystem. +// +// It also returns the device ID of the rootfs superblock. +func localMounts(mountinfo, filesystems string) ([]*osutil.MountInfoEntry, superblock, error) { + local, err := localFilesystems(filesystems) + if err != nil { + return nil, superblock{}, err + } + + mounts, err := osutil.LoadMountInfo(mountinfo) + if err != nil { + return nil, superblock{}, fmt.Errorf("cannot parse %q: %w", mountinfo, err) + } + slices.Reverse(mounts) + + idx := slices.IndexFunc(mounts, func(m *osutil.MountInfoEntry) bool { + return m.MountDir == "/" + }) + if idx < 0 { + return nil, superblock{}, errors.New("root filesystem not mounted") + } + root := superblock{major: mounts[idx].DevMajor, minor: mounts[idx].DevMinor} + + last := make(map[superblock]*osutil.MountInfoEntry, len(mounts)) + for _, m := range mounts { + if isLocal(m, local) { + last[superblock{major: m.DevMajor, minor: m.DevMinor}] = m + } else if m.MountDir == "/" { + return nil, superblock{}, fmt.Errorf("root filesystem %s (%s) is not freezable", root, m.FsType) + } + } + mounts = slices.DeleteFunc(mounts, func(m *osutil.MountInfoEntry) bool { + return last[superblock{major: m.DevMajor, minor: m.DevMinor}] != m + }) + + return mounts, root, nil +} + +// localFilesystems returns a map of filesystems that require block devices. +func localFilesystems(filesystems string) (map[string]bool, error) { + file, err := os.Open(filesystems) + if err != nil { + return nil, err + } + defer file.Close() + + types := map[string]bool{} + scanner := bufio.NewScanner(file) + for scanner.Scan() { + flag, name, ok := strings.Cut(scanner.Text(), "\t") + if !ok { + return nil, fmt.Errorf("cannot parse %q: too few fields", filesystems) + } + // Currently the first column is either "" or "nodev". + if !strings.Contains(flag, "nodev") { + types[name] = true + } + } + if err := scanner.Err(); err != nil { + return nil, fmt.Errorf("cannot parse %q: %w", filesystems, err) + } + return types, nil +} + +// isLocal attempts to determine if the given mount is backed by a block +// device. This is intended as an optimisation: most network-based filesystems +// don't support FIFREEZE and will return EOPNOTSUPP immediately. However, +// opening the file descriptor that we pass to FIFREEZE can potentially block +// for a long time if the network is unstable. +// +// There are a few caveats: CIFS does support FIFREEZE, and loop mounts may +// appear to be local even if the loop file is remote. The implementation is +// based on the QEMU guest agent, which ignores these edge cases. Typically +// mounts with major number 0 are remote and others are local. QEMU carves out +// an exception for btrfs; we treat any filesystem which doesn't specify +// "nodev" in /proc/filesystems as local, which should be more future-proof. +// For example bcachefs is a similar exception to btrfs. +// +// Another exception is ZFS, which uses major number 0 and specifies "nodev," +// but doesn't support FIFREEZE. If in future VMs use ZFS for the rootfs, we +// need to at least call fsync, or preferably the native ZFS equivalent of +// FIFREEZE, to handle this case properly. +func isLocal(m *osutil.MountInfoEntry, local map[string]bool) bool { + // Some filesystems have subtypes, like fuse.sshfs. Only the main type is + // listed in /proc/filesystems. + name, _, _ := strings.Cut(m.FsType, ".") + + return m.DevMajor != 0 || local[name] +} + +func freeze(ctx context.Context, mounts []*osutil.MountInfoEntry, root superblock) ([]*os.File, error) { + rev := revert.New() + defer rev.Fail() + + var frozen []*os.File + rev.Add(func() { + for _, f := range slices.Backward(frozen) { + _ = fsThaw(f) + f.Close() + } + }) + + for _, m := range mounts { + select { + case <-ctx.Done(): + return nil, context.Cause(ctx) + default: + } + + file, err := os.Open(m.MountDir) + if err != nil { + return nil, err + } + + if err := fsFreeze(file); err != nil { + file.Close() + if (superblock{major: m.DevMajor, minor: m.DevMinor} != root) && errors.Is(err, unix.EOPNOTSUPP) { + continue + } + return nil, err + } + + frozen = append(frozen, file) + } + + rev.Success() + return frozen, nil +} + +func thaw(frozen []*os.File) error { + var errs []error + for _, file := range slices.Backward(frozen) { + errs = append(errs, fsThaw(file)) + file.Close() + } + return errors.Join(errs...) +} + +type ReadyWriter struct { + once sync.Once + ready chan struct{} + buf bytes.Buffer +} + +func NewReadyWriter() *ReadyWriter { + return &ReadyWriter{ready: make(chan struct{})} +} + +func (w *ReadyWriter) Ready() <-chan struct{} { + return w.ready +} + +func (w *ReadyWriter) Write(data []byte) (int, error) { + n, err := w.buf.Write(data) + for { + line, _, found := bytes.Cut(w.buf.Bytes(), []byte{'\n'}) + if !found { + break + } + if bytes.Equal(line, []byte("READY=1")) { + w.once.Do(func() { + close(w.ready) + }) + } + w.buf.Next(len(line) + 1) + } + return n, err +} diff --git a/internal/fsfreeze/fsfreeze_test.go b/internal/fsfreeze/fsfreeze_test.go new file mode 100644 index 000000000..2d0cef83c --- /dev/null +++ b/internal/fsfreeze/fsfreeze_test.go @@ -0,0 +1,387 @@ +// Copyright (c) 2026 Canonical Ltd +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License version 3 as +// published by the Free Software Foundation. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +package fsfreeze_test + +import ( + "errors" + "fmt" + "io" + "os" + "path/filepath" + "testing" + "time" + + "golang.org/x/sys/unix" + "gopkg.in/check.v1" + + "github.com/canonical/workshop/internal/fsfreeze" + "github.com/canonical/workshop/internal/osutil" + "github.com/canonical/workshop/internal/revert" + "github.com/canonical/workshop/internal/testutil" +) + +func Test(t *testing.T) { check.TestingT(t) } + +type fsfreezeSuite struct { + scratch string + + frozen map[string]error + canThaw chan struct{} + hangup bool + + restoreFilesystems func() + restoreMountinfo func() + restoreTimeout func() + restoreFsFreeze func() + restoreFsThaw func() +} + +var _ = check.Suite(&fsfreezeSuite{}) + +const fakeFilesystems = ` +nodev sysfs +nodev tmpfs +nodev proc + ext4 + vfat + fuseblk +nodev fuse + btrfs +nodev virtiofs +nodev 9p +nodev nfs4 +nodev zfs +` + +func (s *fsfreezeSuite) SetUpSuite(c *check.C) { + s.scratch = c.MkDir() + + err := os.WriteFile(filepath.Join(s.scratch, "filesystems"), []byte(fakeFilesystems[1:]), 0644) + c.Assert(err, check.IsNil) + s.restoreFilesystems = fsfreeze.MockFilesystems(filepath.Join(s.scratch, "filesystems")) + + s.restoreMountinfo = fsfreeze.MockMountinfo(filepath.Join(s.scratch, "mountinfo")) + + s.restoreTimeout = fsfreeze.MockTimeout(50 * time.Millisecond) + + s.restoreFsFreeze = fsfreeze.MockFsFreeze(s.fsFreeze) + s.restoreFsThaw = fsfreeze.MockFsThaw(s.fsThaw) +} + +func (s *fsfreezeSuite) SetUpTest(c *check.C) { + s.frozen = map[string]error{} + s.canThaw = make(chan struct{}) + s.hangup = false +} + +func (s *fsfreezeSuite) TearDownSuite(c *check.C) { + s.restoreFsThaw() + s.restoreFsFreeze() + s.restoreMountinfo() + s.restoreFilesystems() +} + +func (s *fsfreezeSuite) fsFreeze(file *os.File) error { + if err := s.frozen[file.Name()]; err != nil { + return err + } + s.frozen[file.Name()] = unix.EBUSY + return nil +} + +func (s *fsfreezeSuite) fsThaw(file *os.File) error { + // Ensure the tests have finished checking s.frozen before we touch it + // again. If the freeze times out, the test is very likely to be past that + // point, but `go test -race` can detect the lack of synchronization. + <-s.canThaw + + if err := s.frozen[file.Name()]; !errors.Is(err, unix.EBUSY) { + return err + } + delete(s.frozen, file.Name()) + return nil +} + +func (s *fsfreezeSuite) TestFreezeLocalFilesystemsOK(c *check.C) { + dir := c.MkDir() + mountinfo := fmt.Sprintf(` +28 1 252:1 / / rw,relatime shared:1 - ext4 /dev/vda1 rw +44 28 252:16 / %s rw,relatime shared:22 - ext4 /dev/vda16 rw +`[1:], dir) + err := os.WriteFile(filepath.Join(s.scratch, "mountinfo"), []byte(mountinfo), 0644) + c.Assert(err, check.IsNil) + + thaw, result, err := s.freezeLocalFilesystems() + c.Assert(err, check.IsNil) + + c.Check(s.frozen, check.HasLen, 2) + c.Check(s.frozen["/"], testutil.ErrorIs, unix.EBUSY) + c.Check(s.frozen[dir], testutil.ErrorIs, unix.EBUSY) + close(s.canThaw) + + thaw.Close() + c.Assert(<-result, check.IsNil) + + c.Check(s.frozen, check.HasLen, 0) +} + +func (s *fsfreezeSuite) TestFreezeLocalFilesystemsUnsupported(c *check.C) { + dir := c.MkDir() + mountinfo := fmt.Sprintf(` +28 1 252:1 / / rw,relatime shared:1 - ext4 /dev/vda1 rw +44 28 252:16 / %s rw,relatime shared:22 - ext4 /dev/vda16 rw +`[1:], dir) + err := os.WriteFile(filepath.Join(s.scratch, "mountinfo"), []byte(mountinfo), 0644) + c.Assert(err, check.IsNil) + + s.frozen[dir] = unix.EOPNOTSUPP + + thaw, result, err := s.freezeLocalFilesystems() + c.Assert(err, check.IsNil) + + c.Check(s.frozen, check.HasLen, 2) + c.Check(s.frozen["/"], testutil.ErrorIs, unix.EBUSY) + c.Check(s.frozen[dir], testutil.ErrorIs, unix.EOPNOTSUPP) + close(s.canThaw) + + thaw.Close() + c.Assert(<-result, check.IsNil) + + c.Check(s.frozen, check.HasLen, 1) + c.Check(s.frozen[dir], testutil.ErrorIs, unix.EOPNOTSUPP) +} + +func (s *fsfreezeSuite) TestFreezeLocalFilesystemsRootUnsupported(c *check.C) { + dir := c.MkDir() + mountinfo := fmt.Sprintf(` +28 1 252:1 / / rw,relatime shared:1 - ext4 /dev/vda1 rw +44 28 252:16 / %s rw,relatime shared:22 - ext4 /dev/vda16 rw +`[1:], dir) + err := os.WriteFile(filepath.Join(s.scratch, "mountinfo"), []byte(mountinfo), 0644) + c.Assert(err, check.IsNil) + + s.frozen["/"] = unix.EOPNOTSUPP + close(s.canThaw) + + thaw, result, err := s.freezeLocalFilesystems() + c.Check(err, check.ErrorMatches, "operation not supported") + + if err == nil { + thaw.Close() + c.Assert(<-result, check.ErrorMatches, "operation not supported") + } + + c.Check(s.frozen, check.HasLen, 1) + c.Check(s.frozen["/"], testutil.ErrorIs, unix.EOPNOTSUPP) +} + +func (s *fsfreezeSuite) TestFreezeLocalFilesystemsError(c *check.C) { + dir := c.MkDir() + mountinfo := fmt.Sprintf(` +28 1 252:1 / / rw,relatime shared:1 - ext4 /dev/vda1 rw +44 28 252:16 / %s rw,relatime shared:22 - ext4 /dev/vda16 rw +`[1:], dir) + err := os.WriteFile(filepath.Join(s.scratch, "mountinfo"), []byte(mountinfo), 0644) + c.Assert(err, check.IsNil) + + s.frozen[dir] = errors.New("cannot freeze") + close(s.canThaw) + + thaw, result, err := s.freezeLocalFilesystems() + c.Check(err, check.ErrorMatches, "cannot freeze") + + if err == nil { + thaw.Close() + c.Assert(<-result, check.ErrorMatches, "cannot freeze") + } + + c.Check(s.frozen, check.HasLen, 1) + c.Check(s.frozen[dir], check.ErrorMatches, "cannot freeze") +} + +func (s *fsfreezeSuite) TestFreezeLocalFilesystemsHangup(c *check.C) { + dir := c.MkDir() + mountinfo := fmt.Sprintf(` +28 1 252:1 / / rw,relatime shared:1 - ext4 /dev/vda1 rw +44 28 252:16 / %s rw,relatime shared:22 - ext4 /dev/vda16 rw +`[1:], dir) + err := os.WriteFile(filepath.Join(s.scratch, "mountinfo"), []byte(mountinfo), 0644) + c.Assert(err, check.IsNil) + + s.hangup = true + close(s.canThaw) + + thaw, result, err := s.freezeLocalFilesystems() + c.Check(err, check.ErrorMatches, "cannot report readiness: broken pipe") + + if err == nil { + thaw.Close() + c.Assert(<-result, check.ErrorMatches, "cannot report readiness: broken pipe") + } + + c.Check(s.frozen, check.HasLen, 0) +} + +func (s *fsfreezeSuite) TestFreezeLocalFilesystemsTimeout(c *check.C) { + dir := c.MkDir() + mountinfo := fmt.Sprintf(` +28 1 252:1 / / rw,relatime shared:1 - ext4 /dev/vda1 rw +44 28 252:16 / %s rw,relatime shared:22 - ext4 /dev/vda16 rw +`[1:], dir) + err := os.WriteFile(filepath.Join(s.scratch, "mountinfo"), []byte(mountinfo), 0644) + c.Assert(err, check.IsNil) + + thaw, result, err := s.freezeLocalFilesystems() + c.Assert(err, check.IsNil) + + c.Check(s.frozen, check.HasLen, 2) + c.Check(s.frozen["/"], testutil.ErrorIs, unix.EBUSY) + c.Check(s.frozen[dir], testutil.ErrorIs, unix.EBUSY) + close(s.canThaw) + + c.Check(<-result, check.ErrorMatches, "context deadline exceeded") + thaw.Close() + + c.Check(s.frozen, check.HasLen, 0) +} + +// freezeLocalFilesystems simulates calling fsfreeze.FreezeLocalFilesystems as +// a subprocess, using pipes to receive the ready signal and return. On +// success, returns an io.Closer that signals the "subprocess" to thaw the +// filesystems, and a channel to receive the error it reports (if any). If the +// ready signal isn't received promptly, we wait for the "subprocess" to +// timeout, close the remaining pipes and return an error. +func (s *fsfreezeSuite) freezeLocalFilesystems() (io.Closer, <-chan error, error) { + rev := revert.New() + defer rev.Fail() + + stdin, thaw := io.Pipe() + rev.Add(func() { + thaw.Close() + }) + + var ready <-chan struct{} + var stdout io.Writer + if s.hangup { + stdout = closedWriter{} + } else { + w := fsfreeze.NewReadyWriter() + stdout = w + ready = w.Ready() + } + + result := make(chan error, 1) + go func() { + defer stdin.Close() + result <- fsfreeze.FreezeLocalFilesystems(stdin, stdout) + close(result) + }() + + select { + case <-ready: + rev.Success() + return thaw, result, nil + case err := <-result: + return nil, nil, err + } +} + +type closedWriter struct{} + +func (closedWriter) Write([]byte) (int, error) { + return 0, unix.EPIPE +} + +func (s *fsfreezeSuite) TestLocalMountsOK(c *check.C) { + mountinfo := ` +23 28 0:22 / /proc rw,nosuid,nodev,noexec,relatime shared:12 - proc proc rw +24 28 0:23 / /sys rw,nosuid,nodev,noexec,relatime shared:2 - sysfs sysfs rw +28 1 252:1 / / rw,relatime shared:1 - ext4 /dev/vda1 rw,discard +41 28 0:37 / /tmp rw,nosuid,nodev shared:20 - tmpfs tmpfs rw +44 28 252:16 / /boot rw,relatime shared:22 - ext4 /dev/vda16 rw +46 44 252:15 / /boot/efi rw,relatime shared:24 - vfat /dev/vda15 rw +55 28 0:46 / /usr/local/lib/workshop/guest ro,relatime shared:30 - virtiofs workshop.bin ro +62 28 0:51 / /mnt/share rw,relatime shared:40 - nfs4 server:/export rw +70 28 252:32 / /mnt/backup ro,relatime shared:50 - ext4 /dev/vdc ro +75 28 0:70 /@home /home rw,relatime shared:22 - btrfs /dev/vdd1 rw,subvol=/@home +`[1:] + err := os.WriteFile(filepath.Join(s.scratch, "mountinfo"), []byte(mountinfo), 0644) + c.Assert(err, check.IsNil) + + mounts, root, err := fsfreeze.LocalMounts() + c.Assert(err, check.IsNil) + c.Check(mountDirs(mounts), check.DeepEquals, []string{"/home", "/mnt/backup", "/boot/efi", "/boot", "/"}) + c.Check(root, check.Equals, "252:1") +} + +func (s *fsfreezeSuite) TestLocalMountsCompactsBySuperblock(c *check.C) { + mountinfo := ` +28 1 252:1 / / rw,relatime shared:1 - ext4 /dev/vda1 rw +44 28 252:16 / /srv rw,relatime shared:22 - ext4 /dev/vda16 rw +45 44 252:1 /project /srv/project rw,relatime shared:1 - ext4 /dev/vda1 rw +`[1:] + err := os.WriteFile(filepath.Join(s.scratch, "mountinfo"), []byte(mountinfo), 0644) + c.Assert(err, check.IsNil) + + mounts, root, err := fsfreeze.LocalMounts() + c.Assert(err, check.IsNil) + // It's important that / is chosen over /project, because /srv could be + // backed by a loop file on /, and freezing /project also freezes /. + c.Check(mountDirs(mounts), check.DeepEquals, []string{"/srv", "/"}) + c.Check(root, check.Equals, "252:1") +} + +func mountDirs(mounts []*osutil.MountInfoEntry) []string { + dirs := make([]string, 0, len(mounts)) + for _, m := range mounts { + dirs = append(dirs, m.MountDir) + } + return dirs +} + +func (s *fsfreezeSuite) TestLocalMountsRequiresRootFS(c *check.C) { + mountinfo := ` +23 28 0:22 / /proc rw,relatime shared:12 - proc proc rw +`[1:] + err := os.WriteFile(filepath.Join(s.scratch, "mountinfo"), []byte(mountinfo), 0644) + c.Assert(err, check.IsNil) + + _, _, err = fsfreeze.LocalMounts() + c.Check(err, check.ErrorMatches, "root filesystem not mounted") +} + +func (s *fsfreezeSuite) TestLocalMountsRequiresLocalRootFS(c *check.C) { + mountinfo := ` +28 1 0:51 / / rw,relatime shared:1 - nfs4 server:/export rw +44 28 252:16 / /boot rw,relatime shared:22 - ext4 /dev/vda16 rw +`[1:] + err := os.WriteFile(filepath.Join(s.scratch, "mountinfo"), []byte(mountinfo), 0644) + c.Assert(err, check.IsNil) + + _, _, err = fsfreeze.LocalMounts() + c.Check(err, check.ErrorMatches, `root filesystem 0:51 \(nfs4\) is not freezable`) +} + +func (s *fsfreezeSuite) TestLocalMountsSupportForZFS(c *check.C) { + mountinfo := ` +28 1 0:60 / / rw,relatime shared:1 - zfs rpool/ROOT/ubuntu rw,xattr +44 28 252:16 / /boot rw,relatime shared:22 - ext4 /dev/vda16 rw +`[1:] + err := os.WriteFile(filepath.Join(s.scratch, "mountinfo"), []byte(mountinfo), 0644) + c.Assert(err, check.IsNil) + + _, _, err = fsfreeze.LocalMounts() + c.Check(err, check.ErrorMatches, `root filesystem 0:60 \(zfs\) is not freezable`) +} diff --git a/internal/fsfreeze/sys/generate.sh b/internal/fsfreeze/sys/generate.sh new file mode 100755 index 000000000..bc1482b9d --- /dev/null +++ b/internal/fsfreeze/sys/generate.sh @@ -0,0 +1,26 @@ +#!/usr/bin/env bash + +set -eu +cd -- "$(dirname -- "$0")" + +temp=$(mktemp --directory --tmpdir="$(pwd)") +trap 'rm -rf -- "$temp"' EXIT + +cd "$temp" + +# Limit to supported architectures for safety. Most architectures share the +# _IOWR macro in include/uapi/asm-generic/ioctl.h, but others have a different +# one in arch//include/uapi/asm/ioctl.h. Currently I think FIFREEZE and +# related constants end up the same on all architectures regardless, but it's +# worth checking that when adding support for a new architecture. +cat <zsysnum_linux.go +//go:build amd64 || arm64 || riscv64 + +EOF + +ln -s ../sysnum_linux.go . +go tool cgo -godefs sysnum_linux.go >>zsysnum_linux.go + +gofmt -w zsysnum_linux.go + +mv zsysnum_linux.go .. diff --git a/internal/fsfreeze/sys/syscall_linux.go b/internal/fsfreeze/sys/syscall_linux.go new file mode 100644 index 000000000..eedc2a0b6 --- /dev/null +++ b/internal/fsfreeze/sys/syscall_linux.go @@ -0,0 +1,37 @@ +// Copyright (c) 2026 Canonical Ltd +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License version 3 as +// published by the Free Software Foundation. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +//go:generate ./generate.sh + +package sys + +import ( + "os" + + "golang.org/x/sys/unix" +) + +func FsFreeze(file *os.File) error { + if err := unix.IoctlSetInt(int(file.Fd()), FIFREEZE, 0); err != nil { + return &os.PathError{Op: "fsfreeze", Path: file.Name(), Err: err} + } + return nil +} + +func FsThaw(file *os.File) error { + if err := unix.IoctlSetInt(int(file.Fd()), FITHAW, 0); err != nil { + return &os.PathError{Op: "fsthaw", Path: file.Name(), Err: err} + } + return nil +} diff --git a/internal/fsfreeze/sys/sysnum_linux.go b/internal/fsfreeze/sys/sysnum_linux.go new file mode 100644 index 000000000..f6733fefc --- /dev/null +++ b/internal/fsfreeze/sys/sysnum_linux.go @@ -0,0 +1,29 @@ +// Copyright (c) 2026 Canonical Ltd +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License version 3 as +// published by the Free Software Foundation. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +//go:build ignore + +// This file is used as input to cgo -godefs to define the below constants +// without relying on cgo. +package sys + +/* +#include +*/ +import "C" + +const ( + FIFREEZE = C.FIFREEZE + FITHAW = C.FITHAW +) diff --git a/internal/fsfreeze/sys/zsysnum_linux.go b/internal/fsfreeze/sys/zsysnum_linux.go new file mode 100644 index 000000000..89eaf2d48 --- /dev/null +++ b/internal/fsfreeze/sys/zsysnum_linux.go @@ -0,0 +1,11 @@ +//go:build amd64 || arm64 || riscv64 + +// Code generated by cmd/cgo -godefs; DO NOT EDIT. +// cgo -godefs sysnum_linux.go + +package sys + +const ( + FIFREEZE = 0xc0045877 + FITHAW = 0xc0045878 +) diff --git a/internal/workshop/lxd/lxd_backend_snapshots.go b/internal/workshop/lxd/lxd_backend_snapshots.go index c9abfbd06..913ce6359 100644 --- a/internal/workshop/lxd/lxd_backend_snapshots.go +++ b/internal/workshop/lxd/lxd_backend_snapshots.go @@ -21,6 +21,7 @@ import ( "encoding/hex" "errors" "fmt" + "io" "maps" "net/http" "net/url" @@ -28,17 +29,25 @@ import ( "slices" "strings" "sync" + "time" lxd "github.com/canonical/lxd/client" "github.com/canonical/lxd/shared/api" "github.com/canonical/lxd/shared/entity" + "github.com/canonical/workshop/internal/dirs" + "github.com/canonical/workshop/internal/fsfreeze" "github.com/canonical/workshop/internal/logger" "github.com/canonical/workshop/internal/revert" "github.com/canonical/workshop/internal/sdk" "github.com/canonical/workshop/internal/workshop" ) +const ( + freezeTimeout = 5 * time.Minute + thawTimeout = 2 * time.Minute +) + var ( snapshotGuardsLock sync.Mutex snapshotGuards = map[string]*snapshotGuard{} @@ -303,7 +312,8 @@ func (s *Backend) TakeSnapshot(ctx context.Context, name string, snapshot worksh snapshotName := sdkSnapshotName(snapshot, digest) // Disable cancellation, because the LXD operation will plow on regardless, - // and the lock is supposed to prevent concurrent import operations. + // and the lock is supposed to prevent concurrent import operations. We + // also want to avoid killing the fsfreeze process for VMs. lockedCtx := context.WithoutCancel(ctx) conn, snapshotConn, err := s.snapshotClients(lockedCtx) if err != nil { @@ -368,6 +378,24 @@ func (s *Backend) TakeSnapshot(ctx context.Context, name string, snapshot worksh } defer unlockSnapshot(snapshotName) + thawer := revert.New() + defer thawer.Fail() + var thaw io.Closer + var result <-chan error + + running := inst.StatusCode == api.Running || inst.StatusCode == api.Ready + if running && inst.Type != string(api.InstanceTypeContainer) { + thaw, result, err = s.freezeFilesystems(conn, ctx, name) + if err != nil { + return err + } + thawer.Add(func() { + if err1 := thawFilesystems(thaw, result); err1 != nil { + logger.Noticef("On TakeSnapshot: %v", err1) + } + }) + } + rev := revert.New() defer rev.Fail() @@ -398,6 +426,13 @@ func (s *Backend) TakeSnapshot(ctx context.Context, name string, snapshot worksh } }) + thawer.Success() + if thaw != nil { + if err := thawFilesystems(thaw, result); err != nil { + return err + } + } + if err := s.commitPartialSnapshot(snapshotConn, snapshotName); err != nil { return err } @@ -406,6 +441,84 @@ func (s *Backend) TakeSnapshot(ctx context.Context, name string, snapshot worksh return nil } +// freezeFilesystems runs the fsfreeze facet of workshopctl inside the given +// workshop. See fsfreeze.FreezeLocalFilesystems for details. +func (s *Backend) freezeFilesystems(conn lxd.InstanceServer, ctx context.Context, name string) (io.Closer, <-chan error, error) { + rev := revert.New() + defer rev.Fail() + + stdin, thaw := io.Pipe() + rev.Add(func() { + thaw.Close() + }) + + stdout := fsfreeze.NewReadyWriter() + var stderr strings.Builder + result := make(chan error, 1) + + args := &workshop.Execution{ + ExecArgs: workshop.ExecArgs{ + Command: []string{dirs.FsFreezePath}, + WorkDir: "/", + }, + ExecControls: workshop.ExecControls{ + Stdin: stdin, + Stdout: stdout, + Stderr: &stderr, + }, + } + exectx, err := s.execCommand(conn, ctx, name, args) + if err != nil { + stdin.Close() + return nil, nil, err + } + + // The fsfreeze process has its own timeout. If we interrupt it we risk + // leaving filesystems frozen, but cancelling WaitExecution doesn't + // cancel the execution itself. + waitCtx, cancel := context.WithTimeout(context.Background(), freezeTimeout) + go func() { + defer stdin.Close() + defer cancel() + result <- exectx.WaitExecution(waitCtx) + close(result) + }() + + select { + case <-stdout.Ready(): + rev.Success() + return thaw, result, nil + case err := <-result: + // It's unsafe to access errbuf before the DataDone channel is closed. + if _, ok := errors.AsType[*workshop.ErrExec](err); ok { + if s := stderr.String(); s != "" { + logger.Noticef("On freezeFilesystems: fsfreeze reported %s", s) + } else { + logger.Noticef("On freezeFilesystems: fsfreeze exited unexpectedly") + } + } + + if err == nil { + err = errors.New("fsfreeze never reported ready") + } + return nil, nil, err + } +} + +func thawFilesystems(thaw io.Closer, result <-chan error) error { + thaw.Close() + + waitCtx, cancel := context.WithTimeout(context.Background(), thawTimeout) + defer cancel() + + select { + case err := <-result: + return err + case <-waitCtx.Done(): + return waitCtx.Err() + } +} + func IsInstanceConflict(err error, name string) bool { if err == nil { return false diff --git a/internal/workshop/lxd/tests/helper/helper.go b/internal/workshop/lxd/tests/helper/helper.go index d0a12d77f..e4e1fbb36 100644 --- a/internal/workshop/lxd/tests/helper/helper.go +++ b/internal/workshop/lxd/tests/helper/helper.go @@ -31,6 +31,7 @@ import ( "gopkg.in/check.v1" "github.com/canonical/workshop/internal/dirs" + "github.com/canonical/workshop/internal/fsfreeze" "github.com/canonical/workshop/internal/sdk" "github.com/canonical/workshop/internal/waitready" "github.com/canonical/workshop/internal/workshop" @@ -61,6 +62,14 @@ func RunTestsOrWorkshopCtl(m *testing.M) int { return 0 } + if fsfreeze.IsFsfreezeInvocation() { + if err := fsfreeze.FreezeLocalFilesystems(os.Stdin, os.Stdout); err != nil { + fmt.Fprintf(os.Stderr, "error: %s\n", err) + return 1 + } + return 0 + } + executable, err := os.Executable() if err != nil { panic(fmt.Errorf("cannot get executable path: %w", err)) From a1335e86dff4291e7d496a8dddc3190f0256236c Mon Sep 17 00:00:00 2001 From: Jonathan Conder Date: Tue, 28 Jul 2026 18:20:10 +1200 Subject: [PATCH 06/15] Introduce workshop confinement Initially this field doesn't do much; optionally users can add `confinement: container` to their workshop definition files. But soon we'll add support for `confinement: virtual-machine`. --- internal/daemon/api_connections_test.go | 4 +- internal/daemon/api_exec_test.go | 2 +- internal/daemon/api_sdks_test.go | 8 +-- internal/daemon/api_workshops_test.go | 12 ++--- internal/daemon/snapshot-ingredients.yaml | 2 + .../overlord/healthstate/healthstate_test.go | 10 ++-- internal/overlord/hookstate/handlers_test.go | 8 +-- internal/overlord/ifacestate/ifacemgr_test.go | 2 +- internal/overlord/sdkstate/handlers_test.go | 8 +-- .../overlord/workshopstate/handlers_test.go | 52 +++++++++---------- internal/overlord/workshopstate/manifest.go | 2 +- .../overlord/workshopstate/manifest_test.go | 14 ++--- .../overlord/workshopstate/request_test.go | 8 +-- internal/workshop/backend.go | 8 +-- internal/workshop/fakebackend/backend.go | 8 +-- internal/workshop/lxd/lxd_backend.go | 1 + .../workshop/lxd/lxd_backend_snapshots.go | 9 ++++ internal/workshop/lxd/lxd_base_manager.go | 4 +- internal/workshop/lxd/tests/helper/helper.go | 4 +- .../lxd/tests/integration/snapshot_test.go | 8 +-- .../lxd/tests/integration/workshop_test.go | 48 ++++++++++------- internal/workshop/workshop_file.go | 26 ++++++++++ 22 files changed, 148 insertions(+), 100 deletions(-) diff --git a/internal/daemon/api_connections_test.go b/internal/daemon/api_connections_test.go index 272ca7b0f..5d063f6c9 100644 --- a/internal/daemon/api_connections_test.go +++ b/internal/daemon/api_connections_test.go @@ -91,7 +91,7 @@ func (s *apiSuite) workshopFile(ws string, sdks []*sdk.Info) *workshop.File { func (s *apiSuite) mockInstalledSDK(c *check.C, yaml string, w string) *workshop.Workshop { info := sdk.MockInfo(c, yaml, s.project.ProjectId, w) wf := s.workshopFile(w, []*sdk.Info{info}) - snapshot := workshop.BaseOnly(sdk.R(1), wf.Base, "fakeimage123") + snapshot := workshop.BaseOnly(sdk.R(1), wf.Base, workshop.ConfinementContainer, "fakeimage123") c.Assert(s.b.LaunchOrRebuildWorkshop(s.ctx, wf, snapshot), check.IsNil) wp, err := s.b.Workshop(s.ctx, w) @@ -128,7 +128,7 @@ func (s *apiSuite) mockInstalledSDKBoundPlug(c *check.C, yaml string, w string, Name: to} c.Assert(s.d.overlord.InterfaceManager().Repository().AddSdk(info), check.IsNil) wf := s.workshopFile(w, []*sdk.Info{info}) - snapshot := workshop.BaseOnly(sdk.R(1), wf.Base, "fakeimage123") + snapshot := workshop.BaseOnly(sdk.R(1), wf.Base, workshop.ConfinementContainer, "fakeimage123") c.Assert(s.b.LaunchOrRebuildWorkshop(s.ctx, wf, snapshot), check.IsNil) wp, err := s.b.Workshop(s.ctx, w) c.Check(err, check.IsNil) diff --git a/internal/daemon/api_exec_test.go b/internal/daemon/api_exec_test.go index aad22f031..488e0cbf0 100644 --- a/internal/daemon/api_exec_test.go +++ b/internal/daemon/api_exec_test.go @@ -41,7 +41,7 @@ func (s *apiSuite) setupExec(c *check.C) *Command { s.createWFile(c, "ws", wsYaml) wf := &workshop.File{Name: "ws", Base: "ubuntu@20.04", Actions: map[string]workshop.Action{"lint": "\n\n\ngolangci-lint run\n"}} - snapshot := workshop.BaseOnly(sdk.R(1), wf.Base, "fakeimage123") + snapshot := workshop.BaseOnly(sdk.R(1), wf.Base, workshop.ConfinementContainer, "fakeimage123") err := s.b.LaunchOrRebuildWorkshop(s.ctx, wf, snapshot) c.Assert(err, check.IsNil) diff --git a/internal/daemon/api_sdks_test.go b/internal/daemon/api_sdks_test.go index a022dea5e..26cc3f56e 100644 --- a/internal/daemon/api_sdks_test.go +++ b/internal/daemon/api_sdks_test.go @@ -439,11 +439,11 @@ func (s *apiSuite) TestSdkInfoGetOk(c *check.C) { s.createWFile(c, "lerobot", "name: lerobot\nbase: ubuntu@20.04\n") wf := &workshop.File{Name: "nav2", Base: "ubuntu@20.04"} - snapshot := workshop.BaseOnly(sdk.R(1), wf.Base, "fakeimage123") + snapshot := workshop.BaseOnly(sdk.R(1), wf.Base, workshop.ConfinementContainer, "fakeimage123") c.Assert(s.b.LaunchOrRebuildWorkshop(s.ctx, wf, snapshot), check.IsNil) wf = &workshop.File{Name: "lerobot", Base: "ubuntu@20.04"} - snapshot = workshop.BaseOnly(sdk.R(1), wf.Base, "fakeimage123") + snapshot = workshop.BaseOnly(sdk.R(1), wf.Base, workshop.ConfinementContainer, "fakeimage123") c.Assert(s.b.LaunchOrRebuildWorkshop(s.ctx, wf, snapshot), check.IsNil) // Add SDK setups with channels so the endpoint can report channels. @@ -667,7 +667,7 @@ func (s *apiSuite) TestSdkInfoLocalOnly(c *check.C) { s.createWFile(c, "nav2", "name: nav2\nbase: ubuntu@20.04\n") wf := &workshop.File{Name: "nav2", Base: "ubuntu@20.04"} - snapshot := workshop.BaseOnly(sdk.R(1), wf.Base, "fakeimage123") + snapshot := workshop.BaseOnly(sdk.R(1), wf.Base, workshop.ConfinementContainer, "fakeimage123") c.Assert(s.b.LaunchOrRebuildWorkshop(s.ctx, wf, snapshot), check.IsNil) // Add SDK setup with channels so the endpoint can report channels. @@ -798,7 +798,7 @@ func (s *apiSuite) TestSdkInfoGetInvalidLocalMetadata(c *check.C) { s.createWFile(c, "ws", "name: ws\nbase: ubuntu@20.04\n") wf := &workshop.File{Name: "ws", Base: "ubuntu@20.04"} - snapshot := workshop.BaseOnly(sdk.R(1), wf.Base, "fakeimage123") + snapshot := workshop.BaseOnly(sdk.R(1), wf.Base, workshop.ConfinementContainer, "fakeimage123") c.Assert(s.b.LaunchOrRebuildWorkshop(s.ctx, wf, snapshot), check.IsNil) meta := sdk.Meta{ diff --git a/internal/daemon/api_workshops_test.go b/internal/daemon/api_workshops_test.go index 4954d9905..e6d25c589 100644 --- a/internal/daemon/api_workshops_test.go +++ b/internal/daemon/api_workshops_test.go @@ -3790,8 +3790,8 @@ func (s *apiSuite) TestRefreshBaseUpdate(c *check.C) { defer s.store.SetDownloadCallback(storeDownload(c))() oldGetBase := s.b.GetBaseCallback - s.b.GetBaseCallback = func(ctx context.Context, base string) (workshop.BaseImage, error) { - return workshop.BaseImage{Name: base, Fingerprint: "oldimage123"}, nil + s.b.GetBaseCallback = func(ctx context.Context, base string, confinement workshop.Confinement) (workshop.BaseImage, error) { + return workshop.BaseImage{Name: base, Confinement: confinement, Fingerprint: "oldimage123"}, nil } defer func() { s.b.GetBaseCallback = oldGetBase }() @@ -3810,7 +3810,7 @@ func (s *apiSuite) TestRefreshBaseUpdate(c *check.C) { wp, err := s.b.Workshop(s.ctx, "manysdks") c.Assert(err, check.IsNil) - c.Check(wp.Image, check.Equals, workshop.BaseImage{Name: "ubuntu@22.04", Fingerprint: "oldimage123"}) + c.Check(wp.Image, check.Equals, workshop.BaseImage{Name: "ubuntu@22.04", Confinement: workshop.ConfinementContainer, Fingerprint: "oldimage123"}) requests = []*bytes.Buffer{ bytes.NewBufferString(`{"names":["manysdks"],"action":"refresh"}`), @@ -3824,15 +3824,15 @@ func (s *apiSuite) TestRefreshBaseUpdate(c *check.C) { }, } - s.b.GetBaseCallback = func(ctx context.Context, base string) (workshop.BaseImage, error) { - return workshop.BaseImage{Name: base, Fingerprint: "newimage321"}, nil + s.b.GetBaseCallback = func(ctx context.Context, base string, confinement workshop.Confinement) (workshop.BaseImage, error) { + return workshop.BaseImage{Name: base, Confinement: confinement, Fingerprint: "newimage321"}, nil } s.runActionTest(c, requests, expected) wp, err = s.b.Workshop(s.ctx, "manysdks") c.Assert(err, check.IsNil) - c.Check(wp.Image, check.Equals, workshop.BaseImage{Name: "ubuntu@22.04", Fingerprint: "newimage321"}) + c.Check(wp.Image, check.Equals, workshop.BaseImage{Name: "ubuntu@22.04", Confinement: workshop.ConfinementContainer, Fingerprint: "newimage321"}) want := []expectedWorkshop{{ name: "manysdks", diff --git a/internal/daemon/snapshot-ingredients.yaml b/internal/daemon/snapshot-ingredients.yaml index f1c1e99a0..4e8bd149b 100644 --- a/internal/daemon/snapshot-ingredients.yaml +++ b/internal/daemon/snapshot-ingredients.yaml @@ -7,6 +7,7 @@ File: Name: string Base: string + Confinement: workshop.Confinement Sdks: '[]workshop.SdkRecord' Actions: Connections: @@ -29,6 +30,7 @@ Workshop: Hostname: BaseImage: Name: string + Confinement: workshop.Confinement Fingerprint: string SdkInstallation: Setup: sdk.Setup diff --git a/internal/overlord/healthstate/healthstate_test.go b/internal/overlord/healthstate/healthstate_test.go index 90211ff49..967769b5c 100644 --- a/internal/overlord/healthstate/healthstate_test.go +++ b/internal/overlord/healthstate/healthstate_test.go @@ -112,7 +112,7 @@ var ( func (s *healthSuite) launchWorkshopWithSDKs(c *check.C, sdks []workshop.SdkRecord, hooks map[string]map[string]string) *workshop.Workshop { wf := &workshop.File{Name: "ws", Base: "ubuntu@20.04", Sdks: sdks} - snapshot := workshop.BaseOnly(sdk.R(1), wf.Base, "fakeimage123") + snapshot := workshop.BaseOnly(sdk.R(1), wf.Base, workshop.ConfinementContainer, "fakeimage123") err := s.backend.LaunchOrRebuildWorkshop(s.ctx, wf, snapshot) c.Check(err, check.IsNil) ws, err := s.backend.WorkshopFs(s.ctx, "ws") @@ -252,7 +252,7 @@ func (s *healthSuite) TestWorkshopHealthOperationInProgress(c *check.C) { chg := s.state.NewChange("launch", "test") chg.Set("project-id", s.project.ProjectId) chg.Set("ws_new_format", sdk.R(1)) - chg.Set("ws_new_base", workshop.BaseImage{Name: "ubuntu@20.04", Fingerprint: "fakeimage123"}) + chg.Set("ws_new_base", workshop.BaseImage{Name: "ubuntu@20.04", Confinement: workshop.ConfinementContainer, Fingerprint: "fakeimage123"}) chg.Set("ws_new_sdks", []sdk.Setup{}) task := s.state.NewTask("create-workshop", "test task") task.Set("workshop-file", "name: ws\nbase: ubuntu@20.04\n") @@ -280,7 +280,7 @@ func (s *healthSuite) TestWorkshopHealthOperationWaitingWithNotes(c *check.C) { chg := s.state.NewChange("refresh", "test") chg.Set("project-id", s.project.ProjectId) chg.Set("ws_new_format", sdk.R(1)) - chg.Set("ws_new_base", workshop.BaseImage{Name: "ubuntu@20.04", Fingerprint: "fakeimage123"}) + chg.Set("ws_new_base", workshop.BaseImage{Name: "ubuntu@20.04", Confinement: workshop.ConfinementContainer, Fingerprint: "fakeimage123"}) chg.Set("ws_new_sdks", []sdk.Setup{}) chg.SetStatus(state.WaitStatus) task := s.state.NewTask("create-workshop", "test task") @@ -350,7 +350,7 @@ func (s *healthSuite) TestCheckStatusPending(c *check.C) { chg := s.state.NewChange("refresh", "test") chg.Set("project-id", s.project.ProjectId) chg.Set("ws_new_format", sdk.R(1)) - chg.Set("ws_new_base", workshop.BaseImage{Name: "ubuntu@20.04", Fingerprint: "fakeimage123"}) + chg.Set("ws_new_base", workshop.BaseImage{Name: "ubuntu@20.04", Confinement: workshop.ConfinementContainer, Fingerprint: "fakeimage123"}) chg.Set("ws_new_sdks", []sdk.Setup{}) chg.SetStatus(state.DoingStatus) task := s.state.NewTask("create-workshop", "test task") @@ -384,7 +384,7 @@ func (s *healthSuite) TestCheckStatusWaiting(c *check.C) { chg := s.state.NewChange("refresh", "test") chg.Set("project-id", s.project.ProjectId) chg.Set("ws_new_format", sdk.R(1)) - chg.Set("ws_new_base", workshop.BaseImage{Name: "ubuntu@20.04", Fingerprint: "fakeimage123"}) + chg.Set("ws_new_base", workshop.BaseImage{Name: "ubuntu@20.04", Confinement: workshop.ConfinementContainer, Fingerprint: "fakeimage123"}) chg.Set("ws_new_sdks", []sdk.Setup{}) chg.SetStatus(state.WaitStatus) task := s.state.NewTask("create-workshop", "test task") diff --git a/internal/overlord/hookstate/handlers_test.go b/internal/overlord/hookstate/handlers_test.go index 358e277c8..1cbfe9e16 100644 --- a/internal/overlord/hookstate/handlers_test.go +++ b/internal/overlord/hookstate/handlers_test.go @@ -148,7 +148,7 @@ func (s *hookSuite) TestExecHookDoesNotExist(c *check.C) { // Launch a workshop provinding no hooks wf := &workshop.File{Name: "ws", Base: "ubuntu@20.04"} - snapshot := workshop.BaseOnly(sdk.R(1), wf.Base, "fakeimage123") + snapshot := workshop.BaseOnly(sdk.R(1), wf.Base, workshop.ConfinementContainer, "fakeimage123") err := s.backend.LaunchOrRebuildWorkshop(s.ctx, wf, snapshot) c.Check(err, check.IsNil) @@ -172,7 +172,7 @@ func (s *hookSuite) TestExecHookSkipsStrayHooksFile(c *check.C) { chg.AddTask(t1) wf := &workshop.File{Name: "ws", Base: "ubuntu@20.04"} - snapshot := workshop.BaseOnly(sdk.R(1), wf.Base, "fakeimage123") + snapshot := workshop.BaseOnly(sdk.R(1), wf.Base, workshop.ConfinementContainer, "fakeimage123") err := s.backend.LaunchOrRebuildWorkshop(s.ctx, wf, snapshot) c.Check(err, check.IsNil) @@ -206,7 +206,7 @@ func (s *hookSuite) TestExecHookStatError(c *check.C) { chg.AddTask(t1) wf := &workshop.File{Name: "ws", Base: "ubuntu@20.04"} - snapshot := workshop.BaseOnly(sdk.R(1), wf.Base, "fakeimage123") + snapshot := workshop.BaseOnly(sdk.R(1), wf.Base, workshop.ConfinementContainer, "fakeimage123") err := s.backend.LaunchOrRebuildWorkshop(s.ctx, wf, snapshot) c.Check(err, check.IsNil) @@ -667,7 +667,7 @@ func (s *hookSuite) TestHookWithMultipleHandlersIsError(c *check.C) { func (s *hookSuite) launchWorkshop(c *check.C, newsdk string) { wf := &workshop.File{Name: "ws", Base: "ubuntu@20.04", Sdks: []workshop.SdkRecord{{Name: "one", Channel: "latest/stable"}}} - snapshot := workshop.BaseOnly(sdk.R(1), wf.Base, "fakeimage123") + snapshot := workshop.BaseOnly(sdk.R(1), wf.Base, workshop.ConfinementContainer, "fakeimage123") err := s.backend.LaunchOrRebuildWorkshop(s.ctx, wf, snapshot) c.Check(err, check.IsNil) ws, err := s.backend.WorkshopFs(s.ctx, "ws") diff --git a/internal/overlord/ifacestate/ifacemgr_test.go b/internal/overlord/ifacestate/ifacemgr_test.go index b4416e81f..a1930b8ab 100644 --- a/internal/overlord/ifacestate/ifacemgr_test.go +++ b/internal/overlord/ifacestate/ifacemgr_test.go @@ -145,7 +145,7 @@ func (s *interfaceManagerSuite) launchWorkshop(c *check.C, ws string, sdks []sdk err = yaml.Unmarshal(workshopFile.Bytes(), &wf) c.Assert(err, check.IsNil) - snapshot := workshop.BaseOnly(sdk.R(1), wf.Base, "fakeimage123") + snapshot := workshop.BaseOnly(sdk.R(1), wf.Base, workshop.ConfinementContainer, "fakeimage123") err = s.wsbackend.LaunchOrRebuildWorkshop(ctx, &wf, snapshot) c.Assert(err, check.IsNil) diff --git a/internal/overlord/sdkstate/handlers_test.go b/internal/overlord/sdkstate/handlers_test.go index c59d5c7cd..9c2a93b24 100644 --- a/internal/overlord/sdkstate/handlers_test.go +++ b/internal/overlord/sdkstate/handlers_test.go @@ -195,7 +195,7 @@ func (s *sdkStateSuite) SetUpTest(c *check.C) { {Name: "test", Channel: "latest/stable"}, {Name: "test-broken", Channel: "latest/stable"}, }} - snapshot := workshop.BaseOnly(sdk.R(1), wf.Base, "fakeimage123") + snapshot := workshop.BaseOnly(sdk.R(1), wf.Base, workshop.ConfinementContainer, "fakeimage123") err = s.backend.LaunchOrRebuildWorkshop(s.ctx, wf, snapshot) c.Assert(err, check.IsNil) @@ -456,7 +456,7 @@ func (s *sdkStateSuite) TestRetrieveSystemSdkSuccess(c *check.C) { setWorkshopProject("ws", s.project, t) chg.Set("user", "testuser") chg.Set("ws_new_format", sdk.R(1)) - chg.Set("ws_new_base", workshop.BaseOnly(sdk.R(1), "ubuntu@22.04", "fakeimage123")) + chg.Set("ws_new_base", workshop.BaseOnly(sdk.R(1), "ubuntu@22.04", workshop.ConfinementContainer, "fakeimage123")) chg.Set("ws_new_sdks", []sdk.Setup{newSdk}) chg.AddTask(t) @@ -506,7 +506,7 @@ func (s *sdkStateSuite) TestSnapshotSdkTwice(c *check.C) { s.state.Lock() defer s.state.Unlock() - image := workshop.BaseImage{Name: "ubuntu@22.04", Fingerprint: "fakeimage123"} + image := workshop.BaseImage{Name: "ubuntu@22.04", Confinement: workshop.ConfinementContainer, Fingerprint: "fakeimage123"} sdks := []sdk.Setup{{ Name: "test", Channel: "latest/stable", @@ -601,7 +601,7 @@ func (s *sdkStateSuite) TestSnapshotSkippedPostResume(c *check.C) { chg.Set("user", "testuser") chg.Set("project-id", s.project.ProjectId) chg.Set("ws_new_format", sdk.R(1)) - chg.Set("ws_new_base", workshop.BaseImage{Name: "ubuntu@22.04", Fingerprint: "fakeimage123"}) + chg.Set("ws_new_base", workshop.BaseImage{Name: "ubuntu@22.04", Confinement: workshop.ConfinementContainer, Fingerprint: "fakeimage123"}) chg.Set("ws_new_sdks", sdks) chg.Set("wait-setup", conflict.ChangeSetup{Mode: conflict.ChangeWaitOnError.String()}) chg.AddTask(t1) diff --git a/internal/overlord/workshopstate/handlers_test.go b/internal/overlord/workshopstate/handlers_test.go index 321a87c93..c3cda8d5f 100644 --- a/internal/overlord/workshopstate/handlers_test.go +++ b/internal/overlord/workshopstate/handlers_test.go @@ -156,7 +156,7 @@ func (s *workshopHandlers) TestStopPeriodicProgressUpdate(c *check.C) { defer s.state.Unlock() s.createWFile(c, "ws", wsFocal) wf := &workshop.File{Name: "ws", Base: "ubuntu@20.04"} - snapshot := workshop.BaseOnly(sdk.R(1), wf.Base, "fakeimage123") + snapshot := workshop.BaseOnly(sdk.R(1), wf.Base, workshop.ConfinementContainer, "fakeimage123") err := s.backend.LaunchOrRebuildWorkshop(s.ctx, wf, snapshot) c.Check(err, check.IsNil) @@ -199,7 +199,7 @@ func (s *workshopHandlers) TestUndoStash(c *check.C) { {Name: "test2", Channel: "latest/stable"}, }} - snapshot := workshop.BaseOnly(sdk.R(1), wf.Base, "fakeimage123") + snapshot := workshop.BaseOnly(sdk.R(1), wf.Base, workshop.ConfinementContainer, "fakeimage123") err := s.backend.LaunchOrRebuildWorkshop(s.ctx, wf, snapshot) c.Check(err, check.IsNil) @@ -244,7 +244,7 @@ func (s *workshopHandlers) TestRemoveWorkshop(c *check.C) { userDataDir := workshop.UserDataRootDir(s.user.HomeDir, nil) for _, wf := range wFiles { - snapshot := workshop.BaseOnly(sdk.R(1), wf.Base, "fakeimage123") + snapshot := workshop.BaseOnly(sdk.R(1), wf.Base, workshop.ConfinementContainer, "fakeimage123") err := s.backend.LaunchOrRebuildWorkshop(s.ctx, wf, snapshot) c.Check(err, check.IsNil) @@ -324,7 +324,7 @@ func (s *workshopHandlers) TestCreateWorkshopNoWorkshopDefinitionFound(c *check. setWorkshopProject("ws", s.project, t1) chg.Set("user", "testuser") chg.Set("ws_new_format", sdk.R(1)) - chg.Set("ws_new_base", workshop.BaseImage{Name: "ubuntu@22.04", Fingerprint: "fakeimage123"}) + chg.Set("ws_new_base", workshop.BaseImage{Name: "ubuntu@22.04", Confinement: workshop.ConfinementContainer, Fingerprint: "fakeimage123"}) chg.Set("ws_new_sdks", []sdk.Setup{}) chg.AddTask(t1) @@ -350,7 +350,7 @@ func (s *workshopHandlers) TestCreateWorkshopWithSystemSdk(c *check.C) { setWorkshopProject("ws", s.project, t1) chg.Set("user", "testuser") chg.Set("ws_new_format", sdk.R(1)) - chg.Set("ws_new_base", workshop.BaseImage{Name: "ubuntu@22.04", Fingerprint: "fakeimage123"}) + chg.Set("ws_new_base", workshop.BaseImage{Name: "ubuntu@22.04", Confinement: workshop.ConfinementContainer, Fingerprint: "fakeimage123"}) chg.Set("ws_new_sdks", []sdk.Setup{}) chg.AddTask(t1) @@ -380,7 +380,7 @@ func (s *workshopHandlers) TestCreateWorkshopCleanup(c *check.C) { setWorkshopProject("ws", s.project, t1) chg.Set("user", "testuser") chg.Set("ws_new_format", sdk.R(1)) - chg.Set("ws_new_base", workshop.BaseImage{Name: "ubuntu@22.04", Fingerprint: "fakeimage123"}) + chg.Set("ws_new_base", workshop.BaseImage{Name: "ubuntu@22.04", Confinement: workshop.ConfinementContainer, Fingerprint: "fakeimage123"}) chg.Set("ws_new_sdks", []sdk.Setup{}) chg.AddTask(t1) @@ -412,7 +412,7 @@ func (s *workshopHandlers) TestRebuildWorkshopNoCleanup(c *check.C) { t1.Set("workshop-file", wsJammy) setWorkshopProject("ws", s.project, t1) chg.Set("user", "testuser") - image := workshop.BaseImage{Name: "ubuntu@22.04", Fingerprint: "fakeimage123"} + image := workshop.BaseImage{Name: "ubuntu@22.04", Confinement: workshop.ConfinementContainer, Fingerprint: "fakeimage123"} chg.Set("ws_new_format", sdk.R(1)) chg.Set("ws_new_base", image) chg.Set("ws_new_sdks", []sdk.Setup{}) @@ -452,7 +452,7 @@ func (s *workshopHandlers) TestDownloadBase(c *check.C) { t1 := s.state.NewTask("download-base", "...") setWorkshopProject("ws", s.project, t1) chg.Set("user", "testuser") - chg.Set("ws_new_base", workshop.BaseImage{Name: "ubuntu@22.04", Fingerprint: "fakeimage1234"}) + chg.Set("ws_new_base", workshop.BaseImage{Name: "ubuntu@22.04", Confinement: workshop.ConfinementContainer, Fingerprint: "fakeimage1234"}) chg.AddTask(t1) s.state.Unlock() @@ -660,7 +660,7 @@ func (s *workshopHandlers) TestSnapshotRemovedAfterRemove(c *check.C) { manifest := workshopstate.Manifest{ File: &workshop.File{Name: "ws", Base: "ubuntu@22.04"}, Format: sdk.R(1), - Image: workshop.BaseImage{Name: "ubuntu@22.04", Fingerprint: "fakeimage123"}, + Image: workshop.BaseImage{Name: "ubuntu@22.04", Confinement: workshop.ConfinementContainer, Fingerprint: "fakeimage123"}, Sdks: []sdk.Setup{testSdk, testSdk2}, } snapshot1 := workshop.SdkSnapshot(manifest.Format, manifest.Image, manifest.Sdks[:1]) @@ -712,7 +712,7 @@ func (s *workshopHandlers) TestSnapshotRemovedAfterFailedLaunch(c *check.C) { manifest := workshopstate.Manifest{ File: &workshop.File{Name: "ws", Base: "ubuntu@22.04"}, Format: sdk.R(1), - Image: workshop.BaseImage{Name: "ubuntu@22.04", Fingerprint: "fakeimage123"}, + Image: workshop.BaseImage{Name: "ubuntu@22.04", Confinement: workshop.ConfinementContainer, Fingerprint: "fakeimage123"}, Sdks: []sdk.Setup{testSdk, testSdk2}, } snapshot1 := workshop.SdkSnapshot(manifest.Format, manifest.Image, manifest.Sdks[:1]) @@ -771,7 +771,7 @@ func (s *workshopHandlers) TestSnapshotRemovedAfterRefresh(c *check.C) { current := workshopstate.Manifest{ File: &workshop.File{Name: "ws", Base: "ubuntu@22.04"}, Format: sdk.R(1), - Image: workshop.BaseImage{Name: "ubuntu@22.04", Fingerprint: "fakeimage123"}, + Image: workshop.BaseImage{Name: "ubuntu@22.04", Confinement: workshop.ConfinementContainer, Fingerprint: "fakeimage123"}, Sdks: []sdk.Setup{testSdk}, } snapshot1 := workshop.SdkSnapshot(current.Format, current.Image, current.Sdks) @@ -779,7 +779,7 @@ func (s *workshopHandlers) TestSnapshotRemovedAfterRefresh(c *check.C) { latest := workshopstate.Manifest{ File: &workshop.File{Name: "ws", Base: "ubuntu@22.04"}, Format: sdk.R(1), - Image: workshop.BaseImage{Name: "ubuntu@22.04", Fingerprint: "fakeimage123"}, + Image: workshop.BaseImage{Name: "ubuntu@22.04", Confinement: workshop.ConfinementContainer, Fingerprint: "fakeimage123"}, Sdks: []sdk.Setup{testSdk2}, } snapshot2 := workshop.SdkSnapshot(latest.Format, latest.Image, latest.Sdks) @@ -845,7 +845,7 @@ func (s *workshopHandlers) TestSnapshotRemovedAfterFailedRefresh(c *check.C) { current := workshopstate.Manifest{ File: &workshop.File{Name: "ws", Base: "ubuntu@22.04"}, Format: sdk.R(1), - Image: workshop.BaseImage{Name: "ubuntu@22.04", Fingerprint: "fakeimage123"}, + Image: workshop.BaseImage{Name: "ubuntu@22.04", Confinement: workshop.ConfinementContainer, Fingerprint: "fakeimage123"}, Sdks: []sdk.Setup{testSdk}, } snapshot1 := workshop.SdkSnapshot(current.Format, current.Image, current.Sdks) @@ -853,7 +853,7 @@ func (s *workshopHandlers) TestSnapshotRemovedAfterFailedRefresh(c *check.C) { latest := workshopstate.Manifest{ File: &workshop.File{Name: "ws", Base: "ubuntu@22.04"}, Format: sdk.R(1), - Image: workshop.BaseImage{Name: "ubuntu@22.04", Fingerprint: "fakeimage123"}, + Image: workshop.BaseImage{Name: "ubuntu@22.04", Confinement: workshop.ConfinementContainer, Fingerprint: "fakeimage123"}, Sdks: []sdk.Setup{testSdk2}, } snapshot2 := workshop.SdkSnapshot(latest.Format, latest.Image, latest.Sdks) @@ -920,7 +920,7 @@ func (s *workshopHandlers) TestSnapshotRemovedAfterRemoveMidRefresh(c *check.C) current := workshopstate.Manifest{ File: &workshop.File{Name: "ws", Base: "ubuntu@22.04"}, Format: sdk.R(1), - Image: workshop.BaseImage{Name: "ubuntu@22.04", Fingerprint: "fakeimage123"}, + Image: workshop.BaseImage{Name: "ubuntu@22.04", Confinement: workshop.ConfinementContainer, Fingerprint: "fakeimage123"}, Sdks: []sdk.Setup{testSdk}, } snapshot1 := workshop.SdkSnapshot(current.Format, current.Image, current.Sdks) @@ -928,7 +928,7 @@ func (s *workshopHandlers) TestSnapshotRemovedAfterRemoveMidRefresh(c *check.C) latest := workshopstate.Manifest{ File: &workshop.File{Name: "ws", Base: "ubuntu@22.04"}, Format: sdk.R(1), - Image: workshop.BaseImage{Name: "ubuntu@22.04", Fingerprint: "fakeimage123"}, + Image: workshop.BaseImage{Name: "ubuntu@22.04", Confinement: workshop.ConfinementContainer, Fingerprint: "fakeimage123"}, Sdks: []sdk.Setup{testSdk2}, } snapshot2 := workshop.SdkSnapshot(latest.Format, latest.Image, latest.Sdks) @@ -1049,7 +1049,7 @@ func (s *workshopHandlers) TestSnapshotExitCleanupAfterSuccessfulLaunch(c *check manifest := workshopstate.Manifest{ File: &workshop.File{Name: "ws", Base: "ubuntu@22.04"}, Format: sdk.R(1), - Image: workshop.BaseImage{Name: "ubuntu@22.04", Fingerprint: "fakeimage123"}, + Image: workshop.BaseImage{Name: "ubuntu@22.04", Confinement: workshop.ConfinementContainer, Fingerprint: "fakeimage123"}, Sdks: []sdk.Setup{testSdk}, } snapshot := workshop.SdkSnapshot(manifest.Format, manifest.Image, manifest.Sdks) @@ -1093,7 +1093,7 @@ func (s *workshopHandlers) TestSnapshotNotRemovedBeforeCooldown(c *check.C) { manifest := workshopstate.Manifest{ File: &workshop.File{Name: "ws", Base: "ubuntu@22.04"}, Format: sdk.R(1), - Image: workshop.BaseImage{Name: "ubuntu@22.04", Fingerprint: "fakeimage123"}, + Image: workshop.BaseImage{Name: "ubuntu@22.04", Confinement: workshop.ConfinementContainer, Fingerprint: "fakeimage123"}, Sdks: []sdk.Setup{testSdk}, } snapshot := workshop.SdkSnapshot(manifest.Format, manifest.Image, manifest.Sdks) @@ -1137,7 +1137,7 @@ func (s *workshopHandlers) TestSnapshotExitCleanupIfUsedAgain(c *check.C) { manifest1 := workshopstate.Manifest{ File: &workshop.File{Name: "ws", Base: "ubuntu@22.04"}, Format: sdk.R(1), - Image: workshop.BaseImage{Name: "ubuntu@22.04", Fingerprint: "fakeimage123"}, + Image: workshop.BaseImage{Name: "ubuntu@22.04", Confinement: workshop.ConfinementContainer, Fingerprint: "fakeimage123"}, Sdks: []sdk.Setup{testSdk}, } snapshot1 := workshop.SdkSnapshot(manifest1.Format, manifest1.Image, manifest1.Sdks) @@ -1145,7 +1145,7 @@ func (s *workshopHandlers) TestSnapshotExitCleanupIfUsedAgain(c *check.C) { manifest2 := workshopstate.Manifest{ File: &workshop.File{Name: "ws2", Base: "ubuntu@22.04"}, Format: sdk.R(1), - Image: workshop.BaseImage{Name: "ubuntu@22.04", Fingerprint: "fakeimage123"}, + Image: workshop.BaseImage{Name: "ubuntu@22.04", Confinement: workshop.ConfinementContainer, Fingerprint: "fakeimage123"}, Sdks: []sdk.Setup{testSdk}, } @@ -1203,7 +1203,7 @@ func (s *workshopHandlers) TestSnapshotRetriesCleanupIfBlockingChangesArePresent manifest1 := workshopstate.Manifest{ File: &workshop.File{Name: "ws", Base: "ubuntu@22.04"}, Format: sdk.R(1), - Image: workshop.BaseImage{Name: "ubuntu@22.04", Fingerprint: "fakeimage123"}, + Image: workshop.BaseImage{Name: "ubuntu@22.04", Confinement: workshop.ConfinementContainer, Fingerprint: "fakeimage123"}, Sdks: []sdk.Setup{testSdk}, } snapshot1 := workshop.SdkSnapshot(manifest1.Format, manifest1.Image, manifest1.Sdks) @@ -1211,7 +1211,7 @@ func (s *workshopHandlers) TestSnapshotRetriesCleanupIfBlockingChangesArePresent manifest2 := workshopstate.Manifest{ File: &workshop.File{Name: "ws2", Base: "ubuntu@22.04"}, Format: sdk.R(1), - Image: workshop.BaseImage{Name: "ubuntu@22.04", Fingerprint: "fakeimage123"}, + Image: workshop.BaseImage{Name: "ubuntu@22.04", Confinement: workshop.ConfinementContainer, Fingerprint: "fakeimage123"}, Sdks: []sdk.Setup{testSdk}, } @@ -1289,7 +1289,7 @@ func (s *workshopHandlers) TestSnapshotCleanupPerformedByLatestUser(c *check.C) manifest1 := workshopstate.Manifest{ File: &workshop.File{Name: "ws", Base: "ubuntu@22.04"}, Format: sdk.R(1), - Image: workshop.BaseImage{Name: "ubuntu@22.04", Fingerprint: "fakeimage123"}, + Image: workshop.BaseImage{Name: "ubuntu@22.04", Confinement: workshop.ConfinementContainer, Fingerprint: "fakeimage123"}, Sdks: []sdk.Setup{testSdk}, } snapshot1 := workshop.SdkSnapshot(manifest1.Format, manifest1.Image, manifest1.Sdks) @@ -1297,7 +1297,7 @@ func (s *workshopHandlers) TestSnapshotCleanupPerformedByLatestUser(c *check.C) manifest2 := workshopstate.Manifest{ File: &workshop.File{Name: "ws2", Base: "ubuntu@22.04"}, Format: sdk.R(1), - Image: workshop.BaseImage{Name: "ubuntu@22.04", Fingerprint: "fakeimage123"}, + Image: workshop.BaseImage{Name: "ubuntu@22.04", Confinement: workshop.ConfinementContainer, Fingerprint: "fakeimage123"}, Sdks: []sdk.Setup{testSdk}, } @@ -1351,7 +1351,7 @@ func (s *workshopHandlers) TestSnapshotCleanupWaitsForDependentSnapshots(c *chec manifest1 := workshopstate.Manifest{ File: &workshop.File{Name: "ws", Base: "ubuntu@22.04"}, Format: sdk.R(1), - Image: workshop.BaseImage{Name: "ubuntu@22.04", Fingerprint: "fakeimage123"}, + Image: workshop.BaseImage{Name: "ubuntu@22.04", Confinement: workshop.ConfinementContainer, Fingerprint: "fakeimage123"}, Sdks: []sdk.Setup{testSdk, testSdk2}, } snapshot1 := workshop.SdkSnapshot(manifest1.Format, manifest1.Image, manifest1.Sdks) @@ -1359,7 +1359,7 @@ func (s *workshopHandlers) TestSnapshotCleanupWaitsForDependentSnapshots(c *chec manifest2 := workshopstate.Manifest{ File: &workshop.File{Name: "ws2", Base: "ubuntu@22.04"}, Format: sdk.R(1), - Image: workshop.BaseImage{Name: "ubuntu@22.04", Fingerprint: "fakeimage123"}, + Image: workshop.BaseImage{Name: "ubuntu@22.04", Confinement: workshop.ConfinementContainer, Fingerprint: "fakeimage123"}, Sdks: []sdk.Setup{testSdk}, } snapshot2 := workshop.SdkSnapshot(manifest2.Format, manifest2.Image, manifest2.Sdks) diff --git a/internal/overlord/workshopstate/manifest.go b/internal/overlord/workshopstate/manifest.go index 581c56e9d..0cadafa08 100644 --- a/internal/overlord/workshopstate/manifest.go +++ b/internal/overlord/workshopstate/manifest.go @@ -256,7 +256,7 @@ func (a *artifactFinder) launchOrRefreshManifests(ctx context.Context, names []s } files = append(files, file) - image, err := a.backend.GetBase(ctx, file.Base) + image, err := a.backend.GetBase(ctx, file.Base, file.Confinement) if err != nil { return nil, nil, fmt.Errorf("cannot %s %q: %w", action, name, err) } diff --git a/internal/overlord/workshopstate/manifest_test.go b/internal/overlord/workshopstate/manifest_test.go index 5222201d0..a2d596e7b 100644 --- a/internal/overlord/workshopstate/manifest_test.go +++ b/internal/overlord/workshopstate/manifest_test.go @@ -217,7 +217,7 @@ func (s *manifestSuite) createWFile(c *check.C, ws, base string, sdks []workshop func (s *manifestSuite) launchWorkshopWithSDKs(c *check.C, ws, base string, sdks []workshop.SdkRecord) *workshop.Workshop { wf := s.createWFile(c, ws, base, sdks) - snapshot := workshop.BaseOnly(sdk.R(1), wf.Base, "fakeimage123") + snapshot := workshop.BaseOnly(sdk.R(1), wf.Base, workshop.ConfinementContainer, "fakeimage123") err := s.backend.LaunchOrRebuildWorkshop(s.ctx, wf, snapshot) c.Assert(err, check.IsNil) @@ -250,7 +250,7 @@ func (s *manifestSuite) TestLaunchOK(c *check.C) { Sdks: sdks, }) - c.Check(manifests[0].Image, check.Equals, workshop.BaseImage{Name: "ubuntu@20.04", Fingerprint: "fakeimage123"}) + c.Check(manifests[0].Image, check.Equals, workshop.BaseImage{Name: "ubuntu@20.04", Confinement: workshop.ConfinementContainer, Fingerprint: "fakeimage123"}) c.Check(manifests[1].Image, check.Equals, manifests[0].Image) systemSdk, err := system.SystemSdkMeta() @@ -308,7 +308,7 @@ func (s *manifestSuite) TestRefreshOK(c *check.C) { c.Check(latest[0].File, check.DeepEquals, current[0].File) c.Check(current[0].Format, check.Equals, sdk.R(1)) c.Check(latest[0].Format, check.Equals, sdk.R(2)) - c.Check(current[0].Image, check.Equals, workshop.BaseImage{Name: "ubuntu@20.04", Fingerprint: "fakeimage123"}) + c.Check(current[0].Image, check.Equals, workshop.BaseImage{Name: "ubuntu@20.04", Confinement: workshop.ConfinementContainer, Fingerprint: "fakeimage123"}) c.Check(latest[0].Image, check.Equals, current[0].Image) // Check base was updated for test-2. @@ -325,7 +325,7 @@ func (s *manifestSuite) TestRefreshOK(c *check.C) { c.Check(current[1].Format, check.Equals, sdk.R(1)) c.Check(latest[1].Format, check.Equals, sdk.R(2)) c.Check(current[1].Image, check.Equals, current[0].Image) - c.Check(latest[1].Image, check.Equals, workshop.BaseImage{Name: "ubuntu@22.04", Fingerprint: "fakeimage123"}) + c.Check(latest[1].Image, check.Equals, workshop.BaseImage{Name: "ubuntu@22.04", Confinement: workshop.ConfinementContainer, Fingerprint: "fakeimage123"}) // Check current SDKs are loaded from running workshop. c.Check(current[0].Sdks, check.DeepEquals, []sdk.Setup{oldSdk.Setup}) @@ -376,7 +376,7 @@ func (s *manifestSuite) TestRefreshRestoreOK(c *check.C) { Base: "ubuntu@20.04", Sdks: sdks, }) - c.Check(current[0].Image, check.Equals, workshop.BaseImage{Name: "ubuntu@20.04", Fingerprint: "fakeimage123"}) + c.Check(current[0].Image, check.Equals, workshop.BaseImage{Name: "ubuntu@20.04", Confinement: workshop.ConfinementContainer, Fingerprint: "fakeimage123"}) c.Check(current[0].Sdks, check.DeepEquals, []sdk.Setup{oldSdk.Setup}) } @@ -504,7 +504,7 @@ func (s *manifestSuite) TestLaunchRequiresBase(c *check.C) { s.launchWorkshopWithSDKs(c, "test-1", "ubuntu@20.04", nil) - restoreBase := testutil.FakeFunc(func(ctx context.Context, base string) (workshop.BaseImage, error) { + restoreBase := testutil.FakeFunc(func(ctx context.Context, base string, confinement workshop.Confinement) (workshop.BaseImage, error) { return workshop.BaseImage{}, errors.New("contrived error") }, &s.backend.GetBaseCallback) defer restoreBase() @@ -564,7 +564,7 @@ func (s *manifestSuite) TestLaunchValidRequest(c *check.C) { Base: "ubuntu@20.04", Sdks: sdks, }) - c.Check(manifests[0].Image, check.Equals, workshop.BaseImage{Name: "ubuntu@20.04", Fingerprint: "fakeimage123"}) + c.Check(manifests[0].Image, check.Equals, workshop.BaseImage{Name: "ubuntu@20.04", Confinement: workshop.ConfinementContainer, Fingerprint: "fakeimage123"}) systemSdk, err := system.SystemSdkMeta() c.Assert(err, check.IsNil) diff --git a/internal/overlord/workshopstate/request_test.go b/internal/overlord/workshopstate/request_test.go index 7d2e81a7d..724e8caae 100644 --- a/internal/overlord/workshopstate/request_test.go +++ b/internal/overlord/workshopstate/request_test.go @@ -133,7 +133,7 @@ func (s *requestSuite) launchWorkshopWithSDKs(c *check.C, ws string, sdks []work c.Assert(err, check.IsNil) wf := workshop.File{Name: ws, Base: "ubuntu@20.04", Sdks: sdks} - snapshot := workshop.BaseOnly(sdk.R(1), wf.Base, "fakeimage123") + snapshot := workshop.BaseOnly(sdk.R(1), wf.Base, workshop.ConfinementContainer, "fakeimage123") err = s.backend.LaunchOrRebuildWorkshop(s.ctx, &wf, snapshot) c.Assert(err, check.IsNil) @@ -198,7 +198,7 @@ connections: current := []workshopstate.Manifest{{ File: &oldf, Format: sdk.R(1), - Image: workshop.BaseImage{Name: newf.Base, Fingerprint: "fakeimage123"}, + Image: workshop.BaseImage{Name: newf.Base, Confinement: workshop.ConfinementContainer, Fingerprint: "fakeimage123"}, Sdks: []sdk.Setup{{ Name: "system", Source: sdk.SystemSource, @@ -365,7 +365,7 @@ sdks: err := yaml.Unmarshal([]byte(file), &wf) c.Assert(err, check.IsNil) - image := workshop.BaseImage{Name: "ubuntu@22.04", Fingerprint: "fakeimage123"} + image := workshop.BaseImage{Name: "ubuntu@22.04", Confinement: workshop.ConfinementContainer, Fingerprint: "fakeimage123"} uv := sdk.Setup{ Name: "uv", @@ -523,7 +523,7 @@ sdks: err := yaml.Unmarshal([]byte(file), &wf) c.Assert(err, check.IsNil) - image := workshop.BaseImage{Name: "ubuntu@22.04", Fingerprint: "fakeimage123"} + image := workshop.BaseImage{Name: "ubuntu@22.04", Confinement: workshop.ConfinementContainer, Fingerprint: "fakeimage123"} uv := sdk.Setup{ Name: "uv", diff --git a/internal/workshop/backend.go b/internal/workshop/backend.go index f085782a0..b2beb782c 100644 --- a/internal/workshop/backend.go +++ b/internal/workshop/backend.go @@ -100,13 +100,15 @@ type SdkVolume struct { type BaseImage struct { // Base name (e.g. ubuntu@24.04). Name string `json:"name"` + // Type of sandbox used to run the image (e.g. container). + Confinement Confinement `json:"confinement"` // Base image identifier, typically a hash. Fingerprint string `json:"fingerprint"` } type BaseImageManager interface { // Lookup the latest image for the given base. - GetBase(ctx context.Context, base string) (BaseImage, error) + GetBase(ctx context.Context, base string, confinement Confinement) (BaseImage, error) // Download the given base image. DownloadBase(ctx context.Context, image BaseImage, report *progress.Reporter) error } @@ -137,8 +139,8 @@ func (s Snapshot) IsBasedOn(other Snapshot) bool { } // BaseOnly identifies a "snapshot" which consists of a base image only. -func BaseOnly(format sdk.Revision, name, fingerprint string) Snapshot { - return Snapshot{Format: format, Image: BaseImage{Name: name, Fingerprint: fingerprint}} +func BaseOnly(format sdk.Revision, name string, confinement Confinement, fingerprint string) Snapshot { + return Snapshot{Format: format, Image: BaseImage{Name: name, Confinement: confinement, Fingerprint: fingerprint}} } // SdkSnapshot identifies a snapshot consisting of a base image and a sequence diff --git a/internal/workshop/fakebackend/backend.go b/internal/workshop/fakebackend/backend.go index e17d76449..b93bc2d34 100644 --- a/internal/workshop/fakebackend/backend.go +++ b/internal/workshop/fakebackend/backend.go @@ -121,7 +121,7 @@ type FakeWorkshopBackend struct { WorkshopFsCalls []*FsCall baseLock sync.Mutex - GetBaseCallback func(ctx context.Context, base string) (workshop.BaseImage, error) + GetBaseCallback func(ctx context.Context, base string, confinement workshop.Confinement) (workshop.BaseImage, error) DownloadBaseCallback func(ctx context.Context, image workshop.BaseImage, report *progress.Reporter) error DownloadBaseCalls []*DownloadCall @@ -624,14 +624,14 @@ func (s *FakeWorkshopBackend) userProject(ctx context.Context) (string, string, return userName, projectId, nil } -func (b *FakeWorkshopBackend) GetBase(ctx context.Context, base string) (workshop.BaseImage, error) { +func (b *FakeWorkshopBackend) GetBase(ctx context.Context, base string, confinement workshop.Confinement) (workshop.BaseImage, error) { b.baseLock.Lock() defer b.baseLock.Unlock() if b.GetBaseCallback != nil { - return b.GetBaseCallback(ctx, base) + return b.GetBaseCallback(ctx, base, confinement) } - return workshop.BaseImage{Name: base, Fingerprint: "fakeimage123"}, nil + return workshop.BaseImage{Name: base, Confinement: confinement, Fingerprint: "fakeimage123"}, nil } func (b *FakeWorkshopBackend) DownloadBase(ctx context.Context, image workshop.BaseImage, report *progress.Reporter) error { diff --git a/internal/workshop/lxd/lxd_backend.go b/internal/workshop/lxd/lxd_backend.go index aad709d7a..0e656338f 100644 --- a/internal/workshop/lxd/lxd_backend.go +++ b/internal/workshop/lxd/lxd_backend.go @@ -1137,6 +1137,7 @@ func (b *Backend) loadWorkshop(conn lxd.InstanceServer, inst *api.Instance, p wo image := workshop.BaseImage{ Name: f.Base, + Confinement: f.Confinement, Fingerprint: inst.Config[workshop.ConfigWorkshopBaseFingerprint], } diff --git a/internal/workshop/lxd/lxd_backend_snapshots.go b/internal/workshop/lxd/lxd_backend_snapshots.go index 913ce6359..9eef0fa6a 100644 --- a/internal/workshop/lxd/lxd_backend_snapshots.go +++ b/internal/workshop/lxd/lxd_backend_snapshots.go @@ -259,6 +259,7 @@ func identifySnapshot(inst *api.Instance) (*workshop.Snapshot, error) { Format: format, Image: workshop.BaseImage{ Name: inst.Config[workshop.ConfigWorkshopBase], + Confinement: workshop.ConfinementContainer, Fingerprint: inst.Config[workshop.ConfigWorkshopBaseFingerprint], }, Sdks: sdks[:length], @@ -273,6 +274,14 @@ func compareSnapshots(name string, actual, expected workshop.Snapshot) error { if actual.Image.Name != expected.Image.Name { return fmt.Errorf("%q snapshot has %q base; required: %q", name, actual.Image.Name, expected.Image.Name) } + if actual.Image.Confinement != expected.Image.Confinement { + c1, err1 := actual.Image.Confinement.MarshalText() + c2, err2 := expected.Image.Confinement.MarshalText() + if err := cmp.Or(err1, err2); err != nil { + return fmt.Errorf("%q snapshot: %w", name, err) + } + return fmt.Errorf("%q snapshot has %q confinement; required: %q", name, c1, c2) + } if actual.Image.Fingerprint != expected.Image.Fingerprint { return fmt.Errorf("%q snapshot has %q base fingerprint; required: %q", name, actual.Image.Fingerprint, expected.Image.Fingerprint) } diff --git a/internal/workshop/lxd/lxd_base_manager.go b/internal/workshop/lxd/lxd_base_manager.go index e39f3cf27..896f9c72e 100644 --- a/internal/workshop/lxd/lxd_base_manager.go +++ b/internal/workshop/lxd/lxd_base_manager.go @@ -46,7 +46,7 @@ var ( currentDownloads = map[string]*downloadOp{} ) -func (b *Backend) GetBase(ctx context.Context, base string) (workshop.BaseImage, error) { +func (b *Backend) GetBase(ctx context.Context, base string, confinement workshop.Confinement) (workshop.BaseImage, error) { source, err := baseImageSource(base) if err != nil { return workshop.BaseImage{}, err @@ -63,7 +63,7 @@ func (b *Backend) GetBase(ctx context.Context, base string) (workshop.BaseImage, return workshop.BaseImage{}, fmt.Errorf("base %q not found: %w", base, err) } - return workshop.BaseImage{Name: base, Fingerprint: alias.Target}, nil + return workshop.BaseImage{Name: base, Confinement: confinement, Fingerprint: alias.Target}, nil } func (b *Backend) DownloadBase(ctx context.Context, image workshop.BaseImage, report *progress.Reporter) error { diff --git a/internal/workshop/lxd/tests/helper/helper.go b/internal/workshop/lxd/tests/helper/helper.go index e4e1fbb36..8ebecd24d 100644 --- a/internal/workshop/lxd/tests/helper/helper.go +++ b/internal/workshop/lxd/tests/helper/helper.go @@ -132,7 +132,7 @@ func CreateTestContext(username, projectId string) context.Context { } func LaunchTestWorkshop(c *check.C, ctx context.Context, bd workshop.Backend, dir string) { - image, err := bd.GetBase(ctx, "ubuntu@24.04") + image, err := bd.GetBase(ctx, "ubuntu@24.04", workshop.ConfinementContainer) c.Assert(err, check.IsNil) err = bd.DownloadBase(ctx, image, nil) c.Assert(err, check.IsNil) @@ -161,7 +161,7 @@ printf '%s\n' "$@" _, _, err = bd.CreateOrLoadProject(ctx, dir) c.Assert(err, check.IsNil) - snapshot := workshop.BaseOnly(bd.FormatRevision(), image.Name, image.Fingerprint) + snapshot := workshop.BaseOnly(bd.FormatRevision(), image.Name, workshop.ConfinementContainer, image.Fingerprint) err = bd.LaunchOrRebuildWorkshop(ctx, wf, snapshot) c.Assert(err, check.IsNil) diff --git a/internal/workshop/lxd/tests/integration/snapshot_test.go b/internal/workshop/lxd/tests/integration/snapshot_test.go index 6268ffae4..92c579e07 100644 --- a/internal/workshop/lxd/tests/integration/snapshot_test.go +++ b/internal/workshop/lxd/tests/integration/snapshot_test.go @@ -134,7 +134,7 @@ func (s *snapshotSuite) TestLxdBackendSnapshotFormat(c *check.C) { c.Assert(err, check.IsNil) // Launch workshop. - image, err := s.bd.GetBase(s.ctx, "ubuntu@24.04") + image, err := s.bd.GetBase(s.ctx, "ubuntu@24.04", workshop.ConfinementContainer) c.Assert(err, check.IsNil) err = s.bd.DownloadBase(s.ctx, image, nil) c.Assert(err, check.IsNil) @@ -146,7 +146,7 @@ func (s *snapshotSuite) TestLxdBackendSnapshotFormat(c *check.C) { {Name: "local-sdk", Source: sdk.ProjectSource}, }, } - snapshot := workshop.BaseOnly(s.bd.FormatRevision(), image.Name, image.Fingerprint) + snapshot := workshop.BaseOnly(s.bd.FormatRevision(), image.Name, workshop.ConfinementContainer, image.Fingerprint) remove := s.launchWorkshop(c, wf, snapshot) defer remove() @@ -328,7 +328,7 @@ func (s *snapshotSuite) TestLxdBackendSnapshotDiff(c *check.C) { func (s *snapshotSuite) snapshotDiff(c *check.C, base string) { // Download base image. - image, err := s.bd.GetBase(s.ctx, base) + image, err := s.bd.GetBase(s.ctx, base, workshop.ConfinementContainer) c.Assert(err, check.IsNil) err = s.bd.DownloadBase(s.ctx, image, nil) c.Assert(err, check.IsNil) @@ -338,7 +338,7 @@ func (s *snapshotSuite) snapshotDiff(c *check.C, base string) { Name: "test1", Base: base, } - baseOnly := workshop.BaseOnly(s.bd.FormatRevision(), image.Name, image.Fingerprint) + baseOnly := workshop.BaseOnly(s.bd.FormatRevision(), image.Name, workshop.ConfinementContainer, image.Fingerprint) remove := s.launchWorkshop(c, wf1, baseOnly) defer remove() diff --git a/internal/workshop/lxd/tests/integration/workshop_test.go b/internal/workshop/lxd/tests/integration/workshop_test.go index 0a1787f3c..3cc2ee51d 100644 --- a/internal/workshop/lxd/tests/integration/workshop_test.go +++ b/internal/workshop/lxd/tests/integration/workshop_test.go @@ -155,11 +155,11 @@ func (f *wsOps) TestLxdBackendWorkshopStashUnstash(c *check.C) { Name: "test", Base: "ubuntu@22.04", } - image, err := f.bd.GetBase(f.ctx, wf.Base) + image, err := f.bd.GetBase(f.ctx, wf.Base, workshop.ConfinementContainer) c.Assert(err, check.IsNil) err = f.bd.DownloadBase(f.ctx, image, nil) c.Assert(err, check.IsNil) - snapshot := workshop.BaseOnly(f.bd.FormatRevision(), image.Name, image.Fingerprint) + snapshot := workshop.BaseOnly(f.bd.FormatRevision(), image.Name, workshop.ConfinementContainer, image.Fingerprint) err = f.bd.LaunchOrRebuildWorkshop(f.ctx, wf, snapshot) c.Assert(err, check.IsNil) @@ -532,9 +532,10 @@ func (f *wsOps) TestLxdBackendDownloadBase(c *check.C) { // ensure there is no image in LXD storage f.deleteImages(c, "ubuntu@22.04") - image, err := f.bd.GetBase(f.ctx, "ubuntu@22.04") + image, err := f.bd.GetBase(f.ctx, "ubuntu@22.04", workshop.ConfinementContainer) c.Assert(err, check.IsNil) c.Check(image.Name, check.Equals, "ubuntu@22.04") + c.Check(image.Confinement, check.Equals, workshop.ConfinementContainer) c.Assert(image.Fingerprint, check.Not(check.Equals), "") var wg sync.WaitGroup @@ -562,30 +563,30 @@ func (f *wsOps) TestLxdBackendDownloadBase(c *check.C) { } func (f *wsOps) TestLxdBackendGetOrDownloadMalformedBase(c *check.C) { - image := workshop.BaseImage{Name: "ubuntu:24.04", Fingerprint: ""} - _, err := f.bd.GetBase(f.ctx, image.Name) + image := workshop.BaseImage{Name: "ubuntu:24.04", Confinement: workshop.ConfinementContainer, Fingerprint: ""} + _, err := f.bd.GetBase(f.ctx, image.Name, workshop.ConfinementContainer) c.Check(err, check.ErrorMatches, `invalid base "ubuntu:24.04" \(expected @\)`) err = f.bd.DownloadBase(f.ctx, image, nil) c.Check(err, check.ErrorMatches, `invalid base "ubuntu:24.04" \(expected @\)`) image.Name = "ubuntu@" - _, err = f.bd.GetBase(f.ctx, image.Name) + _, err = f.bd.GetBase(f.ctx, image.Name, workshop.ConfinementContainer) c.Check(err, check.ErrorMatches, `invalid base "ubuntu@" \(expected @\)`) err = f.bd.DownloadBase(f.ctx, image, nil) c.Check(err, check.ErrorMatches, `invalid base "ubuntu@" \(expected @\)`) image.Name = "canonical@ubuntu@24.04" - _, err = f.bd.GetBase(f.ctx, image.Name) + _, err = f.bd.GetBase(f.ctx, image.Name, workshop.ConfinementContainer) c.Check(err, check.ErrorMatches, `invalid base "canonical@ubuntu@24.04" \(expected @\)`) err = f.bd.DownloadBase(f.ctx, image, nil) c.Check(err, check.ErrorMatches, `invalid base "canonical@ubuntu@24.04" \(expected @\)`) } func (f *wsOps) TestLxdBackendDownloadBaseImageNotFound(c *check.C) { - _, err := f.bd.GetBase(f.ctx, "ubuntu@1.01") + _, err := f.bd.GetBase(f.ctx, "ubuntu@1.01", workshop.ConfinementContainer) c.Check(err, check.ErrorMatches, `base "ubuntu@1.01" not found.*`) - image := workshop.BaseImage{Name: "ubuntu@22.04", Fingerprint: "##################"} + image := workshop.BaseImage{Name: "ubuntu@22.04", Confinement: workshop.ConfinementContainer, Fingerprint: "##################"} err = f.bd.DownloadBase(f.ctx, image, nil) c.Check(err, check.ErrorMatches, `"ubuntu@22.04" download failed.*`) } @@ -593,15 +594,15 @@ func (f *wsOps) TestLxdBackendDownloadBaseImageNotFound(c *check.C) { func (f *wsOps) TestLxdBackendDownloadProtocolNotSupported(c *check.C) { defer lxdbackend.FakeImageServer("https://cloud-images.ubuntu.com/minimal/releases")() - image := workshop.BaseImage{Name: "ubuntu@20.04", Fingerprint: ""} - _, err := f.bd.GetBase(f.ctx, image.Name) + image := workshop.BaseImage{Name: "ubuntu@20.04", Confinement: workshop.ConfinementContainer, Fingerprint: ""} + _, err := f.bd.GetBase(f.ctx, image.Name, workshop.ConfinementContainer) c.Check(err, check.ErrorMatches, `unknown image server URL prefix \(supported: simplestreams, lxd\)`) err = f.bd.DownloadBase(f.ctx, image, nil) c.Check(err, check.ErrorMatches, `unknown image server URL prefix \(supported: simplestreams, lxd\)`) } func (f *wsOps) TestLxdBackendDownloadConcurrentErrors(c *check.C) { - image := workshop.BaseImage{Name: "ubuntu@22.04", Fingerprint: "##################"} + image := workshop.BaseImage{Name: "ubuntu@22.04", Confinement: workshop.ConfinementContainer, Fingerprint: "##################"} var wg sync.WaitGroup for range 5 { @@ -617,9 +618,10 @@ func (f *wsOps) TestLxdBackendDownloadBaseResumeAfterCancellation(c *check.C) { // ensure there is no image in LXD storage f.deleteImages(c, "ubuntu@22.04") - image, err := f.bd.GetBase(f.ctx, "ubuntu@22.04") + image, err := f.bd.GetBase(f.ctx, "ubuntu@22.04", workshop.ConfinementContainer) c.Assert(err, check.IsNil) c.Check(image.Name, check.Equals, "ubuntu@22.04") + c.Check(image.Confinement, check.Equals, workshop.ConfinementContainer) c.Assert(image.Fingerprint, check.Not(check.Equals), "") wcancel, cancel := context.WithCancel(f.ctx) @@ -664,9 +666,10 @@ func (f *wsOps) TestLxdBackendDownloadMultipleBasesConcurrently(c *check.C) { var wg sync.WaitGroup for i, b := range workshop.SupportedBases { wg.Go(func() { - image, err := f.bd.GetBase(f.ctx, b) + image, err := f.bd.GetBase(f.ctx, b, workshop.ConfinementContainer) c.Assert(err, check.IsNil) c.Check(image.Name, check.Equals, b) + c.Check(image.Confinement, check.Equals, workshop.ConfinementContainer) c.Assert(image.Fingerprint, check.Not(check.Equals), "") fingerprints[i] = image.Fingerprint @@ -704,9 +707,10 @@ func (f *wsOps) TestLxdBackendReuseDownloadedBase(c *check.C) { images := f.listAllImages(c, "ubuntu@22.04") c.Assert(images, check.HasLen, 0) - image, err := f.bd.GetBase(f.ctx, "ubuntu@22.04") + image, err := f.bd.GetBase(f.ctx, "ubuntu@22.04", workshop.ConfinementContainer) c.Assert(err, check.IsNil) c.Check(image.Name, check.Equals, "ubuntu@22.04") + c.Check(image.Confinement, check.Equals, workshop.ConfinementContainer) c.Assert(image.Fingerprint, check.Not(check.Equals), "") err = f.bd.DownloadBase(f.ctx, image, nil) c.Assert(err, check.IsNil) @@ -767,7 +771,7 @@ func (f *wsOps) TestLxdBackendReuseCachedBase(c *check.C) { c.Check(ok, check.Equals, false) c.Check(imageCached.UpdateSource, check.NotNil) - image := workshop.BaseImage{Name: "ubuntu@22.04", Fingerprint: imageCached.Fingerprint} + image := workshop.BaseImage{Name: "ubuntu@22.04", Confinement: workshop.ConfinementContainer, Fingerprint: imageCached.Fingerprint} err := f.bd.DownloadBase(f.ctx, image, nil) c.Assert(err, check.IsNil) @@ -896,13 +900,13 @@ func (f *wsOps) lsMnt(c *check.C) []os.FileInfo { } func (f *wsOps) TestLxdBackendWorkshopLaunch(c *check.C) { - image, err := f.bd.GetBase(f.ctx, "ubuntu@24.04") + image, err := f.bd.GetBase(f.ctx, "ubuntu@24.04", workshop.ConfinementContainer) c.Assert(err, check.IsNil) err = f.bd.DownloadBase(f.ctx, image, nil) c.Assert(err, check.IsNil) wf := &workshop.File{Name: "test", Base: "ubuntu@24.04"} - snapshot := workshop.BaseOnly(f.bd.FormatRevision(), image.Name, image.Fingerprint) + snapshot := workshop.BaseOnly(f.bd.FormatRevision(), image.Name, workshop.ConfinementContainer, image.Fingerprint) err = f.bd.LaunchOrRebuildWorkshop(f.ctx, wf, snapshot) c.Assert(err, check.IsNil) defer helper.RemoveTestWorkshop(c, f.ctx, f.bd) @@ -949,11 +953,11 @@ func (f *wsOps) TestLxdBackendWorkshopRebuild(c *check.C) { Name: "test", Base: "ubuntu@22.04", } - image, err := f.bd.GetBase(f.ctx, "ubuntu@22.04") + image, err := f.bd.GetBase(f.ctx, "ubuntu@22.04", workshop.ConfinementContainer) c.Assert(err, check.IsNil) err = f.bd.DownloadBase(f.ctx, image, nil) c.Assert(err, check.IsNil) - snapshot := workshop.BaseOnly(f.bd.FormatRevision(), image.Name, image.Fingerprint) + snapshot := workshop.BaseOnly(f.bd.FormatRevision(), image.Name, workshop.ConfinementContainer, image.Fingerprint) err = f.bd.LaunchOrRebuildWorkshop(f.ctx, wf, snapshot) c.Assert(err, check.IsNil) @@ -1150,6 +1154,7 @@ func (f *wsOps) TestLxdBackendSnapshotOK(c *check.C) { Format: f.bd.FormatRevision(), Image: workshop.BaseImage{ Name: "ubuntu@24.04", + Confinement: workshop.ConfinementContainer, Fingerprint: "0b9429c9855cb158b90159bb818e6f98eab9b5b1260ace11b30ddb936e4f78979abc7cdc5e4e9fad51e3e290a2190ac2", }, Sdks: []sdk.ContentID{{ @@ -1204,6 +1209,7 @@ func (f *wsOps) TestLxdBackendSnapshotConflict(c *check.C) { Format: f.bd.FormatRevision(), Image: workshop.BaseImage{ Name: "ubuntu@24.04", + Confinement: workshop.ConfinementContainer, Fingerprint: "0b9429c9855cb158b90159bb818e6f98eab9b5b1260ace11b30ddb936e4f78979abc7cdc5e4e9fad51e3e290a2190ac2", }, Sdks: []sdk.ContentID{{ @@ -1286,6 +1292,7 @@ func (f *wsOps) TestLxdBackendSnapshotInterrupted(c *check.C) { Format: f.bd.FormatRevision(), Image: workshop.BaseImage{ Name: "ubuntu@24.04", + Confinement: workshop.ConfinementContainer, Fingerprint: "0b9429c9855cb158b90159bb818e6f98eab9b5b1260ace11b30ddb936e4f78979abc7cdc5e4e9fad51e3e290a2190ac2", }, Sdks: []sdk.ContentID{{ @@ -1334,6 +1341,7 @@ func (f *wsOps) TestLxdBackendSnapshotHashCollision(c *check.C) { Format: f.bd.FormatRevision(), Image: workshop.BaseImage{ Name: "ubuntu@24.04", + Confinement: workshop.ConfinementContainer, Fingerprint: "0b9429c9855cb158b90159bb818e6f98eab9b5b1260ace11b30ddb936e4f78979abc7cdc5e4e9fad51e3e290a2190ac2", }, Sdks: []sdk.ContentID{{ diff --git a/internal/workshop/workshop_file.go b/internal/workshop/workshop_file.go index 465a33c6e..4fdc7706d 100644 --- a/internal/workshop/workshop_file.go +++ b/internal/workshop/workshop_file.go @@ -209,9 +209,35 @@ type Connection struct { type Action string +type Confinement int + +const ( + ConfinementContainer Confinement = iota +) + +func (c Confinement) MarshalText() ([]byte, error) { + switch c { + case ConfinementContainer: + return []byte("container"), nil + default: + return nil, fmt.Errorf("invalid confinement: %v", int(c)) + } +} + +func (c *Confinement) UnmarshalText(text []byte) error { + switch string(text) { + case "container": + *c = ConfinementContainer + default: + return fmt.Errorf("invalid confinement: %q", string(text)) + } + return nil +} + type File struct { Name string `yaml:"name"` Base string `yaml:"base"` + Confinement Confinement `yaml:"confinement,omitempty"` Sdks []SdkRecord `yaml:"sdks,omitempty"` Connections []Connection `yaml:"connections,omitempty"` Actions map[string]Action `yaml:"actions,omitempty"` From a7b0a14debdb78b07f8d93f65231f3c30fef48b7 Mon Sep 17 00:00:00 2001 From: Jonathan Conder Date: Tue, 28 Jul 2026 18:20:10 +1200 Subject: [PATCH 07/15] Add virtual-machine confinement Add support for LXD VMs in the backend, but keep them disabled in workshop definition files for now. VMs don't currently support SDKs, due to [1], so we even disable the system SDK. [1] https://github.com/canonical/lxd/issues/18686 --- internal/overlord/workshopstate/manifest.go | 17 +- internal/workshop/lxd/lxd_backend.go | 65 ++- .../workshop/lxd/lxd_backend_snapshots.go | 20 +- internal/workshop/lxd/lxd_backend_test.go | 70 ++- internal/workshop/lxd/lxd_base_manager.go | 8 +- internal/workshop/lxd/tests/helper/helper.go | 2 +- .../lxd/tests/integration/snapshot_test.go | 504 ++++++++++++------ internal/workshop/workshop_file.go | 13 + 8 files changed, 504 insertions(+), 195 deletions(-) diff --git a/internal/overlord/workshopstate/manifest.go b/internal/overlord/workshopstate/manifest.go index 0cadafa08..366f5d7df 100644 --- a/internal/overlord/workshopstate/manifest.go +++ b/internal/overlord/workshopstate/manifest.go @@ -266,7 +266,9 @@ func (a *artifactFinder) launchOrRefreshManifests(ctx context.Context, names []s if err != nil { return nil, nil, fmt.Errorf("cannot %s %q: %w", action, name, err) } - sdks = slices.Insert(sdks, 0, systemMeta.Setup) + if file.Confinement == workshop.ConfinementContainer { + sdks = slices.Insert(sdks, 0, systemMeta.Setup) + } storeSdks = append(storeSdks, sdks) } @@ -285,6 +287,16 @@ func (a *artifactFinder) launchOrRefreshManifests(ctx context.Context, names []s if err != nil { return nil, nil, fmt.Errorf("cannot %s %q: %w", action, name, err) } + + if cur.File.Confinement != files[i].Confinement { + c1, err1 := cur.File.Confinement.MarshalText() + c2, err2 := files[i].Confinement.MarshalText() + if err := cmp.Or(err1, err2); err != nil { + return nil, nil, fmt.Errorf("cannot %s %q: %w", action, name, err) + } + return nil, nil, fmt.Errorf("cannot %s %q: confinement changed from %q to %q", action, name, c1, c2) + } + current = append(current, *cur) } else if err := a.checkNotLaunched(ctx, a.project.ProjectId, name); err != nil { return nil, nil, fmt.Errorf("cannot %s %q: %w", action, name, err) @@ -298,6 +310,9 @@ func (a *artifactFinder) launchOrRefreshManifests(ctx context.Context, names []s format := a.backend.FormatRevision() installOrder := sdkInstallOrder(files[i]) sdks := ordered(installOrder, storeSdks[i], localSdks) + if files[i].Confinement != workshop.ConfinementContainer && len(sdks) > 0 { + return nil, nil, fmt.Errorf("cannot %s %q: SDKs are currently unavailable for virtual machines", action, name) + } latest = append(latest, Manifest{File: files[i], Format: format, Image: images[i], Sdks: sdks}) } diff --git a/internal/workshop/lxd/lxd_backend.go b/internal/workshop/lxd/lxd_backend.go index 0e656338f..d0133de4e 100644 --- a/internal/workshop/lxd/lxd_backend.go +++ b/internal/workshop/lxd/lxd_backend.go @@ -411,10 +411,10 @@ func (s *Backend) LaunchOrRebuildWorkshop(ctx context.Context, file *workshop.Fi req := api.InstancesPost{ InstancePut: api.InstancePut{ Config: config, - Devices: defaultDevices(usr, projectId, file.Name), + Devices: defaultDevices(usr, projectId, file.Name, file.Confinement), }, Name: InstanceName(file.Name, projectId), - Type: api.InstanceTypeContainer, + Type: instanceType(file.Confinement), } if !snapshot.IsBase() { @@ -429,7 +429,14 @@ func (s *Backend) LaunchOrRebuildWorkshop(ctx context.Context, file *workshop.Fi return err } - return s.adjustInstanceTemplates(conn, req.Name) + return s.adjustInstanceTemplates(conn, req.Name, file.Confinement) +} + +func instanceType(confinement workshop.Confinement) api.InstanceType { + if confinement == workshop.ConfinementVirtualMachine { + return api.InstanceTypeVM + } + return api.InstanceTypeContainer } func (s *Backend) launchOrRebuildFromImage(conn lxd.InstanceServer, usr *user.User, req api.InstancesPost) error { @@ -514,7 +521,7 @@ var instanceTemplates embed.FS // from an image (although the instance-id is different for 22.04 and up), but // when rebuilding a workshop from a snapshot, it results in both the hostname // and instance-id being taken from the snapshot. -func (s *Backend) adjustInstanceTemplates(conn lxd.InstanceServer, name string) error { +func (s *Backend) adjustInstanceTemplates(conn lxd.InstanceServer, name string, confinement workshop.Confinement) error { fromImage := []string{"create"} fromSnapshot := []string{"create", "copy"} @@ -527,10 +534,6 @@ func (s *Backend) adjustInstanceTemplates(conn lxd.InstanceServer, name string) When: fromSnapshot, Template: "hostname.tpl", }, - "/etc/machine-id": { - When: fromSnapshot, - Template: "machine-id.tpl", - }, "/etc/ssh/ssh_host_ed25519_key": { When: fromSnapshot, Template: "ssh_host_ed25519_key.tpl", @@ -552,11 +555,17 @@ func (s *Backend) adjustInstanceTemplates(conn lxd.InstanceServer, name string) Template: "eth0.network.tpl", Properties: map[string]string{"domain": networkDomain}, }, - dirs.WorkshopSocketPath + ".untrusted": { + } + if confinement == workshop.ConfinementContainer { + templates["/etc/machine-id"] = &api.ImageMetadataTemplate{ + When: fromSnapshot, + Template: "machine-id.tpl", + } + templates[dirs.WorkshopSocketPath+".untrusted"] = &api.ImageMetadataTemplate{ When: fromImage, CreateOnly: true, Template: "workshop.socket.untrusted.tpl", - }, + } } metadata, etag, err := conn.GetInstanceMetadata(name) @@ -583,12 +592,8 @@ func (s *Backend) adjustInstanceTemplates(conn lxd.InstanceServer, name string) } maps.Copy(metadata.Templates, templates) - files, err := instanceTemplates.ReadDir("templates") - if err != nil { - return err - } - for _, entry := range files { - if err := createInstanceTemplateFile(conn, name, entry.Name()); err != nil { + for _, template := range templates { + if err := createInstanceTemplateFile(conn, name, template.Template); err != nil { return err } } @@ -1328,7 +1333,7 @@ func (s *Backend) LxdClient(ctx context.Context) (lxd.InstanceServer, error) { return ConnectLxd(ctx) } -func defaultDevices(usr *user.User, pid, w string) map[string]map[string]string { +func defaultDevices(usr *user.User, pid, w string, confinement workshop.Confinement) map[string]map[string]string { devices := map[string]map[string]string{ "root": {"type": "disk", "pool": storagePool, "path": "/"}, "workshop.network": {"type": "nic", "network": networkName, "name": "eth0"}, @@ -1339,8 +1344,11 @@ func defaultDevices(usr *user.User, pid, w string) map[string]map[string]string devices[mount.Name] = mountToLxdDisk(mount) } - for _, proxy := range proxies { - devices[proxy.Name] = proxyToLxdDevice(usr, proxy) + // LXD VMs have only limited support for proxy devices. + if confinement == workshop.ConfinementContainer { + for _, proxy := range proxies { + devices[proxy.Name] = proxyToLxdDevice(usr, proxy) + } } return devices @@ -1457,6 +1465,9 @@ runcmd: # Put workshopctl on the PATH. - ln -sf {{shquote .WorkshopCtlPath}} /usr/local/bin/workshopctl - ln -sf ../../bin/workshopctl /usr/local/lib/workshop/waitready +{{- if ne .FsFreezePath ""}} + - ln -sf ../../bin/workshopctl {{shquote .FsFreezePath}} +{{- end}} - systemctl enable --now workshop-waitready.service # Linger starts the user manager for the specified user on boot, which then creates /run/user/$UID, # sets $XDG_RUNTIME_DIR and more. Interfaces such as desktop rely on both of these to be present. @@ -1468,10 +1479,16 @@ runcmd: funcs := map[string]any{ "shquote": shlex.Quote, } + var fsFreezePath string + if file.Confinement != workshop.ConfinementContainer { + fsFreezePath = dirs.FsFreezePath + } dot := struct { + FsFreezePath string WorkshopCtlPath string WorkshopStateDir string }{ + FsFreezePath: fsFreezePath, WorkshopCtlPath: filepath.Join(dirs.WorkshopGuestBinDir, filepath.Base(dirs.WorkshopCtlPath)), WorkshopStateDir: dirs.WorkshopStateDir, } @@ -1490,19 +1507,25 @@ runcmd: cfg := map[string]string{ "boot.autostart": "false", "raw.idmap": fmt.Sprintf("uid %s %s\ngid %s %s", userid, workshop.User.Uid, groupid, workshop.User.Gid), - "security.nesting": "true", "cloud-init.user-data": cloudConfig.String(), "user.workshop.format-revision": format.String(), "user.workshop.project-id": projectId, "user.workshop.name": file.Name, "user.workshop.file": string(f), "user.workshop.base-fingerprint": baseFingerprint, + } + + if file.Confinement == workshop.ConfinementContainer { + cfg["security.nesting"] = "true" // LXC appears to have a race condition wherein a proxy device mounted in // a dynamically created directory has the potential to be 'masked' by this // directory. We create an explicit mount for /tmp here (one such dynamic // directory) to allow us to mount X11 sockets reliably. // See: https://github.com/lxc/lxc/issues/434 - "raw.lxc": "lxc.mount.entry = tmpfs tmp tmpfs defaults", + cfg["raw.lxc"] = "lxc.mount.entry = tmpfs tmp tmpfs defaults" + } else { + // Ensure the NIC is named "eth0" so we can configure it. + cfg["agent.nic_config"] = "true" } return cfg, nil diff --git a/internal/workshop/lxd/lxd_backend_snapshots.go b/internal/workshop/lxd/lxd_backend_snapshots.go index 9eef0fa6a..859a1d857 100644 --- a/internal/workshop/lxd/lxd_backend_snapshots.go +++ b/internal/workshop/lxd/lxd_backend_snapshots.go @@ -180,7 +180,7 @@ func (s *Backend) Snapshot(ctx context.Context, snapshot workshop.Snapshot) (*wo workshops := map[string][]string{} usedBy, err := conn.GetInstances(lxd.GetInstancesArgs{ - InstanceType: api.InstanceTypeContainer, + InstanceType: instanceType(snapshot.Image.Confinement), Filters: []string{fmt.Sprintf("config.user.workshop.snapshot-%v=%s", len(snapshot.Sdks), name)}, }) if err != nil { @@ -194,7 +194,7 @@ func (s *Backend) Snapshot(ctx context.Context, snapshot workshop.Snapshot) (*wo // Check for stashed workshops as well. usedBy, err = snapshotConn.GetInstances(lxd.GetInstancesArgs{ - InstanceType: api.InstanceTypeContainer, + InstanceType: instanceType(snapshot.Image.Confinement), Filters: []string{fmt.Sprintf("config.user.workshop.snapshot-%v=%s", len(snapshot.Sdks), name)}, }) if err != nil { @@ -228,6 +228,11 @@ func identifySnapshot(inst *api.Instance) (*workshop.Snapshot, error) { return nil, err } + confinement := workshop.ConfinementContainer + if inst.Type == string(api.InstanceTypeVM) { + confinement = workshop.ConfinementVirtualMachine + } + sdks := make([]sdk.ContentID, len(inst.Devices)) length := 0 maxInstallOrder := 0 @@ -259,7 +264,7 @@ func identifySnapshot(inst *api.Instance) (*workshop.Snapshot, error) { Format: format, Image: workshop.BaseImage{ Name: inst.Config[workshop.ConfigWorkshopBase], - Confinement: workshop.ConfinementContainer, + Confinement: confinement, Fingerprint: inst.Config[workshop.ConfigWorkshopBaseFingerprint], }, Sdks: sdks[:length], @@ -950,7 +955,7 @@ func (s *Backend) snapshotClients(ctx context.Context) (lxd.InstanceServer, lxd. // replay some of the install-sdk and setup-base tasks. These can be handled in // the same way as in-progress launches and refreshes. func (s *Backend) FormatRevision() sdk.Revision { - return sdk.R(11) + return sdk.R(12) } func (s *Backend) HashSnapshot(snapshot workshop.Snapshot) (string, error) { @@ -959,8 +964,13 @@ func (s *Backend) HashSnapshot(snapshot workshop.Snapshot) (string, error) { return "", err } + confinement, err := snapshot.Image.Confinement.MarshalText() + if err != nil { + return "", err + } + hash := sha3.New384() - if _, err := fmt.Fprintf(hash, "%s %s\x00%s", snapshot.Format, snapshot.Image.Name, digest); err != nil { + if _, err := fmt.Fprintf(hash, "%s %s %s\x00%s", snapshot.Format, snapshot.Image.Name, confinement, digest); err != nil { return "", err } diff --git a/internal/workshop/lxd/lxd_backend_test.go b/internal/workshop/lxd/lxd_backend_test.go index fdd365948..cd24d5173 100644 --- a/internal/workshop/lxd/lxd_backend_test.go +++ b/internal/workshop/lxd/lxd_backend_test.go @@ -15,17 +15,23 @@ package lxdbackend_test import ( + "crypto/sha3" + "encoding/hex" + "path/filepath" "testing" "gopkg.in/check.v1" + "gopkg.in/yaml.v3" + "github.com/canonical/workshop/internal/dirs" "github.com/canonical/workshop/internal/testutil" "github.com/canonical/workshop/internal/workshop" lxdbackend "github.com/canonical/workshop/internal/workshop/lxd" ) type LxdBeTests struct { - project workshop.Project + project workshop.Project + workshopCtlPath string } var _ = check.Suite(&LxdBeTests{}) @@ -35,6 +41,15 @@ func TestLxdBackendSuite(t *testing.T) { check.TestingT(t) } func (s *LxdBeTests) SetUpTest(c *check.C) { dir := c.MkDir() s.project = workshop.Project{ProjectId: "42ws42ws", Path: dir} + + // These tests don't require workshopctl, but the snapshot tests do, and + // the basename of the test binary appears in cloud-init.user-data. + s.workshopCtlPath = dirs.WorkshopCtlPath + dirs.WorkshopCtlPath = filepath.Join(dir, "integration.test") +} + +func (s *LxdBeTests) TearDownTest(c *check.C) { + dirs.WorkshopCtlPath = s.workshopCtlPath } func (f *LxdBeTests) TestReadProjectsSuccess(c *check.C) { @@ -64,7 +79,7 @@ func (f *LxdBeTests) TestReadProjectsSuccess(c *check.C) { c.Assert(projects, check.HasLen, 0) } -var marshalledWorkshop = `name: test +var containerFile = `name: test base: ubuntu@22.04 sdks: - name: one @@ -78,7 +93,7 @@ sdks: channel: latest/edge ` -func (f *LxdBeTests) TestDefaultWorkshopConfig(c *check.C) { +func (f *LxdBeTests) TestDefaultContainerConfig(c *check.C) { // Setup b := &lxdbackend.Backend{} file := &workshop.File{ @@ -99,11 +114,58 @@ func (f *LxdBeTests) TestDefaultWorkshopConfig(c *check.C) { // Validate c.Assert(err, check.IsNil) c.Assert(cfg["raw.idmap"], check.Equals, "uid 1001 1000\ngid 1001 1000") + c.Assert(cfg["raw.lxc"], check.Equals, "lxc.mount.entry = tmpfs tmp tmpfs defaults") c.Assert(cfg["security.nesting"], check.Equals, "true") c.Assert(cfg["user.workshop.project-id"], check.Equals, f.project.ProjectId) - c.Assert(cfg["user.workshop.file"], check.Equals, marshalledWorkshop) + c.Assert(cfg["user.workshop.file"], check.Equals, containerFile) + c.Assert(cfg["user.workshop.format-revision"], check.Equals, b.FormatRevision().String()) + c.Assert(cfg["user.workshop.base-fingerprint"], check.Equals, "fakeimage12345") + + // Check hash here so it's easier to update snapshot-format.yaml. + digest := sha3.Sum384([]byte(cfg["cloud-init.user-data"])) + c.Check(hex.EncodeToString(digest[:]), check.Equals, "8cb63e0464ae87dca0a4b43e73fa6420b8b81e758a313b166732c886113af229acbaf83af34ce8b5fc1006772aae84f4") + // Check for syntax errors (e.g. whitespace). + var config map[string]any + err = yaml.Unmarshal([]byte(cfg["cloud-init.user-data"]), &config) + c.Assert(err, check.IsNil) +} + +var vmFile = `name: test +base: ubuntu@22.04 +confinement: virtual-machine +` + +func (f *LxdBeTests) TestDefaultVMConfig(c *check.C) { + // Setup + b := &lxdbackend.Backend{} + file := &workshop.File{ + Name: "test", + Base: "ubuntu@22.04", + Confinement: workshop.ConfinementVirtualMachine, + } + + // Execute + cfg, err := lxdbackend.DefaultConfig(b, f.project.ProjectId, "1002", "1002", file, b.FormatRevision(), "fakeimage12345") + + // Validate + c.Assert(err, check.IsNil) + c.Assert(cfg["raw.idmap"], check.Equals, "uid 1002 1000\ngid 1002 1000") + _, ok := cfg["raw.lxc"] + c.Assert(ok, check.Equals, false) + _, ok = cfg["security.nesting"] + c.Assert(ok, check.Equals, false) + c.Assert(cfg["user.workshop.project-id"], check.Equals, f.project.ProjectId) + c.Assert(cfg["user.workshop.file"], check.Equals, vmFile) c.Assert(cfg["user.workshop.format-revision"], check.Equals, b.FormatRevision().String()) c.Assert(cfg["user.workshop.base-fingerprint"], check.Equals, "fakeimage12345") + + // Check hash here so it's easier to update snapshot-format.yaml. + digest := sha3.Sum384([]byte(cfg["cloud-init.user-data"])) + c.Check(hex.EncodeToString(digest[:]), check.Equals, "7e2a89d65435671a015502795a945615ebf4ddb261d24274937a916e0ef27923723867ca07e5a6b6359dbc7cdba3faaf") + // Check for syntax errors (e.g. whitespace). + var config map[string]any + err = yaml.Unmarshal([]byte(cfg["cloud-init.user-data"]), &config) + c.Assert(err, check.IsNil) } func (f *LxdBeTests) TestCheckLxdVersion(c *check.C) { diff --git a/internal/workshop/lxd/lxd_base_manager.go b/internal/workshop/lxd/lxd_base_manager.go index 896f9c72e..dfb5ba4bc 100644 --- a/internal/workshop/lxd/lxd_base_manager.go +++ b/internal/workshop/lxd/lxd_base_manager.go @@ -47,7 +47,7 @@ var ( ) func (b *Backend) GetBase(ctx context.Context, base string, confinement workshop.Confinement) (workshop.BaseImage, error) { - source, err := baseImageSource(base) + source, err := baseImageSource(base, confinement) if err != nil { return workshop.BaseImage{}, err } @@ -109,7 +109,7 @@ func (b *Backend) tryDownloadBase(ctx context.Context, op *downloadOp, image wor } func (b *Backend) downloadBase(ctx context.Context, op *downloadOp, image workshop.BaseImage) error { - source, err := baseImageSource(image.Name) + source, err := baseImageSource(image.Name, image.Confinement) if err != nil { return err } @@ -193,7 +193,7 @@ func (b *Backend) downloadBase(ctx context.Context, op *downloadOp, image worksh return nil } -func baseImageSource(base string) (*api.ImageSource, error) { +func baseImageSource(base string, confinement workshop.Confinement) (*api.ImageSource, error) { parts := strings.FieldsFunc(base, func(r rune) bool { return r == '@' }) if len(parts) != 2 { return nil, fmt.Errorf("invalid base %q (expected @)", base) @@ -218,7 +218,7 @@ func baseImageSource(base string) (*api.ImageSource, error) { // variants, but it should be OK for x86_64, aarch64 and riscv64. source := api.ImageSource{ Alias: parts[1], - ImageType: string(api.InstanceTypeContainer), + ImageType: string(instanceType(confinement)), Protocol: protocol, Server: url, } diff --git a/internal/workshop/lxd/tests/helper/helper.go b/internal/workshop/lxd/tests/helper/helper.go index 8ebecd24d..313fd5c15 100644 --- a/internal/workshop/lxd/tests/helper/helper.go +++ b/internal/workshop/lxd/tests/helper/helper.go @@ -189,7 +189,7 @@ func ExecOutput(ctx context.Context, bd workshop.Backend, name string, args work return "", err } if err := exectx.WaitExecution(ctx); err != nil { - return "", fmt.Errorf("%w\n%s", err, stderr.String()) + return stdout.String(), fmt.Errorf("%w\n%s", err, stderr.String()) } return stdout.String(), err } diff --git a/internal/workshop/lxd/tests/integration/snapshot_test.go b/internal/workshop/lxd/tests/integration/snapshot_test.go index 92c579e07..c1b050c77 100644 --- a/internal/workshop/lxd/tests/integration/snapshot_test.go +++ b/internal/workshop/lxd/tests/integration/snapshot_test.go @@ -26,19 +26,19 @@ import ( "errors" "fmt" "os" - "os/exec" "os/user" "path/filepath" "strings" - "syscall" "time" "github.com/canonical/lxd/shared/api" + "golang.org/x/sys/unix" "gopkg.in/check.v1" "gopkg.in/yaml.v3" "github.com/canonical/workshop/internal/dirs" "github.com/canonical/workshop/internal/osutil" + "github.com/canonical/workshop/internal/revert" "github.com/canonical/workshop/internal/sdk" "github.com/canonical/workshop/internal/testutil" "github.com/canonical/workshop/internal/workshop" @@ -51,9 +51,8 @@ type snapshotSuite struct { project workshop.Project ctx context.Context - restoreLookupUsr func() - restoreUserEnv func() - restoreImageServer func() + restoreLookupUsr func() + restoreUserEnv func() bd *lxdbackend.Backend } @@ -74,7 +73,6 @@ func (s *snapshotSuite) SetUpSuite(c *check.C) { s.restoreUserEnv = osutil.FakeUserEnvironment(func(user *user.User) (map[string]string, error) { return nil, nil }) - s.restoreImageServer = lxdbackend.FakeImageServer(helper.MinimalImageServer) dirs.SetRootDir(c.MkDir()) dirs.SocketPath = filepath.Join(dirs.DataDir, "workshop.socket") @@ -99,7 +97,6 @@ func (s *snapshotSuite) TearDownSuite(c *check.C) { s.restoreLookupUsr() s.restoreUserEnv() - s.restoreImageServer() } // This suite deliberately doesn't override the default devices, so the test @@ -320,214 +317,389 @@ func (s *snapshotSuite) TestLxdBackendSnapshotDiff(c *check.C) { c.Skip("requires root to mount and compare workshop filesystems") } - for _, base := range workshop.SupportedBases { - c.Logf("Testing snapshot integrity for base %q", base) - s.snapshotDiff(c, base) + for _, confinement := range []workshop.Confinement{workshop.ConfinementContainer, workshop.ConfinementVirtualMachine} { + kind, err := confinement.MarshalText() + c.Assert(err, check.IsNil) + for _, base := range workshop.SupportedBases { + c.Logf("Testing snapshot integrity for %s %ss", base, kind) + s.snapshotDiff(c, base, confinement) + } } } -func (s *snapshotSuite) snapshotDiff(c *check.C, base string) { +func (s *snapshotSuite) snapshotDiff(c *check.C, base string, confinement workshop.Confinement) { // Download base image. - image, err := s.bd.GetBase(s.ctx, base, workshop.ConfinementContainer) + image, err := s.bd.GetBase(s.ctx, base, confinement) c.Assert(err, check.IsNil) err = s.bd.DownloadBase(s.ctx, image, nil) c.Assert(err, check.IsNil) - // Launch first workshop. - wf1 := &workshop.File{ - Name: "test1", - Base: base, + // Launch original workshop. + originFile := &workshop.File{ + Name: "origin", + Base: base, + Confinement: confinement, } - baseOnly := workshop.BaseOnly(s.bd.FormatRevision(), image.Name, workshop.ConfinementContainer, image.Fingerprint) - remove := s.launchWorkshop(c, wf1, baseOnly) + baseOnly := workshop.BaseOnly(s.bd.FormatRevision(), image.Name, confinement, image.Fingerprint) + remove := s.launchWorkshop(c, originFile, baseOnly) defer remove() - // Start first workshop to take snapshot. - err = s.bd.StartWorkshop(s.ctx, "test1") + // Start original workshop and take a snapshot. + err = s.bd.StartWorkshop(s.ctx, "origin") c.Assert(err, check.IsNil) - snapshot := baseOnly - snapshot.Sdks = []sdk.ContentID{{ + originSnapshot := baseOnly + originSnapshot.Sdks = []sdk.ContentID{{ Name: "system", Sha3_384: "6b499970ebf370d4dbc4e9a005c042dee003c19a9420a78944bcbf32653d257f80f7c56bad55b4c967dca68a1ea92be7", IsVolume: true, }} - err1 := s.bd.TakeSnapshot(s.ctx, "test1", snapshot) - mount1, err2 := s.rootfsMount("test1") - err3 := s.bd.StopWorkshop(s.ctx, "test1", true) + err1 := s.bd.TakeSnapshot(s.ctx, "origin", originSnapshot) + originRootFS, err2 := s.workshopRootFS("origin", confinement) + err3 := s.bd.StopWorkshop(s.ctx, "origin", true) c.Assert(cmp.Or(err1, err2, err3), check.IsNil) - // Launch second workshop. - wf2 := &workshop.File{ - Name: "test2", - Base: base, + // Launch a completely independent workshop. + siblingFile := &workshop.File{ + Name: "sibling", + Base: base, + Confinement: confinement, } - remove = s.launchWorkshop(c, wf2, baseOnly) + remove = s.launchWorkshop(c, siblingFile, baseOnly) defer remove() - // Start second workshop to run cloud-init. - err = s.bd.StartWorkshop(s.ctx, "test2") + // Start independent workshop to run cloud-init. + err = s.bd.StartWorkshop(s.ctx, "sibling") c.Assert(err, check.IsNil) - mount2, err1 := s.rootfsMount("test2") - err2 = s.bd.StopWorkshop(s.ctx, "test2", true) + siblingRootFS, err1 := s.workshopRootFS("sibling", confinement) + err2 = s.bd.StopWorkshop(s.ctx, "sibling", true) c.Assert(cmp.Or(err1, err2), check.IsNil) - // Launch third workshop from snapshot. - wf3 := &workshop.File{ - Name: "test3", - Base: base, + // Launch clone of the first workshop. + cloneFile := &workshop.File{ + Name: "clone", + Base: base, + Confinement: confinement, } - remove = s.launchWorkshop(c, wf3, snapshot) + remove = s.launchWorkshop(c, cloneFile, originSnapshot) defer remove() - // Start third workshop to run cloud-init (again). - err = s.bd.StartWorkshop(s.ctx, "test3") + // Start cloned workshop and take another snapshot. + err = s.bd.StartWorkshop(s.ctx, "clone") c.Assert(err, check.IsNil) - snapshot2 := snapshot - snapshot2.Sdks = append(snapshot2.Sdks, sdk.ContentID{ + cloneSnapshot := originSnapshot + cloneSnapshot.Sdks = append(cloneSnapshot.Sdks, sdk.ContentID{ Name: "test-sdk", Sha3_384: "d024fbe91c6b99d0064306d52006c17a5d0406822ff253fbbe6a934ca9be50d3ff9a6ec3bac3be8396006029a1ff453a", IsVolume: false, }) - err1 = s.bd.TakeSnapshot(s.ctx, "test3", snapshot2) - mount3, err2 := s.rootfsMount("test3") - err3 = s.bd.StopWorkshop(s.ctx, "test3", true) + err1 = s.bd.TakeSnapshot(s.ctx, "clone", cloneSnapshot) + cloneRootFS, err2 := s.workshopRootFS("clone", confinement) + err3 = s.bd.StopWorkshop(s.ctx, "clone", true) c.Assert(cmp.Or(err1, err2, err3), check.IsNil) - // Mount each rootfs for comparison - workdir := c.MkDir() - for i := range 3 { - err := os.Mkdir(filepath.Join(workdir, fmt.Sprint(i)), os.ModePerm) - c.Assert(err, check.IsNil) + // Launch another independent workshop. + wf := &workshop.File{ + Name: "test", + Base: base, + Confinement: confinement, } - var roots []string - var files []uniqueFiles - for i, m := range []mount{mount1, mount2, mount3} { - if m.Fstype != "zfs" { - c.Skip("workshop storage pool is not using ZFS") - } - target := filepath.Join(workdir, fmt.Sprint(i)) - err := syscall.Mount(m.Source, target, m.Fstype, 0, "") + remove = s.launchWorkshop(c, wf, baseOnly) + defer remove() + err = s.bd.StartWorkshop(s.ctx, "test") + c.Assert(err, check.IsNil) + defer func() { + err1 := s.bd.StopWorkshop(s.ctx, "test", true) + c.Check(err1, check.IsNil) + }() + + // Mount the other workshops inside the new one. + unmountOrigin := s.mountRootFS(c, originRootFS, "test", "/mnt/origin") + defer unmountOrigin.Fail() + unmountSibling := s.mountRootFS(c, siblingRootFS, "test", "/mnt/sibling") + defer unmountSibling.Fail() + unmountClone := s.mountRootFS(c, cloneRootFS, "test", "/mnt/clone") + defer unmountClone.Fail() + + // Ensure ID files are unique, while most others are identical. + originFiles := s.extractUniqueFiles(c, "test", "/mnt/origin") + siblingFiles := s.extractUniqueFiles(c, "test", "/mnt/sibling") + cloneFiles := s.extractUniqueFiles(c, "test", "/mnt/clone") + + c.Check(originFiles.hostname, check.Not(check.Equals), siblingFiles.hostname) + c.Check(originFiles.machineID, check.Not(check.Equals), siblingFiles.machineID) + c.Check(originFiles.networkCfg, check.Not(check.Equals), siblingFiles.networkCfg) + c.Check(originFiles.sshKey, check.Not(check.Equals), siblingFiles.sshKey) + + c.Check(originFiles.hostname, check.Not(check.Equals), cloneFiles.hostname) + if confinement == workshop.ConfinementContainer { + c.Check(originFiles.machineID, check.Not(check.Equals), cloneFiles.machineID) + } else { + // TODO: fix /etc/machine-id in VMs. + c.Check(originFiles.machineID, check.Equals, cloneFiles.machineID) + } + c.Check(originFiles.networkCfg, check.Not(check.Equals), cloneFiles.networkCfg) + c.Check(originFiles.sshKey, check.Not(check.Equals), cloneFiles.sshKey) + + s.execDiff(c, "test", "/mnt/origin", "/mnt/sibling") + s.execDiff(c, "test", "/mnt/origin", "/mnt/clone") + + names := []string{"origin", "clone"} + for i, snapshot := range []workshop.Snapshot{originSnapshot, cloneSnapshot} { + c.Logf("Restoring snapshot of %q workshop", names[i]) + + // Restore original workshop from snapshot. + clone := unmountOrigin.Clone() + unmountOrigin.Success() + clone.Fail() + _ = s.launchWorkshop(c, originFile, snapshot) + + // Restart it to give services a chance to run. + err = s.bd.StartWorkshop(s.ctx, "origin") c.Assert(err, check.IsNil) - defer func() { - err1 := syscall.Unmount(target, 0) - c.Check(err1, check.IsNil) - }() + originRootFS, err1 := s.workshopRootFS("origin", confinement) + err2 = s.bd.StopWorkshop(s.ctx, "origin", true) + c.Assert(cmp.Or(err1, err2), check.IsNil) + + // Remount the rootfs into the "test" workshop. + unmountOrigin = s.mountRootFS(c, originRootFS, "test", "/mnt/origin") + defer unmountOrigin.Fail() + + // Check ID files, and most others, are preserved. + restoredFiles := s.extractUniqueFiles(c, "test", "/mnt/origin") + + c.Check(restoredFiles.hostname, check.Equals, originFiles.hostname) + if confinement == workshop.ConfinementContainer || i == 0 { + c.Check(restoredFiles.machineID, check.Equals, originFiles.machineID) + } else { + // TODO: fix /etc/machine-id in VMs. + c.Check(restoredFiles.machineID, check.Equals, cloneFiles.machineID) + } + c.Check(restoredFiles.networkCfg, check.Equals, originFiles.networkCfg) + c.Check(restoredFiles.sshKey, check.Equals, originFiles.sshKey) + + s.execDiff(c, "test", "/mnt/sibling", "/mnt/origin") + } +} + +type filesystem struct { + name string + confinement workshop.Confinement + + Fstype string `json:"fstype"` + Source string `json:"source"` + Fsroot string `json:"fsroot"` +} + +// workshopRootFS performs operations while the workshop is running to make it +// easy to mount its rootfs elsewhere once it has stopped. For containers, it +// returns the source ZFS dataset, and the subdirectory of the rootfs within +// that. For VMs, it relabels the filesystem to make it easier to locate when +// mounting the parent block device in another VM. +func (s *snapshotSuite) workshopRootFS(name string, confinement workshop.Confinement) (filesystem, error) { + args := workshop.ExecArgs{ + Command: []string{"findmnt", "--json", "--mountpoint=/", "--nofsroot", "--output=fsroot,fstype,source"}, + WorkDir: "/", + Timeout: time.Second, + } + output, err := helper.ExecOutput(s.ctx, s.bd, name, args) + if err != nil { + return filesystem{}, err + } + + var filesystems struct { + Filesystems []filesystem `json:"filesystems"` + } + if err := json.Unmarshal([]byte(output), &filesystems); err != nil { + return filesystem{}, err + } + if len(filesystems.Filesystems) != 1 { + return filesystem{}, fmt.Errorf("expected 1 filesystem, found:\n%s", output) + } + + rootfs := filesystems.Filesystems[0] + rootfs.name = name + rootfs.confinement = confinement + if confinement == workshop.ConfinementContainer { + return rootfs, nil + } + + if rootfs.Fstype != "ext4" { + return filesystem{}, fmt.Errorf("unexpected rootfs type %q", rootfs.Fstype) + } + args.Command = []string{"e2label", "/dev/disk/by-label/cloudimg-rootfs", "workshop-" + name} + _, err = helper.ExecOutput(s.ctx, s.bd, name, args) + if err != nil { + return filesystem{}, err + } + + rootfs.Source = "/dev/disk/by-label/workshop-" + name + return rootfs, nil +} + +func (s *snapshotSuite) mountRootFS(c *check.C, source filesystem, name, path string) *revert.Reverter { + if source.confinement == workshop.ConfinementContainer { + return s.mountContainerRootFS(c, source, name, path) + } + return s.mountVMRootFS(c, source, name, path) +} + +// mountContainerRootFS mounts the rootfs of one container inside another +// container. LXD doesn't support this directly, so we first mount the rootfs +// on the host and then bind-mount the mountpoint into the other workshop. The +// host mount is ID-mapped so the container can modify the files. +func (s *snapshotSuite) mountContainerRootFS(c *check.C, source filesystem, name, path string) *revert.Reverter { + c.Assert(source.Fstype, check.Equals, "zfs") + + userns := s.workshopUserNS(c, name) + defer userns.Close() + + rev := revert.New() + defer rev.Fail() - root := filepath.Join(target, m.Fsroot) - roots = append(roots, root) - files = append(files, extractUniqueFiles(c, root)) + mountpoint := c.MkDir() + s.idmappedMount(c, source, mountpoint, userns) + rev.Add(func() { + err1 := unix.Unmount(mountpoint, 0) + c.Check(err1, check.IsNil) + }) + + mount := workshop.Mount{ + Name: "root_" + source.name, + Type: workshop.HostWorkshop, + What: filepath.Join(mountpoint, source.Fsroot), + Where: path, + MakeWhere: true, } + err := s.bd.AddWorkshopMount(s.ctx, name, mount) + c.Assert(err, check.IsNil) + rev.Add(func() { + err1 := s.bd.RemoveWorkshopMount(s.ctx, name, mount.Name) + c.Check(err1, check.IsNil) + }) - // Ensure certain files are unique. - c.Check(files[0].hostname, check.Not(check.Equals), files[1].hostname) - c.Check(files[0].machineID, check.Not(check.Equals), files[1].machineID) - c.Check(files[0].networkCfg, check.Not(check.Equals), files[1].networkCfg) - c.Check(files[0].sshKey, check.Not(check.Equals), files[1].sshKey) + clone := rev.Clone() + rev.Success() + return clone +} - c.Check(files[0].hostname, check.Not(check.Equals), files[2].hostname) - c.Check(files[0].machineID, check.Not(check.Equals), files[2].machineID) - c.Check(files[0].networkCfg, check.Not(check.Equals), files[2].networkCfg) - c.Check(files[0].sshKey, check.Not(check.Equals), files[2].sshKey) +func (s *snapshotSuite) workshopUserNS(c *check.C, name string) *os.File { + conn, err := s.bd.LxdClient(s.ctx) + c.Assert(err, check.IsNil) + defer conn.Disconnect() - // Check for unexpected differences. - output, err := exec.Command("diff", "--brief", "--no-dereference", "--recursive", roots[0], roots[1]).CombinedOutput() - c.Check(err, check.IsNil, check.Commentf("%s", output)) + state, _, err := conn.GetInstanceState(lxdbackend.InstanceName(name, s.project.ProjectId)) + c.Assert(err, check.IsNil) + c.Assert(state.Pid > 0, check.Equals, true) - output, err = exec.Command("diff", "--brief", "--no-dereference", "--recursive", roots[0], roots[2]).CombinedOutput() - c.Check(err, check.IsNil, check.Commentf("%s", output)) + userns, err := os.Open(filepath.Join("/proc", fmt.Sprint(state.Pid), "ns", "user")) + c.Assert(err, check.IsNil) + return userns +} - // Restore first workshop from its own snapshot. - err = syscall.Unmount(filepath.Join(workdir, "0"), 0) +func (s *snapshotSuite) idmappedMount(c *check.C, source filesystem, target string, userns *os.File) { + fsfd, err := unix.Fsopen(source.Fstype, unix.FSOPEN_CLOEXEC) c.Assert(err, check.IsNil) - _ = s.launchWorkshop(c, wf1, snapshot) + defer unix.Close(fsfd) - // Restart it to give cloud-init a chance to run. - err = s.bd.StartWorkshop(s.ctx, "test1") + err = unix.FsconfigSetString(fsfd, "source", source.Source) + c.Assert(err, check.IsNil) + err = unix.FsconfigCreate(fsfd) c.Assert(err, check.IsNil) - mount1, err1 = s.rootfsMount("test1") - err2 = s.bd.StopWorkshop(s.ctx, "test1", true) - c.Assert(cmp.Or(err1, err2), check.IsNil) - // Remount the rootfs. - if mount1.Fstype != "zfs" { - c.Skip("workshop storage pool is not using ZFS") + tree, err := unix.Fsmount(fsfd, unix.FSMOUNT_CLOEXEC, 0) + c.Assert(err, check.IsNil) + defer unix.Close(tree) + + attr := &unix.MountAttr{ + Attr_set: unix.MOUNT_ATTR_IDMAP, + Userns_fd: uint64(userns.Fd()), } - err = syscall.Mount(mount1.Source, filepath.Join(workdir, "0"), mount1.Fstype, 0, "") + err = unix.MountSetattr(tree, "", unix.AT_EMPTY_PATH, attr) c.Assert(err, check.IsNil) - restored := extractUniqueFiles(c, roots[0]) - // Check that only Workshop-managed attributes are preserved. - c.Check(files[0].hostname, check.Equals, restored.hostname) - c.Check(files[0].machineID, check.Equals, restored.machineID) - c.Check(files[0].networkCfg, check.Equals, restored.networkCfg) - c.Check(files[0].sshKey, check.Equals, restored.sshKey) + err = unix.MoveMount(tree, "", unix.AT_FDCWD, target, unix.MOVE_MOUNT_F_EMPTY_PATH) + c.Assert(err, check.IsNil) +} - // Check for unexpected differences. - output, err = exec.Command("diff", "--brief", "--no-dereference", "--recursive", roots[1], roots[0]).CombinedOutput() - c.Check(err, check.IsNil, check.Commentf("%s", output)) +// mountVMRootFS mounts the rootfs of one VM inside another VM. LXD supports +// this directly, but only creates the block device without mounting it. +// Luckily we already relabeled the rootfs, so we can use that to mount it +// after udev has created the appropriate symlinks. +func (s *snapshotSuite) mountVMRootFS(c *check.C, source filesystem, name, path string) *revert.Reverter { + conn, err := s.bd.LxdClient(s.ctx) + c.Assert(err, check.IsNil) + defer conn.Disconnect() - // Refresh first workshop from a snapshot of third workshop. - err = syscall.Unmount(filepath.Join(workdir, "0"), 0) + inst, etag1, err := conn.GetInstance(lxdbackend.InstanceName(name, s.project.ProjectId)) c.Assert(err, check.IsNil) - _ = s.launchWorkshop(c, wf1, snapshot2) - // Restart first workshop to run cloud-init. - err = s.bd.StartWorkshop(s.ctx, "test1") + vol, etag2, err := conn.GetStoragePoolVolume(inst.Devices["root"]["pool"], inst.Type, lxdbackend.InstanceName(source.name, s.project.ProjectId)) c.Assert(err, check.IsNil) - mount1, err1 = s.rootfsMount("test1") + vol.Config["security.shared"] = "true" + op, err := conn.UpdateStoragePoolVolume(vol.Pool, vol.Type, vol.Name, vol.Writable(), etag2) + c.Assert(err, check.IsNil) + c.Assert(op.WaitContext(s.ctx), check.IsNil) - err2 = s.bd.StopWorkshop(s.ctx, "test1", true) - c.Assert(cmp.Or(err1, err2), check.IsNil) + rev := revert.New() + defer rev.Fail() - // Remount the rootfs. - if mount1.Fstype != "zfs" { - c.Skip("workshop storage pool is not using ZFS") + inst.Devices["root_"+source.name] = map[string]string{ + "type": "disk", + "pool": vol.Pool, + "source": vol.Name, + "source.type": vol.Type, } - err = syscall.Mount(mount1.Source, filepath.Join(workdir, "0"), mount1.Fstype, 0, "") + op, err = conn.UpdateInstance(inst.Name, inst.Writable(), etag1) c.Assert(err, check.IsNil) - refreshed := extractUniqueFiles(c, roots[0]) - - // Check that only Workshop-managed attributes are preserved. - c.Check(files[0].hostname, check.Equals, refreshed.hostname) - c.Check(files[0].machineID, check.Equals, refreshed.machineID) - c.Check(files[0].networkCfg, check.Equals, refreshed.networkCfg) - c.Check(files[0].sshKey, check.Equals, refreshed.sshKey) + c.Assert(op.WaitContext(s.ctx), check.IsNil) - // Check for unexpected differences. - output, err = exec.Command("diff", "--brief", "--no-dereference", "--recursive", roots[2], roots[0]).CombinedOutput() - c.Check(err, check.IsNil, check.Commentf("%s", output)) -} + rev.Add(func() { + inst1, etag3, err1 := conn.GetInstance(inst.Name) + if c.Check(err1, check.IsNil) { + delete(inst1.Devices, "root_"+source.name) -type mount struct { - Fstype string `json:"fstype"` - Source string `json:"source"` - Fsroot string `json:"fsroot"` -} + op1, err1 := conn.UpdateInstance(inst1.Name, inst1.Writable(), etag3) + if c.Check(err1, check.IsNil) { + c.Check(op1.WaitContext(s.ctx), check.IsNil) + } + } + }) -// rootfsMount returns the source ZFS dataset, and subdirectory within that, of -// the given container's rootfs. -func (s *snapshotSuite) rootfsMount(name string) (mount, error) { args := workshop.ExecArgs{ - Command: []string{"findmnt", "--json", "--mountpoint=/", "--nofsroot", "--output=fsroot,fstype,source"}, + Command: []string{"udevadm", "settle"}, WorkDir: "/", Timeout: time.Second, } - output, err := helper.ExecOutput(s.ctx, s.bd, name, args) - if err != nil { - return mount{}, err - } + _, err = helper.ExecOutput(s.ctx, s.bd, name, args) + c.Assert(err, check.IsNil) - var result struct { - Filesystems []mount `json:"filesystems"` - } - if err := json.Unmarshal([]byte(output), &result); err != nil { - return mount{}, err - } - if len(result.Filesystems) != 1 { - return mount{}, fmt.Errorf("expected 1 filesystem, found:\n%s", output) - } + // Check filesystem integrity; nonzero exit codes can indicate the rootfs + // was repaired successfully, but could be a sign that the filesystem + // wasn't properly frozen or the workshop wasn't stopped cleanly. + args.Command = []string{"fsck.ext4", "-fy", source.Source} + out, err := helper.ExecOutput(s.ctx, s.bd, name, args) + c.Check(err, check.IsNil, check.Commentf("%s", out)) + + args.Command = []string{"mkdir", "-p", path} + _, err = helper.ExecOutput(s.ctx, s.bd, name, args) + c.Assert(err, check.IsNil) - return result.Filesystems[0], nil + args.Command = []string{"mount", source.Source, path} + _, err = helper.ExecOutput(s.ctx, s.bd, name, args) + c.Assert(err, check.IsNil) + rev.Add(func() { + args1 := workshop.ExecArgs{ + Command: []string{"umount", path}, + WorkDir: "/", + Timeout: time.Second, + } + _, err1 := helper.ExecOutput(s.ctx, s.bd, name, args1) + c.Check(err1, check.IsNil) + }) + + clone := rev.Clone() + rev.Success() + return clone } type uniqueFiles struct { @@ -540,14 +712,18 @@ type uniqueFiles struct { // extractUniqueFiles prepares a rootfs for diff comparison. It removes files // that are likely be different (most of which are inconsequential) and returns // the contents of the files that really ought to be different. -func extractUniqueFiles(c *check.C, path string) uniqueFiles { - hostname, err := os.ReadFile(filepath.Join(path, "etc", "hostname")) +func (s *snapshotSuite) extractUniqueFiles(c *check.C, name, path string) uniqueFiles { + fs, err := s.bd.WorkshopFs(s.ctx, name) + c.Assert(err, check.IsNil) + defer fs.Close() + + hostname, err := fs.ReadFile(filepath.Join(path, "etc", "hostname")) c.Assert(err, check.IsNil) - machineID, err := os.ReadFile(filepath.Join(path, "etc", "machine-id")) + machineID, err := fs.ReadFile(filepath.Join(path, "etc", "machine-id")) c.Assert(err, check.IsNil) - networkCfg, err := os.ReadFile(filepath.Join(path, "etc", "systemd", "network", "10-cloud-init-eth0.network.d", "workshop.conf")) + networkCfg, err := fs.ReadFile(filepath.Join(path, "etc", "systemd", "network", "10-cloud-init-eth0.network.d", "workshop.conf")) c.Assert(err, check.IsNil) - sshKey, err := os.ReadFile(filepath.Join(path, "etc", "ssh", "ssh_host_ed25519_key.pub")) + sshKey, err := fs.ReadFile(filepath.Join(path, "etc", "ssh", "ssh_host_ed25519_key.pub")) c.Assert(err, check.IsNil) files := []string{ @@ -559,34 +735,34 @@ func extractUniqueFiles(c *check.C, path string) uniqueFiles { "etc/sudoers.d/90-cloud-init-users", "etc/systemd/network/10-cloud-init-eth0.network.d/workshop.conf", "var/cache/ldconfig/aux-cache", + "var/lib/systemd/random-seed", "var/lib/workshop/run/workshop.socket.untrusted", - "var/log/cloud-init.log", - "var/log/cloud-init-output.log", - "var/log/unattended-upgrades/unattended-upgrades-shutdown.log", - "var/log/wtmp", } for _, file := range files { local, err := filepath.Localize(file) c.Assert(err, check.IsNil) - err = os.Remove(filepath.Join(path, local)) + err = fs.Remove(filepath.Join(path, local)) if !errors.Is(err, os.ErrNotExist) { c.Assert(err, check.IsNil) } } dirs := []string{ + "tmp", + "var/cache/apparmor", "var/cache/snapd", "var/lib/cloud", "var/lib/snapd", - "var/log/journal", + "var/log", + "var/snap/lxd/common", "var/tmp", } for _, dir := range dirs { local, err := filepath.Localize(dir) c.Assert(err, check.IsNil) - err = os.RemoveAll(filepath.Join(path, local)) + err = fs.RemoveAll(filepath.Join(path, local)) c.Assert(err, check.IsNil) } @@ -597,3 +773,13 @@ func extractUniqueFiles(c *check.C, path string) uniqueFiles { sshKey: string(sshKey), } } + +func (s *snapshotSuite) execDiff(c *check.C, name, a, b string) { + args := workshop.ExecArgs{ + Command: []string{"diff", "--brief", "--no-dereference", "--recursive", a, b}, + WorkDir: "/", + Timeout: time.Second, + } + out, err := helper.ExecOutput(s.ctx, s.bd, name, args) + c.Check(err, check.IsNil, check.Commentf("diff %s %s:\n%s", a, b, out)) +} diff --git a/internal/workshop/workshop_file.go b/internal/workshop/workshop_file.go index 4fdc7706d..86469a041 100644 --- a/internal/workshop/workshop_file.go +++ b/internal/workshop/workshop_file.go @@ -213,12 +213,15 @@ type Confinement int const ( ConfinementContainer Confinement = iota + ConfinementVirtualMachine ) func (c Confinement) MarshalText() ([]byte, error) { switch c { case ConfinementContainer: return []byte("container"), nil + case ConfinementVirtualMachine: + return []byte("virtual-machine"), nil default: return nil, fmt.Errorf("invalid confinement: %v", int(c)) } @@ -228,6 +231,8 @@ func (c *Confinement) UnmarshalText(text []byte) error { switch string(text) { case "container": *c = ConfinementContainer + case "virtual-machine": + *c = ConfinementVirtualMachine default: return fmt.Errorf("invalid confinement: %q", string(text)) } @@ -299,6 +304,14 @@ func ValidateFile(file *File) error { return fmt.Errorf("base %q not supported", file.Base) } + if file.Confinement != ConfinementContainer { + confinement, err := file.Confinement.MarshalText() + if err != nil { + return err + } + return fmt.Errorf("confinement %q not supported", confinement) + } + if err := validateSdks(file.Sdks); err != nil { return err } From 307ef48de5d2d0d0436eb9717e827de028a8c9a8 Mon Sep 17 00:00:00 2001 From: Jonathan Conder Date: Fri, 28 Aug 2026 16:18:50 +1200 Subject: [PATCH 08/15] Stop VMs gracefully during refresh Without this, fsck.ext4 is likely to report something like this: cloudimg-rootfs: recovering journal Pass 1: Checking inodes, blocks, and sizes Pass 2: Checking directory structure Pass 3: Checking directory connectivity Pass 4: Checking reference counts Pass 5: Checking group summary information Free blocks count wrong (1843126, counted=1843130). Fix? yes Free inodes count wrong (1094312, counted=1094318). Fix? yes cloudimg-rootfs: ***** FILE SYSTEM WAS MODIFIED ***** cloudimg-rootfs: 85330/1179648 files (0.0% non-contiguous), 515905/2359035 blocks --- internal/overlord/workshopstate/request.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/overlord/workshopstate/request.go b/internal/overlord/workshopstate/request.go index 7d68a86ea..875833ee1 100644 --- a/internal/overlord/workshopstate/request.go +++ b/internal/overlord/workshopstate/request.go @@ -400,7 +400,9 @@ func refresh(st *state.State, project workshop.Project, current, latest Manifest addTaskSet(state.NewTaskSet(discard)) stop := st.NewTask("stop-workshop", fmt.Sprintf("Stop %q workshop", latest.File.Name)) - stop.Set("force", true) + // Using force is fine for containers, but for VMs it can lead to (usually + // repairable) filesystem integrity issues, which are copied to the stash. + stop.Set("force", current.File.Confinement == workshop.ConfinementContainer) addTaskSet(state.NewTaskSet(stop)) // Unmount SDKs and remove plugs and slots from interfaces repository. From f00efaf83dbe98a93aa80c5d25e379b89e391c49 Mon Sep 17 00:00:00 2001 From: Jonathan Conder Date: Tue, 28 Jul 2026 18:20:11 +1200 Subject: [PATCH 09/15] Update /etc/machine-id after restoring a VM snapshot VM instance templates are applied by the LXD agent, which is a systemd service and therefore can't modify the machine ID. When /etc/machine-id is missing, systemd creates one based on the SMBIOS UUID. For us, this means launchOrRebuildFromImage works fine. However, launchOrRebuildFromSnapshot doesn't. Since VMs only support SFTP while running, it's not easy to just delete /etc/machine-id after taking the snapshot. Instead we can tell systemd to always use the UUID via a kernel parameter. Since VM images use GRUB as a bootloader, the kernel command line needs to be configured through GRUB. We accomplish this with a custom GRUB script and drop-in config file. --- internal/workshop/lxd/lxd_backend.go | 23 +++++++++++++++++++ .../workshop/lxd/lxd_backend_snapshots.go | 2 +- internal/workshop/lxd/lxd_backend_test.go | 4 +++- .../lxd/tests/integration/snapshot_test.go | 14 ++--------- 4 files changed, 29 insertions(+), 14 deletions(-) diff --git a/internal/workshop/lxd/lxd_backend.go b/internal/workshop/lxd/lxd_backend.go index d0133de4e..26215271b 100644 --- a/internal/workshop/lxd/lxd_backend.go +++ b/internal/workshop/lxd/lxd_backend.go @@ -1455,6 +1455,24 @@ write_files: [Install] WantedBy=multi-user.target +{{- if .HasGRUB}} + - path: /etc/grub.d/70_workshop + permissions: '0755' + content: | + #!/bin/sh + exec tail --lines=+4 "$0" + + # Extract SMBIOS UUID and store it in a GRUB variable. We use it to set + # the systemd.machine_id kernel parameter to the LXD UUID, which forces + # systemd to use it. By default it prefers reading the machine ID from + # /etc/machine-id, which may be stale when restoring from a snapshot. + insmod smbios + smbios --type 1 --get-uuid 8 --set workshop_machine_id + export workshop_machine_id + - path: /etc/default/grub.d/70-workshop.cfg + content: | + GRUB_CMDLINE_LINUX="${GRUB_CMDLINE_LINUX:+$GRUB_CMDLINE_LINUX }"'systemd.machine_id=${workshop_machine_id}' +{{- end}} runcmd: # Project directory is required for 'workshop exec'. - install --directory --mode=755 /project /usr/local/bin /usr/local/lib/workshop {{shquote .WorkshopStateDir}} @@ -1473,6 +1491,9 @@ runcmd: # sets $XDG_RUNTIME_DIR and more. Interfaces such as desktop rely on both of these to be present. # This does not introduce any additional modification beyond what a login session would normally create. - loginctl enable-linger workshop +{{- if .HasGRUB}} + - update-grub +{{- end}} `[1:] var cloudConfig strings.Builder @@ -1485,10 +1506,12 @@ runcmd: } dot := struct { FsFreezePath string + HasGRUB bool WorkshopCtlPath string WorkshopStateDir string }{ FsFreezePath: fsFreezePath, + HasGRUB: file.Confinement == workshop.ConfinementVirtualMachine, WorkshopCtlPath: filepath.Join(dirs.WorkshopGuestBinDir, filepath.Base(dirs.WorkshopCtlPath)), WorkshopStateDir: dirs.WorkshopStateDir, } diff --git a/internal/workshop/lxd/lxd_backend_snapshots.go b/internal/workshop/lxd/lxd_backend_snapshots.go index 859a1d857..a75312314 100644 --- a/internal/workshop/lxd/lxd_backend_snapshots.go +++ b/internal/workshop/lxd/lxd_backend_snapshots.go @@ -955,7 +955,7 @@ func (s *Backend) snapshotClients(ctx context.Context) (lxd.InstanceServer, lxd. // replay some of the install-sdk and setup-base tasks. These can be handled in // the same way as in-progress launches and refreshes. func (s *Backend) FormatRevision() sdk.Revision { - return sdk.R(12) + return sdk.R(13) } func (s *Backend) HashSnapshot(snapshot workshop.Snapshot) (string, error) { diff --git a/internal/workshop/lxd/lxd_backend_test.go b/internal/workshop/lxd/lxd_backend_test.go index cd24d5173..1cf175bbd 100644 --- a/internal/workshop/lxd/lxd_backend_test.go +++ b/internal/workshop/lxd/lxd_backend_test.go @@ -113,6 +113,7 @@ func (f *LxdBeTests) TestDefaultContainerConfig(c *check.C) { // Validate c.Assert(err, check.IsNil) + c.Assert(cfg["cloud-init.user-data"], check.Not(testutil.Contains), "GRUB_CMDLINE_LINUX") c.Assert(cfg["raw.idmap"], check.Equals, "uid 1001 1000\ngid 1001 1000") c.Assert(cfg["raw.lxc"], check.Equals, "lxc.mount.entry = tmpfs tmp tmpfs defaults") c.Assert(cfg["security.nesting"], check.Equals, "true") @@ -149,6 +150,7 @@ func (f *LxdBeTests) TestDefaultVMConfig(c *check.C) { // Validate c.Assert(err, check.IsNil) + c.Assert(cfg["cloud-init.user-data"], testutil.Contains, "GRUB_CMDLINE_LINUX") c.Assert(cfg["raw.idmap"], check.Equals, "uid 1002 1000\ngid 1002 1000") _, ok := cfg["raw.lxc"] c.Assert(ok, check.Equals, false) @@ -161,7 +163,7 @@ func (f *LxdBeTests) TestDefaultVMConfig(c *check.C) { // Check hash here so it's easier to update snapshot-format.yaml. digest := sha3.Sum384([]byte(cfg["cloud-init.user-data"])) - c.Check(hex.EncodeToString(digest[:]), check.Equals, "7e2a89d65435671a015502795a945615ebf4ddb261d24274937a916e0ef27923723867ca07e5a6b6359dbc7cdba3faaf") + c.Check(hex.EncodeToString(digest[:]), check.Equals, "b2197fbcdb2a08fbb712f3f2a525dac06c50e025793b652ea7f4bc5d8456775cc69bcbc73a12d91e07d18baa5cba0c12") // Check for syntax errors (e.g. whitespace). var config map[string]any err = yaml.Unmarshal([]byte(cfg["cloud-init.user-data"]), &config) diff --git a/internal/workshop/lxd/tests/integration/snapshot_test.go b/internal/workshop/lxd/tests/integration/snapshot_test.go index c1b050c77..e5da4376a 100644 --- a/internal/workshop/lxd/tests/integration/snapshot_test.go +++ b/internal/workshop/lxd/tests/integration/snapshot_test.go @@ -431,12 +431,7 @@ func (s *snapshotSuite) snapshotDiff(c *check.C, base string, confinement worksh c.Check(originFiles.sshKey, check.Not(check.Equals), siblingFiles.sshKey) c.Check(originFiles.hostname, check.Not(check.Equals), cloneFiles.hostname) - if confinement == workshop.ConfinementContainer { - c.Check(originFiles.machineID, check.Not(check.Equals), cloneFiles.machineID) - } else { - // TODO: fix /etc/machine-id in VMs. - c.Check(originFiles.machineID, check.Equals, cloneFiles.machineID) - } + c.Check(originFiles.machineID, check.Not(check.Equals), cloneFiles.machineID) c.Check(originFiles.networkCfg, check.Not(check.Equals), cloneFiles.networkCfg) c.Check(originFiles.sshKey, check.Not(check.Equals), cloneFiles.sshKey) @@ -468,12 +463,7 @@ func (s *snapshotSuite) snapshotDiff(c *check.C, base string, confinement worksh restoredFiles := s.extractUniqueFiles(c, "test", "/mnt/origin") c.Check(restoredFiles.hostname, check.Equals, originFiles.hostname) - if confinement == workshop.ConfinementContainer || i == 0 { - c.Check(restoredFiles.machineID, check.Equals, originFiles.machineID) - } else { - // TODO: fix /etc/machine-id in VMs. - c.Check(restoredFiles.machineID, check.Equals, cloneFiles.machineID) - } + c.Check(restoredFiles.machineID, check.Equals, originFiles.machineID) c.Check(restoredFiles.networkCfg, check.Equals, originFiles.networkCfg) c.Check(restoredFiles.sshKey, check.Equals, originFiles.sshKey) From f3f486c65639a6431fa1ce0ed63cdd8a03e1b464 Mon Sep 17 00:00:00 2001 From: Jonathan Conder Date: Mon, 3 Aug 2026 11:28:54 +1200 Subject: [PATCH 10/15] Use unprivileged container ID-map for VMs --- internal/idmap/idmapset_linux.go | 496 ++++++++++++++++++ internal/idmap/structs.go | 15 + internal/workshop/lxd/lxd_backend.go | 89 +++- internal/workshop/lxd/lxd_backend_test.go | 5 +- .../tests/integration/snapshot-format.yaml | 8 +- tests/main/launch/task.yaml | 15 + 6 files changed, 620 insertions(+), 8 deletions(-) create mode 100644 internal/idmap/idmapset_linux.go create mode 100644 internal/idmap/structs.go diff --git a/internal/idmap/idmapset_linux.go b/internal/idmap/idmapset_linux.go new file mode 100644 index 000000000..97cb9aeec --- /dev/null +++ b/internal/idmap/idmapset_linux.go @@ -0,0 +1,496 @@ +package idmap + +import ( + "bufio" + "errors" + "fmt" + "os" + "reflect" + "sort" + "strconv" + "strings" + + "github.com/canonical/lxd/shared" +) + +var ( + // ErrHostIdIsSubId is returned when an attempt is made to add an idmap entry + // that intersects with an existing entry's host IDs. + ErrHostIdIsSubId = errors.New("host id is in the range of subids") //nolint:revive +) + +type IdRange struct { //nolint:revive + Isuid bool + Isgid bool + Startid int64 + Endid int64 +} + +// Contains checks if the given id is within the range defined by Startid and Endid. +func (i *IdRange) Contains(id int64) bool { + return id >= i.Startid && id <= i.Endid +} + +// ToLxcString returns the idmap entry in a format suitable for lxc.idmap. +func (e *IdmapEntry) ToLxcString() []string { + digits := fmt.Sprintf("%d %d %d", e.Nsid, e.Hostid, e.Maprange) + + if e.Isuid && e.Isgid { + return []string{ + "u " + digits, + "g " + digits, + } + } + + if e.Isuid { + return []string{"u " + digits} + } + + return []string{"g " + digits} +} + +// isBetween returns true if x is in the range [low, high). +func isBetween(x, low, high int64) bool { + return x >= low && x < high +} + +// HostidsIntersect checks if the host IDs of two idmap entries intersect. +func (e *IdmapEntry) HostidsIntersect(i IdmapEntry) bool { + if (e.Isuid && i.Isuid) || (e.Isgid && i.Isgid) { + switch { + case isBetween(e.Hostid, i.Hostid, i.Hostid+i.Maprange): + return true + case isBetween(i.Hostid, e.Hostid, e.Hostid+e.Maprange): + return true + case isBetween(e.Hostid+e.Maprange, i.Hostid, i.Hostid+i.Maprange): + return true + case isBetween(i.Hostid+i.Maprange, e.Hostid, e.Hostid+e.Maprange): + return true + } + } + + return false +} + +// Intersects checks if two idmap entries intersect. +func (e *IdmapEntry) Intersects(i IdmapEntry) bool { + if (e.Isuid && i.Isuid) || (e.Isgid && i.Isgid) { + switch { + case isBetween(e.Hostid, i.Hostid, i.Hostid+i.Maprange-1): + return true + case isBetween(i.Hostid, e.Hostid, e.Hostid+e.Maprange-1): + return true + case isBetween(e.Hostid+e.Maprange-1, i.Hostid, i.Hostid+i.Maprange-1): + return true + case isBetween(i.Hostid+i.Maprange-1, e.Hostid, e.Hostid+e.Maprange-1): + return true + case isBetween(e.Nsid, i.Nsid, i.Nsid+i.Maprange-1): + return true + case isBetween(i.Nsid, e.Nsid, e.Nsid+e.Maprange-1): + return true + case isBetween(e.Nsid+e.Maprange-1, i.Nsid, i.Nsid+i.Maprange-1): + return true + case isBetween(i.Nsid+i.Maprange-1, e.Nsid, e.Nsid+e.Maprange-1): + return true + } + } + return false +} + +// Usable returns whether or not the idmap entry is usable in the current user namespace. +func (e *IdmapEntry) Usable() error { + kernelIdmap, err := CurrentIdmapSet() + if err != nil { + return err + } + + kernelRanges, err := kernelIdmap.ValidRanges() + if err != nil { + return err + } + + // Validate the uid map + if e.Isuid { + valid := false + for _, kernelRange := range kernelRanges { + if !kernelRange.Isuid { + continue + } + + if kernelRange.Contains(e.Hostid) && kernelRange.Contains(e.Hostid+e.Maprange-1) { + valid = true + break + } + } + + if !valid { + return fmt.Errorf("the %q map cannot work in the current user namespace", e.ToLxcString()) + } + } + + // Validate the gid map + if e.Isgid { + valid := false + for _, kernelRange := range kernelRanges { + if !kernelRange.Isgid { + continue + } + + if kernelRange.Contains(e.Hostid) && kernelRange.Contains(e.Hostid+e.Maprange-1) { + valid = true + break + } + } + + if !valid { + return fmt.Errorf("the %q map cannot work in the current user namespace", e.ToLxcString()) + } + } + + return nil +} + +// Len returns the length of the IdmapSet. +func (m IdmapSet) Len() int { + return len(m.Idmap) +} + +// Less compares the elements with indexes i and j. +func (m IdmapSet) Less(i, j int) bool { + if m.Idmap[i].Isuid != m.Idmap[j].Isuid { + return m.Idmap[i].Isuid + } + + if m.Idmap[i].Isgid != m.Idmap[j].Isgid { + return m.Idmap[i].Isgid + } + + return m.Idmap[i].Nsid < m.Idmap[j].Nsid +} + +// Swap swaps the elements with indexes i and j. +func (m IdmapSet) Swap(i, j int) { + m.Idmap[i], m.Idmap[j] = m.Idmap[j], m.Idmap[i] +} + +// Usable checks if all entries in the IdmapSet are usable in the current user namespace. +func (m IdmapSet) Usable() error { + for _, e := range m.Idmap { + err := e.Usable() + if err != nil { + return err + } + } + + return nil +} + +// ValidRanges returns a list of valid ID ranges from the IdmapSet. +func (m IdmapSet) ValidRanges() ([]*IdRange, error) { + ranges := []*IdRange{} + + // Sort the map + idmap := IdmapSet{} + err := shared.DeepCopy(&m, &idmap) + if err != nil { + return nil, err + } + + sort.Sort(idmap) + + for _, mapEntry := range idmap.Idmap { + var entry *IdRange + for _, idEntry := range ranges { + if mapEntry.Isuid != idEntry.Isuid || mapEntry.Isgid != idEntry.Isgid { + continue + } + + if idEntry.Endid+1 == mapEntry.Nsid { + entry = idEntry + break + } + } + + if entry != nil { + entry.Endid = entry.Endid + mapEntry.Maprange + continue + } + + ranges = append(ranges, &IdRange{ + Isuid: mapEntry.Isuid, + Isgid: mapEntry.Isgid, + Startid: mapEntry.Nsid, + Endid: mapEntry.Nsid + mapEntry.Maprange - 1, + }) + } + + return ranges, nil +} + +// AddSafe adds an entry to the idmap set, breaking apart any ranges that the +// new idmap intersects with in the process. +func (m *IdmapSet) AddSafe(i IdmapEntry) error { + // doAddSafe() can't properly handle mappings that + // both UID and GID, because in this case the "i" idmapping + // will be inserted twice which may result to a further bugs and issues. + // Simplest solution is to split a "both" mapping into two separate ones + // one for UIDs and another one for GIDs. + newUidIdmapEntry := i //nolint:revive + newUidIdmapEntry.Isgid = false + err := m.doAddSafe(newUidIdmapEntry) + if err != nil { + return err + } + + newGidIdmapEntry := i + newGidIdmapEntry.Isuid = false + err = m.doAddSafe(newGidIdmapEntry) + if err != nil { + return err + } + + return nil +} + +func (m *IdmapSet) doAddSafe(i IdmapEntry) error { + result := []IdmapEntry{} + added := false + + if !i.Isuid && !i.Isgid { + return nil + } + + for _, e := range m.Idmap { + if !e.Intersects(i) { + result = append(result, e) + continue + } + + if e.HostidsIntersect(i) { + return ErrHostIdIsSubId + } + + added = true + + lower := IdmapEntry{ + Isuid: e.Isuid, + Isgid: e.Isgid, + Hostid: e.Hostid, + Nsid: e.Nsid, + Maprange: i.Nsid - e.Nsid, + } + + upper := IdmapEntry{ + Isuid: e.Isuid, + Isgid: e.Isgid, + Hostid: e.Hostid + lower.Maprange + i.Maprange, + Nsid: i.Nsid + i.Maprange, + Maprange: e.Maprange - i.Maprange - lower.Maprange, + } + + if lower.Maprange > 0 { + result = append(result, lower) + } + + result = append(result, i) + if upper.Maprange > 0 { + result = append(result, upper) + } + } + + if !added { + result = append(result, i) + } + + m.Idmap = result + return nil +} + +// getFromProc gets a uid or gid mapping from /proc/self/{g,u}id_map. +func getFromProc(fname string) ([][]int64, error) { + entries := [][]int64{} + + f, err := os.Open(fname) + if err != nil { + return nil, err + } + + defer func() { _ = f.Close() }() + + scanner := bufio.NewScanner(f) + for scanner.Scan() { + // Validate format + s := strings.Fields(scanner.Text()) + if len(s) < 3 { + return nil, fmt.Errorf("unexpected values in %q: %q", fname, s) + } + + // Get range start + entryStart, err := strconv.ParseUint(s[0], 10, 32) + if err != nil { + continue + } + + // Get range size + entryHost, err := strconv.ParseUint(s[1], 10, 32) + if err != nil { + continue + } + + // Get range size + entrySize, err := strconv.ParseUint(s[2], 10, 32) + if err != nil { + continue + } + + entries = append(entries, []int64{int64(entryStart), int64(entryHost), int64(entrySize)}) + } + + if err := scanner.Err(); err != nil { + return nil, err + } + if len(entries) == 0 { + return nil, errors.New("namespace does not have any map set") + } + + return entries, nil +} + +func KernelDefaultMap() (*IdmapSet, error) { + idmapset := new(IdmapSet) + + kernelMap, err := CurrentIdmapSet() + if err != nil { + // Hardcoded fallback map + e := IdmapEntry{Isuid: true, Isgid: false, Nsid: 0, Hostid: 1000000, Maprange: 1000000000} + idmapset.Idmap = append(idmapset.Idmap, e) + + e = IdmapEntry{Isuid: false, Isgid: true, Nsid: 0, Hostid: 1000000, Maprange: 1000000000} + idmapset.Idmap = append(idmapset.Idmap, e) + return idmapset, nil //nolint:nilerr + } + + // Look for mapped ranges + kernelRanges, err := kernelMap.ValidRanges() + if err != nil { + return nil, err + } + + // Special case for when we have the full kernel range + fullKernelRanges := []*IdRange{ + {true, false, int64(0), int64(4294967294)}, + {false, true, int64(0), int64(4294967294)}} + + if reflect.DeepEqual(kernelRanges, fullKernelRanges) { + // Hardcoded fallback map + e := IdmapEntry{Isuid: true, Isgid: false, Nsid: 0, Hostid: 1000000, Maprange: 1000000000} + idmapset.Idmap = append(idmapset.Idmap, e) + + e = IdmapEntry{Isuid: false, Isgid: true, Nsid: 0, Hostid: 1000000, Maprange: 1000000000} + idmapset.Idmap = append(idmapset.Idmap, e) + return idmapset, nil + } + + // Find a suitable uid range + for _, entry := range kernelRanges { + // We only care about uids right now + if !entry.Isuid { + continue + } + + // We want a map that's separate from the system's own POSIX allocation + if entry.Endid < 100000 { + continue + } + + // Don't use the first 100000 ids + if entry.Startid < 100000 { + entry.Startid = 100000 + } + + // Check if we have enough ids + if entry.Endid-entry.Startid < 65536 { + continue + } + + // Add the map + e := IdmapEntry{Isuid: true, Isgid: false, Nsid: 0, Hostid: entry.Startid, Maprange: entry.Endid - entry.Startid + 1} + idmapset.Idmap = append(idmapset.Idmap, e) + + // NOTE: Remove once LXD can deal with multiple shadow maps + break + } + + // Find a suitable gid range + for _, entry := range kernelRanges { + // We only care about gids right now + if !entry.Isgid { + continue + } + + // We want a map that's separate from the system's own POSIX allocation + if entry.Endid < 100000 { + continue + } + + // Don't use the first 65536 ids + if entry.Startid < 100000 { + entry.Startid = 100000 + } + + // Check if we have enough ids + if entry.Endid-entry.Startid < 65536 { + continue + } + + // Add the map + e := IdmapEntry{Isuid: false, Isgid: true, Nsid: 0, Hostid: entry.Startid, Maprange: entry.Endid - entry.Startid + 1} + idmapset.Idmap = append(idmapset.Idmap, e) + + // NOTE: Remove once LXD can deal with multiple shadow maps + break + } + + return idmapset, nil +} + +// CurrentIdmapSet creates an idmap of the current allocation. +func CurrentIdmapSet() (*IdmapSet, error) { + idmapset := new(IdmapSet) + + // Parse the uidmap + entries, err := getFromProc("/proc/self/uid_map") + if err != nil { + if !errors.Is(err, os.ErrNotExist) { + return nil, err + } + + // Fallback map + e := IdmapEntry{Isuid: true, Nsid: 0, Hostid: 0, Maprange: 0} + idmapset.Idmap = append(idmapset.Idmap, e) + } else { + for _, entry := range entries { + e := IdmapEntry{Isuid: true, Nsid: entry[0], Hostid: entry[1], Maprange: entry[2]} + idmapset.Idmap = append(idmapset.Idmap, e) + } + } + + // Parse the gidmap + entries, err = getFromProc("/proc/self/gid_map") + if err != nil { + if !errors.Is(err, os.ErrNotExist) { + return nil, err + } + + // Fallback map + e := IdmapEntry{Isgid: true, Nsid: 0, Hostid: 0, Maprange: 0} + idmapset.Idmap = append(idmapset.Idmap, e) + } else { + for _, entry := range entries { + e := IdmapEntry{Isgid: true, Nsid: entry[0], Hostid: entry[1], Maprange: entry[2]} + idmapset.Idmap = append(idmapset.Idmap, e) + } + } + + return idmapset, nil +} diff --git a/internal/idmap/structs.go b/internal/idmap/structs.go new file mode 100644 index 000000000..e29637247 --- /dev/null +++ b/internal/idmap/structs.go @@ -0,0 +1,15 @@ +package idmap + +// IdmapEntry is a single idmap entry (line). +type IdmapEntry struct { + Isuid bool `json:"Isuid"` + Isgid bool `json:"Isgid"` + Hostid int64 `json:"Hostid"` // id as seen on the host - i.e. 100000 + Nsid int64 `json:"Nsid"` // id as seen in the ns - i.e. 0 + Maprange int64 `json:"Maprange"` +} + +// IdmapSet is a list of IdmapEntry with some functions on it. +type IdmapSet struct { + Idmap []IdmapEntry `json:"Idmap"` +} diff --git a/internal/workshop/lxd/lxd_backend.go b/internal/workshop/lxd/lxd_backend.go index 26215271b..0d243d7fe 100644 --- a/internal/workshop/lxd/lxd_backend.go +++ b/internal/workshop/lxd/lxd_backend.go @@ -42,6 +42,7 @@ import ( "github.com/canonical/workshop/internal/dirs" "github.com/canonical/workshop/internal/fsutil" + "github.com/canonical/workshop/internal/idmap" "github.com/canonical/workshop/internal/logger" "github.com/canonical/workshop/internal/osutil" "github.com/canonical/workshop/internal/revert" @@ -1522,15 +1523,20 @@ runcmd: f, err := yaml.Marshal(file) if err != nil { - return map[string]string{}, err + return nil, err + } + + idmapSet, err := workshopIdmap(file.Confinement, userid, groupid) + if err != nil { + return nil, err } // Include all options we might change, even those with default values, // so that workshops can be rebuilt. cfg := map[string]string{ "boot.autostart": "false", - "raw.idmap": fmt.Sprintf("uid %s %s\ngid %s %s", userid, workshop.User.Uid, groupid, workshop.User.Gid), "cloud-init.user-data": cloudConfig.String(), + "raw.idmap": formatIdmap(idmapSet), "user.workshop.format-revision": format.String(), "user.workshop.project-id": projectId, "user.workshop.name": file.Name, @@ -1553,3 +1559,82 @@ runcmd: return cfg, nil } + +func workshopIdmap(confinement workshop.Confinement, userid, groupid string) (*idmap.IdmapSet, error) { + hostUid, err1 := strconv.ParseInt(userid, 10, 64) + nsUid, err2 := strconv.ParseInt(workshop.User.Uid, 10, 64) + hostGid, err3 := strconv.ParseInt(groupid, 10, 64) + nsGid, err4 := strconv.ParseInt(workshop.User.Gid, 10, 64) + if err := cmp.Or(err1, err2, err3, err4); err != nil { + return nil, fmt.Errorf("invalid user or group ID: %w", err) + } + entries := []idmap.IdmapEntry{ + {Isuid: true, Hostid: hostUid, Nsid: nsUid, Maprange: 1}, + {Isgid: true, Hostid: hostGid, Nsid: nsGid, Maprange: 1}, + } + + idmapSet := &idmap.IdmapSet{} + if confinement != workshop.ConfinementContainer { + // TODO: query LXD for the default idmap somehow. The current + // implementation only works because the LXD snap runs in a mount + // namespace where /etc/ is a tmpfs, so it effectively ignores + // /etc/subuid and /etc/subgid. It would be more correct to call + // DefaultIdmapSet("/proc//root", "root"), but traversing + // /proc//root is a privileged operation. + var err error + idmapSet, err = idmap.KernelDefaultMap() + if err != nil { + return nil, err + } + if idmapSet.Len() == 0 { + return nil, errors.New("no available uid/gid map could be found") + } + if err := idmapSet.Usable(); err != nil { + return nil, err + } + } + + for _, entry := range entries { + if err := idmapSet.AddSafe(entry); err != nil { + singleton := &idmap.IdmapSet{Idmap: []idmap.IdmapEntry{entry}} + return nil, fmt.Errorf("raw.idmap %q: %w", strings.TrimSpace(formatIdmap(singleton)), err) + } + } + + return idmapSet, nil +} + +func formatIdmap(idmapSet *idmap.IdmapSet) string { + var entries strings.Builder + for _, ent := range idmapSet.Idmap { + switch { + case ent.Maprange <= 0, !ent.Isuid && !ent.Isgid: + continue + case ent.Isuid && !ent.Isgid: + entries.WriteString("uid") + case !ent.Isuid && ent.Isgid: + entries.WriteString("gid") + case ent.Isuid && ent.Isgid: + entries.WriteString("both") + } + + entries.WriteByte(' ') + + entries.WriteString(strconv.FormatInt(ent.Hostid, 10)) + if ent.Maprange > 1 { + entries.WriteByte('-') + entries.WriteString(strconv.FormatInt(ent.Hostid+ent.Maprange-1, 10)) + } + + entries.WriteByte(' ') + + entries.WriteString(strconv.FormatInt(ent.Nsid, 10)) + if ent.Maprange > 1 { + entries.WriteByte('-') + entries.WriteString(strconv.FormatInt(ent.Nsid+ent.Maprange-1, 10)) + } + + entries.WriteByte('\n') + } + return entries.String() +} diff --git a/internal/workshop/lxd/lxd_backend_test.go b/internal/workshop/lxd/lxd_backend_test.go index 1cf175bbd..18f7e15e4 100644 --- a/internal/workshop/lxd/lxd_backend_test.go +++ b/internal/workshop/lxd/lxd_backend_test.go @@ -114,7 +114,7 @@ func (f *LxdBeTests) TestDefaultContainerConfig(c *check.C) { // Validate c.Assert(err, check.IsNil) c.Assert(cfg["cloud-init.user-data"], check.Not(testutil.Contains), "GRUB_CMDLINE_LINUX") - c.Assert(cfg["raw.idmap"], check.Equals, "uid 1001 1000\ngid 1001 1000") + c.Assert(cfg["raw.idmap"], check.Equals, "uid 1001 1000\ngid 1001 1000\n") c.Assert(cfg["raw.lxc"], check.Equals, "lxc.mount.entry = tmpfs tmp tmpfs defaults") c.Assert(cfg["security.nesting"], check.Equals, "true") c.Assert(cfg["user.workshop.project-id"], check.Equals, f.project.ProjectId) @@ -151,7 +151,8 @@ func (f *LxdBeTests) TestDefaultVMConfig(c *check.C) { // Validate c.Assert(err, check.IsNil) c.Assert(cfg["cloud-init.user-data"], testutil.Contains, "GRUB_CMDLINE_LINUX") - c.Assert(cfg["raw.idmap"], check.Equals, "uid 1002 1000\ngid 1002 1000") + c.Assert(cfg["raw.idmap"], testutil.Contains, "uid 1002 1000\n") + c.Assert(cfg["raw.idmap"], testutil.Contains, "gid 1002 1000\n") _, ok := cfg["raw.lxc"] c.Assert(ok, check.Equals, false) _, ok = cfg["security.nesting"] diff --git a/internal/workshop/lxd/tests/integration/snapshot-format.yaml b/internal/workshop/lxd/tests/integration/snapshot-format.yaml index 7b7c9d769..0bdf9dcb1 100644 --- a/internal/workshop/lxd/tests/integration/snapshot-format.yaml +++ b/internal/workshop/lxd/tests/integration/snapshot-format.yaml @@ -10,7 +10,7 @@ launched: architecture: '' config: boot.autostart: 'false' - raw.idmap: |- + raw.idmap: | uid 1000 1000 gid 1000 1000 raw.lxc: lxc.mount.entry = tmpfs tmp tmpfs defaults @@ -50,7 +50,7 @@ started: architecture: '' config: boot.autostart: 'true' - raw.idmap: |- + raw.idmap: | uid 1000 1000 gid 1000 1000 raw.lxc: lxc.mount.entry = tmpfs tmp tmpfs defaults @@ -90,7 +90,7 @@ sdk-attached: architecture: '' config: boot.autostart: 'true' - raw.idmap: |- + raw.idmap: | uid 1000 1000 gid 1000 1000 raw.lxc: lxc.mount.entry = tmpfs tmp tmpfs defaults @@ -142,7 +142,7 @@ sdk-mounted: architecture: '' config: boot.autostart: 'true' - raw.idmap: |- + raw.idmap: | uid 1000 1000 gid 1000 1000 raw.lxc: lxc.mount.entry = tmpfs tmp tmpfs defaults diff --git a/tests/main/launch/task.yaml b/tests/main/launch/task.yaml index ab56021d4..b386db663 100644 --- a/tests/main/launch/task.yaml +++ b/tests/main/launch/task.yaml @@ -17,18 +17,32 @@ execute: | function launch_and_validate() { workshop_exec launch "$1" workshop_exec list | MATCH "$1[[:space:]]*Ready[[:space:]]*-" + + # ensure user idmap is applied + rm -f build-artifact + workshop_exec exec "$1" touch build-artifact + stat --format='%U %G' build-artifact | MATCH '^ubuntu ubuntu$' + workshop_exec exec "$1" sudo chown 0:0 build-artifact + stat --format='%u %g' build-artifact | MATCH '^1000000 1000000$' + workshop_exec exec "$1" sudo chown 1001:1001 build-artifact + stat --format='%u %g' build-artifact | MATCH '^1001001 1001001$' + # ensure the workshop has a socket for workshopctl workshop_exec exec "$1" test -S /var/lib/workshop/run/workshop.socket.untrusted + # ensure the apt cache is mounted workshop_exec exec "$1" mountpoint /var/cache/apt/archives workshop_exec exec "$1" stat --format='%A %U %G' /var/cache/apt/archives | MATCH '^drwxr-xr-x workshop workshop$' + # ensure apt is configured to exclude suggested and recommended packages workshop_exec run "$1" check-apt + # ensure systemd-resolved is configured for the workshopbr0 network resolvectl domain workshopbr0 | MATCH '^Link [0-9]+ \(workshopbr0\): ~wp$' resolvectl query "$1-$(< .workshop.lock).wp" | MATCH 'link: workshopbr0' resolvectl query --type=CNAME "${1}.$(< .workshop.lock).wp" | MATCH "IN CNAME $1-$(< .workshop.lock).wp" resolvectl query --type=CNAME "${1}.launch.wp" | MATCH "IN CNAME $1-$(< .workshop.lock).wp" + # ensure systemd-networkd is configured not to release DHCP on stop # just skip if `networkctl cat` is missing, since old bases have been manually tested sd_ver="$(workshop_exec exec "$1" networkctl --version | awk 'FNR == 1 {print $2}')" @@ -37,6 +51,7 @@ execute: | extract_section DHCPv4 < eth0.log | MATCH '^SendRelease=false$' extract_section DHCPv6 < eth0.log | MATCH '^SendRelease=false$' fi + # ensure timezone is inherited from host workshop_exec exec "$1" timedatectl show | MATCH '^Timezone=Pacific/Auckland$' } From 326f78e445c72ee60ffa808c0b4fc78b05e893fe Mon Sep 17 00:00:00 2001 From: Jonathan Conder Date: Tue, 28 Jul 2026 18:20:11 +1200 Subject: [PATCH 11/15] Enable VM support --- docs/reference/definition-files/schema.json | 9 +++++ internal/overlord/workshopstate/manifest.go | 6 +++ .../overlord/workshopstate/manifest_test.go | 37 +++++++++++++++++++ internal/workshop/workshop_file.go | 8 ---- internal/workshop/workshop_file_test.go | 29 +++++++++++++++ snap/local/commands/run_daemon | 3 ++ tests/lib/utils.sh | 1 + tests/main/launch/.workshop/vm-20.yaml | 12 ++++++ tests/main/launch/.workshop/vm-22.yaml | 12 ++++++ tests/main/launch/.workshop/vm-24.yaml | 12 ++++++ tests/main/launch/.workshop/vm-26.yaml | 12 ++++++ tests/main/launch/.workshop/ws-26.yaml | 11 ++++++ tests/main/launch/task.yaml | 13 +++++-- 13 files changed, 154 insertions(+), 11 deletions(-) create mode 100644 tests/main/launch/.workshop/vm-20.yaml create mode 100644 tests/main/launch/.workshop/vm-22.yaml create mode 100644 tests/main/launch/.workshop/vm-24.yaml create mode 100644 tests/main/launch/.workshop/vm-26.yaml create mode 100644 tests/main/launch/.workshop/ws-26.yaml diff --git a/docs/reference/definition-files/schema.json b/docs/reference/definition-files/schema.json index 8b677bf8b..0f5abab9c 100644 --- a/docs/reference/definition-files/schema.json +++ b/docs/reference/definition-files/schema.json @@ -23,6 +23,15 @@ ], "errorMessage": "The base must be one of the supported values: ubuntu@20.04, ubuntu@22.04, ubuntu@24.04, ubuntu@26.04." }, + "confinement": { + "type": "string", + "description": "Type of sandboxing to use to run the workshop.", + "enum": [ + "container", + "virtual-machine" + ], + "errorMessage": "The confinement must be one of the supported values: container, virtual-machine." + }, "sdks": { "type": "array", "description": "Ordered list of SDKs to install on top of the base. Each entry references an existing SDK; names must be unique within the list. The system SDK is installed first implicitly and need not be listed.", diff --git a/internal/overlord/workshopstate/manifest.go b/internal/overlord/workshopstate/manifest.go index 366f5d7df..0c6914746 100644 --- a/internal/overlord/workshopstate/manifest.go +++ b/internal/overlord/workshopstate/manifest.go @@ -268,6 +268,12 @@ func (a *artifactFinder) launchOrRefreshManifests(ctx context.Context, names []s } if file.Confinement == workshop.ConfinementContainer { sdks = slices.Insert(sdks, 0, systemMeta.Setup) + } else if !refresh && !osutil.GetenvBool("WORKSHOP_EXPERIMENTAL_VMS") { + confinement, err := file.Confinement.MarshalText() + if err == nil { + err = fmt.Errorf("confinement %q is experimental\nTo opt in: %q", confinement, "snap set workshop workshop.experimental-vms=1") + } + return nil, nil, fmt.Errorf("cannot %s %q: %w", action, name, err) } storeSdks = append(storeSdks, sdks) } diff --git a/internal/overlord/workshopstate/manifest_test.go b/internal/overlord/workshopstate/manifest_test.go index a2d596e7b..f0f16a41b 100644 --- a/internal/overlord/workshopstate/manifest_test.go +++ b/internal/overlord/workshopstate/manifest_test.go @@ -442,6 +442,21 @@ func (s *manifestSuite) TestRefreshRequiresStatusReady(c *check.C) { c.Assert(err, check.ErrorMatches, `cannot refresh "test-2": not running`) } +func (s *manifestSuite) TestRefreshRequiresContainer(c *check.C) { + s.state.Lock() + defer s.state.Unlock() + + s.launchWorkshopWithSDKs(c, "test", "ubuntu@20.04", nil) + f, err := os.OpenFile(workshop.Filepath(s.project.Path, "test"), os.O_APPEND|os.O_WRONLY, 0644) + c.Assert(err, check.IsNil) + _, err = f.WriteString("confinement: virtual-machine\n") + c.Assert(f.Close(), check.IsNil) + c.Assert(err, check.IsNil) + + _, _, err = s.manager.RefreshManifests(s.ctx, s.project, []string{"test"}, conflict.RefreshUpdate) + c.Check(err, check.ErrorMatches, `cannot refresh "test": confinement changed from "container" to "virtual-machine"`) +} + func (s *manifestSuite) TestRestoreRequiresCurrentFormat(c *check.C) { s.state.Lock() defer s.state.Unlock() @@ -1102,3 +1117,25 @@ func (s *manifestSuite) TestRefreshSortsSdks(c *check.C) { } c.Check(sorted, check.DeepEquals, expected) } + +func (s *manifestSuite) TestLaunchRejectsVMsWithSDKs(c *check.C) { + s.state.Lock() + defer s.state.Unlock() + + sdks := []workshop.SdkRecord{{Name: "test", Channel: "latest/edge"}} + s.createWFile(c, "test", "ubuntu@20.04", sdks) + f, err := os.OpenFile(workshop.Filepath(s.project.Path, "test"), os.O_APPEND|os.O_WRONLY, 0644) + c.Assert(err, check.IsNil) + _, err = f.WriteString("confinement: virtual-machine\n") + c.Assert(f.Close(), check.IsNil) + c.Assert(err, check.IsNil) + + os.Setenv("WORKSHOP_EXPERIMENTAL_VMS", "1") + _, err = s.manager.LaunchManifests(s.ctx, s.project, []string{"test"}) + c.Check(err, check.ErrorMatches, `cannot launch "test": SDKs are currently unavailable for virtual machines`) + + os.Unsetenv("WORKSHOP_EXPERIMENTAL_VMS") + _, err = s.manager.LaunchManifests(s.ctx, s.project, []string{"test"}) + c.Check(err, check.ErrorMatches, `cannot launch "test": confinement "virtual-machine" is experimental +To opt in: "snap set workshop workshop.experimental-vms=1"`) +} diff --git a/internal/workshop/workshop_file.go b/internal/workshop/workshop_file.go index 86469a041..cb77df72b 100644 --- a/internal/workshop/workshop_file.go +++ b/internal/workshop/workshop_file.go @@ -304,14 +304,6 @@ func ValidateFile(file *File) error { return fmt.Errorf("base %q not supported", file.Base) } - if file.Confinement != ConfinementContainer { - confinement, err := file.Confinement.MarshalText() - if err != nil { - return err - } - return fmt.Errorf("confinement %q not supported", confinement) - } - if err := validateSdks(file.Sdks); err != nil { return err } diff --git a/internal/workshop/workshop_file_test.go b/internal/workshop/workshop_file_test.go index 9720b3586..5f0910a9e 100644 --- a/internal/workshop/workshop_file_test.go +++ b/internal/workshop/workshop_file_test.go @@ -95,6 +95,7 @@ actions: c.Assert(err, check.Equals, nil) c.Assert(file.Name, check.Equals, "xbert-gpu") c.Assert(file.Base, check.Equals, "ubuntu@20.04") + c.Assert(file.Confinement, check.Equals, workshop.ConfinementContainer) c.Assert(file.Sdks[0], check.DeepEquals, workshop.SdkRecord{Name: "system", Source: sdk.SystemSource}) c.Assert(file.Sdks[1], check.DeepEquals, workshop.SdkRecord{Name: "huggingface"}) c.Assert(file.Sdks[2], check.DeepEquals, workshop.SdkRecord{Name: "cuda", Channel: "latest/edge"}) @@ -186,6 +187,34 @@ func (f *workshopFile) TestSingleWorkshopFileError(c *check.C) { c.Assert(err, check.ErrorMatches, ".*is a directory") } +func (f *workshopFile) TestConfinement(c *check.C) { + yaml := `name: xbert-gpu +base: ubuntu@20.04 +confinement: container +` + f.createSingleWFile(c, "workshop.yaml", yaml) + file, err := f.project.Workshop("xbert-gpu") + c.Assert(err, check.IsNil) + c.Check(file.Confinement, check.Equals, workshop.ConfinementContainer) + + yaml = strings.Replace(yaml, "container", "virtual-machine", 1) + f.createSingleWFile(c, "workshop.yaml", yaml) + file, err = f.project.Workshop("xbert-gpu") + c.Assert(err, check.IsNil) + c.Check(file.Confinement, check.Equals, workshop.ConfinementVirtualMachine) +} + +func (f *workshopFile) TestConfinementError(c *check.C) { + yaml := `name: xbert-gpu +base: ubuntu@20.04 +confinement: classic +` + f.createSingleWFile(c, "workshop.yaml", yaml) + file, err := f.project.Workshop("xbert-gpu") + c.Check(file, check.IsNil) + c.Check(err, check.ErrorMatches, `invalid file ".*": invalid confinement: "classic"`) +} + func (f *workshopFile) TestWorkshopFileDuplicate(c *check.C) { yaml := `name: xbert-gpu base: ubuntu@22.04 diff --git a/snap/local/commands/run_daemon b/snap/local/commands/run_daemon index cf7018d70..d82625527 100755 --- a/snap/local/commands/run_daemon +++ b/snap/local/commands/run_daemon @@ -9,6 +9,9 @@ export WORKSHOP_DEBUG WORKSHOP_IMAGE_SERVER=$(snapctl get workshop.image.server.url) export WORKSHOP_IMAGE_SERVER +WORKSHOP_EXPERIMENTAL_VMS=$(snapctl get workshop.experimental-vms) +export WORKSHOP_EXPERIMENTAL_VMS + read -r sbpid _ < /proc/self/stat export LISTEN_PID="${sbpid}" diff --git a/tests/lib/utils.sh b/tests/lib/utils.sh index c3977f819..52f386ed8 100644 --- a/tests/lib/utils.sh +++ b/tests/lib/utils.sh @@ -88,6 +88,7 @@ function setup_workshop() { snap install --dangerous --classic /workshop/tests/*.snap snap set workshop workshop.debug=1 + snap set workshop workshop.experimental-vms=1 snap set workshop workshop.image.server.url="$IMAGE_SERVER" snap alias workshop.sdk sdk snap restart workshop diff --git a/tests/main/launch/.workshop/vm-20.yaml b/tests/main/launch/.workshop/vm-20.yaml new file mode 100644 index 000000000..203e47a3e --- /dev/null +++ b/tests/main/launch/.workshop/vm-20.yaml @@ -0,0 +1,12 @@ +name: vm-20 +base: ubuntu@20.04 +confinement: virtual-machine +actions: + check-apt: | + dry_run() { + sudo apt-get -o 'Debug::NoLocking=true' -o 'APT::Get::Assume-Yes=0' --assume-no "$@" install gnome + } + + [ -f /etc/apt/apt.conf.d/94cloud-init-config ] + sudo apt-get update + diff <(dry_run --no-install-recommends --no-install-suggests) <(dry_run) diff --git a/tests/main/launch/.workshop/vm-22.yaml b/tests/main/launch/.workshop/vm-22.yaml new file mode 100644 index 000000000..61fec3a09 --- /dev/null +++ b/tests/main/launch/.workshop/vm-22.yaml @@ -0,0 +1,12 @@ +name: vm-22 +base: ubuntu@22.04 +confinement: virtual-machine +actions: + check-apt: | + dry_run() { + sudo apt-get -o 'Debug::NoLocking=true' -o 'APT::Get::Assume-Yes=0' --assume-no "$@" install gnome + } + + [ -f /etc/apt/apt.conf.d/94cloud-init-config ] + sudo apt-get update + diff <(dry_run --no-install-recommends --no-install-suggests) <(dry_run) diff --git a/tests/main/launch/.workshop/vm-24.yaml b/tests/main/launch/.workshop/vm-24.yaml new file mode 100644 index 000000000..dc317acef --- /dev/null +++ b/tests/main/launch/.workshop/vm-24.yaml @@ -0,0 +1,12 @@ +name: vm-24 +base: ubuntu@24.04 +confinement: virtual-machine +actions: + check-apt: | + dry_run() { + sudo apt-get -o 'Debug::NoLocking=true' -o 'APT::Get::Assume-Yes=0' --assume-no "$@" install gnome + } + + [ -f /etc/apt/apt.conf.d/94cloud-init-config ] + sudo apt-get update + diff <(dry_run --no-install-recommends --no-install-suggests) <(dry_run) diff --git a/tests/main/launch/.workshop/vm-26.yaml b/tests/main/launch/.workshop/vm-26.yaml new file mode 100644 index 000000000..d13f2cc16 --- /dev/null +++ b/tests/main/launch/.workshop/vm-26.yaml @@ -0,0 +1,12 @@ +name: vm-26 +base: ubuntu@26.04 +confinement: virtual-machine +actions: + check-apt: | + dry_run() { + sudo apt-get -o 'Debug::NoLocking=true' -o 'APT::Get::Assume-Yes=0' --assume-no "$@" install gnome + } + + [ -f /etc/apt/apt.conf.d/94cloud-init-config ] + sudo apt-get update + diff <(dry_run --no-install-recommends --no-install-suggests) <(dry_run) diff --git a/tests/main/launch/.workshop/ws-26.yaml b/tests/main/launch/.workshop/ws-26.yaml new file mode 100644 index 000000000..f587e913e --- /dev/null +++ b/tests/main/launch/.workshop/ws-26.yaml @@ -0,0 +1,11 @@ +name: ws-26 +base: ubuntu@26.04 +actions: + check-apt: | + dry_run() { + sudo apt-get -o 'Debug::NoLocking=true' -o 'APT::Get::Assume-Yes=0' --assume-no "$@" install gnome + } + + [ -f /etc/apt/apt.conf.d/94cloud-init-config ] + sudo apt-get update + diff <(dry_run --no-install-recommends --no-install-suggests) <(dry_run) diff --git a/tests/main/launch/task.yaml b/tests/main/launch/task.yaml index b386db663..cf348b477 100644 --- a/tests/main/launch/task.yaml +++ b/tests/main/launch/task.yaml @@ -4,7 +4,7 @@ prepare: | restore: | . "$TESTSLIB"/utils.sh timedatectl set-timezone Etc/UTC - workshop_exec remove ws-20 ws-22 ws-24 + workshop_exec remove ws-20 ws-22 ws-24 ws-26 vm-20 vm-22 vm-24 vm-26 execute: | . "$TESTSLIB"/utils.sh @@ -28,7 +28,9 @@ execute: | stat --format='%u %g' build-artifact | MATCH '^1001001 1001001$' # ensure the workshop has a socket for workshopctl - workshop_exec exec "$1" test -S /var/lib/workshop/run/workshop.socket.untrusted + if [[ "$1" == ws-* ]]; then + workshop_exec exec "$1" test -S /var/lib/workshop/run/workshop.socket.untrusted + fi # ensure the apt cache is mounted workshop_exec exec "$1" mountpoint /var/cache/apt/archives @@ -59,8 +61,13 @@ execute: | launch_and_validate ws-20 launch_and_validate ws-22 launch_and_validate ws-24 + launch_and_validate ws-26 + launch_and_validate vm-20 + launch_and_validate vm-22 + launch_and_validate vm-24 + launch_and_validate vm-26 # ensure workshops can resolve each other by name mkdir -p srv/qwertyuiop workshop_exec exec ws-22 -- systemd-run --user --working-directory=/project/srv -- python3 -m http.server 8877 - workshop_exec exec ws-24 -- curl http://ws-22:8877 | MATCH qwertyuiop + workshop_exec exec vm-24 -- curl http://ws-22:8877 | MATCH qwertyuiop From ccbf64cd2511eb6f06442b9575ce427cb894c56e Mon Sep 17 00:00:00 2001 From: Jonathan Conder Date: Mon, 3 Aug 2026 11:59:54 +1200 Subject: [PATCH 12/15] Add workshop init --vm flag --- cmd/workshop/init.go | 13 +++++++++++++ cmd/workshop/init_test.go | 15 +++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/cmd/workshop/init.go b/cmd/workshop/init.go index d824c25c6..6fc65207e 100644 --- a/cmd/workshop/init.go +++ b/cmd/workshop/init.go @@ -19,6 +19,7 @@ type CmdInit struct { root *CmdRoot sdks []string base string + vm bool } func (c *CmdInit) Command() *cobra.Command { @@ -53,6 +54,9 @@ $ workshop init dev --base ubuntu@22.04 --sdks go`, cmd.Flags().StringSliceVar(&c.sdks, "sdks", nil, `Comma-separated list of SDKs (e.g., "go,uv/latest/stable").`) cmd.Flags().StringVar(&c.base, "base", defaultBase, "Base image for the workshop.") + cmd.Flags().BoolVar(&c.vm, "vm", false, "Use a virtual machine instead of a container.") + + cmd.MarkFlagsMutuallyExclusive("sdks", "vm") return cmd } @@ -66,6 +70,11 @@ func (c *CmdInit) Run(cmd *cobra.Command, args []string) error { return err } + confinement := workshop.ConfinementContainer + if c.vm { + confinement = workshop.ConfinementVirtualMachine + } + wfile := &workshop.File{ Name: name, Base: c.base, @@ -76,6 +85,10 @@ func (c *CmdInit) Run(cmd *cobra.Command, args []string) error { return err } + // TODO: include confinement in the validation. We skip validation here + // because we might not have the experimental flag in the environment. + wfile.Confinement = confinement + if err := ensureCanCreate(projectDir, name); err != nil { return err } diff --git a/cmd/workshop/init_test.go b/cmd/workshop/init_test.go index 3345df03a..c45df21af 100644 --- a/cmd/workshop/init_test.go +++ b/cmd/workshop/init_test.go @@ -85,6 +85,21 @@ base: ubuntu@24.04 `) } +func (s *workshopInit) TestInitVM(c *check.C) { + projectDir := c.MkDir() + cmd := s.makeCmd(projectDir) + cmd.vm = true + + err := s.run(cmd, "dev") + c.Assert(err, check.IsNil) + + path := workshop.Filepath(projectDir, "dev") + c.Check(path, testutil.FileEquals, `name: dev +base: ubuntu@24.04 +confinement: virtual-machine +`) +} + func (s *workshopInit) TestInitWithSdkChannel(c *check.C) { projectDir := c.MkDir() cmd := s.makeCmd(projectDir) From 261f3195be2e37fe1743d349cb9e163ad0247f3e Mon Sep 17 00:00:00 2001 From: Jonathan Conder Date: Mon, 31 Aug 2026 16:49:43 +1200 Subject: [PATCH 13/15] Add confinement to workshop info --- client/workshop.go | 15 ++++---- cmd/workshop/info.go | 1 + cmd/workshop/info_test.go | 50 ++++++++++++++++----------- internal/daemon/api_workshops.go | 27 +++++++++++---- internal/daemon/api_workshops_test.go | 40 +++++++++++---------- 5 files changed, 80 insertions(+), 53 deletions(-) diff --git a/client/workshop.go b/client/workshop.go index 3a7fc71e9..2e6971802 100644 --- a/client/workshop.go +++ b/client/workshop.go @@ -71,13 +71,14 @@ type Workshops struct { } type WorkshopInfo struct { - ProjectId string `json:"project-id"` - Name string `json:"name"` - Base string `json:"base"` - Status string `json:"status"` - Sdks []*Sdk `json:"sdks,omitempty"` - Hostname string `json:"hostname,omitempty"` - Notes []string `json:"notes,omitempty"` + ProjectId string `json:"project-id"` + Name string `json:"name"` + Base string `json:"base"` + Confinement string `json:"confinement"` + Status string `json:"status"` + Sdks []*Sdk `json:"sdks,omitempty"` + Hostname string `json:"hostname,omitempty"` + Notes []string `json:"notes,omitempty"` } type WorkshopFile struct { diff --git a/cmd/workshop/info.go b/cmd/workshop/info.go index bfab11ea3..993c3ee01 100644 --- a/cmd/workshop/info.go +++ b/cmd/workshop/info.go @@ -154,6 +154,7 @@ func (c *CmdInfo) Run(cmd *cobra.Command, av []string) error { fmt.Fprintf(w, "hostname:\t%s\n", workshop.Hostname) } fmt.Fprintf(w, "status:\t%s\n", strings.ToLower(workshop.Status)) + fmt.Fprintf(w, "confinement:\t%s\n", workshop.Confinement) // get the workshop notes notes := workshop.Notes diff --git a/cmd/workshop/info_test.go b/cmd/workshop/info_test.go index e690e84cd..f46a9ca22 100644 --- a/cmd/workshop/info_test.go +++ b/cmd/workshop/info_test.go @@ -41,6 +41,7 @@ func (m *workshopInfo) SetUpTest(c *check.C) { var mockWorkshopWithSdks = `{"type":"sync","status-code":200,"status":"OK","result":{ "name":"ws", "base":"ubuntu@22.04", + "confinement":"container", "project-id":"42424242", "status":"Error", "hostname":"ws.sdkcraft.wp", @@ -94,12 +95,13 @@ func (m *workshopInfo) TestWorkshopInfo(c *check.C) { err = cmd.Run(cmd.Command(), nil) c.Assert(err, check.IsNil) - c.Assert(m.stdout.String(), check.Matches, fmt.Sprintf(`name: ws -base: ubuntu@22.04 -project: %s -hostname: ws\.sdkcraft\.wp -status: error -notes: missing-project + c.Assert(m.stdout.String(), check.Matches, fmt.Sprintf(`name: ws +base: ubuntu@22.04 +project: %s +hostname: ws\.sdkcraft\.wp +status: error +confinement: container +notes: missing-project sdks: go: tracking: latest/edge @@ -114,6 +116,7 @@ sdks: var mockWorkshopWithHealth = `{"type":"sync","status-code":200,"status":"OK","result":{ "name":"ws", "base":"ubuntu@22.04", + "confinement":"container", "project-id":"42424242", "status":"Pending", "notes":["workshop-note"], @@ -152,11 +155,12 @@ func (m *workshopInfo) TestWorkshopInfoWithSdkHealthReport(c *check.C) { err := cmd.Run(cmd.Command(), []string{workshop}) c.Assert(err, check.IsNil) - c.Assert(m.stdout.String(), check.Matches, fmt.Sprintf(`name: ws -base: ubuntu@22.04 -project: %s -status: pending -notes: workshop-note,try-later + c.Assert(m.stdout.String(), check.Matches, fmt.Sprintf(`name: ws +base: ubuntu@22.04 +project: %s +status: pending +confinement: container +notes: workshop-note,try-later sdks: go: tracking: latest/edge @@ -169,6 +173,7 @@ sdks: var mockWorkshopWithMounts = `{"type":"sync","status-code":200,"status":"OK","result":{ "name":"ws", "base":"ubuntu@22.04", + "confinement":"container", "project-id":"42424242", "status":"Ready", "sdks":[{ @@ -200,11 +205,12 @@ var mockWorkshopWithMounts = `{"type":"sync","status-code":200,"status":"OK","re }] }}` -var mockWorkshopWithMountsOutput = `name: ws -base: ubuntu@22.04 -project: %s -status: ready -notes: %s +var mockWorkshopWithMountsOutput = `name: ws +base: ubuntu@22.04 +project: %s +status: ready +confinement: container +notes: %s sdks: go: tracking: latest/edge @@ -307,6 +313,7 @@ var mockWorkshopWithTunnels = `{ "result": { "name": "ws", "base": "ubuntu@22.04", + "confinement": "container", "project-id": "42424242", "status": "Ready", "sdks": [ @@ -392,11 +399,12 @@ func (m *workshopInfo) TestWorkshopInfoWithSdkTunnels(c *check.C) { err = cmd.Run(cmd.Command(), []string{workshop}) c.Assert(err, check.IsNil) - c.Assert(m.stdout.String(), check.Matches, fmt.Sprintf(`name: ws -base: ubuntu@22.04 -project: %s -status: ready -notes: -- + c.Assert(m.stdout.String(), check.Matches, fmt.Sprintf(`name: ws +base: ubuntu@22.04 +project: %s +status: ready +confinement: container +notes: -- sdks: system: installed: \(1\) diff --git a/internal/daemon/api_workshops.go b/internal/daemon/api_workshops.go index ca8fe1aea..4b219a44c 100644 --- a/internal/daemon/api_workshops.go +++ b/internal/daemon/api_workshops.go @@ -106,13 +106,14 @@ type Workshops struct { } type WorkshopInfo struct { - ProjectId string `json:"project-id"` - Name string `json:"name"` - Base string `json:"base"` - Status string `json:"status"` - Sdks []*SdkInfo `json:"sdks,omitempty"` - Hostname string `json:"hostname,omitempty"` - Notes []string `json:"notes,omitempty"` + ProjectId string `json:"project-id"` + Name string `json:"name"` + Base string `json:"base"` + Confinement string `json:"confinement"` + Status string `json:"status"` + Sdks []*SdkInfo `json:"sdks,omitempty"` + Hostname string `json:"hostname,omitempty"` + Notes []string `json:"notes,omitempty"` } type WorkshopFileInfo struct { @@ -202,6 +203,12 @@ func workshopToInfo(username string, w *workshop.Workshop, health healthstate.He info.ProjectId = w.Project.ProjectId info.Base = w.File.Base + confinement, err := w.File.Confinement.MarshalText() + if err != nil { + return nil, err + } + info.Confinement = string(confinement) + sdkSetups := w.SdksByInstallOrder() usr, env, err := osutil.UserAndEnv(username) @@ -255,6 +262,12 @@ func workshopToInfoFull(ctx context.Context, username string, w *workshop.Worksh info.ProjectId = w.Project.ProjectId info.Base = w.File.Base + confinement, err := w.File.Confinement.MarshalText() + if err != nil { + return nil, err + } + info.Confinement = string(confinement) + sdks, err := w.SdkInfosByInstallOrder(ctx) if err != nil { return nil, err diff --git a/internal/daemon/api_workshops_test.go b/internal/daemon/api_workshops_test.go index e6d25c589..2a231dad8 100644 --- a/internal/daemon/api_workshops_test.go +++ b/internal/daemon/api_workshops_test.go @@ -551,10 +551,11 @@ func (s *apiSuite) TestGetWorkshops(c *check.C) { info := rsp.Result.(Workshops) c.Check(info.Workshops, testutil.DeepUnsortedMatches, []*WorkshopInfo{{ - Name: "manysdks", - Base: "ubuntu@24.04", - ProjectId: s.project.ProjectId, - Status: "Ready", + Name: "manysdks", + Base: "ubuntu@24.04", + Confinement: "container", + ProjectId: s.project.ProjectId, + Status: "Ready", Sdks: []*SdkInfo{ { Name: "system", @@ -569,10 +570,11 @@ func (s *apiSuite) TestGetWorkshops(c *check.C) { }, }, }, { - Name: "basic", - Base: "ubuntu@22.04", - ProjectId: s.project.ProjectId, - Status: "Ready", + Name: "basic", + Base: "ubuntu@22.04", + Confinement: "container", + ProjectId: s.project.ProjectId, + Status: "Ready", Sdks: []*SdkInfo{{ Name: "system", Revision: system.SystemSdkRevision.String(), @@ -672,11 +674,12 @@ func (s *apiSuite) TestGetWorkshopInfo(c *check.C) { c.Assert(err, check.IsNil) c.Check(result, check.DeepEquals, Workshop{ WorkshopInfo: WorkshopInfo{ - Name: "tunnels", - Base: "ubuntu@22.04", - ProjectId: s.project.ProjectId, - Status: "Ready", - Notes: nil, + Name: "tunnels", + Base: "ubuntu@22.04", + Confinement: "container", + ProjectId: s.project.ProjectId, + Status: "Ready", + Notes: nil, Sdks: []*SdkInfo{ { Name: "system", @@ -824,11 +827,12 @@ func (s *apiSuite) TestGetWorkshopInfoSomePlugsBound(c *check.C) { } c.Check(result, check.DeepEquals, Workshop{ WorkshopInfo: WorkshopInfo{ - Name: "somebound", - Base: "ubuntu@22.04", - ProjectId: s.project.ProjectId, - Status: "Ready", - Notes: nil, + Name: "somebound", + Base: "ubuntu@22.04", + Confinement: "container", + ProjectId: s.project.ProjectId, + Status: "Ready", + Notes: nil, Sdks: []*SdkInfo{ { Name: "mount-conflict", From 46f852e66166102f445baff74e2b1f40f1e27306 Mon Sep 17 00:00:00 2001 From: Jonathan Conder Date: Mon, 31 Aug 2026 15:31:16 +1200 Subject: [PATCH 14/15] Split tests/main/launch into multiple variants --- tests/main/launch-multiple/task.yaml | 8 +- tests/main/launch/.workshop/vm-20.yaml | 12 -- tests/main/launch/.workshop/vm-22.yaml | 12 -- tests/main/launch/.workshop/vm-24.yaml | 12 -- tests/main/launch/.workshop/vm-26.yaml | 12 -- tests/main/launch/.workshop/ws-22.yaml | 11 -- tests/main/launch/.workshop/ws-24.yaml | 11 -- tests/main/launch/.workshop/ws-26.yaml | 11 -- tests/main/launch/task.yaml | 103 ++++++++---------- .../ws-20.yaml => workshop.yaml.in} | 5 +- 10 files changed, 57 insertions(+), 140 deletions(-) delete mode 100644 tests/main/launch/.workshop/vm-20.yaml delete mode 100644 tests/main/launch/.workshop/vm-22.yaml delete mode 100644 tests/main/launch/.workshop/vm-24.yaml delete mode 100644 tests/main/launch/.workshop/vm-26.yaml delete mode 100644 tests/main/launch/.workshop/ws-22.yaml delete mode 100644 tests/main/launch/.workshop/ws-24.yaml delete mode 100644 tests/main/launch/.workshop/ws-26.yaml rename tests/main/launch/{.workshop/ws-20.yaml => workshop.yaml.in} (85%) diff --git a/tests/main/launch-multiple/task.yaml b/tests/main/launch-multiple/task.yaml index ee8b2cf37..efba10a47 100644 --- a/tests/main/launch-multiple/task.yaml +++ b/tests/main/launch-multiple/task.yaml @@ -1,4 +1,7 @@ summary: Ensure that launch for multiple workshops works correctly +restore: | + . "$TESTSLIB"/utils.sh + workshop_exec remove ws-one ws-two ws-three execute: | . "$TESTSLIB"/utils.sh @@ -8,4 +11,7 @@ execute: | workshop_exec list | MATCH "ws-two[[:space:]]*Ready[[:space:]]*-" workshop_exec list | MATCH "ws-three[[:space:]]*Ready[[:space:]]*-" - workshop_exec remove ws-one ws-two ws-three + # ensure workshops can resolve each other by name + mkdir -p srv/qwertyuiop + workshop_exec exec ws-one -- systemd-run --user --working-directory=/project/srv -- python3 -m http.server 8877 + workshop_exec exec ws-two -- curl http://ws-one:8877 | MATCH qwertyuiop diff --git a/tests/main/launch/.workshop/vm-20.yaml b/tests/main/launch/.workshop/vm-20.yaml deleted file mode 100644 index 203e47a3e..000000000 --- a/tests/main/launch/.workshop/vm-20.yaml +++ /dev/null @@ -1,12 +0,0 @@ -name: vm-20 -base: ubuntu@20.04 -confinement: virtual-machine -actions: - check-apt: | - dry_run() { - sudo apt-get -o 'Debug::NoLocking=true' -o 'APT::Get::Assume-Yes=0' --assume-no "$@" install gnome - } - - [ -f /etc/apt/apt.conf.d/94cloud-init-config ] - sudo apt-get update - diff <(dry_run --no-install-recommends --no-install-suggests) <(dry_run) diff --git a/tests/main/launch/.workshop/vm-22.yaml b/tests/main/launch/.workshop/vm-22.yaml deleted file mode 100644 index 61fec3a09..000000000 --- a/tests/main/launch/.workshop/vm-22.yaml +++ /dev/null @@ -1,12 +0,0 @@ -name: vm-22 -base: ubuntu@22.04 -confinement: virtual-machine -actions: - check-apt: | - dry_run() { - sudo apt-get -o 'Debug::NoLocking=true' -o 'APT::Get::Assume-Yes=0' --assume-no "$@" install gnome - } - - [ -f /etc/apt/apt.conf.d/94cloud-init-config ] - sudo apt-get update - diff <(dry_run --no-install-recommends --no-install-suggests) <(dry_run) diff --git a/tests/main/launch/.workshop/vm-24.yaml b/tests/main/launch/.workshop/vm-24.yaml deleted file mode 100644 index dc317acef..000000000 --- a/tests/main/launch/.workshop/vm-24.yaml +++ /dev/null @@ -1,12 +0,0 @@ -name: vm-24 -base: ubuntu@24.04 -confinement: virtual-machine -actions: - check-apt: | - dry_run() { - sudo apt-get -o 'Debug::NoLocking=true' -o 'APT::Get::Assume-Yes=0' --assume-no "$@" install gnome - } - - [ -f /etc/apt/apt.conf.d/94cloud-init-config ] - sudo apt-get update - diff <(dry_run --no-install-recommends --no-install-suggests) <(dry_run) diff --git a/tests/main/launch/.workshop/vm-26.yaml b/tests/main/launch/.workshop/vm-26.yaml deleted file mode 100644 index d13f2cc16..000000000 --- a/tests/main/launch/.workshop/vm-26.yaml +++ /dev/null @@ -1,12 +0,0 @@ -name: vm-26 -base: ubuntu@26.04 -confinement: virtual-machine -actions: - check-apt: | - dry_run() { - sudo apt-get -o 'Debug::NoLocking=true' -o 'APT::Get::Assume-Yes=0' --assume-no "$@" install gnome - } - - [ -f /etc/apt/apt.conf.d/94cloud-init-config ] - sudo apt-get update - diff <(dry_run --no-install-recommends --no-install-suggests) <(dry_run) diff --git a/tests/main/launch/.workshop/ws-22.yaml b/tests/main/launch/.workshop/ws-22.yaml deleted file mode 100644 index 475fe1fa0..000000000 --- a/tests/main/launch/.workshop/ws-22.yaml +++ /dev/null @@ -1,11 +0,0 @@ -name: ws-22 -base: ubuntu@22.04 -actions: - check-apt: | - dry_run() { - sudo apt-get -o 'Debug::NoLocking=true' -o 'APT::Get::Assume-Yes=0' --assume-no "$@" install gnome - } - - [ -f /etc/apt/apt.conf.d/94cloud-init-config ] - sudo apt-get update - diff <(dry_run --no-install-recommends --no-install-suggests) <(dry_run) diff --git a/tests/main/launch/.workshop/ws-24.yaml b/tests/main/launch/.workshop/ws-24.yaml deleted file mode 100644 index 5279443ff..000000000 --- a/tests/main/launch/.workshop/ws-24.yaml +++ /dev/null @@ -1,11 +0,0 @@ -name: ws-24 -base: ubuntu@24.04 -actions: - check-apt: | - dry_run() { - sudo apt-get -o 'Debug::NoLocking=true' -o 'APT::Get::Assume-Yes=0' --assume-no "$@" install gnome - } - - [ -f /etc/apt/apt.conf.d/94cloud-init-config ] - sudo apt-get update - diff <(dry_run --no-install-recommends --no-install-suggests) <(dry_run) diff --git a/tests/main/launch/.workshop/ws-26.yaml b/tests/main/launch/.workshop/ws-26.yaml deleted file mode 100644 index f587e913e..000000000 --- a/tests/main/launch/.workshop/ws-26.yaml +++ /dev/null @@ -1,11 +0,0 @@ -name: ws-26 -base: ubuntu@26.04 -actions: - check-apt: | - dry_run() { - sudo apt-get -o 'Debug::NoLocking=true' -o 'APT::Get::Assume-Yes=0' --assume-no "$@" install gnome - } - - [ -f /etc/apt/apt.conf.d/94cloud-init-config ] - sudo apt-get update - diff <(dry_run --no-install-recommends --no-install-suggests) <(dry_run) diff --git a/tests/main/launch/task.yaml b/tests/main/launch/task.yaml index cf348b477..9bb4e78d4 100644 --- a/tests/main/launch/task.yaml +++ b/tests/main/launch/task.yaml @@ -1,73 +1,64 @@ summary: Test launch for the supported workshop bases +environment: + # TODO: enable VM variants + BASE/ctr20: ubuntu@20.04 + BASE/ctr22: ubuntu@22.04 + BASE/ctr24: ubuntu@24.04 + BASE/ctr26: ubuntu@26.04 + CONFINEMENT: container prepare: | timedatectl set-timezone Pacific/Auckland restore: | . "$TESTSLIB"/utils.sh timedatectl set-timezone Etc/UTC - workshop_exec remove ws-20 ws-22 ws-24 ws-26 vm-20 vm-22 vm-24 vm-26 + workshop_exec remove || true execute: | . "$TESTSLIB"/utils.sh - echo "Test launch for all the supported bases" + echo "Test launch for $BASE ${CONFINEMENT}s" + envsubst '${BASE}${CONFINEMENT}' < workshop.yaml.in > workshop.yaml + chown ubuntu:ubuntu workshop.yaml + workshop_exec launch ws + workshop_exec list | MATCH 'ws[[:space:]]*Ready[[:space:]]*-' - function extract_section() { - awk --assign title="[$1]" '/\[/ {p=0} $0 == title {p=1; next} p' - } - - function launch_and_validate() { - workshop_exec launch "$1" - workshop_exec list | MATCH "$1[[:space:]]*Ready[[:space:]]*-" + # ensure user idmap is applied + rm -f build-artifact + workshop_exec exec ws touch build-artifact + stat --format='%U %G' build-artifact | MATCH '^ubuntu ubuntu$' + workshop_exec exec ws sudo chown 0:0 build-artifact + stat --format='%u %g' build-artifact | MATCH '^1000000 1000000$' + workshop_exec exec ws sudo chown 1001:1001 build-artifact + stat --format='%u %g' build-artifact | MATCH '^1001001 1001001$' - # ensure user idmap is applied - rm -f build-artifact - workshop_exec exec "$1" touch build-artifact - stat --format='%U %G' build-artifact | MATCH '^ubuntu ubuntu$' - workshop_exec exec "$1" sudo chown 0:0 build-artifact - stat --format='%u %g' build-artifact | MATCH '^1000000 1000000$' - workshop_exec exec "$1" sudo chown 1001:1001 build-artifact - stat --format='%u %g' build-artifact | MATCH '^1001001 1001001$' + # ensure the workshop has a socket for workshopctl + if [ "$CONFINEMENT" = container ]; then + workshop_exec exec ws test -S /var/lib/workshop/run/workshop.socket.untrusted + fi - # ensure the workshop has a socket for workshopctl - if [[ "$1" == ws-* ]]; then - workshop_exec exec "$1" test -S /var/lib/workshop/run/workshop.socket.untrusted - fi + # ensure the apt cache is mounted + workshop_exec exec ws mountpoint /var/cache/apt/archives + workshop_exec exec ws stat --format='%A %U %G' /var/cache/apt/archives | MATCH '^drwxr-xr-x workshop workshop$' - # ensure the apt cache is mounted - workshop_exec exec "$1" mountpoint /var/cache/apt/archives - workshop_exec exec "$1" stat --format='%A %U %G' /var/cache/apt/archives | MATCH '^drwxr-xr-x workshop workshop$' + # ensure apt is configured to exclude suggested and recommended packages + workshop_exec run ws check-apt - # ensure apt is configured to exclude suggested and recommended packages - workshop_exec run "$1" check-apt + # ensure systemd-resolved is configured for the workshopbr0 network + resolvectl domain workshopbr0 | MATCH '^Link [0-9]+ \(workshopbr0\): ~wp$' + resolvectl query "ws-$(< .workshop.lock).wp" | MATCH 'link: workshopbr0' + resolvectl query --type=CNAME "ws.$(< .workshop.lock).wp" | MATCH "IN CNAME ws-$(< .workshop.lock).wp" + resolvectl query --type=CNAME 'ws.launch.wp' | MATCH "IN CNAME ws-$(< .workshop.lock).wp" - # ensure systemd-resolved is configured for the workshopbr0 network - resolvectl domain workshopbr0 | MATCH '^Link [0-9]+ \(workshopbr0\): ~wp$' - resolvectl query "$1-$(< .workshop.lock).wp" | MATCH 'link: workshopbr0' - resolvectl query --type=CNAME "${1}.$(< .workshop.lock).wp" | MATCH "IN CNAME $1-$(< .workshop.lock).wp" - resolvectl query --type=CNAME "${1}.launch.wp" | MATCH "IN CNAME $1-$(< .workshop.lock).wp" - - # ensure systemd-networkd is configured not to release DHCP on stop - # just skip if `networkctl cat` is missing, since old bases have been manually tested - sd_ver="$(workshop_exec exec "$1" networkctl --version | awk 'FNR == 1 {print $2}')" - if [ "$sd_ver" -ge 254 ]; then - workshop_exec exec "$1" sudo networkctl cat @eth0 > eth0.log - extract_section DHCPv4 < eth0.log | MATCH '^SendRelease=false$' - extract_section DHCPv6 < eth0.log | MATCH '^SendRelease=false$' - fi - - # ensure timezone is inherited from host - workshop_exec exec "$1" timedatectl show | MATCH '^Timezone=Pacific/Auckland$' + # ensure systemd-networkd is configured not to release DHCP on stop + # just skip if `networkctl cat` is missing, since old bases have been manually tested + function extract_section() { + awk --assign title="[$1]" '/\[/ {p=0} $0 == title {p=1; next} p' } - - launch_and_validate ws-20 - launch_and_validate ws-22 - launch_and_validate ws-24 - launch_and_validate ws-26 - launch_and_validate vm-20 - launch_and_validate vm-22 - launch_and_validate vm-24 - launch_and_validate vm-26 + sd_ver="$(workshop_exec exec ws networkctl --version | awk 'FNR == 1 {print $2}')" + if [ "$sd_ver" -ge 254 ]; then + workshop_exec exec ws sudo networkctl cat @eth0 > eth0.log + extract_section DHCPv4 < eth0.log | MATCH '^SendRelease=false$' + extract_section DHCPv6 < eth0.log | MATCH '^SendRelease=false$' + fi - # ensure workshops can resolve each other by name - mkdir -p srv/qwertyuiop - workshop_exec exec ws-22 -- systemd-run --user --working-directory=/project/srv -- python3 -m http.server 8877 - workshop_exec exec vm-24 -- curl http://ws-22:8877 | MATCH qwertyuiop + # ensure timezone is inherited from host + workshop_exec exec ws timedatectl show | MATCH '^Timezone=Pacific/Auckland$' diff --git a/tests/main/launch/.workshop/ws-20.yaml b/tests/main/launch/workshop.yaml.in similarity index 85% rename from tests/main/launch/.workshop/ws-20.yaml rename to tests/main/launch/workshop.yaml.in index 072331f9f..8a727edae 100644 --- a/tests/main/launch/.workshop/ws-20.yaml +++ b/tests/main/launch/workshop.yaml.in @@ -1,5 +1,6 @@ -name: ws-20 -base: ubuntu@20.04 +name: ws +base: ${BASE} +confinement: ${CONFINEMENT} actions: check-apt: | dry_run() { From 8bbe22b037b37d219af91fb32c2188226d24dbb7 Mon Sep 17 00:00:00 2001 From: Jonathan Conder Date: Mon, 31 Aug 2026 15:50:08 +1200 Subject: [PATCH 15/15] Split snapshot integration tests into multiple variants --- .github/workflows/spread.yaml | 17 ++++++++- .../lxd/tests/integration/snapshot_test.go | 38 +++++++++++++------ .../integration/workshop-snapshots/task.yaml | 10 ++++- 3 files changed, 50 insertions(+), 15 deletions(-) diff --git a/.github/workflows/spread.yaml b/.github/workflows/spread.yaml index 675ddb0e7..09f56d8f9 100644 --- a/.github/workflows/spread.yaml +++ b/.github/workflows/spread.yaml @@ -150,10 +150,23 @@ jobs: env: SUITE_PATHS: ${{ matrix.suite.paths }} LXD_CHANNEL: ${{ inputs.lxd_channel || '6/stable' }} + ARTIFACTS: ${{ github.workspace }}.cover run: | - mkdir ${{ github.workspace }}.cover + mkdir "$ARTIFACTS" read -r -a suite_paths <<< "$SUITE_PATHS" - spread -artifacts=${{ github.workspace }}.cover "${suite_paths[@]}" + spread -artifacts="$ARTIFACTS" "${suite_paths[@]}" + + # Workaround https://github.com/actions/upload-artifact/issues/546 + cd "$ARTIFACTS/lxd:ubuntu-24.04:tests" || exit 0 + tasks=(*/*/) + for task in "${tasks[@]}"; do + suite=${task%%/*} + task=${task#*/} + task=${task%/} + safe=${task//:/.} + [ "$task" = "$safe" ] || (cd "$suite" && mv "$task" "$safe") + done + shell: bash - name: Upload Coverage uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: diff --git a/internal/workshop/lxd/tests/integration/snapshot_test.go b/internal/workshop/lxd/tests/integration/snapshot_test.go index e5da4376a..0770cfdc5 100644 --- a/internal/workshop/lxd/tests/integration/snapshot_test.go +++ b/internal/workshop/lxd/tests/integration/snapshot_test.go @@ -310,24 +310,38 @@ func (s *snapshotSuite) snapshotFormat(c *check.C, snapshot workshop.Snapshot) a return inst } +func (s *snapshotSuite) TestLxdBackendSnapshotDiffContainer20(c *check.C) { + s.snapshotDiff(c, "ubuntu@20.04", workshop.ConfinementContainer) +} +func (s *snapshotSuite) TestLxdBackendSnapshotDiffContainer22(c *check.C) { + s.snapshotDiff(c, "ubuntu@22.04", workshop.ConfinementContainer) +} +func (s *snapshotSuite) TestLxdBackendSnapshotDiffContainer24(c *check.C) { + s.snapshotDiff(c, "ubuntu@24.04", workshop.ConfinementContainer) +} +func (s *snapshotSuite) TestLxdBackendSnapshotDiffContainer26(c *check.C) { + s.snapshotDiff(c, "ubuntu@26.04", workshop.ConfinementContainer) +} +func (s *snapshotSuite) TestLxdBackendSnapshotDiffVM20(c *check.C) { + s.snapshotDiff(c, "ubuntu@20.04", workshop.ConfinementVirtualMachine) +} +func (s *snapshotSuite) TestLxdBackendSnapshotDiffVM22(c *check.C) { + s.snapshotDiff(c, "ubuntu@22.04", workshop.ConfinementVirtualMachine) +} +func (s *snapshotSuite) TestLxdBackendSnapshotDiffVM24(c *check.C) { + s.snapshotDiff(c, "ubuntu@24.04", workshop.ConfinementVirtualMachine) +} +func (s *snapshotSuite) TestLxdBackendSnapshotDiffVM26(c *check.C) { + s.snapshotDiff(c, "ubuntu@26.04", workshop.ConfinementVirtualMachine) +} + // Launches 2 workshops from scratch and another from a snapshot of the first, // then checks that the third workshop is indistinguishable from the other two. -func (s *snapshotSuite) TestLxdBackendSnapshotDiff(c *check.C) { +func (s *snapshotSuite) snapshotDiff(c *check.C, base string, confinement workshop.Confinement) { if os.Geteuid() != 0 { c.Skip("requires root to mount and compare workshop filesystems") } - for _, confinement := range []workshop.Confinement{workshop.ConfinementContainer, workshop.ConfinementVirtualMachine} { - kind, err := confinement.MarshalText() - c.Assert(err, check.IsNil) - for _, base := range workshop.SupportedBases { - c.Logf("Testing snapshot integrity for %s %ss", base, kind) - s.snapshotDiff(c, base, confinement) - } - } -} - -func (s *snapshotSuite) snapshotDiff(c *check.C, base string, confinement workshop.Confinement) { // Download base image. image, err := s.bd.GetBase(s.ctx, base, confinement) c.Assert(err, check.IsNil) diff --git a/tests/integration/workshop-snapshots/task.yaml b/tests/integration/workshop-snapshots/task.yaml index 0c9d02c57..0bba65d72 100644 --- a/tests/integration/workshop-snapshots/task.yaml +++ b/tests/integration/workshop-snapshots/task.yaml @@ -1,10 +1,18 @@ summary: Run LXD integration tests for workshop snapshots +environment: + METHOD/format: TestLxdBackendSnapshotFormat + # TODO: enable VM variants + METHOD/ctr20: TestLxdBackendSnapshotDiffContainer20 + METHOD/ctr22: TestLxdBackendSnapshotDiffContainer22 + METHOD/ctr24: TestLxdBackendSnapshotDiffContainer24 + METHOD/ctr26: TestLxdBackendSnapshotDiffContainer26 + execute: | rm -rf cover mkdir cover - go test -cover "$SPREAD_PATH"/internal/workshop/lxd/tests/integration -timeout=30m -tags=integration -check.v -check.f snapshotSuite -coverpkg=github.com/canonical/workshop/internal/workshop/lxd -args -test.gocoverdir="$PWD"/cover + go test -cover "$SPREAD_PATH"/internal/workshop/lxd/tests/integration -timeout=30m -tags=integration -check.v -check.f "snapshotSuite.$METHOD" -coverpkg=github.com/canonical/workshop/internal/workshop/lxd -args -test.gocoverdir="$PWD"/cover artifacts: - cover