diff --git a/docs/contributing/development.rst b/docs/contributing/development.rst index df87101bc..e2b105cdd 100644 --- a/docs/contributing/development.rst +++ b/docs/contributing/development.rst @@ -48,7 +48,7 @@ Your work is split across two environments: and the LXD integration tests. :program:`workshopd` connects to LXD -and expects the ZFS storage driver, +and expects the ZFS or btrfs storage driver, neither of which the :samp:`dev` workshop ships, so the host-side commands run outside it. diff --git a/docs/explanation/architecture/components.rst b/docs/explanation/architecture/components.rst index 113f85b12..7a1d7fd62 100644 --- a/docs/explanation/architecture/components.rst +++ b/docs/explanation/architecture/components.rst @@ -259,8 +259,9 @@ used for temporary storage during workshop rebuild operations. Storage backends ~~~~~~~~~~~~~~~~ -|ws_markup| uses ZFS for storage on Linux, -with automatic Btrfs fallback on Windows Subsystem for Linux (WSL). +|ws_markup| uses ZFS for storage where it detects that ZFS is available, +with automatic btrfs fallback otherwise, +for example on Windows Subsystem for Linux (WSL). Storage is managed via LXD and requires a minimum pool size of 5 GiB. The storage backend manages container root filesystems, workshop-specific data volumes, diff --git a/docs/reference/workshops.rst b/docs/reference/workshops.rst index aa9bdd7cd..e6b67d9be 100644 --- a/docs/reference/workshops.rst +++ b/docs/reference/workshops.rst @@ -187,8 +187,9 @@ Storage pools and drivers ------------------------- |ws_markup| stores its containers and data on a storage pool. -On Linux, |ws_markup| uses ZFS, -while on Windows Subsystem for Linux it automatically uses Btrfs. +It uses ZFS on systems where |ws_markup| detects that ZFS is available, +and otherwise falls back to btrfs +(for example on Windows Subsystem for Linux). This approach consolidates container images, :program:`apt` caches, SDKs and other workshop content under a single system. diff --git a/docs/tutorial/part-1-get-started.rst b/docs/tutorial/part-1-get-started.rst index 57a07327d..d8f8210fc 100644 --- a/docs/tutorial/part-1-get-started.rst +++ b/docs/tutorial/part-1-get-started.rst @@ -47,8 +47,9 @@ Prerequisites |ws_markup| is supported on Ubuntu and other :program:`snap`-enabled Linux distributions; -it is also compatible with Windows Subsystem for Linux (WSL2), -where it uses Btrfs instead of ZFS for storage. +it is also compatible with Windows Subsystem for Linux (WSL2). +For storage, it uses ZFS where |ws_markup| detects that ZFS is available, +and otherwise falls back to btrfs. |ws_markup| relies on `LXD 6.8+ `_ diff --git a/internal/workshop/lxd/export_test.go b/internal/workshop/lxd/export_test.go index 9ccf39553..1cf064f9f 100644 --- a/internal/workshop/lxd/export_test.go +++ b/internal/workshop/lxd/export_test.go @@ -23,8 +23,21 @@ var ( HandleImageUpdate = handleImageUpdate CheckServerVersion = checkVersion GenerateCNAME = generateCNAME + PreferredDriver = preferredDriver + CheckStoragePool = checkStoragePool + AdoptPoolDriver = adoptPoolDriver + PoolDriver = poolDriver ) +// MockPoolDriver clears the recorded storage pool driver and returns a +// function restoring it. +func MockPoolDriver() (restore func()) { + old := storagePoolDriver.Swap(nil) + return func() { + storagePoolDriver.Store(old) + } +} + func MockFirewallChecker(f func(string) string) func() { old := firewallChecker firewallChecker = f diff --git a/internal/workshop/lxd/lxd_backend.go b/internal/workshop/lxd/lxd_backend.go index c0980166a..018e16ade 100644 --- a/internal/workshop/lxd/lxd_backend.go +++ b/internal/workshop/lxd/lxd_backend.go @@ -30,6 +30,8 @@ import ( "slices" "strconv" "strings" + "sync" + "sync/atomic" "time" lxd "github.com/canonical/lxd/client" @@ -82,16 +84,38 @@ const ( ) var ( - storagePoolDriver = "zfs" + // storageDrivers lists the supported pool drivers, most preferred first. + storageDrivers = []string{"zfs", "btrfs"} + + storagePoolDriver atomic.Pointer[string] + + // backendReadyMu serializes ensureBackendReady. + backendReadyMu sync.Mutex workshopFormatsChecked = false ) -func init() { - if osutil.IsWSL() { - storagePoolDriver = "btrfs" +// poolDriver returns the storage pool driver, or "" if unknown. +func poolDriver() string { + if driver := storagePoolDriver.Load(); driver != nil { + return *driver + } + return "" +} + +// adoptPoolDriver records driver as the storage pool driver if supported. +func adoptPoolDriver(driver string) error { + if !slices.Contains(storageDrivers, driver) { + return fmt.Errorf("cannot use storage pool %q: unsupported driver %q (expected one of %s)", storagePool, driver, strings.Join(storageDrivers, ", ")) } + if old := poolDriver(); old != "" && old != driver { + logger.Noticef("Storage pool %q changed driver from %q to %q", storagePool, old, driver) + } + storagePoolDriver.Store(&driver) + return nil +} +func init() { // Order matters: capabilities (version and storage) must be validated // before ensureBackendReady attempts to create the storage pool and // network. Registering it as a check also lets the daemon recover after @@ -102,8 +126,7 @@ func init() { syscheck.RegisterCheck(checkStorageSpace) } -type Backend struct { -} +type Backend struct{} var _ workshop.Backend = (*Backend)(nil) @@ -174,19 +197,30 @@ func checkVersion(version string) error { return nil } -func checkStorageDriver(drivers []api.ServerStorageDriverInfo) error { - hasDriver := func(driver api.ServerStorageDriverInfo) bool { - return driver.Name == storagePoolDriver +// preferredDriver returns the first supported storage driver in order of +// preference (ZFS, then btrfs), or an error if none is supported. +func preferredDriver(supported []api.ServerStorageDriverInfo) (string, error) { + for _, driver := range storageDrivers { + if slices.ContainsFunc(supported, func(d api.ServerStorageDriverInfo) bool { + return d.Name == driver + }) { + return driver, nil + } } - if slices.ContainsFunc(drivers, hasDriver) { + return "", fmt.Errorf("suitable storage backend not found: none of %s is available", strings.Join(storageDrivers, ", ")) +} + +// checkStoragePool checks that the storage pool is usable or can be created. +func checkStoragePool(conn lxd.InstanceServer, supported []api.ServerStorageDriverInfo) error { + if _, _, err := conn.GetStoragePool(storagePool); err == nil { return nil + } else if !api.StatusErrorCheck(err, http.StatusNotFound) { + return fmt.Errorf("%q storage pool unavailable: %w", storagePool, err) } - // The LXD error message when creating a pool is: - // Error: Error loading "zfs" module: Failed to run: modprobe -b zfs: - // exit status 1 (modprobe: FATAL: Module zfs not found ...) - // We keep the first part for consistency, the rest doesn't add much. - return fmt.Errorf(`suitable storage backend not found: error loading %q module`, storagePoolDriver) + // The pool is missing; make sure it can be created. + _, err := preferredDriver(supported) + return err } // checkStorageSpace puts the daemon into degraded mode when the workshop @@ -250,7 +284,7 @@ func checkServerCapabilities() error { workshop.WorkshopVMsSupportSDKs.Store(checkVMsSupportSDKs(conn)) - return checkStorageDriver(info.Environment.StorageSupportedDrivers) + return checkStoragePool(conn, info.Environment.StorageSupportedDrivers) } // vmDiskShiftSupported checks whether LXD supports mounting shifted SDK @@ -315,6 +349,9 @@ func New() (*Backend, error) { // ensureBackendReady creates the LXD storage pool and network the backend // relies on, it is idempotent. func ensureBackendReady() error { + backendReadyMu.Lock() + defer backendReadyMu.Unlock() + // TODO: consider separating the storage pool or network for different users. conn, err := lxd.ConnectLXDUnix("", nil) if err != nil { @@ -322,15 +359,23 @@ func ensureBackendReady() error { } defer conn.Disconnect() - // Create LXD storage pool if it doesn't exist. - pools, err := conn.GetStoragePools() - if err != nil { - return err - } - if idx := slices.IndexFunc(pools, func(p api.StoragePool) bool { return p.Name == storagePool }); idx < 0 { + // Create the LXD storage pool if it doesn't exist, and record its driver so + // pool operations can reuse it. + existingPool, _, err := conn.GetStoragePool(storagePool) + if api.StatusErrorCheck(err, http.StatusNotFound) { + // Pick from the drivers LXD supports now, as they may have changed + // since the pool was last created. + info, _, err := conn.GetServer() + if err != nil { + return err + } + driver, err := preferredDriver(info.Environment.StorageSupportedDrivers) + if err != nil { + return err + } req := api.StoragePoolsPost{ Name: storagePool, - Driver: storagePoolDriver, + Driver: driver, } op, err := conn.CreateStoragePool(req) if err != nil { @@ -339,6 +384,9 @@ func ensureBackendReady() error { if err := op.Wait(); err != nil { return err } + if err := adoptPoolDriver(driver); err != nil { + return err + } // Ensure the new pool has enough total space available. pool, etag, err := conn.GetStoragePool(storagePool) @@ -369,8 +417,10 @@ func ensureBackendReady() error { logger.Noticef("On ensureBackendReady: set storage pool to the minimal size: %dGiB", storagePoolMinimalGiB) } - } else if pools[idx].Driver != storagePoolDriver { - return fmt.Errorf("storage pool %q already exists with a different driver: %q (expected %q)", storagePool, pools[idx].Driver, storagePoolDriver) + } else if err != nil { + return err + } else if err := adoptPoolDriver(existingPool.Driver); err != nil { + return err } network, etag, err := conn.GetNetwork(networkName) @@ -1014,7 +1064,7 @@ func (s *Backend) execCommand(conn lxd.InstanceServer, ctx context.Context, name } opmeta := op.Get() - var env = map[string]string{} + env := map[string]string{} for k, v := range opmeta.Metadata["environment"].(map[string]any) { if value, ok := v.(string); ok { env[k] = value @@ -1038,7 +1088,7 @@ func (s *Backend) execCommand(conn lxd.InstanceServer, ctx context.Context, name // unconditionally right after the operation has exited, so it will not be // blocked if we are here <-done - var status = int(op.Get().Metadata["return"].(float64)) + status := int(op.Get().Metadata["return"].(float64)) if status != 0 { return &workshop.ErrExec{Status: status} } diff --git a/internal/workshop/lxd/lxd_backend_snapshots.go b/internal/workshop/lxd/lxd_backend_snapshots.go index 69544d502..b96528318 100644 --- a/internal/workshop/lxd/lxd_backend_snapshots.go +++ b/internal/workshop/lxd/lxd_backend_snapshots.go @@ -731,7 +731,7 @@ func mergeDevices(source map[string]map[string]string, sdks []sdk.ContentID, w s return k != "root" }) - if storagePoolDriver == "zfs" { + if poolDriver() == "zfs" { root := maps.Clone(source["root"]) if source == nil || root == nil { return fmt.Errorf("internal error: %q workshop has no rootfs", w) @@ -862,7 +862,7 @@ func (s *Backend) copyInstance(src, dst lxd.InstanceServer, srcName, dstName str req := *srcInst - if storagePoolDriver == "zfs" { + if poolDriver() == "zfs" { req.Devices = maps.Clone(req.Devices) root := maps.Clone(req.Devices["root"]) if req.Devices == nil || root == nil { diff --git a/internal/workshop/lxd/lxd_backend_test.go b/internal/workshop/lxd/lxd_backend_test.go index 91983d28f..6e01a3e27 100644 --- a/internal/workshop/lxd/lxd_backend_test.go +++ b/internal/workshop/lxd/lxd_backend_test.go @@ -18,8 +18,11 @@ import ( "crypto/sha3" "encoding/hex" "fmt" + "net/http" "testing" + lxd "github.com/canonical/lxd/client" + "github.com/canonical/lxd/shared/api" "gopkg.in/check.v1" "github.com/canonical/workshop/internal/sdk" @@ -262,3 +265,85 @@ func (f *LxdBeTests) TestCheckLxdVersion(c *check.C) { err = lxdbackend.CheckServerVersion("6.7.9") c.Assert(err, check.ErrorMatches, `(?s).*LXD server version.*is not supported.*`) } + +func driverInfo(names ...string) []api.ServerStorageDriverInfo { + infos := make([]api.ServerStorageDriverInfo, 0, len(names)) + for _, n := range names { + infos = append(infos, api.ServerStorageDriverInfo{Name: n}) + } + return infos +} + +// preferredDriver keeps ZFS whenever LXD reports it, falls back to Btrfs +// otherwise, and errors when neither is supported. It trusts LXD's +// supported-drivers list (which LXD builds by trying to load each driver's +// module) rather than probing modules. +func (f *LxdBeTests) TestPreferredDriver(c *check.C) { + driver, err := lxdbackend.PreferredDriver(driverInfo("zfs", "btrfs", "dir")) + c.Check(err, check.IsNil) + c.Check(driver, check.Equals, "zfs") + + driver, err = lxdbackend.PreferredDriver(driverInfo("btrfs", "zfs")) + c.Check(err, check.IsNil) + c.Check(driver, check.Equals, "zfs") + + driver, err = lxdbackend.PreferredDriver(driverInfo("btrfs", "dir")) + c.Check(err, check.IsNil) + c.Check(driver, check.Equals, "btrfs") + + driver, err = lxdbackend.PreferredDriver(driverInfo("dir")) + c.Check(err, check.ErrorMatches, "suitable storage backend not found.*") + c.Check(driver, check.Equals, "") + + driver, err = lxdbackend.PreferredDriver(nil) + c.Check(err, check.ErrorMatches, "suitable storage backend not found.*") + c.Check(driver, check.Equals, "") +} + +// poolServer is an lxd.InstanceServer whose GetStoragePool returns err. +type poolServer struct { + lxd.InstanceServer + err error +} + +func (s poolServer) GetStoragePool(string) (*api.StoragePool, string, error) { + if s.err != nil { + return nil, "", s.err + } + return &api.StoragePool{}, "", nil +} + +func (f *LxdBeTests) TestCheckStoragePool(c *check.C) { + notFound := api.StatusErrorf(http.StatusNotFound, "not found") + + // An existing pool doesn't need a supported driver to be listed. + c.Check(lxdbackend.CheckStoragePool(poolServer{}, nil), check.IsNil) + + // A missing pool is fine as long as it can be created. + c.Check(lxdbackend.CheckStoragePool(poolServer{err: notFound}, driverInfo("btrfs")), check.IsNil) + + // A missing pool with no usable driver can't be created. + err := lxdbackend.CheckStoragePool(poolServer{err: notFound}, driverInfo("dir")) + c.Check(err, check.ErrorMatches, "suitable storage backend not found.*") + + // Any other error means the existing pool is unusable. + err = lxdbackend.CheckStoragePool(poolServer{err: fmt.Errorf("zfs module not loaded")}, driverInfo("zfs")) + c.Check(err, check.ErrorMatches, `"workshop" storage pool unavailable: zfs module not loaded`) +} + +func (f *LxdBeTests) TestAdoptPoolDriver(c *check.C) { + defer lxdbackend.MockPoolDriver()() + c.Check(lxdbackend.PoolDriver(), check.Equals, "") + + c.Assert(lxdbackend.AdoptPoolDriver("zfs"), check.IsNil) + c.Check(lxdbackend.PoolDriver(), check.Equals, "zfs") + + // A pool recreated on another supported driver is adopted. + c.Assert(lxdbackend.AdoptPoolDriver("btrfs"), check.IsNil) + c.Check(lxdbackend.PoolDriver(), check.Equals, "btrfs") + + // An unsupported driver is rejected and the recorded one is kept. + err := lxdbackend.AdoptPoolDriver("dir") + c.Check(err, check.ErrorMatches, `cannot use storage pool "workshop": unsupported driver "dir" \(expected one of zfs, btrfs\)`) + c.Check(lxdbackend.PoolDriver(), check.Equals, "btrfs") +}