Enable sagas by default, keeping the flag as an escape hatch

cli/cli.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"log/slog"
 	"os"
 	"strings"
 
@@ -23,9 +24,11 @@ func Run(args []string) int {
 		labs.EnableAll()
 	}
 
-	// Initialize labs feature flags from environment before registering commands
+	// Initialize labs feature flags from environment before registering commands.
+	// Discard the log: this only decides which commands get registered, and the
+	// server logs the same thing properly when it starts.
 	if labsEnv := os.Getenv("MIREN_LABS"); labsEnv != "" {
-		labs.Init(nil, strings.Split(labsEnv, ","))
+		labs.Init(slog.New(slog.DiscardHandler), strings.Split(labsEnv, ","))
 	}
 
 	d := mflags.NewDispatcher("miren")

controllers/sandbox/create_saga.go

@@ -253,7 +253,7 @@ func bootTask(ctx context.Context, in bootTaskIn) (bootTaskOut, error) {
 		return bootTaskOut{}, fmt.Errorf("booting task: %w", err)
 	}
 
-	rootSpec, err := container.Spec(ctx)
+	rootSpec, err := deps.runtime.ContainerSpec(ctx, container)
 	if err != nil {
 		return bootTaskOut{}, fmt.Errorf("getting container spec: %w", err)
 	}

controllers/sandbox/create_saga_test.go

@@ -11,6 +11,7 @@ import (
 	"time"
 
 	apitypes "github.com/containerd/containerd/api/types"
+	"github.com/containerd/containerd/namespaces"
 	containerd "github.com/containerd/containerd/v2/client"
 	"github.com/containerd/containerd/v2/core/containers"
 	"github.com/containerd/containerd/v2/pkg/cio"
@@ -183,6 +184,12 @@ func (m *mockContainerRuntime) CreateContainer(ctx context.Context, id string, o
 	return id, nil
 }
 
+// ContainerSpec mirrors sandboxOps: the namespace is the adapter's job to
+// supply, not the saga action's.
+func (m *mockContainerRuntime) ContainerSpec(ctx context.Context, cont containerd.Container) (*oci.Spec, error) {
+	return cont.Spec(namespaces.WithNamespace(ctx, "miren-test"))
+}
+
 func (m *mockContainerRuntime) LoadContainer(ctx context.Context, id string) (containerd.Container, error) {
 	m.mu.Lock()
 	defer m.mu.Unlock()
@@ -324,6 +331,10 @@ type sagaMockContainer struct {
 	id     string
 	spec   *oci.Spec
 	taskFn func(context.Context, cio.Attach) (containerd.Task, error)
+
+	// specNamespace records the containerd namespace the last Spec call
+	// arrived with, so tests can assert the caller supplied one.
+	specNamespace string
 }
 
 func (c *sagaMockContainer) ID() string { return c.id }
@@ -334,7 +345,16 @@ func (c *sagaMockContainer) Delete(context.Context, ...containerd.DeleteOpts) er
 func (c *sagaMockContainer) NewTask(context.Context, cio.Creator, ...containerd.NewTaskOpts) (containerd.Task, error) {
 	return nil, nil
 }
+
+// Spec refuses an un-namespaced context the way containerd does, so a caller
+// that goes around sandboxOps fails here instead of silently passing on a
+// client that happens to have a default namespace configured.
 func (c *sagaMockContainer) Spec(ctx context.Context) (*oci.Spec, error) {
+	ns, ok := namespaces.Namespace(ctx)
+	if !ok {
+		return nil, fmt.Errorf("namespace is required: %w", errdefs.ErrFailedPrecondition)
+	}
+	c.specNamespace = ns
 	return c.spec, nil
 }
 func (c *sagaMockContainer) Task(ctx context.Context, attach cio.Attach) (containerd.Task, error) {

controllers/sandbox/interface.go

@@ -6,6 +6,7 @@ import (
 	"time"
 
 	containerd "github.com/containerd/containerd/v2/client"
+	"github.com/containerd/containerd/v2/pkg/oci"
 
 	compute "miren.dev/runtime/api/compute/compute_v1alpha"
 	"miren.dev/runtime/network"
@@ -45,6 +46,11 @@ type SandboxContainerRuntime interface {
 	BuildSpec(ctx context.Context, sb *compute.Sandbox, ep *network.EndpointConfig, meta *entity.Meta) ([]containerd.NewContainerOpts, error)
 	CreateContainer(ctx context.Context, id string, opts ...containerd.NewContainerOpts) (string, error)
 	LoadContainer(ctx context.Context, id string) (containerd.Container, error)
+	// ContainerSpec fetches a loaded container's OCI spec. Callers must go
+	// through here rather than calling cont.Spec directly: containerd resolves
+	// the container out of the namespace on the context, and only this
+	// implementation knows which namespace that is.
+	ContainerSpec(ctx context.Context, cont containerd.Container) (*oci.Spec, error)
 	CleanupContainer(ctx context.Context, cont containerd.Container)
 	BootInitialTask(ctx context.Context, sb *compute.Sandbox, ep *network.EndpointConfig, container containerd.Container, shortID string) (containerd.Task, error)
 	ConfigureVolumes(ctx context.Context, sb *compute.Sandbox, meta *entity.Meta) (map[string]string, error)

controllers/sandbox/sandbox_ops.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/containerd/containerd/namespaces"
 	containerd "github.com/containerd/containerd/v2/client"
+	"github.com/containerd/containerd/v2/pkg/oci"
 	"github.com/containerd/errdefs"
 
 	compute "miren.dev/runtime/api/compute/compute_v1alpha"
@@ -107,6 +108,11 @@ func (o *sandboxOps) LoadContainer(ctx context.Context, id string) (containerd.C
 	return o.ctrl.CC.LoadContainer(ctx, id)
 }
 
+func (o *sandboxOps) ContainerSpec(ctx context.Context, cont containerd.Container) (*oci.Spec, error) {
+	ctx = namespaces.WithNamespace(ctx, o.ctrl.Namespace)
+	return cont.Spec(ctx)
+}
+
 func (o *sandboxOps) CleanupContainer(ctx context.Context, cont containerd.Container) {
 	ctx = namespaces.WithNamespace(ctx, o.ctrl.Namespace)
 	o.ctrl.CleanupContainer(ctx, cont)

controllers/sandbox/sandbox_ops_test.go

@@ -0,0 +1,35 @@
+package sandbox
+
+import (
+	"context"
+	"testing"
+
+	"github.com/containerd/containerd/v2/pkg/oci"
+	"github.com/opencontainers/runtime-spec/specs-go"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// The saga actions hold a context that nobody has namespaced, so every
+// containerd call has to pick up the namespace on its way through sandboxOps.
+// ContainerSpec used to be missing from that adapter and the saga called
+// cont.Spec directly, which only worked because the server happens to build
+// its containerd client with a default namespace. Anyone constructing a client
+// without one (the test harness, for instance) got "namespace is required"
+// instead.
+func TestSandboxOpsContainerSpecCarriesNamespace(t *testing.T) {
+	cont := &sagaMockContainer{
+		id: "test-sandbox-1",
+		spec: &oci.Spec{
+			Linux: &specs.Linux{CgroupsPath: "/sys/fs/cgroup/sandbox/test-sandbox-1"},
+		},
+	}
+
+	ops := &sandboxOps{ctrl: &SandboxController{Namespace: "miren-test"}}
+
+	spec, err := ops.ContainerSpec(context.Background(), cont)
+	require.NoError(t, err)
+	assert.Equal(t, "/sys/fs/cgroup/sandbox/test-sandbox-1", spec.Linux.CgroupsPath)
+	assert.Equal(t, "miren-test", cont.specNamespace,
+		"ContainerSpec must namespace the context before handing it to containerd")
+}

docs/docs/changelog.md

@@ -11,6 +11,9 @@ All notable changes to Miren Runtime will be documented in this file.
 ## Unreleased
 *main*
 
+**Improvements**
+- **Builds and sandbox startup survive a restart** - Both are now driven by the saga engine, which journals each step as it goes, so a control-plane restart in the middle of one resumes it (or unwinds it cleanly) instead of stranding a half-built image or a sandbox that never finishes coming up. This ran behind the `sagas` labs flag on our own clusters for the last couple of releases and is now the default everywhere. The flag stays for one release as an escape hatch: start the server with `--labs -sagas` (or `MIREN_LABS=-sagas`) to go back to the old path, and please tell us if you need to, because the old path goes away in the release after this one. See [Miren Labs](/labs). ([#972](https://github.com/mirendev/runtime/pull/972))
+
 ---
 
 ## v0.12.0

docs/docs/labs.md

@@ -15,10 +15,13 @@ Miren Labs is where we ship experimental features that aren't quite ready for pr
 Labs features are:
 
 - **Experimental** — APIs and behavior may change based on feedback
-- **Opt-in** — Disabled by default, you choose when to try them
+- **Opt-in** — Off by default, you choose when to try them
+- **Reversible** — Anything you turn on you can turn back off
 - **Supported** — We want to hear about bugs and rough edges
 - **On a path** — Most labs features are headed toward stable release
 
+When a feature graduates to stable it flips on by default and stops being opt-in, but we leave its flag in place for a release so you can turn it back off if the new behavior causes trouble. After that release the flag and the old behavior both go away.
+
 ## Enabling Labs Features
 
 Labs features are controlled via the `--labs` flag or `MIREN_LABS` environment variable when starting the Miren server.
@@ -39,6 +42,25 @@ MIREN_LABS=distributedrunners,sagas miren server
 ```
 </CliCommand>
 
+## Turning a Feature Off
+
+Prefix a feature name with `-` to disable it. This is how you back out of a feature that's on by default, and it takes the same flag and environment variable as enabling.
+
+<CliCommand context="server">
+```miren
+# Turn off a feature that is on by default
+miren server --labs -sagas
+
+# Via environment variable
+MIREN_LABS=-sagas miren server
+
+# Mix and match: distributed runners on, sagas off
+MIREN_LABS=distributedrunners,-sagas miren server
+```
+</CliCommand>
+
+Check the server log to confirm the setting landed. Every boot logs a `labs features` line listing where each feature ended up, whether or not you named it.
+
 ## Giving Feedback
 
 We'd love to hear how labs features work for you:

pkg/labs/cmd/labsgen/main.go

@@ -89,6 +89,7 @@ package labs
 
 import (
 	"log/slog"
+	"sort"
 	"strings"
 	"sync"
 )
@@ -134,6 +135,8 @@ var featureDefaults = map[string]bool{
 // Each flag can be a feature name to enable it, or prefixed with "-" to disable it.
 // Unknown feature names are logged as warnings but do not cause an error.
 // This function should be called once at startup.
+//
+// log is required; pass slog.New(slog.DiscardHandler) to silence it.
 func Init(log *slog.Logger, flags []string) {
 	mu.Lock()
 	defer mu.Unlock()
@@ -164,25 +167,31 @@ func Init(log *slog.Logger, flags []string) {
 			for featureName := range featureDefaults {
 				enabledFeatures[featureName] = !disable
 			}
-			if log != nil {
-				log.Info("labs feature configured", "feature", "all", "enabled", !disable)
-			}
 			continue
 		}
 
 		// Check if it's a known feature
 		if _, known := featureDefaults[name]; !known {
-			if log != nil {
-				log.Warn("unknown labs feature flag", "flag", flag)
-			}
+			log.Warn("unknown labs feature flag", "flag", flag)
 			continue
 		}
 
 		enabledFeatures[name] = !disable
-		if log != nil {
-			log.Info("labs feature configured", "feature", name, "enabled", !disable)
-		}
 	}
+
+	// Report where every feature landed, not just the ones that were named.
+	// Features that default to on say nothing on their own, so without this
+	// the boot log is silent about the state an operator most wants to see.
+	names := AllFeatures()
+	sort.Strings(names)
+
+	attrs := make([]any, 0, len(names)*2)
+	for _, name := range names {
+		// Read the map directly: IsEnabled takes the read lock we already hold.
+		attrs = append(attrs, name, enabledFeatures[name])
+	}
+
+	log.Info("labs features", attrs...)
 }
 
 // Reset resets the feature flags to their default state.

pkg/labs/features.yaml

@@ -4,7 +4,9 @@ features:
     description: Schedule jobs across multiple runner nodes
     default: false
 
+  # Sagas are GA (MIR-953). The flag survives only as an escape hatch (-sagas)
+  # for one release; MIR-1460 removes it and the pre-saga code path with it.
   - name: sagas
     predicate: Sagas
     description: Use saga-based crash-recoverable workflows
-    default: false
+    default: true