diff --git a/api/compute/rpc.yml b/api/compute/rpc.yml index e7a889513..972e2297d 100644 --- a/api/compute/rpc.yml +++ b/api/compute/rpc.yml @@ -89,7 +89,7 @@ interfaces: parameters: - name: app type: string - doc: Empty lists all apps for unscoped callers, or the bound app for app-scoped callers. + doc: Empty lists all apps for unscoped callers, or the bound app for app-scoped callers. Otherwise matches the app each pool reports, which for addon pools comes from their app label. results: - name: pools type: list diff --git a/cli/commands/sandbox_pool_list.go b/cli/commands/sandbox_pool_list.go index fce3f7cde..7e811a19d 100644 --- a/cli/commands/sandbox_pool_list.go +++ b/cli/commands/sandbox_pool_list.go @@ -2,6 +2,7 @@ package commands import ( "fmt" + "slices" "time" "miren.dev/runtime/api/compute/compute_v1alpha" @@ -29,6 +30,8 @@ type sandboxPoolListEntry struct { } func SandboxPoolList(ctx *Context, opts struct { + App string `long:"app" description:"Only show pools belonging to this app"` + Service string `long:"service" description:"Only show pools of this service (e.g. web, worker)"` FormatOptions ConfigCentric }) error { @@ -42,7 +45,7 @@ func SandboxPoolList(ctx *Context, opts struct { } var pools []*compute_v1alpha.SandboxPoolInfo if err == nil { - res, callErr := compute_v1alpha.NewSandboxPoolsClient(client).List(ctx, "") + res, callErr := compute_v1alpha.NewSandboxPoolsClient(client).List(ctx, opts.App) err = callErr if err == nil { pools = res.Pools() @@ -50,10 +53,17 @@ func SandboxPoolList(ctx *Context, opts struct { } if isMissingRPC(err) { pools, err = sandboxPoolsLegacy(ctx) + // Servers without the RPC cannot filter by app. + pools = slices.DeleteFunc(pools, func(pool *compute_v1alpha.SandboxPoolInfo) bool { + return opts.App != "" && pool.App() != opts.App + }) } if err != nil { return err } + pools = slices.DeleteFunc(pools, func(pool *compute_v1alpha.SandboxPoolInfo) bool { + return opts.Service != "" && pool.Service() != opts.Service + }) if opts.IsJSON() { entries := make([]sandboxPoolListEntry, 0, len(pools)) @@ -71,7 +81,7 @@ func SandboxPoolList(ctx *Context, opts struct { } var rows []ui.Row - headers := []string{"ID", "VERSION", "SERVICE", "MODE", "DESIRED", "CURRENT", "READY", "CREATED", "UPDATED"} + headers := []string{"ID", "APP", "VERSION", "SERVICE", "MODE", "DESIRED", "CURRENT", "READY", "CREATED", "UPDATED"} for _, pool := range pools { versionDisplay := ui.DisplayAppVersion(pool.Version()) @@ -81,8 +91,9 @@ func SandboxPoolList(ctx *Context, opts struct { rows = append(rows, ui.Row{ ui.DisplayShortID(pool.ShortId(), pool.Id()), - versionDisplay, - pool.Service(), + orDash(pool.App()), + orDash(versionDisplay), + orDash(pool.Service()), pool.ScalingMode(), fmt.Sprintf("%d", pool.DesiredInstances()), fmt.Sprintf("%d", pool.CurrentInstances()), @@ -93,7 +104,7 @@ func SandboxPoolList(ctx *Context, opts struct { } if len(rows) == 0 { - ctx.Printf("No sandbox pools found\n") + ctx.Printf("No sandbox pools found%s\n", sandboxFilter{app: opts.App, service: opts.Service}.describe()) return nil } diff --git a/cli/commands/sandbox_pool_test.go b/cli/commands/sandbox_pool_test.go index e99652aaf..5773e7bc4 100644 --- a/cli/commands/sandbox_pool_test.go +++ b/cli/commands/sandbox_pool_test.go @@ -5,9 +5,9 @@ import ( "context" "encoding/json" "fmt" - "io" "log/slog" "path/filepath" + "slices" "strings" "testing" @@ -15,9 +15,11 @@ import ( "miren.dev/runtime/api/app/app_v1alpha" "miren.dev/runtime/api/compute/compute_v1alpha" "miren.dev/runtime/api/core/core_v1alpha" + "miren.dev/runtime/api/entityserver" "miren.dev/runtime/api/entityserver/entityserver_v1alpha" "miren.dev/runtime/pkg/entity" "miren.dev/runtime/pkg/entity/testutils" + "miren.dev/runtime/pkg/entity/types" "miren.dev/runtime/pkg/rpc" appserver "miren.dev/runtime/servers/app" sandboxserver "miren.dev/runtime/servers/sandbox" @@ -45,16 +47,18 @@ func TestAppPoolSizeSelectsOnlyActiveService(t *testing.T) { server.Server().ExposeValue("dev.miren.runtime/app", app_v1alpha.AdaptCrud(&appserver.AppInfo{Log: slog.Default(), EC: store.Client})) server.Server().ExposeValue("dev.miren.runtime/sandbox-pools", compute_v1alpha.AdaptSandboxPools(sandboxserver.NewPoolServer(store.Client))) var stdout bytes.Buffer - ctx := &Context{Context: t.Context(), Log: slog.Default(), Stdout: &stdout, Stderr: io.Discard} - ctx.Config.ServerAddress = server.ListenAddr() + base := []string{"--config", filepath.Join(t.TempDir(), "config.yaml"), "--server-address", server.ListenAddr(), "--app", "shop"} run := func(size, service string, json bool) error { - stdout.Reset() - return AppPoolSize(ctx, struct { - AppCentric - FormatOptions - Size string `position:"0" usage:"Desired instance count (omit to show current size)"` - Service string `short:"s" long:"service" description:"Service to scale" default:"web"` - }{AppCentric: AppCentric{App: "shop"}, FormatOptions: FormatOptions{JSON: json}, Size: size, Service: service}) + args := append(slices.Clone(base), "--service", service) + if json { + args = append(args, "--json") + } + if size != "" { + args = append(args, "--", size) + } + out, err := RunCommand(AppPoolSize, args...) + stdout = out.Stdout + return err } require.NoError(t, run("5", "web", true)) var result map[string]any @@ -141,30 +145,27 @@ func TestSandboxPoolCommandsWithoutEntityAccess(t *testing.T) { // exclusively through the safe operation-specific boundary. server.Server().ExposeValue("dev.miren.runtime/sandbox-pools", compute_v1alpha.AdaptSandboxPools(sandboxserver.NewPoolServer(ec))) var stdout bytes.Buffer - ctx := &Context{Context: t.Context(), Log: slog.Default(), Stdout: &stdout, Stderr: io.Discard} - ctx.Config.ServerAddress = server.ListenAddr() - list := func(format FormatOptions) error { - stdout.Reset() - return SandboxPoolList(ctx, struct { - FormatOptions - ConfigCentric - }{FormatOptions: format}) + base := []string{"--config", filepath.Join(t.TempDir(), "config.yaml"), "--server-address", server.ListenAddr()} + list := func(format ...string) error { + out, err := RunCommand(SandboxPoolList, append(slices.Clone(base), format...)...) + stdout = out.Stdout + return err } setDesired := func(id, desired string, raw bool) error { - stdout.Reset() - return SandboxPoolSetDesired(ctx, struct { - RawID bool `long:"raw-id" description:"Use the provided ID as-is without adding the pool/ prefix"` - PoolID string `position:"0" usage:"Pool ID (e.g., pool-CUSkT8J58BmgkDeGyPP2e or pool/pool-CUSkT8J58BmgkDeGyPP2e)" required:"true"` - Desired string `position:"1" usage:"Desired instance count (absolute number, +N to increase, or -N to decrease)" required:"true"` - ConfigCentric - }{PoolID: id, Desired: desired, RawID: raw}) + args := slices.Clone(base) + if raw { + args = append(args, "--raw-id") + } + out, err := RunCommand(SandboxPoolSetDesired, append(args, "--", id, desired)...) + stdout = out.Stdout + return err } - require.NoError(t, list(FormatOptions{})) - for _, text := range []string{"ID", "VERSION", "SERVICE", "MODE", "DESIRED", "CURRENT", "READY", "CREATED", "UPDATED", ent.Entity().ShortId(), verEnt.Entity().ShortId(), "web", "fixed"} { + require.NoError(t, list()) + for _, text := range []string{"ID", "APP", "VERSION", "SERVICE", "MODE", "DESIRED", "CURRENT", "READY", "CREATED", "UPDATED", ent.Entity().ShortId(), "shop", verEnt.Entity().ShortId(), "web", "fixed"} { require.Contains(t, stdout.String(), text) } - for _, format := range []FormatOptions{{JSON: true}, {Format: "json"}} { - require.NoError(t, list(format)) + for _, format := range [][]string{{"--json"}, {"--format", "json"}} { + require.NoError(t, list(format...)) var items []map[string]any require.NoError(t, json.Unmarshal(stdout.Bytes(), &items)) require.Len(t, items, 1) @@ -223,12 +224,64 @@ func TestSandboxPoolCommandsWithoutEntityAccess(t *testing.T) { require.EqualValues(t, 1, pool.DesiredInstances) require.NoError(t, ec.Delete(t.Context(), rawID)) require.NoError(t, ec.Delete(t.Context(), poolID)) - require.NoError(t, list(FormatOptions{})) + require.NoError(t, list()) require.Equal(t, "No sandbox pools found\n", stdout.String()) - require.NoError(t, list(FormatOptions{JSON: true})) + require.NoError(t, list("--json")) require.Equal(t, "[]\n", stdout.String()) } +func TestSandboxPoolListFilters(t *testing.T) { + inmem, cleanup := testutils.NewInMemEntityServer(t) + t.Cleanup(cleanup) + ec := inmem.Client + for _, app := range []string{"shop", "blog"} { + appID, err := ec.Create(t.Context(), app, &core_v1alpha.App{}) + require.NoError(t, err) + versionID, err := ec.Create(t.Context(), app+"-v1", &core_v1alpha.AppVersion{App: appID}) + require.NoError(t, err) + for _, service := range []string{"web", "worker"} { + poolID := entity.Id("pool/" + app + "-" + service) + require.NoError(t, ec.UpdateAttrs(t.Context(), poolID, (&compute_v1alpha.SandboxPool{ + App: appID, Service: service, SandboxSpec: compute_v1alpha.SandboxSpec{Version: versionID}, + }).Encode(), entity.Ref(entity.DBId, poolID))) + } + } + // Addon pools name their app only in a label, with no owner or service. + addonID, err := ec.Create(t.Context(), "shop-db", &compute_v1alpha.SandboxPool{}, entityserver.WithLabels(types.LabelSet("addon", "miren-postgresql", "app", "shop"))) + require.NoError(t, err) + addon := addonID.String() + server, err := rpc.NewState(t.Context(), rpc.WithBindAddr("localhost:0")) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, server.Close()) }) + server.Server().ExposeValue("dev.miren.runtime/sandbox-pools", compute_v1alpha.AdaptSandboxPools(sandboxserver.NewPoolServer(ec))) + args := []string{"--config", filepath.Join(t.TempDir(), "config.yaml"), "--server-address", server.ListenAddr(), "--format", "json"} + list := func(extra ...string) []string { + out, err := RunCommand(SandboxPoolList, append(args, extra...)...) + require.NoError(t, err) + var items []sandboxPoolListEntry + require.NoError(t, json.Unmarshal(out.Stdout.Bytes(), &items)) + ids := make([]string, 0, len(items)) + for _, item := range items { + ids = append(ids, item.ID) + } + return ids + } + require.ElementsMatch(t, []string{"pool/shop-web", "pool/shop-worker", "pool/blog-web", "pool/blog-worker", addon}, list()) + require.ElementsMatch(t, []string{"pool/shop-web", "pool/shop-worker", addon}, list("--app", "shop")) + require.ElementsMatch(t, []string{"pool/shop-worker", "pool/blog-worker"}, list("--service", "worker")) + require.Equal(t, []string{"pool/blog-web"}, list("--app", "blog", "--service", "web")) + require.Empty(t, list("--app", "missing")) + + table := args[:len(args)-2] + out, err := RunCommand(SandboxPoolList, table...) + require.NoError(t, err) + require.Contains(t, out.Stdout.String(), "APP") + require.Contains(t, out.Stdout.String(), "blog") + out, err = RunCommand(SandboxPoolList, append(slices.Clone(table), "--app", "missing")...) + require.NoError(t, err) + require.Equal(t, "No sandbox pools found for app \"missing\"\n", out.Stdout.String()) +} + type deniedPoolServer struct{} func (deniedPoolServer) List(context.Context, *compute_v1alpha.SandboxPoolsList) error { @@ -248,33 +301,18 @@ func TestSandboxPoolCommandsDoNotFallback(t *testing.T) { if available { server.Server().ExposeValue("dev.miren.runtime/sandbox-pools", compute_v1alpha.AdaptSandboxPools(deniedPoolServer{})) } - var stdout bytes.Buffer - ctx := &Context{Context: t.Context(), Log: slog.Default(), Stdout: &stdout, Stderr: io.Discard} - ctx.Config.ServerAddress = server.ListenAddr() - err = SandboxPoolList(ctx, struct { - FormatOptions - ConfigCentric - }{}) - require.Error(t, err) - if available { - require.ErrorContains(t, err, "unauthorized") - } else { - require.ErrorContains(t, err, "entities") - } - require.Empty(t, stdout.String()) - err = SandboxPoolSetDesired(ctx, struct { - RawID bool `long:"raw-id" description:"Use the provided ID as-is without adding the pool/ prefix"` - PoolID string `position:"0" usage:"Pool ID (e.g., pool-CUSkT8J58BmgkDeGyPP2e or pool/pool-CUSkT8J58BmgkDeGyPP2e)" required:"true"` - Desired string `position:"1" usage:"Desired instance count (absolute number, +N to increase, or -N to decrease)" required:"true"` - ConfigCentric - }{PoolID: "pool-1", Desired: "+2"}) - require.Error(t, err) - if available { - require.ErrorContains(t, err, "unauthorized") - } else { - require.ErrorContains(t, err, "entities") + base := []string{"--config", filepath.Join(t.TempDir(), "config.yaml"), "--server-address", server.ListenAddr()} + check := func(out *CommandOutput, err error) { + require.Error(t, err) + if available { + require.ErrorContains(t, err, "unauthorized") + } else { + require.ErrorContains(t, err, "entities") + } + require.Empty(t, out.Stdout.String()) } - require.Empty(t, stdout.String()) + check(RunCommand(SandboxPoolList, base...)) + check(RunCommand(SandboxPoolSetDesired, append(base, "pool-1", "+2")...)) }) } } diff --git a/docs/docs/command/debug-sandbox-pool-list.md b/docs/docs/command/debug-sandbox-pool-list.md index 8a29814df..a3eac20d8 100644 --- a/docs/docs/command/debug-sandbox-pool-list.md +++ b/docs/docs/command/debug-sandbox-pool-list.md @@ -16,10 +16,12 @@ miren debug sandbox-pool list [flags] ## Flags +- `--app` — Only show pools belonging to this app - `--cluster, -C` — Cluster name - `--config` — Path to the config file - `--format` — Output format (text, json) (default: `text`) - `--json` — Shorthand for --format json +- `--service` — Only show pools of this service (e.g. web, worker) ## Global Options diff --git a/docs/docs/command/sandbox-pool-list.md b/docs/docs/command/sandbox-pool-list.md index 88c97b555..63c60f91d 100644 --- a/docs/docs/command/sandbox-pool-list.md +++ b/docs/docs/command/sandbox-pool-list.md @@ -16,10 +16,12 @@ miren sandbox-pool list [flags] ## Flags +- `--app` — Only show pools belonging to this app - `--cluster, -C` — Cluster name - `--config` — Path to the config file - `--format` — Output format (text, json) (default: `text`) - `--json` — Shorthand for --format json +- `--service` — Only show pools of this service (e.g. web, worker) ## Global Options diff --git a/servers/sandbox/pools.go b/servers/sandbox/pools.go index a6a1de7ee..ac244edfb 100644 --- a/servers/sandbox/pools.go +++ b/servers/sandbox/pools.go @@ -43,33 +43,16 @@ func (s *PoolServer) List(ctx context.Context, state *compute_v1alpha.SandboxPoo if err != nil { return err } - index := entity.Ref(entity.EntityKind, compute_v1alpha.KindSandboxPool) - if appName != "" { - var appID entity.Id - for id, owner := range apps { - if owner.Instance == appName { - appID = id - break - } - } - if entity.Empty(appID) { - state.Results().SetPools([]*compute_v1alpha.SandboxPoolInfo{}) - return nil - } - allowed, err := filterInventoryApp(ctx, apps[appID]) - if err != nil { - return err - } - if !allowed { - state.Results().SetPools([]*compute_v1alpha.SandboxPoolInfo{}) - return nil - } - index = entity.Ref(compute_v1alpha.SandboxPoolAppId, appID) - } - pools, err := s.ec.List(ctx, index) + // Labels are not indexed, so an app's addon pools can only be found by + // walking every pool. The filter runs before version lookups, so excluded + // pools cost no store calls. + pools, err := s.ec.List(ctx, entity.Ref(entity.EntityKind, compute_v1alpha.KindSandboxPool)) if err != nil { return err } + // Pools for one version (web, worker, ...) share a version and its config, + // so resolve each version once rather than once per pool. + versions := make(map[entity.Id]*poolVersion) items := make([]*compute_v1alpha.SandboxPoolInfo, 0, pools.Length()) for pools.Next() { var pool compute_v1alpha.SandboxPool @@ -85,23 +68,33 @@ func (s *PoolServer) List(ctx context.Context, state *compute_v1alpha.SandboxPoo continue } ent := pools.Entity() + app := owner.Instance + if app == "" { + // Addon pools name their app only in a label. It identifies the + // pool but grants nothing, so read it after authorization. + var md core_v1alpha.Metadata + md.Decode(ent) + app, _ = md.Labels.Get("app") + } + if appName != "" && app != appName { + continue + } item := new(compute_v1alpha.SandboxPoolInfo) item.SetId(pool.ID.String()) item.SetShortId(ent.ShortId()) - item.SetApp(owner.Instance) + item.SetApp(app) item.SetVersion(pool.SandboxSpec.Version.String()) item.SetService(pool.Service) item.SetScalingMode("auto") - var version core_v1alpha.AppVersion - if verEnt, err := s.ec.GetByIdWithEntity(ctx, pool.SandboxSpec.Version, &version); err == nil && version.App == pool.App { - item.SetVersionShortId(verEnt.Entity().ShortId()) - if cfg, err := coreutil.ResolveRuntimeConfig(ctx, s.ec.EAC(), &version); err == nil { - for _, svc := range cfg.Services { - if svc.Name == pool.Service && svc.Concurrency.Mode == "fixed" { - item.SetScalingMode("fixed") - break - } - } + ver, ok := versions[pool.SandboxSpec.Version] + if !ok { + ver = s.poolVersion(ctx, pool.SandboxSpec.Version) + versions[pool.SandboxSpec.Version] = ver + } + if ver != nil && ver.app == pool.App { + item.SetVersionShortId(ver.shortID) + if ver.fixed[pool.Service] { + item.SetScalingMode("fixed") } } item.SetDesiredInstances(pool.DesiredInstances) @@ -116,6 +109,39 @@ func (s *PoolServer) List(ctx context.Context, state *compute_v1alpha.SandboxPoo return nil } +// poolVersion is what inventory needs from a pool's version: its owner, short +// ID, and which services run at a fixed scale. +type poolVersion struct { + app entity.Id + shortID string + fixed map[string]bool +} + +// poolVersion returns nil when the version cannot be read; inventory then +// shows the pool without version details rather than failing the listing. +// Concurrency comes from the stored config alone. Addon bindings only supply +// variables and disks, so resolving them would cost a store round trip and +// load credentials without changing the result. +func (s *PoolServer) poolVersion(ctx context.Context, id entity.Id) *poolVersion { + if entity.Empty(id) { + return nil + } + var version core_v1alpha.AppVersion + verEnt, err := s.ec.GetByIdWithEntity(ctx, id, &version) + if err != nil { + return nil + } + ver := &poolVersion{app: version.App, shortID: verEnt.Entity().ShortId(), fixed: map[string]bool{}} + if cfg, err := coreutil.ResolveConfig(ctx, s.ec.EAC(), &version); err == nil { + for _, svc := range cfg.Services { + if svc.Concurrency.Mode == "fixed" { + ver.fixed[svc.Name] = true + } + } + } + return ver +} + // An empty owner denotes an orphan: operators may manage it, but app-bound // callers must fail the AllowApp check. Store failures are not orphan evidence. func (s *PoolServer) poolApp(ctx context.Context, pool *compute_v1alpha.SandboxPool) (string, error) { diff --git a/servers/sandbox/pools_test.go b/servers/sandbox/pools_test.go index fc28236ed..722fd6602 100644 --- a/servers/sandbox/pools_test.go +++ b/servers/sandbox/pools_test.go @@ -18,6 +18,7 @@ import ( "miren.dev/runtime/pkg/cond" "miren.dev/runtime/pkg/entity" "miren.dev/runtime/pkg/entity/testutils" + "miren.dev/runtime/pkg/entity/types" "miren.dev/runtime/pkg/rpc" ) @@ -371,3 +372,66 @@ func TestPoolScalingConcurrentReadPatch(t *testing.T) { require.NoError(t, inmem.Client.GetById(ctx, poolID, &pool)) require.EqualValues(t, 5, pool.DesiredInstances) } + +func TestPoolInventorySharedVersion(t *testing.T) { + ctx := context.Background() + inmem, cleanup := testutils.NewInMemEntityServer(t) + t.Cleanup(cleanup) + ec := inmem.Client + client := compute_v1alpha.NewSandboxPoolsClient(rpc.LocalClient(compute_v1alpha.AdaptSandboxPools(NewPoolServer(ec)))) + appID, err := ec.Create(ctx, "shop", &core_v1alpha.App{}) + require.NoError(t, err) + versionID, err := ec.Create(ctx, "shop-v1", &core_v1alpha.AppVersion{App: appID, Config: core_v1alpha.Config{ + Services: []core_v1alpha.Services{ + {Name: "web"}, + {Name: "worker", ServiceConcurrency: core_v1alpha.ServiceConcurrency{Mode: "fixed"}}, + }, + }}) + require.NoError(t, err) + for _, service := range []string{"web", "worker"} { + _, err = ec.Create(ctx, "shop-"+service, &compute_v1alpha.SandboxPool{App: appID, Service: service, SandboxSpec: compute_v1alpha.SandboxSpec{Version: versionID}}) + require.NoError(t, err) + } + + res, err := client.List(ctx, "") + require.NoError(t, err) + modes := map[string]string{} + for _, pool := range res.Pools() { + require.NotEmpty(t, pool.VersionShortId()) + modes[pool.Service()] = pool.ScalingMode() + } + require.Equal(t, map[string]string{"web": "auto", "worker": "fixed"}, modes) +} + +func TestPoolInventoryAddonPoolApp(t *testing.T) { + ctx := context.Background() + inmem, cleanup := testutils.NewInMemEntityServer(t) + t.Cleanup(cleanup) + ec := inmem.Client + client := compute_v1alpha.NewSandboxPoolsClient(rpc.LocalClient(compute_v1alpha.AdaptSandboxPools(NewPoolServer(ec)))) + _, err := ec.Create(ctx, "shop", &core_v1alpha.App{}) + require.NoError(t, err) + _, err = ec.Create(ctx, "shop-db", &compute_v1alpha.SandboxPool{}, entityserver.WithLabels(types.LabelSet("addon", "miren-postgresql", "app", "shop"))) + require.NoError(t, err) + + res, err := client.List(ctx, "") + require.NoError(t, err) + require.Len(t, res.Pools(), 1) + require.Equal(t, "shop", res.Pools()[0].App()) + + res, err = client.List(ctx, "shop") + require.NoError(t, err) + require.Len(t, res.Pools(), 1, "the app filter matches the reported app") + res, err = client.List(ctx, "blog") + require.NoError(t, err) + require.Empty(t, res.Pools()) + + // The label names the app. It grants nothing, so app-bound callers still + // cannot see a pool with no real owner. + own := rpc.ContextWithIdentity(ctx, &rpc.Identity{Method: rpc.AuthMethodWorkload, Metadata: map[string]any{"app": "shop", "bound_app": "shop"}}) + for _, filter := range []string{"", "shop"} { + res, err = client.List(own, filter) + require.NoError(t, err) + require.Empty(t, res.Pools()) + } +}