cli: refactor sqldb ns to use docker volumes

We've observed permission-related issues when using `colima`
with volume mounts. See if it works better with docker volumes.

cli/daemon/db.go

@@ -98,10 +98,10 @@ func (s *Server) dbConnectLocal(ctx context.Context, req *daemonpb.DBConnectRequ
 	}
 
 	clusterType := sqldb.Run
-	passwd := "local-" + string(clusterNS.Name)
+	passwd := "local-" + string(clusterNS.ID)
 	if req.Test {
 		clusterType = sqldb.Test
-		passwd = "test-" + string(clusterNS.Name)
+		passwd = "test-" + string(clusterNS.ID)
 	}
 
 	clusterID := sqldb.GetClusterID(app, clusterType, clusterNS)

cli/daemon/namespace/namespace.go

@@ -187,6 +187,7 @@ func (m *Manager) Delete(ctx context.Context, app *apps.Instance, name Name) err
 	if ns.Active {
 		return ErrActive
 	}
+	ns.App = app
 
 	// Check all the deletion handlers.
 	for _, h := range m.handlers {
@@ -282,6 +283,7 @@ func (m *Manager) GetActive(ctx context.Context, app *apps.Instance) (*Namespace
 	if err != nil && !errors.Is(err, sql.ErrNoRows) {
 		return nil, err
 	} else if err == nil {
+		ns.App = app
 		return &ns, nil
 	}
 

cli/daemon/sqldb/cluster.go

@@ -205,12 +205,12 @@ func (c *Cluster) initDBs(md *meta.Data, reinit bool) {
 func (c *Cluster) initDB(encoreName string) *DB {
 	driverName := encoreName
 	if !c.driver.Meta().ClusterIsolation {
-		driverName += fmt.Sprintf("-%s-%s", c.ID.App.PlatformOrLocalID(), c.ID.Type)
+		driverName += fmt.Sprintf("-%s-%s", c.ID.NS.App.PlatformOrLocalID(), c.ID.Type)
 
 		// Add the namespace id, as long as it's not the default namespace
 		// (for backwards compatibility).
-		if c.ID.NSName != "default" {
-			driverName += "-" + string(c.ID.NSID)
+		if c.ID.NS.Name != "default" {
+			driverName += "-" + string(c.ID.NS.ID)
 		}
 	}
 

cli/daemon/sqldb/docker/docker.go

@@ -8,17 +8,15 @@ import (
 	"io"
 	"os"
 	"os/exec"
-	"path/filepath"
 	"strings"
 	"time"
 
-	"github.com/cenkalti/backoff/v4"
 	"github.com/cockroachdb/errors"
 	"github.com/jackc/pgx/v5"
 	"github.com/rs/zerolog"
 
+	"encr.dev/cli/daemon/namespace"
 	"encr.dev/cli/daemon/sqldb"
-	"encr.dev/internal/conf"
 	"encr.dev/pkg/idents"
 )
 
@@ -149,15 +147,12 @@ func (d *Driver) CreateCluster(ctx context.Context, p *sqldb.CreateParams, log z
 				"-c", "fsync=off",
 			)
 		} else {
-			clusterDataDir, err := ClusterDataDir(cid)
-			if err != nil {
-				return nil, err
-			} else if err := os.MkdirAll(filepath.Dir(clusterDataDir), 0o755); err != nil {
-				return nil, errors.Wrap(err, "could not create cluster data dir")
+			volumeName := clusterVolumeNames(p.ClusterID.NS)[0] // guaranteed to be non-empty
+			if err := d.createVolumeIfNeeded(ctx, volumeName); err != nil {
+				return nil, errors.Wrap(err, "create data volume")
 			}
-
 			args = append(args,
-				"-v", fmt.Sprintf("%s:%s", clusterDataDir, defaultDataDir),
+				"-v", fmt.Sprintf("%s:%s", volumeName, defaultDataDir),
 				Image)
 		}
 
@@ -282,11 +277,6 @@ func (d *Driver) CanDestroyCluster(ctx context.Context, id sqldb.ClusterID) erro
 }
 
 func (d *Driver) DestroyCluster(ctx context.Context, id sqldb.ClusterID) error {
-	dataDir, err := ClusterDataDir(id)
-	if err != nil {
-		return err
-	}
-
 	cnames := containerNames(id)
 	for _, cname := range cnames {
 		out, err := exec.CommandContext(ctx, "docker", "rm", "-f", cname).CombinedOutput()
@@ -297,13 +287,28 @@ func (d *Driver) DestroyCluster(ctx context.Context, id sqldb.ClusterID) error {
 			return errors.Wrapf(err, "could not delete cluster: %s", out)
 		}
 	}
+	return nil
+}
 
-	// Delete the data dir. Retry a few times, mainly for Windows.
-	b := backoff.NewExponentialBackOff()
-	b.MaxElapsedTime = 5 * time.Second
-	return backoff.Retry(func() error {
-		return os.RemoveAll(dataDir)
-	}, b)
+func (d *Driver) DestroyNamespaceData(ctx context.Context, ns *namespace.Namespace) error {
+	candidates := clusterVolumeNames(ns)
+	for _, c := range candidates {
+		if err := exec.CommandContext(ctx, "docker", "volume", "rm", "-f", c).Run(); err != nil {
+			if strings.Contains(strings.ToLower(err.Error()), "no such volume") {
+				continue
+			}
+			return errors.Wrapf(err, "could not delete volume %s", c)
+		}
+	}
+	return nil
+}
+
+func (d *Driver) createVolumeIfNeeded(ctx context.Context, name string) error {
+	if err := exec.CommandContext(ctx, "docker", "volume", "inspect", name).Run(); err == nil {
+		return nil
+	}
+	out, err := exec.CommandContext(ctx, "docker", "volume", "create", name).CombinedOutput()
+	return errors.Wrapf(err, "create volume %s: %s", name, out)
 }
 
 func (d *Driver) Meta() sqldb.DriverMeta {
@@ -321,22 +326,22 @@ func containerNames(id sqldb.ClusterID) []string {
 		}
 
 		// Convert the namespace to kebab case to remove invalid characters like ':'.
-		nsName := idents.Convert(string(id.NSName), idents.KebabCase)
+		nsName := idents.Convert(string(id.NS.Name), idents.KebabCase)
 
-		names = []string{base + "-" + nsName + "-" + string(id.NSID)}
+		names = []string{base + "-" + nsName + "-" + string(id.NS.ID)}
 		// If this is the default namespace look up the container without
 		// the namespace suffix as well, for backwards compatibility.
-		if id.NSName == "default" {
+		if id.NS.Name == "default" {
 			names = append(names, base)
 		}
 		return names
 	}
 
 	var names []string
-	if pid := id.App.PlatformID(); pid != "" {
+	if pid := id.NS.App.PlatformID(); pid != "" {
 		names = append(names, candidates(pid)...)
 	}
-	names = append(names, candidates(id.App.LocalID())...)
+	names = append(names, candidates(id.NS.App.LocalID())...)
 	return names
 }
 
@@ -368,13 +373,15 @@ func isDockerRunning(ctx context.Context) bool {
 	return err == nil
 }
 
-// ClusterDataDir reports the data directory for the given cluster id.
-func ClusterDataDir(id sqldb.ClusterID) (string, error) {
-	dataDir, err := conf.DataDir()
-	if err != nil {
-		return "", errors.Wrap(err, "unable to determine database dir")
-	}
+// clusterVolumeName reports the candidate names for the docker volume.
+func clusterVolumeNames(ns *namespace.Namespace) (candidates []string) {
+	nsName := idents.Convert(string(ns.Name), idents.KebabCase)
+	suffix := fmt.Sprintf("%s-%s", ns.ID, nsName)
 
-	name := fmt.Sprintf("%s-%s", id.App.PlatformOrLocalID(), id.Type)
-	return filepath.Join(dataDir, string(id.NSID), "sqldb", name), nil
+	for _, id := range [...]string{ns.App.PlatformID(), ns.App.LocalID()} {
+		if id != "" {
+			candidates = append(candidates, fmt.Sprintf("sqldb-%s-%s", id, suffix))
+		}
+	}
+	return candidates
 }

cli/daemon/sqldb/driver.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/rs/zerolog"
 
+	"encr.dev/cli/daemon/namespace"
 	"encr.dev/internal/optracker"
 )
 
@@ -26,6 +27,10 @@ type Driver interface {
 	// If a Driver doesn't support destroying the cluster it reports ErrUnsupported.
 	DestroyCluster(ctx context.Context, id ClusterID) error
 
+	// DestroyNamespaceData destroys the data associated with a namespace.
+	// If a Driver doesn't support destroying data it reports ErrUnsupported.
+	DestroyNamespaceData(ctx context.Context, ns *namespace.Namespace) error
+
 	// ClusterStatus reports the current status of a cluster.
 	ClusterStatus(ctx context.Context, id ClusterID) (*ClusterStatus, error)
 

cli/daemon/sqldb/external/external.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/rs/zerolog"
 
+	"encr.dev/cli/daemon/namespace"
 	"encr.dev/cli/daemon/sqldb"
 )
 
@@ -47,6 +48,10 @@ func (d *Driver) DestroyCluster(ctx context.Context, id sqldb.ClusterID) error {
 	return sqldb.ErrUnsupported
 }
 
+func (d *Driver) DestroyNamespaceData(ctx context.Context, ns *namespace.Namespace) error {
+	return sqldb.ErrUnsupported
+}
+
 func (d *Driver) CheckRequirements(ctx context.Context) error {
 	return nil
 }

cli/daemon/sqldb/manager.go

@@ -4,9 +4,10 @@ import (
 	"context"
 	"crypto/rand"
 	"encoding/base64"
-	"errors"
+	"fmt"
 	"sync"
 
+	"github.com/cockroachdb/errors"
 	"github.com/rs/zerolog"
 	"github.com/rs/zerolog/log"
 	"golang.org/x/sync/singleflight"
@@ -46,15 +47,19 @@ type ClusterManager struct {
 
 // ClusterID uniquely identifies a cluster.
 type ClusterID struct {
-	App  *apps.Instance
+	NS   *namespace.Namespace
 	Type ClusterType
+}
+
+// clusterKey is the key to use to store a cluster in the cluster map.
+type clusterKey string
 
-	NSID   namespace.ID
-	NSName namespace.Name
+func (id ClusterID) clusterKey() clusterKey {
+	return clusterKey(fmt.Sprintf("%s-%s", id.NS.ID, id.Type))
 }
 
 func GetClusterID(app *apps.Instance, typ ClusterType, ns *namespace.Namespace) ClusterID {
-	return ClusterID{app, typ, ns.ID, ns.Name}
+	return ClusterID{ns, typ}
 }
 
 // Ready reports whether the cluster manager is ready and all requirements are met.
@@ -80,7 +85,7 @@ func (cm *ClusterManager) Create(ctx context.Context, params *CreateParams) *Clu
 
 	if !ok {
 		ctx, cancel := context.WithCancel(context.Background())
-		key := clusterKeys(params.ClusterID)[0] // guaranteed to be non-empty
+		key := params.ClusterID.clusterKey()
 		passwd := genPassword()
 		c = &Cluster{
 			ID:       params.ClusterID,
@@ -123,12 +128,8 @@ func (cm *ClusterManager) Get(id ClusterID) (*Cluster, bool) {
 // get retrieves the cluster keyed by id.
 // cm.mu must be held.
 func (cm *ClusterManager) get(id ClusterID) (*Cluster, bool) {
-	for _, key := range clusterKeys(id) {
-		if c, ok := cm.clusters[key]; ok {
-			return c, true
-		}
-	}
-	return nil, false
+	c, ok := cm.clusters[id.clusterKey()]
+	return c, ok
 }
 
 // CanDeleteNamespace implements namespace.DeletionHandler.
@@ -147,33 +148,33 @@ func (cm *ClusterManager) CanDeleteNamespace(ctx context.Context, app *apps.Inst
 
 // DeleteNamespace implements namespace.DeletionHandler.
 func (cm *ClusterManager) DeleteNamespace(ctx context.Context, app *apps.Instance, ns *namespace.Namespace) error {
-	c, ok := cm.Get(GetClusterID(app, Run, ns))
-	if !ok {
-		return nil
+	// Find all clusters matching this namespace.
+	// Use a closure for the lock to avoid holding it while we destroy the clusters.
+	var clusters []*Cluster
+	(func() {
+		cm.mu.Lock()
+		defer cm.mu.Unlock()
+		for _, c := range cm.clusters {
+			if c.ID.NS.ID == ns.ID {
+				clusters = append(clusters, c)
+			}
+		}
+	})()
+
+	// Destroy the clusters.
+	for _, c := range clusters {
+		if err := c.driver.DestroyCluster(ctx, c.ID); err != nil && !errors.Is(err, ErrUnsupported) {
+			return errors.Wrapf(err, "destroy cluster %s", c.ID)
+		}
+		c.cancel()
 	}
 
-	err := c.driver.DestroyCluster(ctx, c.ID)
+	// If that succeeded, destroy the namespace data.
+	err := cm.driver.DestroyNamespaceData(ctx, ns)
 	if errors.Is(err, ErrUnsupported) {
 		err = nil
 	}
-	return nil
-}
-
-type clusterKey string
-
-// clusterKeys computes clusterKey candidates for a given id.
-func clusterKeys(id ClusterID) []clusterKey {
-	typeSuffix := "-" + string(id.Type)
-	nsSuffix := "-" + string(id.NSID)
-
-	var keys []clusterKey
-
-	if pid := id.App.PlatformID(); pid != "" {
-		keys = append(keys, clusterKey(pid+typeSuffix+nsSuffix))
-	}
-	keys = append(keys, clusterKey(id.App.LocalID()+typeSuffix+nsSuffix))
-
-	return keys
+	return err
 }
 
 func genPassword() string {

cli/daemon/sqldb/proxy.go

@@ -97,14 +97,14 @@ func (cm *ClusterManager) ProxyConn(client net.Conn, waitForSetup bool) error {
 
 		ctx := context.Background()
 
-		clusterType, nsName, ok := strings.Cut(startup.Password, "-")
+		clusterType, nsID, ok := strings.Cut(startup.Password, "-")
 
 		// Look up the namespace to use.
 		var ns *namespace.Namespace
 		if !ok {
 			ns, err = cm.ns.GetActive(ctx, app)
 		} else {
-			ns, err = cm.ns.GetByName(ctx, app, namespace.Name(nsName))
+			ns, err = cm.ns.GetByID(ctx, app, namespace.ID(nsID))
 		}
 		if err != nil {
 			cm.log.Error().Err(err).Msg("dbproxy: could not find infra namespace")