From 533f8e4e491f017faae2a0fb2fbfbc82fa94888b Mon Sep 17 00:00:00 2001 From: Evan Phoenix Date: Thu, 24 Sep 2026 18:02:43 +0000 Subject: [PATCH 1/8] Back off sandbox failures before RUNNING Persist startup outcomes and counted sandbox identities so long readiness failures enter per-pool cooldown exactly once. Stop port waits on process exit and guard terminal transitions against concurrent RUNNING updates. --- api/compute/compute_v1alpha/schema.gen.go | 116 +++++++++++++------ api/compute/schema.yml | 14 ++- controllers/nodehealth/controller.go | 17 ++- controllers/nodehealth/controller_test.go | 15 +++ controllers/sandbox/create_saga.go | 5 + controllers/sandbox/create_saga_test.go | 23 ++++ controllers/sandbox/saga_controller.go | 10 +- controllers/sandbox/sandbox.go | 83 ++++++++++--- controllers/sandbox/sandbox_test.go | 31 +++++ controllers/sandbox/waitforport_race_test.go | 23 ++++ controllers/sandboxpool/manager.go | 79 +++++++------ controllers/sandboxpool/manager_test.go | 110 ++++++++++++++++++ 12 files changed, 432 insertions(+), 94 deletions(-) diff --git a/api/compute/compute_v1alpha/schema.gen.go b/api/compute/compute_v1alpha/schema.gen.go index 280700d4e..2a769f106 100644 --- a/api/compute/compute_v1alpha/schema.gen.go +++ b/api/compute/compute_v1alpha/schema.gen.go @@ -1075,44 +1075,58 @@ func (o *Node) InitSchema(sb *schema.SchemaBuilder) { } const ( - SandboxBoundPortId = entity.Id("dev.miren.compute/sandbox.bound_port") - SandboxContainerId = entity.Id("dev.miren.compute/sandbox.container") - SandboxExitId = entity.Id("dev.miren.compute/sandbox.exit") - SandboxHostNetworkId = entity.Id("dev.miren.compute/sandbox.hostNetwork") - SandboxLabelsId = entity.Id("dev.miren.compute/sandbox.labels") - SandboxLastActivityId = entity.Id("dev.miren.compute/sandbox.last_activity") - SandboxLogAttributeId = entity.Id("dev.miren.compute/sandbox.logAttribute") - SandboxLogEntityId = entity.Id("dev.miren.compute/sandbox.logEntity") - SandboxNetworkId = entity.Id("dev.miren.compute/sandbox.network") - SandboxRouteId = entity.Id("dev.miren.compute/sandbox.route") - SandboxSpecId = entity.Id("dev.miren.compute/sandbox.spec") - SandboxStaticHostId = entity.Id("dev.miren.compute/sandbox.static_host") - SandboxStatusId = entity.Id("dev.miren.compute/sandbox.status") - SandboxStatusPendingId = entity.Id("dev.miren.compute/status.pending") - SandboxStatusNotReadyId = entity.Id("dev.miren.compute/status.not_ready") - SandboxStatusRunningId = entity.Id("dev.miren.compute/status.running") - SandboxStatusStoppedId = entity.Id("dev.miren.compute/status.stopped") - SandboxStatusDeadId = entity.Id("dev.miren.compute/status.dead") - SandboxVolumeId = entity.Id("dev.miren.compute/sandbox.volume") + SandboxBoundPortId = entity.Id("dev.miren.compute/sandbox.bound_port") + SandboxContainerId = entity.Id("dev.miren.compute/sandbox.container") + SandboxExitId = entity.Id("dev.miren.compute/sandbox.exit") + SandboxHostNetworkId = entity.Id("dev.miren.compute/sandbox.hostNetwork") + SandboxLabelsId = entity.Id("dev.miren.compute/sandbox.labels") + SandboxLastActivityId = entity.Id("dev.miren.compute/sandbox.last_activity") + SandboxLogAttributeId = entity.Id("dev.miren.compute/sandbox.logAttribute") + SandboxLogEntityId = entity.Id("dev.miren.compute/sandbox.logEntity") + SandboxNetworkId = entity.Id("dev.miren.compute/sandbox.network") + SandboxRouteId = entity.Id("dev.miren.compute/sandbox.route") + SandboxSpecId = entity.Id("dev.miren.compute/sandbox.spec") + SandboxStartupOutcomeId = entity.Id("dev.miren.compute/sandbox.startup_outcome") + SandboxStartupOutcomeStartupRunningId = entity.Id("dev.miren.compute/startup_outcome.startup_running") + SandboxStartupOutcomeStartupFailedId = entity.Id("dev.miren.compute/startup_outcome.startup_failed") + SandboxStaticHostId = entity.Id("dev.miren.compute/sandbox.static_host") + SandboxStatusId = entity.Id("dev.miren.compute/sandbox.status") + SandboxStatusPendingId = entity.Id("dev.miren.compute/status.pending") + SandboxStatusNotReadyId = entity.Id("dev.miren.compute/status.not_ready") + SandboxStatusRunningId = entity.Id("dev.miren.compute/status.running") + SandboxStatusStoppedId = entity.Id("dev.miren.compute/status.stopped") + SandboxStatusDeadId = entity.Id("dev.miren.compute/status.dead") + SandboxVolumeId = entity.Id("dev.miren.compute/sandbox.volume") ) type Sandbox struct { - ID entity.Id `json:"id"` - BoundPort []BoundPort `cbor:"bound_port,omitempty" json:"bound_port,omitempty"` - Container []Container `cbor:"container" json:"container"` - Exit Exit `cbor:"exit,omitempty" json:"exit"` - HostNetwork bool `cbor:"hostNetwork,omitempty" json:"hostNetwork,omitempty"` - Labels []string `cbor:"labels,omitempty" json:"labels,omitempty"` - LastActivity time.Time `cbor:"last_activity,omitempty" json:"last_activity"` - LogAttribute types.Labels `cbor:"logAttribute,omitempty" json:"logAttribute,omitempty"` - LogEntity string `cbor:"logEntity,omitempty" json:"logEntity,omitempty"` - Network []Network `cbor:"network,omitempty" json:"network,omitempty"` - Route []Route `cbor:"route,omitempty" json:"route,omitempty"` - Spec SandboxSpec `cbor:"spec,omitempty" json:"spec"` - StaticHost []StaticHost `cbor:"static_host,omitempty" json:"static_host,omitempty"` - Status SandboxStatus `cbor:"status,omitempty" json:"status,omitempty"` - Volume []Volume `cbor:"volume,omitempty" json:"volume,omitempty"` -} + ID entity.Id `json:"id"` + BoundPort []BoundPort `cbor:"bound_port,omitempty" json:"bound_port,omitempty"` + Container []Container `cbor:"container" json:"container"` + Exit Exit `cbor:"exit,omitempty" json:"exit"` + HostNetwork bool `cbor:"hostNetwork,omitempty" json:"hostNetwork,omitempty"` + Labels []string `cbor:"labels,omitempty" json:"labels,omitempty"` + LastActivity time.Time `cbor:"last_activity,omitempty" json:"last_activity"` + LogAttribute types.Labels `cbor:"logAttribute,omitempty" json:"logAttribute,omitempty"` + LogEntity string `cbor:"logEntity,omitempty" json:"logEntity,omitempty"` + Network []Network `cbor:"network,omitempty" json:"network,omitempty"` + Route []Route `cbor:"route,omitempty" json:"route,omitempty"` + Spec SandboxSpec `cbor:"spec,omitempty" json:"spec"` + StartupOutcome SandboxStartupOutcome `cbor:"startup_outcome,omitempty" json:"startup_outcome,omitempty"` + StaticHost []StaticHost `cbor:"static_host,omitempty" json:"static_host,omitempty"` + Status SandboxStatus `cbor:"status,omitempty" json:"status,omitempty"` + Volume []Volume `cbor:"volume,omitempty" json:"volume,omitempty"` +} + +type SandboxStartupOutcome string + +const ( + STARTUP_RUNNING SandboxStartupOutcome = "startup_outcome.startup_running" + STARTUP_FAILED SandboxStartupOutcome = "startup_outcome.startup_failed" +) + +var sandboxstartup_outcomeFromId = map[entity.Id]SandboxStartupOutcome{SandboxStartupOutcomeStartupRunningId: STARTUP_RUNNING, SandboxStartupOutcomeStartupFailedId: STARTUP_FAILED} +var sandboxstartup_outcomeToId = map[SandboxStartupOutcome]entity.Id{STARTUP_RUNNING: SandboxStartupOutcomeStartupRunningId, STARTUP_FAILED: SandboxStartupOutcomeStartupFailedId} type SandboxStatus string @@ -1182,6 +1196,9 @@ func (o *Sandbox) Decode(e entity.AttrGetter) { if a, ok := e.Get(SandboxSpecId); ok && a.Value.Kind() == entity.KindComponent { o.Spec.Decode(a.Value.Component()) } + if a, ok := e.Get(SandboxStartupOutcomeId); ok && a.Value.Kind() == entity.KindId { + o.StartupOutcome = sandboxstartup_outcomeFromId[a.Value.Id()] + } for _, a := range e.GetAll(SandboxStaticHostId) { if a.Value.Kind() == entity.KindComponent { var v StaticHost @@ -1249,6 +1266,9 @@ func (o *Sandbox) Encode() (attrs []entity.Attr) { if !o.Spec.Empty() { attrs = append(attrs, entity.Component(SandboxSpecId, o.Spec.Encode())) } + if a, ok := sandboxstartup_outcomeToId[o.StartupOutcome]; ok { + attrs = append(attrs, entity.Ref(SandboxStartupOutcomeId, a)) + } for _, v := range o.StaticHost { attrs = append(attrs, entity.Component(SandboxStaticHostId, v.Encode())) } @@ -1296,6 +1316,9 @@ func (o *Sandbox) Empty() bool { if !o.Spec.Empty() { return false } + if o.StartupOutcome != "" { + return false + } if len(o.StaticHost) != 0 { return false } @@ -1325,6 +1348,9 @@ func (o *Sandbox) InitSchema(sb *schema.SchemaBuilder) { sb.Component("route", "dev.miren.compute/sandbox.route", schema.Doc("A network route the container uses"), schema.Many) (&Route{}).InitSchema(sb.Builder("sandbox.route")) sb.Component("spec", "dev.miren.compute/sandbox.spec", schema.Doc("Immutable sandbox configuration")) + sb.Singleton("dev.miren.compute/startup_outcome.startup_running") + sb.Singleton("dev.miren.compute/startup_outcome.startup_failed") + sb.Ref("startup_outcome", "dev.miren.compute/sandbox.startup_outcome", schema.Doc("Durable distinction between a sandbox that reached RUNNING and one that failed before then"), schema.Choices(SandboxStartupOutcomeStartupRunningId, SandboxStartupOutcomeStartupFailedId)) sb.Component("static_host", "dev.miren.compute/sandbox.static_host", schema.Doc("A name to ip mapping configured staticly for the sandbox"), schema.Many) (&StaticHost{}).InitSchema(sb.Builder("sandbox.static_host")) sb.Singleton("dev.miren.compute/status.pending") @@ -2008,6 +2034,7 @@ const ( SandboxPoolAppId = entity.Id("dev.miren.compute/sandbox_pool.app") SandboxPoolConsecutiveCrashCountId = entity.Id("dev.miren.compute/sandbox_pool.consecutive_crash_count") SandboxPoolCooldownUntilId = entity.Id("dev.miren.compute/sandbox_pool.cooldown_until") + SandboxPoolCountedFailuresId = entity.Id("dev.miren.compute/sandbox_pool.counted_failures") SandboxPoolCurrentInstancesId = entity.Id("dev.miren.compute/sandbox_pool.current_instances") SandboxPoolDesiredInstancesId = entity.Id("dev.miren.compute/sandbox_pool.desired_instances") SandboxPoolEphemeralId = entity.Id("dev.miren.compute/sandbox_pool.ephemeral") @@ -2025,6 +2052,7 @@ type SandboxPool struct { App entity.Id `cbor:"app,omitempty" json:"app,omitempty"` ConsecutiveCrashCount int64 `cbor:"consecutive_crash_count,omitempty" json:"consecutive_crash_count,omitempty"` CooldownUntil time.Time `cbor:"cooldown_until,omitempty" json:"cooldown_until"` + CountedFailures []entity.Id `cbor:"counted_failures,omitempty" json:"counted_failures,omitempty"` CurrentInstances int64 `cbor:"current_instances,omitempty" json:"current_instances,omitempty"` DesiredInstances int64 `cbor:"desired_instances,omitempty" json:"desired_instances,omitempty"` Ephemeral bool `cbor:"ephemeral,omitempty" json:"ephemeral,omitempty"` @@ -2048,6 +2076,11 @@ func (o *SandboxPool) Decode(e entity.AttrGetter) { if a, ok := e.Get(SandboxPoolCooldownUntilId); ok && a.Value.Kind() == entity.KindTime { o.CooldownUntil = a.Value.Time() } + for _, a := range e.GetAll(SandboxPoolCountedFailuresId) { + if a.Value.Kind() == entity.KindId { + o.CountedFailures = append(o.CountedFailures, a.Value.Id()) + } + } if a, ok := e.Get(SandboxPoolCurrentInstancesId); ok && a.Value.Kind() == entity.KindInt64 { o.CurrentInstances = a.Value.Int64() } @@ -2110,6 +2143,9 @@ func (o *SandboxPool) Encode() (attrs []entity.Attr) { if !entity.Empty(o.CooldownUntil) { attrs = append(attrs, entity.Time(SandboxPoolCooldownUntilId, o.CooldownUntil)) } + for _, v := range o.CountedFailures { + attrs = append(attrs, entity.Ref(SandboxPoolCountedFailuresId, v)) + } if !entity.Empty(o.CurrentInstances) { attrs = append(attrs, entity.Int64(SandboxPoolCurrentInstancesId, o.CurrentInstances)) } @@ -2152,6 +2188,9 @@ func (o *SandboxPool) Empty() bool { if !entity.Empty(o.CooldownUntil) { return false } + if len(o.CountedFailures) != 0 { + return false + } if !entity.Empty(o.CurrentInstances) { return false } @@ -2187,12 +2226,13 @@ func (o *SandboxPool) Empty() bool { func (o *SandboxPool) InitSchema(sb *schema.SchemaBuilder) { sb.Ref("app", "dev.miren.compute/sandbox_pool.app", schema.Doc("Reference to the app this pool belongs to"), schema.Indexed, schema.Tags("dev.miren.app_ref")) - sb.Int64("consecutive_crash_count", "dev.miren.compute/sandbox_pool.consecutive_crash_count", schema.Doc("Number of consecutive quick crashes (sandboxes that died within 60s of creation)")) + sb.Int64("consecutive_crash_count", "dev.miren.compute/sandbox_pool.consecutive_crash_count", schema.Doc("Number of consecutive sandbox failures before reaching RUNNING")) sb.Time("cooldown_until", "dev.miren.compute/sandbox_pool.cooldown_until", schema.Doc("Timestamp until which new sandbox creation is paused due to crash loop")) + sb.Ref("counted_failures", "dev.miren.compute/sandbox_pool.counted_failures", schema.Doc("Sandbox IDs already included in the failure streak; kept across streak resets"), schema.Many) sb.Int64("current_instances", "dev.miren.compute/sandbox_pool.current_instances", schema.Doc("Current number of sandbox instances (non-STOPPED)")) sb.Int64("desired_instances", "dev.miren.compute/sandbox_pool.desired_instances", schema.Doc("Target number of sandbox instances")) sb.Bool("ephemeral", "dev.miren.compute/sandbox_pool.ephemeral", schema.Doc("True when this pool backs an ephemeral AppVersion. Ephemeral pools never scale beyond 1 instance.")) - sb.Time("last_crash_time", "dev.miren.compute/sandbox_pool.last_crash_time", schema.Doc("Timestamp of the most recent quick crash")) + sb.Time("last_crash_time", "dev.miren.compute/sandbox_pool.last_crash_time", schema.Doc("Timestamp of the most recently counted sandbox failure")) sb.Int64("ready_instances", "dev.miren.compute/sandbox_pool.ready_instances", schema.Doc("Number of RUNNING sandboxes")) sb.Ref("referenced_by_versions", "dev.miren.compute/sandbox_pool.referenced_by_versions", schema.Doc("AppVersions that reference this pool (enables reuse when specs match)"), schema.Many, schema.Indexed) sb.Label("sandbox_labels", "dev.miren.compute/sandbox_pool.sandbox_labels", schema.Doc("Labels that will be added to the metadata of sandboxes created from this pool"), schema.Many) @@ -2312,5 +2352,5 @@ func init() { (&SandboxPool{}).InitSchema(sb) (&Schedule{}).InitSchema(sb) }) - schema.RegisterEncodedSchema("dev.miren.compute", "v1alpha", []byte("\x1f\x8b\b\x00\x00\x00\x00\x00\x00\xff\xec\\َ\xec<\x11~\r\x0e\xcba\xdf\x04d\x00\xb1\x8b\xed\b\xb8\xe5\x15\"w\\\x9d\xf6tbgl\xa7\x17\xee\x00!!\x04<\x04g\x867\x84k\xe4%\x89\x93؉\xe3F\xfaor3\xb2ݮ\xcfv\xb9\\U.\xd7\xe4\x15ST\xc3\v\x86KV\x13\x0e4+Xݴ\x12\xe0L(\x16o\xb7\xcf\xcc~yR\xbfd\x94a\xf8\xb7\xa6\xbd\xcc{\xa8\x1f\r\xc0\x7f\x8f\x98Ո\xd0\xf9\x00\xc7#\x81\n\x8b\xbf~<\x10|\xfb\x92\x1f#C\r\xc9\x11\xc6\x1c\x84\xd0c\x9d\xdd\x06yo\xe0($'\xb4|]\x02)\x18\x15\x92#B\xa5\xc05\xa2\xf7\xff\x18(\xb7YAA\x85\x0eP\xe9\xe9x\x16\xad\x91\x14\x1d\x1e\xfe8\x13\xd0d_\t\x90q(\x89\x90\xc0\x01\xe7Hj\xd2zܤ\x80\xb0$5h\x98/\x84`ZJ\x81\xe7\x04k\b2T\xa7\x8c\xf8b\x00@\x14'\xc0mEh\xa9\x11\x9e\x9d\xba\x9e\x01ж>\xab?\xf9\x05U-\x88\x7f\x9d\n\xc61\xa3\x80o_\x9dC\x0e\xd4Y\xd7\xedl\xdbС\x82\xdb7\x16I\x9c\x9ezΟ\v\xcdY\"ٚ\xcd?ڲw\xae\xaf\xc0\x01\xe1\xfb\xed\xbdgTM\x96\xe9\xdf˖\x9e)\xbbR\x1f\x93m?\xdbㄉP\xb3\xf3\xb2\xd3v\xed\xba\x90\x96\x9e\x00U\xf2t\xf7\ta\x8fk\xfb\xe8M\xf6\xccS\xaf\xf7\x02\\\x10F\xf5\x82ˮ\xe2lp\xd7V^\xbe\x87\xaa愪\x86\x93\x1a\xf1{\xae\x8e\x1bV\x10\xb7\xcf\x06\x8el\x05H\xd83{\x9dwѿF\x1e\xda?\xeb%|-\x00\x92UH\xc8\xfc\x04\x88\xcb\x03X\x81\xa7\x93\xb6\xb1\xc4\x7f>\x84\xd4p\xf6\f\x85\x81(\xbb\x8a\xa2=\x10\xbcL)\x10\xc5\av3\x94]\xc5R.\xf2\x104\xbdo\x7f4\x13-\x94a\xe3\xed\x9dg\xb7M\x87HN\xfe\xf3-\xa08,Lv`-\xc5yøtt׳Ӫ\xd6D\x14\x15\xa3@\xe5P\xb23\x9cCgs\xe8\xc8\xc9\xfe)\xa4\xe5\x06\xa4\xcc\xd5֥GS\x7f\f((\aB/K+\xd8~\x81\x05\xa1rq\u05c8\xa6W\xfd5C\xbf\x1cfh\xc1\xa8D\x84\x02w\xf8I\x86\xc6\x15v\u0381\xb3\x19p$7\xff\xf110\xd3\x1eH\xb5Ԉ\x1a}_v\x15\x87\x99z\xad__F\xa0GR\xe6GR\xc1\xc4\xf6\xf5\xcd++~\x8aX\xb1;\xcc&\x15\xe2і\x0eT\x86\x91DF\x12ti*F+\xd45\xc3\xd6P\xeb\xd2F\xea\x06ɓ\x95BU\x8aU\xc1\xcf\x06CA\xe8Q<\x86s`\x1b&\x1c\n\xc9\xf8\xddH\xe1P\x9dZt\x8f\x9a\x1bP\x80^\x9c\xbd-T5\xe2\xc0\r\xf4\xa4F\xa5a\x14\x98\xe2T\xc2\x16\xa9k\xd6RW7\x81iX\x91\xaao\xc6H\x95Fڤ\x9b<\xa7I\x83d\x18\x84$\x14\xc9δ\x9e݆)\xb7<\xbaߠ\b\xd6\xf2\x02\xac3bʱra\xd8\x12\xf2\xef\x865\xaf\xb8\x98\x8b\xe2\xc4X\x9d\x8b\x82qCK\x86j\xa7B\xdfV\x87\x9f\x18\x1a\x1ccb掞g/7\x98\x99\xbf\x84|pm\x1dV\x18\xe4Y\x9d!c\x18\xf2\u07b2\x90\xa1\xda\xf1fqЀI\n\x9dMCÙd\x05\xab4ݩ\xaf\xf9=\xedB\x16\x8dO\xee:\xb2L\x16M\xd1\xe2\xe5>-n\x16W\xa1\x87\xee\xb9\x16\xefU*\xe2\x90\xc7\xe7\xec0'\x17RA\t\xc6^=;u=܁1s\xbb\xfa\xf6\x12\x8a8\xb5\x12\xb3+͕s\xc8Z\xc3\xf5fֺI\xc1\t\x89\x899\xf6`\x8a\xe3\xf9,*W)\x8dr.T\xa1\xa7[\xf6Ez\xea׀6\xe9\x8e\a܈\x15+]Z9gs\xa0\xcc\x05\xdadx=\x8e\xab\xc2Ȭ\xbb~\x98\xb9\xe8\x1e\x99\xd2\x04Eod\x8b\xce\xc8\xf6\xa7\xc9s\x16-M\xe7my\x9c\xaf(\x89T0!}\xd8\xf1\xe4Ą\xfc=\xc8+\xe3g\xa3\xf2݆~'_\x03\x13\xedPtP\xc0\x8d\x1b\x1cm\xcbT\x02=n\u0600!d\x8e\nI.\xc4JS=n\xeaY\xfd\x1a8e=\x12+?H\xc9ɡ\x95\xae?W\x8d\xda'\xc1\x8c\x05_\xb8b\xe5\xef\xa8\xec&E\x86j\x84\a\xd0aP\xcb\xd1a6%u\x98\xbc \xcfs\xd0l\x02\xba\xc9\xf6\xfb\xa2\x1e\x06&\xeeR\xe2\v\xbbXz\xd1\x1e(Hk\xf7M9VT;f\xbc\x054M\xb7b\xce\xc6[\n\xa6a\x85\x85s\xc0l\x04\xf8\xa8\xf3\xa4A\xb69O\x9e5\x1a\x94\x12I\xb8\"#jeW\x89v\x9f4ƚ>\x15\r\x14F\x19\xe9\xd2f\xbf\xe5\xa9\xefұ1W@\x91\\\xfc\x9b\xde\xe3\xefǢ>x\x13\x9d\x8f\x93\xad\x8d\x13\x1b\x93\xd0l\xfe\xf1\xf6ud\x88\x97\xae\xaaĺ>\x15\x90\x9f%\x00G]\x84\x7f\x99\x04\xfc\xe8\xfdx>\xea\xea>$_\x97\x7f\xf3\xd8\n\xd7\xeeӏ¯\\\xb8\x1f\x85O\xbc\x91\xdf\xde[l\x05\xdd#O\xae\xe9?O\x98[\xf4\xed\xfdG\t\xe0\x11\x97\xfa\x9f$\xc0\xae\xde\xf5S@\xd3B\x00\xf3\x91\xd6\x0f\xce\xf6\x88\xc0oS׳\xcd\xea\xfd*y\x98\ab\n\xb7w>\xc9\x1e\x02\r)*|\xe5z\x9drN\xe2\xc2\x12)\x93M\x89V\xcc\xc7Y\x17\xbb\xcd\xc1\x8b\x146\xc5D7>$\xe3F\x85?\x92\xa7\xbd\x14\x1f\xf9u2\xe8\xe6\x00J\xcaa\x1f\rՇY\x1eG\xea\x821\xc9\xe5S\n}\b\xe7\x17)Ӊ\x8d\xec\xa4X\xf8ĀO\x8a\x9dZ\x89\x03\xa5\x98\xe9\x84\xf0\x90\xf4폞\xc0\x0f\xa2'\xb0!\xb6\xf1\xc3hД\xe0B\xfcm'&ְ\xf1~\xa0\x84:\xbf\"\"G\xb2\xf32o\x9e:d\U00076183\x90\x88˼a\x15)\xcc\xd4\xe9\xa4ͯ\x8f\x80\xc2\x05\xf8\x86\xe36F\xcd4\xf9\x11UWt\x17\x1bn6\x13\x14C\xaf\xedk\x16\x8f\x91\x14\x8a\x98\xe3\x87\xec\xea\xf6\xc8D\xbcH$\x04,\xe2\x8f\xfd\xff!\x8e\xd18\x87_ým;\xf8B\"I\x8a\\\x1dw\xf7\xbe\xea6\xaf\xec\xd3|\xac\xd0>9\xa0\x9bv\xeb\xa7)\xab\xd1*͘\xbb~\x15\xee.\xc5\x1fY\x17\x944&\x9cM\x9a\xe8\x1dj\x9d\x1d2P\nI\xcf\xe1\xbb\xd1s\bf\xf1\x1c\b\xd6;>\x7fU\x0fB\xb1\xaa\xad\xdd\xe3x\xb4-\xdb\xdf\xed\x17G\x88\xdc\xe2\xbfo\xdc\r\x03\x9e\xe1\x83\t\xa6h\x86t\x95\xe9\x1e\xc7\vN\x87J\xc49\xef\xbde2T\xd3m\x8aEV\xf3\x13w!\xa16.\x8fSO\xbf\xdd[\xec\xc5'\x05Ƕ\xc6ێ\x0e\x18\x90\x80\x91-\xac\xc7M\x0f\xb3E\xdf-\xf3>\x10\xf3\xecԧ\xd8\xf1:\xcdb\xaf\\}\xe2\xfd\x18\x8bǮ\xdd\v\x13\x98b\xba\vi\x11\x1b\xce.\x04[\xd0S_{X\x8c9 \x9c3ZY\xd7h\xa8\x8e\xfd\xd3͇N\x90?@^\x1el\x0e\x9c\xad\x8c.|\x9b\xa7*^*\"\xa1\xcfA\x1d\xaa\xb1\xea\xf5\xc5Q\xaf\x06s\xb1{\xe5N\xe2m\xe5\xc5\xefq\xe38\a\xcf<\xe0\x9b\xaca !4\xd2\xec\x05\xd2T\x1f\xb0oσQ[{\xfb\x8cͽ}\xc3\x18\x907\x85\xb7˓\x05\x84\xcb\x06(&\xb4\\ȼ\xb5=J\xdeR\xba\xdc\xd3\xf6(\x85dM\x03A6\xb5\"\xb3=\be27\x19\xc2\xe1\fݾO(O\xa6cL\xaaM\x9eCfc\xc8\xd8׀P\xe2\xc9F\v\xe3ٱ8e\xecyk\x8d֑\x8b\xd2y\x8c\xd0\t]:\xaf\x8f\x01&G\xd7\xe4\x96\xdb\\\xe7ۧ=\x1bi{D\xf2\xfb\x8f\xc1\x87G\x8b\x93\x9d\xc1\xc6\x01T!!\x8f\xc3\xc1٤]\xdb\xe5rU\xb9\\ӯ\x98\xa2\v\b\xec\xb6\xe9\x90\xc8\xc9\x7f\xbeE\x14\x87\x85)\x0e\xac\xa3\xb8l\x19\x97\x9e\xeez\xf2Z՚\x88\xa2b\x14\xa8\x1cJv\x86s\xe8b\x0e\x9d8\xd9?ƴ܀T\xf8ں\x0eh\xea\xf7\x11\x05\xe5A\xe8ei\x05\xdb/\xb0\"T.\xee\x1a\xd1\xf4\xaa\xbff\xe8\x97\xe3\f\xad\x18\x95\x88P\xe0\x1e?\xc9и\xc2\xce9p1\x03N\xe4\xe6\xdf\xdfGf\xda\x03\xa9\x96\v\xa2F\xdf\u05ee\xe21S\xaf\xf5\xeb\xcb\b\xf4H\xea\xf2H\x1a\x98ؾ\xbeye\xc5\xef\x12V\xec\x0f\xb3I\x85\x04\xb4\xa5\aU`$\x91\x91\x04]\x9a\x8a\xd1\n\xf5\x85ak\xa8ui#u\x8b\xe4\xc9J\xa1*\xa5\xaa\xe0'\x83\xa1 \xf4(\x01\xc39\xb0\r\x13\x0e\x95d\xfcn\xa4p\xa8N-z@\xcd\r(@\xaf\xde\xdeV\xaa\x9ap\xe0\x06zrA\xb5a\x14\x98\xe2T\xc2\x16\xa9/\xac\xa3\xben\x02Ӱ\"U\xdfL\x91*\x8d\xb4I7\x05N\x93\x06)0\bI(\x92δ\x9e\xfd\x86)\xb7\x02\xbaߠ\b\xd6\xf1\n\xac3bʩra\xd8\x12\xf3\xef\x865\xaf\xb8\x98\x8b\xe2\xc4إ\x14\x15㆖\fU\xa7B\xdfV\x87\x9f\x18\x1a\x9cbb\xe6\x8e^`/7\x98\x99?\xc7|pm\x1dV\x18\x14X\x9d!c\x18\xca\u07b2\x90\xa1\xeax\xb38h\xc4$\xc5Φ\xa1\xe1L\xb2\x8a5\x9a\xee\xd4\xd7\u009ev%\xab6$w\x8e\xac\x90U[ux\xb9O\x87\xdb\xc5U\xe8\xa1{\xae\xa5{\x95\x8a8\xe6\xf1y;\xccɕ4P\x83\xb1WO^]\x0fw`\xccܮ\xbe\xbd\x84\"N\x9d\xc4셖\xca9d\x9d\xe1z;kݤ\xe0\x84\xc4\xc4\x1c{0\xc5\xf1|\x16\x95\xab\x94F9W\xaa\xd0\xd3-\xfb\"=\xf5kD\x9b\xb8\xe3\x017b\xc5J\x97V\xce\xd9\x1c\xa8\xf0\x816\x19ހ\xe3\xaa0\n\xeb\xae\x1ff.z@\xa64A\xd5\x1b\xd9\xca\x19\xd9\xfe4\x05\u03a2\xa5q\xdeV\xc0\xf9J\x92H\x05\x13Ӈ\x8e''&\xe4\xef@\xbe0~6*\xdfo\xe8w\xf252Q\x87\xa2\x83\x02~\xdc\xe0h[\xa6\x12\x18p\xc3\x06\f!KTIr%V\x9a.㦞կ\x91S\xd6#\xb1\xfac)99t\xd2\xf7\xe7\x9aQ\xfb$\x98\xb1\xe0\v7\xac\xfe-\x95nRd\xa8&x\x00\x0e\x83Z\x8e\x0e\xb3\xa9\xa9\xc7\xe4\x05y\x9e\x83\x16\x13\xd0M\xb6?\x14\xf500i\x97\x92P\xd8\xc5ҋ\xee@AZ\xbboʩ\xa2\xea\x98\xf1\x16\xd14nŜ\x8d\xb7\x14L\xc3\n\v\xe7\x80\xc5\b\xf0Q\xe7I\x83ls\x9e\x02k4(5\x92\xf0\x82\x8c\xa8ծ\x92\xec>i\x8c5}*Z\xa8\x8c2ҥ\xcd~˻\xbe\x8bcc\xa9\x80\x12\xb9\xf8W\xbd\xc7\xdfKE}\xf0&:\x1f\xa7X\x1b'q\x1d\xff\xd0l\xfe\xd1\xf6u\x14\x88\u05fe\xaaĺ>\x15\x90\x9ff\x00']\x84\x7f\x91\x05\xfc\xe8\xfdx>\xea\xea>d_\x97\x7f\xfd\xd8\n\xd7\xeeӏ¯\\\xb8\x1f\x85ϼ\x91\xdf>\xb4\xd8\n\xbaG\x9e\\\xd3\x7f\x961\xb7\xe4\xdb\xfb\x0f3\xc0\x13.\xf5?\u0380]\xbd\xeb\xe7\x80\xe6\x85\x00\xe6#\xad\x1f\x9c\xed\x11\x81\xdf\xe4\xaeg\x9b\xd5\xfbe\xf60\x0f\xc4\x14n\x1f\x84${\b4\xe4\xa8\xf0\x95\xebu\xce9I\vK\xe4L6'Z1\x1fg]\xec6\a/rؔ\x12\xdd\xf88\x1b7)\xfc\x91=\xed\xa5\xf8ȯ\xb2A7\aPr\x0e\xfbh\xa8>\xcc\xf28\x92\v\xc6d\xf343Zs\xfbTH)\xf4!\x9c\x9f\xe7L'5\xb2\x93c\xe13\x03>9vj%\x0e\x94c\xa63\xc2C2\xb4?z\x02\xdfO\x9e\xc0\x86\xd8\xc6\x0f\x92As\x82\v鷝\x94X\xc3\xc6\xfb\x81\x12\xea\xf2\x05\x119\x92\x9d\xe7y\xf3\xd4!K\xb75\x1c\x84D\\\x96-kHe\xa6N'ma}\x04\x14\xae\xc07\x1c\xb71j\xa1ɏ\xa8yAw\xb1\xe1f3A1\xf4ھ\x16\xe9\x18Y\xa1\x889~̮n\x8fL\xa4\x8bDF\xc0\"\xfd\xd8\xff\x1f\xe2\x18\xadw\xf85\xdc۶\x83/$\x92\xa4*\xd5q\xf7\xef\xab~\xf3\xca>\xcdNJ\xed\x93\a\xbai\xb7~\x92\xb3\x1a\xadҌ\xb9\xebW\xe1\xefR\xfa\x91\xf5AIk\xc2٤Mޡ\xce\xdb!\x03\xa5\x90\xf4\x1c\xbe\x9b<\x87h\x16ρ`\xbd\xe3\xf3W\xf5(\x14k\xba\x8b\x7f\x1c\x8f\xb6e\xfb\xbb\xfd\xe2\b\x89[\xfc\xb7\x8d\xbba\xc0\v|0\xc1\x14\xcd\x10W\x99\xeeq\xba\xe08T\"\xcee\xef-\x93\xa1\x9aoS,\xb2\x9a\x9f\xb8\v\t\x17\xe3\xf2x\xf5\xfc۽\xc5^|R\xf0lk\xba\xedp\xc0\x80\x04\x8cl\xe1e\xdc\xf40[\xf4ݲ\xec\x031O^}\x8a\x9d\xae\xd3,\xf6\xca\xd5'ݏ\xb1x\xecŽ0\x81)滐\x16\xb1\xe5\xecJ\xb0\x05=\xf5\xb5\x87Ř\x03\xc2%\xa3\x8du\x8d\x86\xea\xd8?\xdd|\xe8\x04\xf9=\x94\xf5\xc1\xe6\xc0\xd9\xca\xe8·y\xaa\xe2\xb9!\x12\xfa\x1cԡ\x9a\xaa^\x9f=\xf5j0\x17\xbb7\xfe$\xf4q\x9bg\x8e\f\xe1\x7f\xe5\xfatm\xc9:Y1+Gl\xda\x18vר\xebvDJ3\x85T\xfd\x04\xa8\x18S\xf4\xe3\xf0\x8eRB\xeb\xdbG\xe9\b\x96\xe4m\xe5A\xf3q\xdb?\a/\x02\xe0\x9b\x8c}$\xdf5ѪG\xb2p\x1f0\xdfO\x83\xcd^{\xdaMM-~\xc3\x18P0C٥\x01\x03\xc2u\v\x14\xab}\x8f'\x16\xdb\x1e\xb5\x93\x90xOۣ\x16\x92\xb5-D\xd9ԉ\xc2\xf6 \x94\xc9\xd2$@\xc7\x13\x90\xfb>\xb14 ǘ\\\x97c\x0eY\x8c!S\x1f;by5\x1b\rh`\xc7\xd2lM\xe0)9\xd9\x04,J\xe71A\xe5\xb9l\xe5\x10\x03L\n\xb2I\x9d\xb7\xa9ܷO\a6\xd2\xf6H\xe4\xf7\x1f\xa2\xef\xaa\x16\xa78\x83\rs\xa8BF\x9a\x8a\x87\xb3I\xbb\x04\xd6v\x86{ar\xdd\xd5\xce钟\x10\x1e\xa1\xa0\xfd\xd3\x14uOSk\x89\xe0j\xad\x8b\x1dNnY\xa1\x97s?Y\xbcl\x19k\xa2\xdcy\xe7\xf7J}\xa2\x8d\xea^\x0f\xab@\xadџ\x95*\xf8L\n\xb8\xac#\u008aQ\x01U'\xc9\x15ʊ#q*+\xfd\xa2\xa3\xc0^b\x1fG\x8e\xc5wVG`\x8d\x8e\xefuT\x12\x13ᥓ\xb6qRL\xe0\xc24\x01\xec\xa8\x04\xac\xadq\xc7\xc1\xd7\x0e\xed\xec\x9bϋ\x90\xa5\x1f\x01w\x9c\x03\x95%Q.\x02\xad\xc0\x18\x8c\xe7y\xf3h\xfdk\xa8\x18\x04ဧ\xa8\xf3\xe6\x11j\xe8_j|ThOp\x01\x8e\fC\xc9P\x1d;\x92\x81P\xd0\bF''\x99\xadU\xfc7~Դq\x9c\x1e\xb6\x06\xa9\xed\xced\xb5l\xda8z6\b\xf8\xfb\x13\xc4#p\xa0\x15\xe0\xf2p/\xed9\xf5\xb7\xfd\x1a\xe9a7\xff5EL]efq\xe8\xe4\xcb\xc4\xf2\xa4\xe2\xb6\x1c\x8e\xe46F\xb4m\xd3k\xe6\xb7\x12!\xfb\x1c\x98\x91\xeb\xbc\xe7\xc2\xec\xb90{.̞\v\xb3\xe7\xc2\xec\xb90{.̞\v\xb3\xe7\xc2\xec\xb90{.̞\v\xb3\xe7\xc2\xec\xb90{.̞\v\xb3\xe7\xc2\xec\xb90{.̞\v\xb3\xe7\xc2\xec\xb90{.L\"\xe4'\x99\v\x13\xfb=\x86\xf1\x9b\x00\xf0+\xb1\x01\x95\xdaURg\xd7\xf8PӞgqR\x9e\xa5\xf9\x91@\xf3\xb3cK\xbf\x14h\x7fTk\xf1\x97\xc9\xfaw\uf55f\xde\x1a\x9e]\xd7\x1e\xc8G+Hz\xa4\xfd\x1f\x00\x00\x00\xff\xff\x01\x00\x00\xff\xffr\xd3-\x11\x0eQ\x00\x00")) } diff --git a/api/compute/schema.yml b/api/compute/schema.yml index 68c22bd76..d9fac2605 100644 --- a/api/compute/schema.yml +++ b/api/compute/schema.yml @@ -239,6 +239,11 @@ kinds: doc: The status of the pod choices: [pending, not_ready, running, stopped, dead] + startup_outcome: + type: enum + choices: [startup_running, startup_failed] + doc: Durable distinction between a sandbox that reached RUNNING and one that failed before then + last_activity: type: time doc: Last lease activity (throttled updates, ~30s granularity for scale-down) @@ -503,16 +508,21 @@ kinds: # Crash loop detection and cooldown (managed by SandboxPoolManager) consecutive_crash_count: type: int - doc: Number of consecutive quick crashes (sandboxes that died within 60s of creation) + doc: Number of consecutive sandbox failures before reaching RUNNING last_crash_time: type: time - doc: Timestamp of the most recent quick crash + doc: Timestamp of the most recently counted sandbox failure cooldown_until: type: time doc: Timestamp until which new sandbox creation is paused due to crash loop + counted_failures: + type: ref + many: true + doc: Sandbox IDs already included in the failure streak; kept across streak resets + # Version references for pool reuse across deployments referenced_by_versions: type: ref diff --git a/controllers/nodehealth/controller.go b/controllers/nodehealth/controller.go index cf1b7c7fa..8b1cac127 100644 --- a/controllers/nodehealth/controller.go +++ b/controllers/nodehealth/controller.go @@ -242,12 +242,21 @@ func (c *Controller) markNodeSandboxesDead(ctx context.Context, nodeID entity.Id "node", nodeID, "previous_status", sb.Status) + dead := &compute_v1alpha.Sandbox{Status: compute_v1alpha.DEAD} + if sb.StartupOutcome == "" { + switch sb.Status { + case compute_v1alpha.PENDING, compute_v1alpha.NOT_READY: + dead.StartupOutcome = compute_v1alpha.STARTUP_FAILED + case compute_v1alpha.RUNNING: + dead.StartupOutcome = compute_v1alpha.STARTUP_RUNNING + case compute_v1alpha.STOPPED, compute_v1alpha.DEAD: + // STOPPED may be an intentional retirement; DEAD was skipped above. + } + } _, err := c.eac.Patch(ctx, entity.New( entity.DBId, sb.ID, - (&compute_v1alpha.Sandbox{ - Status: compute_v1alpha.DEAD, - }).Encode, - ).Attrs(), 0) + dead.Encode, + ).Attrs(), e.Revision()) if err != nil { c.log.Error("failed to mark sandbox dead", "sandbox", sb.ID, "error", err) patchErr = errors.Join(patchErr, err) diff --git a/controllers/nodehealth/controller_test.go b/controllers/nodehealth/controller_test.go index 34c89e5c2..b3680dddf 100644 --- a/controllers/nodehealth/controller_test.go +++ b/controllers/nodehealth/controller_test.go @@ -156,7 +156,9 @@ func TestNonReadyNodeGracePeriodExpired(t *testing.T) { require.NoError(t, err) sbRunning := createScheduledSandbox(t, ctx, server, "sb-running", nodeID, compute_v1alpha.RUNNING) + server.Store.NowFunc = func() time.Time { return time.Now().Add(-6 * time.Minute) } sbPending := createScheduledSandbox(t, ctx, server, "sb-pending", nodeID, compute_v1alpha.PENDING) + server.Store.NowFunc = nil ctrl := NewController(testutils.TestLogger(t), server.EAC) ctrl.gracePeriod = 5 * time.Minute @@ -175,6 +177,19 @@ func TestNonReadyNodeGracePeriodExpired(t *testing.T) { "RUNNING sandbox should be marked DEAD after grace period") assert.Equal(t, compute_v1alpha.DEAD, getSandboxStatus(t, ctx, server, sbPending), "PENDING sandbox should be marked DEAD after grace period") + for _, tc := range []struct { + id entity.Id + want compute_v1alpha.SandboxStartupOutcome + }{ + {sbRunning, compute_v1alpha.STARTUP_RUNNING}, + {sbPending, compute_v1alpha.STARTUP_FAILED}, + } { + resp, err := server.EAC.Get(ctx, tc.id.String()) + require.NoError(t, err) + var sb compute_v1alpha.Sandbox + sb.Decode(resp.Entity().Entity()) + assert.Equal(t, tc.want, sb.StartupOutcome) + } } func TestNodeRecoversWithinGracePeriod(t *testing.T) { diff --git a/controllers/sandbox/create_saga.go b/controllers/sandbox/create_saga.go index 3708471fe..b7a512700 100644 --- a/controllers/sandbox/create_saga.go +++ b/controllers/sandbox/create_saga.go @@ -2,6 +2,7 @@ package sandbox import ( "context" + "errors" "fmt" "log/slog" "net/netip" @@ -493,6 +494,9 @@ func waitPorts(ctx context.Context, in waitPortsIn) (waitPortsOut, error) { if err == nil { continue // configured port bound — the normal case } + if errors.Is(err, errProcessExited) { + return waitPortsOut{}, err + } if ctx.Err() != nil { // We're shutting down, not looking at a port mismatch — skip // diagnosis so we don't emit misleading "listening elsewhere" events. @@ -616,6 +620,7 @@ func setRunning(ctx context.Context, in setRunningIn) (setRunningOut, error) { func() []entity.Attr { attrs := []entity.Attr{ entity.Ref(compute.SandboxStatusId, compute.SandboxStatusRunningId), + entity.Ref(compute.SandboxStartupOutcomeId, compute.SandboxStartupOutcomeStartupRunningId), } for _, op := range in.ObservedPorts { bp := compute.BoundPort{Port: int64(op.Port), Address: op.Address} diff --git a/controllers/sandbox/create_saga_test.go b/controllers/sandbox/create_saga_test.go index 7b2e6bdbc..cfb332d64 100644 --- a/controllers/sandbox/create_saga_test.go +++ b/controllers/sandbox/create_saga_test.go @@ -556,6 +556,15 @@ func TestCreateSandboxSaga_HappyPath(t *testing.T) { assert.Equal(t, 1, h.runtime.waitForPortCalls) assert.Equal(t, 1, h.obs.addMetricsCalls) assert.Equal(t, 1, h.obs.updateSvcsCalls) + runningOutcome := false + for _, attrs := range h.entities.patchCalls { + for _, attr := range attrs { + if attr.ID == compute.SandboxStartupOutcomeId && attr.Value.Id() == compute.SandboxStartupOutcomeStartupRunningId { + runningOutcome = true + } + } + } + assert.True(t, runningOutcome, "reaching RUNNING must persist the startup outcome") // No undo actions called assert.Equal(t, 0, h.networking.releaseCalls) @@ -717,6 +726,20 @@ func TestCreateSandboxSaga_WaitPortsFails(t *testing.T) { assert.Equal(t, 1, h.networking.releaseCalls) } +func TestCreateSandboxSaga_WaitPortsProcessExit(t *testing.T) { + h := newTestHarness(t) + h.runtime.waitForPortErr = errProcessExited + h.runtime.mockContainer.taskFn = func(ctx context.Context, attach cio.Attach) (containerd.Task, error) { + return h.runtime.mockTask, nil + } + + err := h.execute(t) + require.ErrorContains(t, err, errProcessExited.Error()) + assert.Equal(t, 0, h.runtime.diagnoseListeningCalls, + "an exited process cannot be auto-routed to a different port") + assert.Equal(t, saga.StatusFailed, h.execution(t).Status) +} + func TestSingleAlternativePort(t *testing.T) { // Exactly one routable port that isn't the configured one -> auto-routable. alt, ok := singleAlternativePort([]int{3000}, 8080) diff --git a/controllers/sandbox/saga_controller.go b/controllers/sandbox/saga_controller.go index d38b47ae2..f989ba323 100644 --- a/controllers/sandbox/saga_controller.go +++ b/controllers/sandbox/saga_controller.go @@ -89,9 +89,17 @@ func (c *SandboxController) createSandboxViaSaga(ctx context.Context, co *comput // NOTE: this runs at the call site, so a crash between saga completion // and this patch leaves the entity PENDING (retried by reconciler). // Durable saga outcome declaration is future work. + current, _, getErr := c.ops.GetSandbox(ctx, co.ID.String()) + if getErr != nil { + return fmt.Errorf("fetching sandbox after saga failure: %w", getErr) + } + failure := &compute.Sandbox{Status: compute.DEAD} + if current.StartupOutcome != compute.STARTUP_RUNNING && current.Status != compute.RUNNING { + failure.StartupOutcome = compute.STARTUP_FAILED + } patchAttrs := entity.New( entity.Ref(entity.DBId, co.ID), - (&compute.Sandbox{Status: compute.DEAD}).Encode, + failure.Encode, ) if _, patchErr := c.ops.PatchSandbox(ctx, patchAttrs.Attrs(), 0); patchErr != nil { c.Log.Error("failed to mark sandbox DEAD after saga failure", "id", co.ID, "error", patchErr) diff --git a/controllers/sandbox/sandbox.go b/controllers/sandbox/sandbox.go index c6276ecfc..a5fe0e18a 100644 --- a/controllers/sandbox/sandbox.go +++ b/controllers/sandbox/sandbox.go @@ -2,6 +2,7 @@ package sandbox import ( "context" + stderrors "errors" "fmt" "io" "log/slog" @@ -71,9 +72,12 @@ func cleanupAttach() cio.Attach { } type containerPorts struct { - Ports []observability.BoundPort + Ports []observability.BoundPort + exited bool } +var errProcessExited = stderrors.New("sandbox process exited") + // SandboxControllerDeps holds required dependencies for SandboxController. type SandboxControllerDeps struct { Log *slog.Logger @@ -365,8 +369,8 @@ func (c *SandboxController) SetPortStatus(id string, port observability.BoundPor func (c *SandboxController) WaitForPort(ctx context.Context, id string, port int, timeout time.Duration) error { deadline := time.Now().Add(timeout) - // Create a channel to signal when port is ready - done := make(chan struct{}) + // Signal either readiness or process exit. + done := make(chan error, 1) cancelled := make(chan struct{}) go func() { @@ -385,10 +389,14 @@ func (c *SandboxController) WaitForPort(ctx context.Context, id string, port int ports = &containerPorts{} c.portMap[id] = ports } + if ports.exited { + done <- fmt.Errorf("%w: %s while waiting for port %d", errProcessExited, id, port) + return + } for _, p := range ports.Ports { if p.Port == port { - close(done) + done <- nil return } } @@ -398,8 +406,8 @@ func (c *SandboxController) WaitForPort(ctx context.Context, id string, port int }() select { - case <-done: - return nil + case err := <-done: + return err case <-ctx.Done(): close(cancelled) c.portCond.Broadcast() // Wake up the waiting goroutine @@ -412,6 +420,12 @@ func (c *SandboxController) WaitForPort(ctx context.Context, id string, port int // return a spurious timeout even though the port is in fact bound. c.portMu.Lock() if ports, ok := c.portMap[id]; ok { + if ports.exited { + c.portMu.Unlock() + close(cancelled) + c.portCond.Broadcast() + return fmt.Errorf("%w: %s while waiting for port %d", errProcessExited, id, port) + } for _, p := range ports.Ports { if p.Port == port { c.portMu.Unlock() @@ -426,6 +440,18 @@ func (c *SandboxController) WaitForPort(ctx context.Context, id string, port int } } +func (c *SandboxController) setProcessExited(id string) { + c.portMu.Lock() + defer c.portMu.Unlock() + ports := c.portMap[id] + if ports == nil { + ports = &containerPorts{} + c.portMap[id] = ports + } + ports.exited = true + c.portCond.Broadcast() +} + // mapLegacyProtocol converts legacy PortProtocol values to SandboxSpecContainerPortProtocol func mapLegacyProtocol(legacy compute.PortProtocol) compute.SandboxSpecContainerPortProtocol { switch legacy { @@ -1327,7 +1353,7 @@ func (c *SandboxController) Create(ctx context.Context, co *compute.Sandbox, met "id", co.ID, "createdAt", createdAt, "age", age) patchAttrs := entity.New( entity.Ref(entity.DBId, co.ID), - (&compute.Sandbox{Status: compute.RUNNING}).Encode, + (&compute.Sandbox{Status: compute.RUNNING, StartupOutcome: compute.STARTUP_RUNNING}).Encode, ) _, err := c.ops.PatchSandbox(ctx, patchAttrs.Attrs(), meta.Revision) if err != nil { @@ -1356,7 +1382,7 @@ func (c *SandboxController) Create(ctx context.Context, co *compute.Sandbox, met c.Log.Info("marking unhealthy sandbox as DEAD", "id", co.ID) patchAttrs := entity.New( entity.Ref(entity.DBId, co.ID), - (&compute.Sandbox{Status: compute.DEAD}).Encode, + (&compute.Sandbox{Status: compute.DEAD, StartupOutcome: compute.STARTUP_RUNNING}).Encode, ) _, err := c.ops.PatchSandbox(ctx, patchAttrs.Attrs(), 0) if err != nil { @@ -1416,7 +1442,7 @@ func (c *SandboxController) markDeadNoRestart(ctx context.Context, co *compute.S if co.Status != compute.DEAD { patchAttrs := entity.New( entity.Ref(entity.DBId, co.ID), - (&compute.Sandbox{Status: compute.DEAD}).Encode, + (&compute.Sandbox{Status: compute.DEAD, StartupOutcome: compute.STARTUP_RUNNING}).Encode, ) result, err := c.EAC.Patch(ctx, patchAttrs.Attrs(), 0) if err != nil { @@ -2253,6 +2279,9 @@ func (c *SandboxController) BootContainers( } c.Log.Info("container started", "id", cc.ID()) + c.portMu.Lock() + c.portMap[cc.ID()] = &containerPorts{} + c.portMu.Unlock() if hub != nil { hub.SetResizer(task) @@ -2321,6 +2350,7 @@ func (c *SandboxController) monitorTaskExit( "exit_code", exitStatus.ExitCode(), "exit_time", exitStatus.ExitTime(), ) + c.setProcessExited(containerID) // We don't delete the task here so that our destroySubContainers function // has a consistent view of the state of containers and tasks. @@ -2441,9 +2471,27 @@ func (c *SandboxController) recordExit( return nil, err } + var current compute.Sandbox + current.Decode(resp.Entity().Entity()) + stopped := &compute.Sandbox{Status: compute.STOPPED, Exit: exit} + if current.Status == compute.DEAD { + // A failure or node-loss handler already retired it. A delayed + // exit report may attach the result, but must not revive STOPPED. + stopped.Status = "" + } + if current.StartupOutcome == "" { + switch current.Status { + case compute.RUNNING: + stopped.StartupOutcome = compute.STARTUP_RUNNING + case compute.PENDING: + stopped.StartupOutcome = compute.STARTUP_FAILED + case compute.NOT_READY, compute.STOPPED, compute.DEAD: + // No lifecycle conclusion from these states alone. + } + } patchAttrs := entity.New( entity.Ref(entity.DBId, id), - (&compute.Sandbox{Status: compute.STOPPED, Exit: exit}).Encode, + stopped.Encode, ) result, err := c.EAC.Patch(ctx, patchAttrs.Attrs(), resp.Entity().Revision()) @@ -3263,12 +3311,19 @@ func (c *SandboxController) StopSandbox(ctx context.Context, id entity.Id, sb *c tmpDir := filepath.Join(c.Tempdir, "containerd", id.PathSafe()) _ = os.RemoveAll(tmpDir) - // Mark sandbox as DEAD in entity store + // Preserve the lifecycle outcome when retiring a sandbox. Intentional + // STOPPED sandboxes (including scale-down) acquire no failure outcome. + retired := &compute.Sandbox{Status: compute.DEAD} + if sb != nil { + if sb.Status == compute.PENDING && sb.StartupOutcome != compute.STARTUP_RUNNING { + retired.StartupOutcome = compute.STARTUP_FAILED + } else if sb.Status == compute.RUNNING && sb.StartupOutcome == "" { + retired.StartupOutcome = compute.STARTUP_RUNNING + } + } result, err := c.EAC.Patch(ctx, entity.New( entity.Ref(entity.DBId, id), - (&compute.Sandbox{ - Status: compute.DEAD, - }).Encode, + retired.Encode, ).Attrs(), 0) if err != nil { // We ignore if the entity is not found as we run this code path when detecting diff --git a/controllers/sandbox/sandbox_test.go b/controllers/sandbox/sandbox_test.go index 8ac8f316f..c8756072f 100644 --- a/controllers/sandbox/sandbox_test.go +++ b/controllers/sandbox/sandbox_test.go @@ -28,6 +28,7 @@ import ( "miren.dev/runtime/api/entityserver/entityserver_v1alpha" "miren.dev/runtime/observability" "miren.dev/runtime/pkg/entity" + entitytestutils "miren.dev/runtime/pkg/entity/testutils" "miren.dev/runtime/pkg/entity/types" "miren.dev/runtime/pkg/idgen" "miren.dev/runtime/pkg/saga" @@ -1694,6 +1695,36 @@ func TestMonitorTaskExitIgnoresErrorStatus(t *testing.T) { }, 5*time.Second, 50*time.Millisecond, "sandbox should remain RUNNING when exit status has an error") } +func TestRecordExitStartupOutcome(t *testing.T) { + ctx := context.Background() + server, cleanup := entitytestutils.NewInMemEntityServer(t) + defer cleanup() + c := &SandboxController{EAC: server.EAC} + + for _, tc := range []struct { + status compute.SandboxStatus + want compute.SandboxStartupOutcome + final compute.SandboxStatus + }{ + {compute.PENDING, compute.STARTUP_FAILED, compute.STOPPED}, + {compute.RUNNING, compute.STARTUP_RUNNING, compute.STOPPED}, + {compute.DEAD, "", compute.DEAD}, + } { + t.Run(string(tc.status), func(t *testing.T) { + id, err := server.Client.Create(ctx, string(tc.status), &compute.Sandbox{Status: tc.status}) + require.NoError(t, err) + _, err = c.recordExit(ctx, id, compute.Exit{At: time.Now(), Container: "app"}) + require.NoError(t, err) + resp, err := server.EAC.Get(ctx, id.String()) + require.NoError(t, err) + var sb compute.Sandbox + sb.Decode(resp.Entity().Entity()) + require.Equal(t, tc.final, sb.Status) + require.Equal(t, tc.want, sb.StartupOutcome) + }) + } +} + // TestMonitorTaskExitHandlesValidExit verifies that monitorTaskExit correctly // marks a sandbox as STOPPED when receiving a valid exit status (no error). func TestMonitorTaskExitHandlesValidExit(t *testing.T) { diff --git a/controllers/sandbox/waitforport_race_test.go b/controllers/sandbox/waitforport_race_test.go index 652ba9e7f..20b1e7fe1 100644 --- a/controllers/sandbox/waitforport_race_test.go +++ b/controllers/sandbox/waitforport_race_test.go @@ -2,6 +2,7 @@ package sandbox import ( "context" + "errors" "log/slog" "sync" "testing" @@ -30,6 +31,28 @@ func boundPort(port int) observability.BoundPort { return observability.BoundPort{Port: port} } +func TestWaitForPortProcessExit(t *testing.T) { + c := newPortTestController() + result := make(chan error, 1) + go func() { result <- c.WaitForPort(context.Background(), "app", 8080, time.Minute) }() + c.setProcessExited("app") + select { + case err := <-result: + require.True(t, errors.Is(err, errProcessExited), "got %v", err) + case <-time.After(time.Second): + t.Fatal("port wait did not stop on process exit") + } + + // A later wait must not forget an exit or mistake a stale bound port for readiness. + c.SetPortStatus("app", boundPort(8080), observability.PortStatusBound) + require.ErrorIs(t, c.WaitForPort(context.Background(), "app", 8080, time.Minute), errProcessExited) + + other := newPortTestController() + err := other.WaitForPort(context.Background(), "still-running", 8080, 20*time.Millisecond) + require.ErrorContains(t, err, "timeout waiting for port 8080") + require.NotErrorIs(t, err, errProcessExited) +} + // TestWaitForPortNoSpuriousTimeoutWhenPortBound is the deterministic regression // guard for the WaitForPort deadline race (controllers/sandbox/sandbox.go). // diff --git a/controllers/sandboxpool/manager.go b/controllers/sandboxpool/manager.go index b2e14e299..4e98433a3 100644 --- a/controllers/sandboxpool/manager.go +++ b/controllers/sandboxpool/manager.go @@ -86,7 +86,7 @@ func (m *Manager) Reconcile(ctx context.Context, pool *compute_v1alpha.SandboxPo // Skip crash detection for decommissioned pools (desired=0, no references). // Sandbox deaths during intentional scale-down are expected, not crashes. if pool.DesiredInstances > 0 || len(pool.ReferencedByVersions) > 0 { - newCrashes := m.countQuickCrashes(sandboxes, pool.LastCrashTime) + newCrashes := m.countStartupFailures(sandboxes, pool) if newCrashes > 0 { pool.ConsecutiveCrashCount += int64(newCrashes) pool.LastCrashTime = time.Now() @@ -781,56 +781,65 @@ func (m *Manager) checkForStalePendingSandboxes(ctx context.Context) error { if createdAt.After(threshold) { continue } + m.retireStalePending(ctx, ent) + } - m.log.Warn("marking stale PENDING sandbox as STOPPED", - "sandbox", sb.ID, - "created_at", createdAt, - "age", now.Sub(createdAt)) + return nil +} - if _, err := m.eac.Patch(ctx, entity.New( - entity.DBId, sb.ID, - (&compute_v1alpha.Sandbox{ - Status: compute_v1alpha.STOPPED, - }).Encode, - ).Attrs(), 0); err != nil { - if errors.Is(err, cond.ErrNotFound{}) { - m.log.Warn("sandbox already deleted during stale pending check", - "sandbox", sb.ID) - } else { - m.log.Error("failed to stop stale PENDING sandbox", - "sandbox", sb.ID, - "error", err) - } - continue +func (m *Manager) retireStalePending(ctx context.Context, ent *entityserver_v1alpha.Entity) { + var sb compute_v1alpha.Sandbox + sb.Decode(ent.Entity()) + createdAt := time.UnixMilli(ent.CreatedAt()) + m.log.Warn("marking stale PENDING sandbox as STOPPED", + "sandbox", sb.ID, + "created_at", createdAt, + "age", time.Since(createdAt)) + + if _, err := m.eac.Patch(ctx, entity.New( + entity.DBId, sb.ID, + (&compute_v1alpha.Sandbox{ + Status: compute_v1alpha.STOPPED, + StartupOutcome: compute_v1alpha.STARTUP_FAILED, + }).Encode, + ).Attrs(), ent.Revision()); err != nil { + if errors.Is(err, cond.ErrNotFound{}) { + m.log.Warn("sandbox already deleted during stale pending check", "sandbox", sb.ID) + } else if errors.Is(err, cond.ErrConflict{}) { + m.log.Debug("sandbox changed during stale pending check", "sandbox", sb.ID) + } else { + m.log.Error("failed to stop stale PENDING sandbox", "sandbox", sb.ID, "error", err) } } - - return nil } -// countQuickCrashes counts sandboxes that died within 60 seconds of creation -// and occurred after lastCrashTime -func (m *Manager) countQuickCrashes(sandboxes []*sandboxWithMeta, lastCrashTime time.Time) int64 { +// countStartupFailures records each sandbox ID with the counter in the pool +// update. UpdatedAt is mutable and cannot identify an already counted failure. +// Legacy sandboxes without an outcome retain the old quick-crash heuristic. +func (m *Manager) countStartupFailures(sandboxes []*sandboxWithMeta, pool *compute_v1alpha.SandboxPool) int64 { count := int64(0) - crashThreshold := 60 * time.Second + counted := make(map[entity.Id]bool, len(pool.CountedFailures)) + for _, id := range pool.CountedFailures { + counted[id] = true + } for _, sbm := range sandboxes { - if sbm.sandbox.Status != compute_v1alpha.DEAD { + if sbm.sandbox.Status != compute_v1alpha.DEAD || counted[sbm.sandbox.ID] { continue } - // Check if this is a quick crash (died within 60s of creation) - lifetime := sbm.updatedAt.Sub(sbm.createdAt) - - if lifetime >= crashThreshold { - continue // Lived long enough, not a quick crash + if sbm.sandbox.StartupOutcome == compute_v1alpha.STARTUP_RUNNING || + (sbm.sandbox.StartupOutcome == "" && sbm.updatedAt.Sub(sbm.createdAt) >= 60*time.Second) { + continue } - // Check if this crash is new (after lastCrashTime) - if !lastCrashTime.IsZero() && !sbm.updatedAt.After(lastCrashTime) { - continue // Already counted this crash + // Pre-upgrade crashes did not have durable IDs. The old watermark + // protects them across deploys that reset the streak. + if sbm.sandbox.StartupOutcome == "" && !pool.LastCrashTime.IsZero() && !sbm.updatedAt.After(pool.LastCrashTime) { + continue } + pool.CountedFailures = append(pool.CountedFailures, sbm.sandbox.ID) count++ } diff --git a/controllers/sandboxpool/manager_test.go b/controllers/sandboxpool/manager_test.go index ce0c9f178..cb64d75e0 100644 --- a/controllers/sandboxpool/manager_test.go +++ b/controllers/sandboxpool/manager_test.go @@ -1110,6 +1110,32 @@ func TestCheckForStalePendingSandboxes(t *testing.T) { "RUNNING sandbox should remain RUNNING") } +func TestStalePendingDoesNotOverwriteRunning(t *testing.T) { + ctx := context.Background() + server, cleanup := testutils.NewInMemEntityServer(t) + defer cleanup() + server.Store.NowFunc = func() time.Time { return time.Now().Add(-6 * time.Minute) } + id, err := server.Client.Create(ctx, "pending", &compute_v1alpha.Sandbox{Status: compute_v1alpha.PENDING}) + require.NoError(t, err) + server.Store.NowFunc = nil + + // The monitor's snapshot predates a successful setRunning patch. + stale, err := server.EAC.Get(ctx, id.String()) + require.NoError(t, err) + _, err = server.EAC.Patch(ctx, entity.New(entity.DBId, id, + (&compute_v1alpha.Sandbox{Status: compute_v1alpha.RUNNING, StartupOutcome: compute_v1alpha.STARTUP_RUNNING}).Encode).Attrs(), stale.Entity().Revision()) + require.NoError(t, err) + + manager := NewManager(testutils.TestLogger(t), server.EAC) + manager.retireStalePending(ctx, stale.Entity()) + resp, err := server.EAC.Get(ctx, id.String()) + require.NoError(t, err) + var sb compute_v1alpha.Sandbox + sb.Decode(resp.Entity().Entity()) + assert.Equal(t, compute_v1alpha.RUNNING, sb.Status) + assert.Equal(t, compute_v1alpha.STARTUP_RUNNING, sb.StartupOutcome) +} + // TestStalePendingSandboxUnblocksPoolCapacity tests that marking a stale // PENDING sandbox as STOPPED frees pool capacity, allowing the pool to // create a replacement sandbox on the next reconcile. @@ -1482,3 +1508,87 @@ func TestManagerCrashResetDoesNotRecount(t *testing.T) { assert.True(t, updatedPool.CooldownUntil.IsZero(), "pool should not re-enter cooldown from old dead sandboxes") } + +func TestCountStartupFailures(t *testing.T) { + now := time.Now() + manager := &Manager{} + sandboxes := []*sandboxWithMeta{ + {sandbox: &compute_v1alpha.Sandbox{ID: "first", Status: compute_v1alpha.DEAD, StartupOutcome: compute_v1alpha.STARTUP_FAILED}, createdAt: now.Add(-6 * time.Minute), updatedAt: now.Add(-time.Minute)}, + {sandbox: &compute_v1alpha.Sandbox{ID: "healthy", Status: compute_v1alpha.DEAD, StartupOutcome: compute_v1alpha.STARTUP_RUNNING}, createdAt: now.Add(-time.Hour), updatedAt: now.Add(-30 * time.Second)}, + {sandbox: &compute_v1alpha.Sandbox{ID: "fast-healthy", Status: compute_v1alpha.DEAD, StartupOutcome: compute_v1alpha.STARTUP_RUNNING}, createdAt: now.Add(-20 * time.Second), updatedAt: now.Add(-10 * time.Second)}, + {sandbox: &compute_v1alpha.Sandbox{ID: "legacy", Status: compute_v1alpha.DEAD}, createdAt: now.Add(-20 * time.Second), updatedAt: now.Add(-10 * time.Second)}, + {sandbox: &compute_v1alpha.Sandbox{ID: "old-legacy", Status: compute_v1alpha.DEAD}, createdAt: now.Add(-6 * time.Minute), updatedAt: now.Add(-time.Minute)}, + {sandbox: &compute_v1alpha.Sandbox{ID: "old-failure", Status: compute_v1alpha.DEAD, StartupOutcome: compute_v1alpha.STARTUP_FAILED}, createdAt: now.Add(-10 * time.Minute), updatedAt: now.Add(-2 * time.Minute)}, + } + pool := &compute_v1alpha.SandboxPool{LastCrashTime: now.Add(-90 * time.Second), CountedFailures: []entity.Id{"old-failure"}} + assert.Equal(t, int64(2), manager.countStartupFailures(sandboxes, pool), + "long pre-running failure and legacy quick crash count, but running and previously counted failures do not") + assert.ElementsMatch(t, []entity.Id{"old-failure", "first", "legacy"}, pool.CountedFailures) +} + +func TestManagerLongStartupFailureBackoff(t *testing.T) { + ctx := context.Background() + server, cleanup := testutils.NewInMemEntityServer(t) + defer cleanup() + pool := &compute_v1alpha.SandboxPool{ + Service: "web", DesiredInstances: 1, + SandboxSpec: compute_v1alpha.SandboxSpec{Version: entity.Id("ver-1")}, + } + id, err := server.Client.Create(ctx, "pool", pool) + require.NoError(t, err) + pool.ID = id + manager := NewManager(testutils.TestLogger(t), server.EAC) + + for streak := int64(1); streak <= 2; streak++ { + if streak == 2 { + // A failure can become DEAD after the pool's List snapshot but + // before its watermark advances. Its older update time must not + // suppress a never-accounted-for sandbox. + _, err = server.EAC.Patch(ctx, entity.New(entity.DBId, id, + (&compute_v1alpha.SandboxPool{LastCrashTime: time.Now().Add(time.Minute)}).Encode).Attrs(), 0) + require.NoError(t, err) + } + created := time.Now().Add(-6 * time.Minute) + server.Store.NowFunc = func() time.Time { return created } + sbID, err := server.Client.Create(ctx, fmt.Sprintf("failed-%d", streak), + &compute_v1alpha.Sandbox{Status: compute_v1alpha.PENDING, Spec: pool.SandboxSpec}, + entityserver.WithLabels(types.LabelSet("service", "web", "pool", id.String()))) + require.NoError(t, err) + server.Store.NowFunc = nil + _, err = server.EAC.Patch(ctx, entity.New(entity.DBId, sbID, + (&compute_v1alpha.Sandbox{Status: compute_v1alpha.DEAD, StartupOutcome: compute_v1alpha.STARTUP_FAILED}).Encode).Attrs(), 0) + require.NoError(t, err) + before := time.Now() + reconcilePool(t, ctx, server, manager, pool) + updated := getPool(t, ctx, server, id) + assert.Equal(t, streak, updated.ConsecutiveCrashCount) + assert.WithinDuration(t, before.Add(backoffDuration(streak)), updated.CooldownUntil, 2*time.Second) + assert.Equal(t, int64(0), updated.CurrentInstances) + assert.Len(t, listSandboxesForPool(t, ctx, server, pool), int(streak), "no replacement during cooldown") + assert.Len(t, updated.CountedFailures, int(streak)) + // An exit recorder may rewrite DEAD to STOPPED after the pool + // counted it, and cleanup can then restore DEAD with a new UpdatedAt. + _, err = server.EAC.Patch(ctx, entity.New(entity.DBId, sbID, + (&compute_v1alpha.Sandbox{Status: compute_v1alpha.STOPPED}).Encode).Attrs(), 0) + require.NoError(t, err) + _, err = server.EAC.Patch(ctx, entity.New(entity.DBId, sbID, + (&compute_v1alpha.Sandbox{Status: compute_v1alpha.DEAD}).Encode).Attrs(), 0) + require.NoError(t, err) + // The same terminal entity must not increase the streak on another reconcile. + reconcilePool(t, ctx, server, manager, pool) + updated = getPool(t, ctx, server, id) + assert.Equal(t, streak, updated.ConsecutiveCrashCount) + assert.Len(t, updated.CountedFailures, int(streak)) + } + // A deployment clears the streak, not the identities already counted. + _, err = server.EAC.Patch(ctx, []entity.Attr{ + entity.Ref(entity.DBId, id), + entity.Int64(compute_v1alpha.SandboxPoolConsecutiveCrashCountId, 0), + entity.Time(compute_v1alpha.SandboxPoolCooldownUntilId, time.Time{}), + }, 0) + require.NoError(t, err) + reconcilePool(t, ctx, server, manager, pool) + updated := getPool(t, ctx, server, id) + assert.Equal(t, int64(0), updated.ConsecutiveCrashCount) + assert.Len(t, updated.CountedFailures, 2) +} From e91bb2ce7c93471e3af96ea1c66ee2bd6a44dcfd Mon Sep 17 00:00:00 2001 From: Evan Phoenix Date: Thu, 24 Sep 2026 18:27:45 +0000 Subject: [PATCH 2/8] Restore fast crash backoff and guard sandbox retirement Keep the prior quick-crash rule alongside durable pre-running failures. Mark saga failures DEAD when the follow-up fetch fails without guessing a stale startup outcome, and derive teardown outcomes from a revision-guarded current entity. Amp-Thread-ID: T-01a0cbe2-e330-77cb-bfe8-2363e67399f0 --- api/compute/compute_v1alpha/schema.gen.go | 2 +- api/compute/schema.yml | 2 +- controllers/sandbox/saga_controller.go | 17 ++--- controllers/sandbox/saga_controller_test.go | 32 ++++++++++ controllers/sandbox/sandbox.go | 70 ++++++++++++++------- controllers/sandbox/sandbox_test.go | 30 +++++++++ controllers/sandboxpool/manager.go | 4 +- controllers/sandboxpool/manager_test.go | 6 +- 8 files changed, 126 insertions(+), 37 deletions(-) diff --git a/api/compute/compute_v1alpha/schema.gen.go b/api/compute/compute_v1alpha/schema.gen.go index 2a769f106..f7703aed1 100644 --- a/api/compute/compute_v1alpha/schema.gen.go +++ b/api/compute/compute_v1alpha/schema.gen.go @@ -2226,7 +2226,7 @@ func (o *SandboxPool) Empty() bool { func (o *SandboxPool) InitSchema(sb *schema.SchemaBuilder) { sb.Ref("app", "dev.miren.compute/sandbox_pool.app", schema.Doc("Reference to the app this pool belongs to"), schema.Indexed, schema.Tags("dev.miren.app_ref")) - sb.Int64("consecutive_crash_count", "dev.miren.compute/sandbox_pool.consecutive_crash_count", schema.Doc("Number of consecutive sandbox failures before reaching RUNNING")) + sb.Int64("consecutive_crash_count", "dev.miren.compute/sandbox_pool.consecutive_crash_count", schema.Doc("Number of consecutive sandbox failures, including quick crashes after RUNNING")) sb.Time("cooldown_until", "dev.miren.compute/sandbox_pool.cooldown_until", schema.Doc("Timestamp until which new sandbox creation is paused due to crash loop")) sb.Ref("counted_failures", "dev.miren.compute/sandbox_pool.counted_failures", schema.Doc("Sandbox IDs already included in the failure streak; kept across streak resets"), schema.Many) sb.Int64("current_instances", "dev.miren.compute/sandbox_pool.current_instances", schema.Doc("Current number of sandbox instances (non-STOPPED)")) diff --git a/api/compute/schema.yml b/api/compute/schema.yml index d9fac2605..87221d5f4 100644 --- a/api/compute/schema.yml +++ b/api/compute/schema.yml @@ -508,7 +508,7 @@ kinds: # Crash loop detection and cooldown (managed by SandboxPoolManager) consecutive_crash_count: type: int - doc: Number of consecutive sandbox failures before reaching RUNNING + doc: Number of consecutive sandbox failures, including quick crashes after RUNNING last_crash_time: type: time diff --git a/controllers/sandbox/saga_controller.go b/controllers/sandbox/saga_controller.go index f989ba323..ea1327cbd 100644 --- a/controllers/sandbox/saga_controller.go +++ b/controllers/sandbox/saga_controller.go @@ -89,19 +89,22 @@ func (c *SandboxController) createSandboxViaSaga(ctx context.Context, co *comput // NOTE: this runs at the call site, so a crash between saga completion // and this patch leaves the entity PENDING (retried by reconciler). // Durable saga outcome declaration is future work. - current, _, getErr := c.ops.GetSandbox(ctx, co.ID.String()) - if getErr != nil { - return fmt.Errorf("fetching sandbox after saga failure: %w", getErr) - } + current, meta, getErr := c.ops.GetSandbox(ctx, co.ID.String()) failure := &compute.Sandbox{Status: compute.DEAD} - if current.StartupOutcome != compute.STARTUP_RUNNING && current.Status != compute.RUNNING { - failure.StartupOutcome = compute.STARTUP_FAILED + var revision int64 + if getErr != nil { + c.Log.Warn("failed to fetch sandbox after saga failure; leaving startup outcome unchanged", "id", co.ID, "error", getErr) + } else { + revision = meta.GetRevision() + if current.StartupOutcome != compute.STARTUP_RUNNING && current.Status != compute.RUNNING { + failure.StartupOutcome = compute.STARTUP_FAILED + } } patchAttrs := entity.New( entity.Ref(entity.DBId, co.ID), failure.Encode, ) - if _, patchErr := c.ops.PatchSandbox(ctx, patchAttrs.Attrs(), 0); patchErr != nil { + if _, patchErr := c.ops.PatchSandbox(ctx, patchAttrs.Attrs(), revision); patchErr != nil { c.Log.Error("failed to mark sandbox DEAD after saga failure", "id", co.ID, "error", patchErr) } diff --git a/controllers/sandbox/saga_controller_test.go b/controllers/sandbox/saga_controller_test.go index 183e4b5d5..a8ef2947c 100644 --- a/controllers/sandbox/saga_controller_test.go +++ b/controllers/sandbox/saga_controller_test.go @@ -2,6 +2,7 @@ package sandbox import ( "context" + "errors" "log/slog" "testing" "time" @@ -14,6 +15,37 @@ import ( "miren.dev/runtime/pkg/saga" ) +type sagaFailureOps struct { + SandboxEntityStore + SandboxNetworking + SandboxContainerRuntime + SandboxObservability +} + +func TestSagaFailureMarksDeadWhenFinalFetchFails(t *testing.T) { + h := newTestHarness(t) + h.networking.allocateErr = errors.New("no IPs available") + h.entities.getSandboxFunc = func(_ context.Context, _ string) (*compute.Sandbox, *entity.Meta, error) { + if h.entities.getCalls > 1 { + return nil, nil, errors.New("transient fetch failure") + } + return h.entities.sandbox, h.entities.meta, nil + } + c := &SandboxController{ + Log: slog.Default(), ops: sagaFailureOps{SandboxEntityStore: h.entities}, + executor: h.executor, sagaStorage: h.storage, + } + err := c.createSandboxViaSaga(context.Background(), h.entities.sandbox, false) + require.ErrorContains(t, err, "no IPs available") + require.Len(t, h.entities.patchCalls, 1) + patch := entity.New(h.entities.patchCalls[0]) + status, ok := patch.Get(compute.SandboxStatusId) + require.True(t, ok) + assert.Equal(t, compute.SandboxStatusDeadId, status.Value.Id()) + _, outcomeSet := patch.Get(compute.SandboxStartupOutcomeId) + assert.False(t, outcomeSet, "a failed fetch must not derive an outcome from the stale snapshot") +} + // newSagaControllerForResume wires up only what sagaResumeNeeded reads // (storage + log), so no live containerd client is needed. func newSagaControllerForResume(t *testing.T) *SandboxController { diff --git a/controllers/sandbox/sandbox.go b/controllers/sandbox/sandbox.go index a5fe0e18a..9a23de217 100644 --- a/controllers/sandbox/sandbox.go +++ b/controllers/sandbox/sandbox.go @@ -199,7 +199,12 @@ type SandboxController struct { // or unwinds on restart rather than stranding containers, addresses, and // disk leases. ops adapts this controller to the domain interfaces the // saga's actions are written against. - ops *sandboxOps + ops interface { + SandboxEntityStore + SandboxNetworking + SandboxContainerRuntime + SandboxObservability + } executor *saga.Executor sagaRegistry *saga.Registry sagaStorage saga.Storage @@ -3311,28 +3316,10 @@ func (c *SandboxController) StopSandbox(ctx context.Context, id entity.Id, sb *c tmpDir := filepath.Join(c.Tempdir, "containerd", id.PathSafe()) _ = os.RemoveAll(tmpDir) - // Preserve the lifecycle outcome when retiring a sandbox. Intentional - // STOPPED sandboxes (including scale-down) acquire no failure outcome. - retired := &compute.Sandbox{Status: compute.DEAD} - if sb != nil { - if sb.Status == compute.PENDING && sb.StartupOutcome != compute.STARTUP_RUNNING { - retired.StartupOutcome = compute.STARTUP_FAILED - } else if sb.Status == compute.RUNNING && sb.StartupOutcome == "" { - retired.StartupOutcome = compute.STARTUP_RUNNING - } - } - result, err := c.EAC.Patch(ctx, entity.New( - entity.Ref(entity.DBId, id), - retired.Encode, - ).Attrs(), 0) - if err != nil { - // We ignore if the entity is not found as we run this code path when detecting - // the sandbox entity has already been deleted. - if !errors.Is(err, cond.ErrNotFound{}) { - c.Log.Error("failed to mark sandbox as DEAD", "id", id, "error", err) - } - } else if c.writeTracker != nil && result.HasRevision() { - c.writeTracker.RecordWrite(result.Revision()) + // Use the current entity, not the cleanup snapshot: boot or an exit may + // have changed the lifecycle while resources were being torn down. + if err := c.retireSandbox(ctx, id); err != nil && !errors.Is(err, cond.ErrNotFound{}) { + c.Log.Error("failed to mark sandbox as DEAD", "id", id, "error", err) } c.Log.Info("sandbox retired", "id", id, "status", compute.DEAD) @@ -3346,6 +3333,43 @@ func (c *SandboxController) StopSandbox(ctx context.Context, id entity.Id, sb *c return nil } +func (c *SandboxController) retireSandbox(ctx context.Context, id entity.Id) error { + for attempt := range 10 { + resp, err := c.EAC.Get(ctx, id.String()) + if err != nil { + return err + } + var current compute.Sandbox + current.Decode(resp.Entity().Entity()) + retired := &compute.Sandbox{Status: compute.DEAD} + if current.StartupOutcome == "" { + switch current.Status { + case compute.PENDING: + retired.StartupOutcome = compute.STARTUP_FAILED + case compute.RUNNING: + retired.StartupOutcome = compute.STARTUP_RUNNING + case compute.NOT_READY, compute.STOPPED, compute.DEAD: + // No lifecycle conclusion from these states alone. + } + } + result, err := c.EAC.Patch(ctx, entity.New( + entity.Ref(entity.DBId, id), + retired.Encode, + ).Attrs(), resp.Entity().Revision()) + if errors.Is(err, cond.ErrConflict{}) && attempt < 9 { + continue + } + if err != nil { + return err + } + if c.writeTracker != nil && result.HasRevision() { + c.writeTracker.RecordWrite(result.Revision()) + } + return nil + } + return nil +} + // reregisterSqliteDisks restores replication for a sandbox that outlived the // runner process. It mirrors what configureSqliteVolume did when the sandbox // first started, minus creating anything: the directory and database are diff --git a/controllers/sandbox/sandbox_test.go b/controllers/sandbox/sandbox_test.go index c8756072f..dec066de3 100644 --- a/controllers/sandbox/sandbox_test.go +++ b/controllers/sandbox/sandbox_test.go @@ -1972,6 +1972,36 @@ func TestDeadPatchPreservesRecordedExit(t *testing.T) { r.Equal("app", got.Exit.Container) } +func TestRetireSandboxUsesCurrentLifecycle(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + deps, cleanup := testutils.NewTestDeps() + defer cleanup() + c, err := newSandboxController(deps) + require.NoError(t, err) + defer c.Close() + require.NoError(t, c.Init(ctx)) + id := entity.Id(idgen.GenNS("sb")) + stale := &compute.Sandbox{ID: id, Status: compute.PENDING} + var rpcE entityserver_v1alpha.Entity + rpcE.SetId(id.String()) + rpcE.SetAttrs(entity.New(entity.DBId, id, stale.Encode).Attrs()) + _, err = c.EAC.Put(ctx, &rpcE) + require.NoError(t, err) + + // The cleanup caller still holds PENDING, but boot has already persisted RUNNING. + _, err = c.EAC.Patch(ctx, entity.New(entity.DBId, id, + (&compute.Sandbox{Status: compute.RUNNING, StartupOutcome: compute.STARTUP_RUNNING}).Encode).Attrs(), 0) + require.NoError(t, err) + require.NoError(t, c.StopSandbox(ctx, id, stale)) + resp, err := c.EAC.Get(ctx, id.String()) + require.NoError(t, err) + var got compute.Sandbox + got.Decode(resp.Entity().Entity()) + require.Equal(t, compute.DEAD, got.Status) + require.Equal(t, compute.STARTUP_RUNNING, got.StartupOutcome) +} + // A sandbox whose command must execute at most once is finished when its // containers vanish. Rebooting it would re-run the command -- for a migration, // not a recoverable mistake. diff --git a/controllers/sandboxpool/manager.go b/controllers/sandboxpool/manager.go index 4e98433a3..fa36e53d1 100644 --- a/controllers/sandboxpool/manager.go +++ b/controllers/sandboxpool/manager.go @@ -828,8 +828,8 @@ func (m *Manager) countStartupFailures(sandboxes []*sandboxWithMeta, pool *compu continue } - if sbm.sandbox.StartupOutcome == compute_v1alpha.STARTUP_RUNNING || - (sbm.sandbox.StartupOutcome == "" && sbm.updatedAt.Sub(sbm.createdAt) >= 60*time.Second) { + if sbm.sandbox.StartupOutcome != compute_v1alpha.STARTUP_FAILED && + sbm.updatedAt.Sub(sbm.createdAt) >= 60*time.Second { continue } diff --git a/controllers/sandboxpool/manager_test.go b/controllers/sandboxpool/manager_test.go index cb64d75e0..3f9e73d52 100644 --- a/controllers/sandboxpool/manager_test.go +++ b/controllers/sandboxpool/manager_test.go @@ -1521,9 +1521,9 @@ func TestCountStartupFailures(t *testing.T) { {sandbox: &compute_v1alpha.Sandbox{ID: "old-failure", Status: compute_v1alpha.DEAD, StartupOutcome: compute_v1alpha.STARTUP_FAILED}, createdAt: now.Add(-10 * time.Minute), updatedAt: now.Add(-2 * time.Minute)}, } pool := &compute_v1alpha.SandboxPool{LastCrashTime: now.Add(-90 * time.Second), CountedFailures: []entity.Id{"old-failure"}} - assert.Equal(t, int64(2), manager.countStartupFailures(sandboxes, pool), - "long pre-running failure and legacy quick crash count, but running and previously counted failures do not") - assert.ElementsMatch(t, []entity.Id{"old-failure", "first", "legacy"}, pool.CountedFailures) + assert.Equal(t, int64(3), manager.countStartupFailures(sandboxes, pool), + "long pre-running failure and quick crashes count, but long-running and previously counted failures do not") + assert.ElementsMatch(t, []entity.Id{"old-failure", "first", "fast-healthy", "legacy"}, pool.CountedFailures) } func TestManagerLongStartupFailureBackoff(t *testing.T) { From e4f12eea18ab5b4eca7327e33f2e41c0e8999279 Mon Sep 17 00:00:00 2001 From: Evan Phoenix Date: Thu, 24 Sep 2026 18:36:52 +0000 Subject: [PATCH 3/8] Preserve healthy pool capacity during partial failure cooldown Keep desired instances when a referenced pool still has running sandboxes, including when a boot fails on a lost node. Empty pools retain the existing activator accumulation reset. Amp-Thread-ID: T-01a0cbe2-e330-77cb-bfe8-2363e67399f0 --- controllers/sandboxpool/manager.go | 9 ++++-- controllers/sandboxpool/manager_test.go | 38 +++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/controllers/sandboxpool/manager.go b/controllers/sandboxpool/manager.go index fa36e53d1..c8feddb66 100644 --- a/controllers/sandboxpool/manager.go +++ b/controllers/sandboxpool/manager.go @@ -108,11 +108,14 @@ func (m *Manager) Reconcile(ctx context.Context, pool *compute_v1alpha.SandboxPo // Unreferenced pools should be allowed to scale to 0 even during cooldown isUnreferenced := len(pool.ReferencedByVersions) == 0 - // Reset DesiredInstances to prevent activator-driven accumulation - // Allow desired: 0 for unreferenced pools (deployment cleanup) - targetDesired := int64(1) + // Reset empty pools to prevent activator-driven accumulation, but do + // not shrink a pool that still has healthy RUNNING instances after a + // partial failure (including node loss). + targetDesired := pool.DesiredInstances if isUnreferenced { targetDesired = 0 + } else if ready == 0 { + targetDesired = 1 } if pool.DesiredInstances != targetDesired { diff --git a/controllers/sandboxpool/manager_test.go b/controllers/sandboxpool/manager_test.go index 3f9e73d52..96ab105f4 100644 --- a/controllers/sandboxpool/manager_test.go +++ b/controllers/sandboxpool/manager_test.go @@ -1526,6 +1526,44 @@ func TestCountStartupFailures(t *testing.T) { assert.ElementsMatch(t, []entity.Id{"old-failure", "first", "fast-healthy", "legacy"}, pool.CountedFailures) } +func TestPartialNodeLossKeepsPoolDesiredDuringCooldown(t *testing.T) { + ctx := context.Background() + server, cleanup := testutils.NewInMemEntityServer(t) + defer cleanup() + pool := &compute_v1alpha.SandboxPool{ + Service: "web", DesiredInstances: 5, + ReferencedByVersions: []entity.Id{"ver-1"}, + SandboxSpec: compute_v1alpha.SandboxSpec{Version: "ver-1"}, + } + id, err := server.Client.Create(ctx, "pool", pool) + require.NoError(t, err) + pool.ID = id + for i := range 4 { + _, err = server.Client.Create(ctx, fmt.Sprintf("healthy-%d", i), + &compute_v1alpha.Sandbox{Status: compute_v1alpha.RUNNING, Spec: pool.SandboxSpec}, + entityserver.WithLabels(types.LabelSet("service", "web", "pool", id.String()))) + require.NoError(t, err) + } + server.Store.NowFunc = func() time.Time { return time.Now().Add(-6 * time.Minute) } + failedID, err := server.Client.Create(ctx, "lost-node", + &compute_v1alpha.Sandbox{Status: compute_v1alpha.PENDING, Spec: pool.SandboxSpec}, + entityserver.WithLabels(types.LabelSet("service", "web", "pool", id.String()))) + require.NoError(t, err) + server.Store.NowFunc = nil + _, err = server.EAC.Patch(ctx, entity.New(entity.DBId, failedID, + (&compute_v1alpha.Sandbox{Status: compute_v1alpha.DEAD, StartupOutcome: compute_v1alpha.STARTUP_FAILED}).Encode).Attrs(), 0) + require.NoError(t, err) + + manager := NewManager(testutils.TestLogger(t), server.EAC) + reconcilePool(t, ctx, server, manager, pool) + updated := getPool(t, ctx, server, id) + assert.Equal(t, int64(1), updated.ConsecutiveCrashCount) + assert.Equal(t, int64(5), updated.DesiredInstances, "a failed boot must not downsize four healthy instances") + assert.Equal(t, int64(4), updated.ReadyInstances) + assert.WithinDuration(t, time.Now().Add(10*time.Second), updated.CooldownUntil, 2*time.Second) + assert.Len(t, listSandboxesForPool(t, ctx, server, pool), 5, "replacement waits for cooldown") +} + func TestManagerLongStartupFailureBackoff(t *testing.T) { ctx := context.Background() server, cleanup := testutils.NewInMemEntityServer(t) From e4f5318352430daeab315007ebf884df26c6e7a7 Mon Sep 17 00:00:00 2001 From: Evan Phoenix Date: Thu, 24 Sep 2026 18:45:23 +0000 Subject: [PATCH 4/8] Limit cooldown capacity to one replacement beyond running siblings Preserve existing live capacity after partial node loss while preventing activator growth from relaunching a whole crash-looping pool when cooldown expires. Amp-Thread-ID: T-01a0cbe2-e330-77cb-bfe8-2363e67399f0 --- controllers/sandboxpool/manager.go | 12 ++++----- controllers/sandboxpool/manager_test.go | 35 +++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/controllers/sandboxpool/manager.go b/controllers/sandboxpool/manager.go index c8feddb66..780f18fc8 100644 --- a/controllers/sandboxpool/manager.go +++ b/controllers/sandboxpool/manager.go @@ -108,14 +108,14 @@ func (m *Manager) Reconcile(ctx context.Context, pool *compute_v1alpha.SandboxPo // Unreferenced pools should be allowed to scale to 0 even during cooldown isUnreferenced := len(pool.ReferencedByVersions) == 0 - // Reset empty pools to prevent activator-driven accumulation, but do - // not shrink a pool that still has healthy RUNNING instances after a - // partial failure (including node loss). - targetDesired := pool.DesiredInstances + // Allow one replacement beyond the RUNNING count. This preserves + // healthy siblings after a partial failure (including node loss), + // without letting activator requests accumulate a full crash loop. + targetDesired := int64(1) if isUnreferenced { targetDesired = 0 - } else if ready == 0 { - targetDesired = 1 + } else if ready > 0 { + targetDesired = max(1, min(pool.DesiredInstances, ready+1)) } if pool.DesiredInstances != targetDesired { diff --git a/controllers/sandboxpool/manager_test.go b/controllers/sandboxpool/manager_test.go index 96ab105f4..2234d027e 100644 --- a/controllers/sandboxpool/manager_test.go +++ b/controllers/sandboxpool/manager_test.go @@ -1564,6 +1564,41 @@ func TestPartialNodeLossKeepsPoolDesiredDuringCooldown(t *testing.T) { assert.Len(t, listSandboxesForPool(t, ctx, server, pool), 5, "replacement waits for cooldown") } +func TestCrashLoopCooldownCapsDesiredWithRunningSiblings(t *testing.T) { + ctx := context.Background() + server, cleanup := testutils.NewInMemEntityServer(t) + defer cleanup() + pool := &compute_v1alpha.SandboxPool{ + Service: "web", DesiredInstances: 8, + ReferencedByVersions: []entity.Id{"ver-1"}, + SandboxSpec: compute_v1alpha.SandboxSpec{Version: "ver-1"}, + } + id, err := server.Client.Create(ctx, "pool", pool) + require.NoError(t, err) + pool.ID = id + for i := range 2 { + _, err = server.Client.Create(ctx, fmt.Sprintf("running-%d", i), + &compute_v1alpha.Sandbox{Status: compute_v1alpha.RUNNING, Spec: pool.SandboxSpec}, + entityserver.WithLabels(types.LabelSet("service", "web", "pool", id.String()))) + require.NoError(t, err) + } + server.Store.NowFunc = func() time.Time { return time.Now().Add(-20 * time.Second) } + failedID, err := server.Client.Create(ctx, "quick-crash", + &compute_v1alpha.Sandbox{Status: compute_v1alpha.RUNNING, StartupOutcome: compute_v1alpha.STARTUP_RUNNING, Spec: pool.SandboxSpec}, + entityserver.WithLabels(types.LabelSet("service", "web", "pool", id.String()))) + require.NoError(t, err) + server.Store.NowFunc = nil + _, err = server.EAC.Patch(ctx, entity.New(entity.DBId, failedID, + (&compute_v1alpha.Sandbox{Status: compute_v1alpha.DEAD}).Encode).Attrs(), 0) + require.NoError(t, err) + + reconcilePool(t, ctx, server, NewManager(testutils.TestLogger(t), server.EAC), pool) + updated := getPool(t, ctx, server, id) + assert.Equal(t, int64(1), updated.ConsecutiveCrashCount) + assert.Equal(t, int64(3), updated.DesiredInstances, "only one replacement may be queued beyond the two running siblings") + assert.Equal(t, int64(2), updated.ReadyInstances) +} + func TestManagerLongStartupFailureBackoff(t *testing.T) { ctx := context.Background() server, cleanup := testutils.NewInMemEntityServer(t) From a451dc93309deff9afbc5ce00a3713e22dbf20e0 Mon Sep 17 00:00:00 2001 From: Evan Phoenix Date: Thu, 24 Sep 2026 18:54:20 +0000 Subject: [PATCH 5/8] Preserve pending siblings in cooldown capacity cap Base the one-replacement cap on RUNNING plus PENDING sandboxes so siblings that finish booting during cooldown do not get scaled down. Amp-Thread-ID: T-01a0cbe2-e330-77cb-bfe8-2363e67399f0 --- controllers/sandboxpool/manager.go | 8 ++-- controllers/sandboxpool/manager_test.go | 51 +++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/controllers/sandboxpool/manager.go b/controllers/sandboxpool/manager.go index 780f18fc8..231f394d1 100644 --- a/controllers/sandboxpool/manager.go +++ b/controllers/sandboxpool/manager.go @@ -108,14 +108,14 @@ func (m *Manager) Reconcile(ctx context.Context, pool *compute_v1alpha.SandboxPo // Unreferenced pools should be allowed to scale to 0 even during cooldown isUnreferenced := len(pool.ReferencedByVersions) == 0 - // Allow one replacement beyond the RUNNING count. This preserves - // healthy siblings after a partial failure (including node loss), + // Allow one replacement beyond the live count (RUNNING or PENDING). + // This preserves siblings after a partial failure (including node loss), // without letting activator requests accumulate a full crash loop. targetDesired := int64(1) if isUnreferenced { targetDesired = 0 - } else if ready > 0 { - targetDesired = max(1, min(pool.DesiredInstances, ready+1)) + } else if actual > 0 { + targetDesired = max(1, min(pool.DesiredInstances, actual+1)) } if pool.DesiredInstances != targetDesired { diff --git a/controllers/sandboxpool/manager_test.go b/controllers/sandboxpool/manager_test.go index 2234d027e..9b0462a41 100644 --- a/controllers/sandboxpool/manager_test.go +++ b/controllers/sandboxpool/manager_test.go @@ -1599,6 +1599,57 @@ func TestCrashLoopCooldownCapsDesiredWithRunningSiblings(t *testing.T) { assert.Equal(t, int64(2), updated.ReadyInstances) } +func TestCooldownPreservesPendingSiblingsAfterTheyStart(t *testing.T) { + ctx := context.Background() + server, cleanup := testutils.NewInMemEntityServer(t) + defer cleanup() + pool := &compute_v1alpha.SandboxPool{ + Service: "web", DesiredInstances: 5, + ReferencedByVersions: []entity.Id{"ver-1"}, + SandboxSpec: compute_v1alpha.SandboxSpec{Version: "ver-1"}, + } + id, err := server.Client.Create(ctx, "pool", pool) + require.NoError(t, err) + pool.ID = id + for i := range 4 { + status := compute_v1alpha.RUNNING + if i >= 2 { + status = compute_v1alpha.PENDING + } + _, err = server.Client.Create(ctx, fmt.Sprintf("sibling-%d", i), + &compute_v1alpha.Sandbox{Status: status, Spec: pool.SandboxSpec}, + entityserver.WithLabels(types.LabelSet("service", "web", "pool", id.String()))) + require.NoError(t, err) + } + _, err = server.Client.Create(ctx, "failed", + &compute_v1alpha.Sandbox{Status: compute_v1alpha.DEAD, StartupOutcome: compute_v1alpha.STARTUP_FAILED, Spec: pool.SandboxSpec}, + entityserver.WithLabels(types.LabelSet("service", "web", "pool", id.String()))) + require.NoError(t, err) + manager := NewManager(testutils.TestLogger(t), server.EAC) + reconcilePool(t, ctx, server, manager, pool) + updated := getPool(t, ctx, server, id) + assert.Equal(t, int64(5), updated.DesiredInstances) + assert.Equal(t, int64(4), updated.CurrentInstances) + assert.Equal(t, int64(2), updated.ReadyInstances) + + // Both pending siblings finish booting before the cooldown ends. + for _, sb := range listSandboxesForPool(t, ctx, server, pool) { + if sb.Status == compute_v1alpha.PENDING { + _, err = server.EAC.Patch(ctx, entity.New(entity.DBId, sb.ID, + (&compute_v1alpha.Sandbox{Status: compute_v1alpha.RUNNING, StartupOutcome: compute_v1alpha.STARTUP_RUNNING}).Encode).Attrs(), 0) + require.NoError(t, err) + } + } + _, err = server.EAC.Patch(ctx, entity.New(entity.DBId, id, + (&compute_v1alpha.SandboxPool{CooldownUntil: time.Now().Add(-time.Second)}).Encode).Attrs(), 0) + require.NoError(t, err) + reconcilePool(t, ctx, server, manager, pool) + updated = getPool(t, ctx, server, id) + assert.Equal(t, int64(5), updated.DesiredInstances) + assert.Equal(t, int64(5), updated.CurrentInstances, "a replacement may start without retiring a healthy sibling") + assert.Equal(t, int64(4), updated.ReadyInstances) +} + func TestManagerLongStartupFailureBackoff(t *testing.T) { ctx := context.Background() server, cleanup := testutils.NewInMemEntityServer(t) From 258ab97d7789bdbbda78731d01416972b53001b1 Mon Sep 17 00:00:00 2001 From: Evan Phoenix Date: Thu, 24 Sep 2026 19:08:58 +0000 Subject: [PATCH 6/8] Prune persisted counted sandbox IDs after entity deletion Store the new deduplication field as a replaceable JSON array rather than additive many-valued references. Retain listed terminal sandboxes, prune deleted IDs, and persist an explicit empty array through the real reconcile path. Amp-Thread-ID: T-01a0cbe2-e330-77cb-bfe8-2363e67399f0 --- api/compute/compute_v1alpha/schema.gen.go | 18 +++---- api/compute/schema.yml | 5 +- controllers/sandboxpool/manager.go | 40 ++++++++++++--- controllers/sandboxpool/manager_test.go | 60 ++++++++++++++++++++--- 4 files changed, 96 insertions(+), 27 deletions(-) diff --git a/api/compute/compute_v1alpha/schema.gen.go b/api/compute/compute_v1alpha/schema.gen.go index f7703aed1..2522fb2f4 100644 --- a/api/compute/compute_v1alpha/schema.gen.go +++ b/api/compute/compute_v1alpha/schema.gen.go @@ -2052,7 +2052,7 @@ type SandboxPool struct { App entity.Id `cbor:"app,omitempty" json:"app,omitempty"` ConsecutiveCrashCount int64 `cbor:"consecutive_crash_count,omitempty" json:"consecutive_crash_count,omitempty"` CooldownUntil time.Time `cbor:"cooldown_until,omitempty" json:"cooldown_until"` - CountedFailures []entity.Id `cbor:"counted_failures,omitempty" json:"counted_failures,omitempty"` + CountedFailures string `cbor:"counted_failures,omitempty" json:"counted_failures,omitempty"` CurrentInstances int64 `cbor:"current_instances,omitempty" json:"current_instances,omitempty"` DesiredInstances int64 `cbor:"desired_instances,omitempty" json:"desired_instances,omitempty"` Ephemeral bool `cbor:"ephemeral,omitempty" json:"ephemeral,omitempty"` @@ -2076,10 +2076,8 @@ func (o *SandboxPool) Decode(e entity.AttrGetter) { if a, ok := e.Get(SandboxPoolCooldownUntilId); ok && a.Value.Kind() == entity.KindTime { o.CooldownUntil = a.Value.Time() } - for _, a := range e.GetAll(SandboxPoolCountedFailuresId) { - if a.Value.Kind() == entity.KindId { - o.CountedFailures = append(o.CountedFailures, a.Value.Id()) - } + if a, ok := e.Get(SandboxPoolCountedFailuresId); ok && a.Value.Kind() == entity.KindString { + o.CountedFailures = a.Value.String() } if a, ok := e.Get(SandboxPoolCurrentInstancesId); ok && a.Value.Kind() == entity.KindInt64 { o.CurrentInstances = a.Value.Int64() @@ -2143,8 +2141,8 @@ func (o *SandboxPool) Encode() (attrs []entity.Attr) { if !entity.Empty(o.CooldownUntil) { attrs = append(attrs, entity.Time(SandboxPoolCooldownUntilId, o.CooldownUntil)) } - for _, v := range o.CountedFailures { - attrs = append(attrs, entity.Ref(SandboxPoolCountedFailuresId, v)) + if !entity.Empty(o.CountedFailures) { + attrs = append(attrs, entity.String(SandboxPoolCountedFailuresId, o.CountedFailures)) } if !entity.Empty(o.CurrentInstances) { attrs = append(attrs, entity.Int64(SandboxPoolCurrentInstancesId, o.CurrentInstances)) @@ -2188,7 +2186,7 @@ func (o *SandboxPool) Empty() bool { if !entity.Empty(o.CooldownUntil) { return false } - if len(o.CountedFailures) != 0 { + if !entity.Empty(o.CountedFailures) { return false } if !entity.Empty(o.CurrentInstances) { @@ -2228,7 +2226,7 @@ func (o *SandboxPool) InitSchema(sb *schema.SchemaBuilder) { sb.Ref("app", "dev.miren.compute/sandbox_pool.app", schema.Doc("Reference to the app this pool belongs to"), schema.Indexed, schema.Tags("dev.miren.app_ref")) sb.Int64("consecutive_crash_count", "dev.miren.compute/sandbox_pool.consecutive_crash_count", schema.Doc("Number of consecutive sandbox failures, including quick crashes after RUNNING")) sb.Time("cooldown_until", "dev.miren.compute/sandbox_pool.cooldown_until", schema.Doc("Timestamp until which new sandbox creation is paused due to crash loop")) - sb.Ref("counted_failures", "dev.miren.compute/sandbox_pool.counted_failures", schema.Doc("Sandbox IDs already included in the failure streak; kept across streak resets"), schema.Many) + sb.String("counted_failures", "dev.miren.compute/sandbox_pool.counted_failures", schema.Doc("JSON array of sandbox IDs already included in the failure streak; kept across streak resets")) sb.Int64("current_instances", "dev.miren.compute/sandbox_pool.current_instances", schema.Doc("Current number of sandbox instances (non-STOPPED)")) sb.Int64("desired_instances", "dev.miren.compute/sandbox_pool.desired_instances", schema.Doc("Target number of sandbox instances")) sb.Bool("ephemeral", "dev.miren.compute/sandbox_pool.ephemeral", schema.Doc("True when this pool backs an ephemeral AppVersion. Ephemeral pools never scale beyond 1 instance.")) @@ -2352,5 +2350,5 @@ func init() { (&SandboxPool{}).InitSchema(sb) (&Schedule{}).InitSchema(sb) }) - schema.RegisterEncodedSchema("dev.miren.compute", "v1alpha", []byte("\x1f\x8b\b\x00\x00\x00\x00\x00\x00\xff\xec\\َ\xf4:\x11~\r\x0e\xcba\xdf\x04\xe4\xe7 v\xb1\x1d\x01\xb7\xbcB䎫ӞN\xdb\x19\xdb\xe9\xe9\xe6\x0e\x10\x12B,\xef\xc0?\xc3\x1b\xc25\xf2\x968\x89\x9d8n\xa4s\x93\x9b\x91\xed\xb8>\xdb\xe5rU\xb9\\ӯ\x98\xa2\v\b\xec\xb6\xe9\x90\xc8\xc9\x7f\xbeE\x14\x87\x85)\x0e\xac\xa3\xb8l\x19\x97\x9e\xeez\xf2Z՚\x88\xa2b\x14\xa8\x1cJv\x86s\xe8b\x0e\x9d8\xd9?ƴ܀T\xf8ں\x0eh\xea\xf7\x11\x05\xe5A\xe8ei\x05\xdb/\xb0\"T.\xee\x1a\xd1\xf4\xaa\xbff\xe8\x97\xe3\f\xad\x18\x95\x88P\xe0\x1e?\xc9и\xc2\xce9p1\x03N\xe4\xe6\xdf\xdfGf\xda\x03\xa9\x96\v\xa2F\xdf\u05ee\xe21S\xaf\xf5\xeb\xcb\b\xf4H\xea\xf2H\x1a\x98ؾ\xbeye\xc5\xef\x12V\xec\x0f\xb3I\x85\x04\xb4\xa5\aU`$\x91\x91\x04]\x9a\x8a\xd1\n\xf5\x85ak\xa8ui#u\x8b\xe4\xc9J\xa1*\xa5\xaa\xe0'\x83\xa1 \xf4(\x01\xc39\xb0\r\x13\x0e\x95d\xfcn\xa4p\xa8N-z@\xcd\r(@\xaf\xde\xdeV\xaa\x9ap\xe0\x06zrA\xb5a\x14\x98\xe2T\xc2\x16\xa9/\xac\xa3\xben\x02Ӱ\"U\xdfL\x91*\x8d\xb4I7\x05N\x93\x06)0\bI(\x92δ\x9e\xfd\x86)\xb7\x02\xbaߠ\b\xd6\xf1\n\xac3bʩra\xd8\x12\xf3\xef\x865\xaf\xb8\x98\x8b\xe2\xc4إ\x14\x15㆖\fU\xa7B\xdfV\x87\x9f\x18\x1a\x9cbb\xe6\x8e^`/7\x98\x99?\xc7|pm\x1dV\x18\x14X\x9d!c\x18\xca\u07b2\x90\xa1\xeax\xb38h\xc4$\xc5Φ\xa1\xe1L\xb2\x8a5\x9a\xee\xd4\xd7\u009ev%\xab6$w\x8e\xac\x90U[ux\xb9O\x87\xdb\xc5U\xe8\xa1{\xae\xa5{\x95\x8a8\xe6\xf1y;\xccɕ4P\x83\xb1WO^]\x0fw`\xccܮ\xbe\xbd\x84\"N\x9d\xc4셖\xca9d\x9d\xe1z;kݤ\xe0\x84\xc4\xc4\x1c{0\xc5\xf1|\x16\x95\xab\x94F9W\xaa\xd0\xd3-\xfb\"=\xf5kD\x9b\xb8\xe3\x017b\xc5J\x97V\xce\xd9\x1c\xa8\xf0\x816\x19ހ\xe3\xaa0\n\xeb\xae\x1ff.z@\xa64A\xd5\x1b\xd9\xca\x19\xd9\xfe4\x05\u03a2\xa5q\xdeV\xc0\xf9J\x92H\x05\x13Ӈ\x8e''&\xe4\xef@\xbe0~6*\xdfo\xe8w\xf252Q\x87\xa2\x83\x02~\xdc\xe0h[\xa6\x12\x18p\xc3\x06\f!KTIr%V\x9a.㦞կ\x91S\xd6#\xb1\xfac)99t\xd2\xf7\xe7\x9aQ\xfb$\x98\xb1\xe0\v7\xac\xfe-\x95nRd\xa8&x\x00\x0e\x83Z\x8e\x0e\xb3\xa9\xa9\xc7\xe4\x05y\x9e\x83\x16\x13\xd0M\xb6?\x14\xf500i\x97\x92P\xd8\xc5ҋ\xee@AZ\xbboʩ\xa2\xea\x98\xf1\x16\xd14nŜ\x8d\xb7\x14L\xc3\n\v\xe7\x80\xc5\b\xf0Q\xe7I\x83ls\x9e\x02k4(5\x92\xf0\x82\x8c\xa8ծ\x92\xec>i\x8c5}*Z\xa8\x8c2ҥ\xcd~˻\xbe\x8bcc\xa9\x80\x12\xb9\xf8W\xbd\xc7\xdfKE}\xf0&:\x1f\xa7X\x1b'q\x1d\xff\xd0l\xfe\xd1\xf6u\x14\x88\u05fe\xaaĺ>\x15\x90\x9ff\x00']\x84\x7f\x91\x05\xfc\xe8\xfdx>\xea\xea>d_\x97\x7f\xfd\xd8\n\xd7\xeeӏ¯\\\xb8\x1f\x85ϼ\x91\xdf>\xb4\xd8\n\xbaG\x9e\\\xd3\x7f\x961\xb7\xe4\xdb\xfb\x0f3\xc0\x13.\xf5?\u0380]\xbd\xeb\xe7\x80\xe6\x85\x00\xe6#\xad\x1f\x9c\xed\x11\x81\xdf\xe4\xaeg\x9b\xd5\xfbe\xf60\x0f\xc4\x14n\x1f\x84${\b4\xe4\xa8\xf0\x95\xebu\xce9I\vK\xe4L6'Z1\x1fg]\xec6\a/rؔ\x12\xdd\xf88\x1b7)\xfc\x91=\xed\xa5\xf8ȯ\xb2A7\aPr\x0e\xfbh\xa8>\xcc\xf28\x92\v\xc6d\xf343Zs\xfbTH)\xf4!\x9c\x9f\xe7L'5\xb2\x93c\xe13\x03>9vj%\x0e\x94c\xa63\xc2C2\xb4?z\x02\xdfO\x9e\xc0\x86\xd8\xc6\x0f\x92As\x82\v鷝\x94X\xc3\xc6\xfb\x81\x12\xea\xf2\x05\x119\x92\x9d\xe7y\xf3\xd4!K\xb75\x1c\x84D\\\x96-kHe\xa6N'ma}\x04\x14\xae\xc07\x1c\xb71j\xa1ɏ\xa8yAw\xb1\xe1f3A1\xf4ھ\x16\xe9\x18Y\xa1\x889~̮n\x8fL\xa4\x8bDF\xc0\"\xfd\xd8\xff\x1f\xe2\x18\xadw\xf85\xdc۶\x83/$\x92\xa4*\xd5q\xf7\xef\xab~\xf3\xca>\xcdNJ\xed\x93\a\xbai\xb7~\x92\xb3\x1a\xadҌ\xb9\xebW\xe1\xefR\xfa\x91\xf5AIk\xc2٤Mޡ\xce\xdb!\x03\xa5\x90\xf4\x1c\xbe\x9b<\x87h\x16ρ`\xbd\xe3\xf3W\xf5(\x14k\xba\x8b\x7f\x1c\x8f\xb6e\xfb\xbb\xfd\xe2\b\x89[\xfc\xb7\x8d\xbba\xc0\v|0\xc1\x14\xcd\x10W\x99\xeeq\xba\xe08T\"\xcee\xef-\x93\xa1\x9aoS,\xb2\x9a\x9f\xb8\v\t\x17\xe3\xf2x\xf5\xfc۽\xc5^|R\xf0lk\xba\xedp\xc0\x80\x04\x8cl\xe1e\xdc\xf40[\xf4ݲ\xec\x031O^}\x8a\x9d\xae\xd3,\xf6\xca\xd5'ݏ\xb1x\xecŽ0\x81)滐\x16\xb1\xe5\xecJ\xb0\x05=\xf5\xb5\x87Ř\x03\xc2%\xa3\x8du\x8d\x86\xea\xd8?\xdd|\xe8\x04\xf9=\x94\xf5\xc1\xe6\xc0\xd9\xca\xe8·y\xaa\xe2\xb9!\x12\xfa\x1cԡ\x9a\xaa^\x9f=\xf5j0\x17\xbb7\xfe$\xf4q\x9bg\x8e\f\xe1\x7f\xe5\xfatm\xc9:Y1+Gl\xda\x18vר\xebvDJ3\x85T\xfd\x04\xa8\x18S\xf4\xe3\xf0\x8eRB\xeb\xdbG\xe9\b\x96\xe4m\xe5A\xf3q\xdb?\a/\x02\xe0\x9b\x8c}$\xdf5ѪG\xb2p\x1f0\xdfO\x83\xcd^{\xdaMM-~\xc3\x18P0C٥\x01\x03\xc2u\v\x14\xab}\x8f'\x16\xdb\x1e\xb5\x93\x90xOۣ\x16\x92\xb5-D\xd9ԉ\xc2\xf6 \x94\xc9\xd2$@\xc7\x13\x90\xfb>\xb14 ǘ\\\x97c\x0eY\x8c!S\x1f;by5\x1b\rh`\xc7\xd2lM\xe0)9\xd9\x04,J\xe71A\xe5\xb9l\xe5\x10\x03L\n\xb2I\x9d\xb7\xa9ܷO\a6\xd2\xf6H\xe4\xf7\x1f\xa2\xef\xaa\x16\xa78\x83\rs\xa8BF\x9a\x8a\x87\xb3I\xbb\x04\xd6v\x86{ar\xdd\xd5\xce钟\x10\x1e\xa1\xa0\xfd\xd3\x14uOSk\x89\xe0j\xad\x8b\x1dNnY\xa1\x97s?Y\xbcl\x19k\xa2\xdcy\xe7\xf7J}\xa2\x8d\xea^\x0f\xab@\xadџ\x95*\xf8L\n\xb8\xac#\u008aQ\x01U'\xc9\x15ʊ#q*+\xfd\xa2\xa3\xc0^b\x1fG\x8e\xc5wVG`\x8d\x8e\xefuT\x12\x13ᥓ\xb6qRL\xe0\xc24\x01\xec\xa8\x04\xac\xadq\xc7\xc1\xd7\x0e\xed\xec\x9bϋ\x90\xa5\x1f\x01w\x9c\x03\x95%Q.\x02\xad\xc0\x18\x8c\xe7y\xf3h\xfdk\xa8\x18\x04ဧ\xa8\xf3\xe6\x11j\xe8_j|ThOp\x01\x8e\fC\xc9P\x1d;\x92\x81P\xd0\bF''\x99\xadU\xfc7~Դq\x9c\x1e\xb6\x06\xa9\xed\xced\xb5l\xda8z6\b\xf8\xfb\x13\xc4#p\xa0\x15\xe0\xf2p/\xed9\xf5\xb7\xfd\x1a\xe9a7\xff5EL]efq\xe8\xe4\xcb\xc4\xf2\xa4\xe2\xb6\x1c\x8e\xe46F\xb4m\xd3k\xe6\xb7\x12!\xfb\x1c\x98\x91\xeb\xbc\xe7\xc2\xec\xb90{.̞\v\xb3\xe7\xc2\xec\xb90{.̞\v\xb3\xe7\xc2\xec\xb90{.̞\v\xb3\xe7\xc2\xec\xb90{.̞\v\xb3\xe7\xc2\xec\xb90{.̞\v\xb3\xe7\xc2\xec\xb90{.L\"\xe4'\x99\v\x13\xfb=\x86\xf1\x9b\x00\xf0+\xb1\x01\x95\xdaURg\xd7\xf8PӞgqR\x9e\xa5\xf9\x91@\xf3\xb3cK\xbf\x14h\x7fTk\xf1\x97\xc9\xfaw\uf55f\xde\x1a\x9e]\xd7\x1e\xc8G+Hz\xa4\xfd\x1f\x00\x00\x00\xff\xff\x01\x00\x00\xff\xffr\xd3-\x11\x0eQ\x00\x00")) + schema.RegisterEncodedSchema("dev.miren.compute", "v1alpha", []byte("\x1f\x8b\b\x00\x00\x00\x00\x00\x00\xff\xec\\َ\xf4:\x11~\r\x0e\xcba\xdf\x04\xe4\xe7 v\xb1\x1d\x01\xb7\xbcB䎫ӞN\xdb\x19\xdb\xe9\xe9\xe6\x0e\x10\x12B,\xef\xc0?\xc3\x1b\xc25\xf2\x968\x89\x9d8n\xa4s\x93\x9b\x91\xedv}\xb6\xcb\xe5\xaar\xb9&\xaf\x98\xa2\v\x1b9\xb2\r a\xcf\xec˼\x8b\xfe5\xf1\xd0\xfeI/\xe1k\x11\x90\xa2AB\x96'@\\\x1e\xc0\n<\x9d\xb4\x8d%\xfe\xf31\xa4\x96\xb3'\xa8\fD\xed*\x8a\xf6@\xf02\xa5@\x14\x1f\xd8\xcdP\xba\x8a\xa5\\\xe4!h\xfa\xd0\xfeh&Z(\xc3\xc6\xdb\a\x81\xdd6\x1d\x129\xf9Ϸ\x88\xe2\xb00Łu\x14\x97-\xe3\xd2\xd3]O^\xabZ\x13QT\x8c\x02\x95C\xc9\xcep\x0e]̡\x13'\xfbǘ\x96\x1b\x90\n_[\xd7\x01M\xfd>\xa2\xa0<\b\xbd,\xad`\xfb\x05V\x84\xca\xc5]#\x9a^\xf5\xd7\f\xfdr\x9c\xa1\x15\xa3\x12\x11\n\xdc\xe3'\x19\x1aW\xd89\a.f\xc0\x89\xdc\xfc\xfb\xfb\xc8L{ \xd5rA\xd4\xe8\xfb\xdaUo\x879\xb9\x92\x06j0\xf6\xeaɫ\xeb\xe1\x0e\x8c\x99\xdbշ\x97Pĩ\x93\x98\xbd\xd0R9\x87\xac3\\og\xad\x9b\x14\x9c\x90\x98\x98c\x0f\xa68\x9eϢr\x95\xd2(\xe7J\x15z\xbae_\xa4\xa7~\x8dh\x13w<\xe0F\xacX\xe9\xd2\xca9\x9b\x03\x15>\xd0&\xc3\x1bp\\\x15Fa\xdd\xf5\xc3\xccE\x0fȔ&\xa8z#[9#۟\xa6\xc0Y\xb44\xce\xdb\n8_I\x12\xa9`b\xfa\xd0\xf1\xe4Ą\xfc\x1d\xc8\x17\xc6\xcfF\xe5\xfb\r\xfdN\xbeF&\xeaPtP\xc0\x8f\x1b\x1cm\xcbT\x02\x03n\u0600!d\x89*I\xae\xc4J\xd3e\xdcԳ\xfa5r\xcaz$V\x7f,%'\x87N\xfa\xfe\\3j\x9f\x043\x16|\xe1\x86տ\xa5\xd2M\x8a\f\xd5\x04\x0f\xc0aP\xcb\xd1a65\xf5\x98\xbc \xcfs\xd0b\x02\xba\xc9\xf6\x87\xa2\x1e\x06&\xedR\x12\n\xbbXz\xd1\x1d(Hk\xf7M9UT\x1d3\xde\"\x9aƭ\x98\xb3\xf1\x96\x82iXa\xe1\x1c\xb0\x18\x01>\xea\x84\xf3\xf3\x9c\xe9\xa4Fvr,|f\xc0'\xc7N\xadār\xcctFxH\x86\xf6GO\xe0\xfb\xc9\x13\xd8\x10\xdb\xf8A2hNp!\xfd\xb6\x93\x12k\xd8x?PB]\xbe \"G\xb2\xf3(iM8\x9b\xb4\xc9;\xd4y;d\xa0\x14\x92\x9e\xc3w\x93\xe7\x10\xcd\xe29\x10\xacw|\xfe\xaa\x1e\x85bMw\xf1\x8f\xe3Ѷl\x7f\xb7_\x1c!q\x8b\xff\xb6q7\fx\x81\x0f&\x98\xa2\x19\xe2*\xd3=N\x17\x1c\x87JĹ\xec\xbde2T\xf3m\x8aEV\xf3\x13w!\xe1b\\\x1e\xaf\x9e\x7f\xbb\xb7؋O\n\x9emM\xb7\x1d\x0e\x18\x90\x80\x91-\xbc\x8c\x9b\x1ef\x8b\xbe[\x96} \xe6ɫO\xb1\xd3u\x9a\xc5^\xb9\xfa\xa4\xfb1\x16\x8f\xbd\xb8\x17&0\xc5|\x17\xd2\"\xb6\x9c]\t\xb6\xa0\xa7\xbe\xf6\xb0\x18s@\xb8d\xb4\xb1\xae\xd1P\x1d\xfb\xa7\x9b\x0f\x9d \xbf\x87\xb2>\xd8\x1c8[\x19]\xf86OU<7DB\x9f\x83:TS\xd5볧^\r\xe6b\xf7Ɵ\x84>n\xf3̑!\xfc\xaf\\\x9f\xae-Y'+f\xe5\x88M\x1b\xc3\xee\x1auݎHi\xa6\x90\xaa\x9f\x00\x15c\x8a~\x1c\xdeQJh}\xfb(\x1d\xc1\x92\xbc\xadn\xfb\xe7\xe0E\x00|\x93\xb1\x8f\xe4\xbb&Z\xf5H\x16\xee\x03\xe6\xfbi\xb0\xd9kO\xbb\xa9\xa9\xc5o\x18\x03\nf(\xbb4`@\xb8n\x81b\xb5\xef\xf1\xc4bۣv\x12\x12\xefi{\xd4B\xb2\xb6\x85(\x9b:Q\xd8\x1e\x842Y\x9a\x04\xe8x\x02r\xdf'\x96\x06\xe4\x18\x93\xebr\xcc!\x8b1d\xeacG,\xaff\xa3\x01\r\xecX\x9a\xad\t<%'\x9b\x80E\xe9<&\xa8<\x97\xad\x1cb\x80IA6\xa9\xf36\x95\xfb\xf6\xe9\xc0F\xda\x1e\x89\xfc\xfeC\xf4]\xd5\xe2\x14g\xb0a\x0eU\xc8HS\xf1p6i\x97\xc0\xda\xcep/L\xae\xbb\xda9]\xf2\x13\xc2#\x14\xb4\x7f\x9a\xa2\xeeij-\x11\\\xadu\xb1\xc3\xc9-+\xf4r\xee'\x8b\x97-cM\x94;\xef\xfc^\xa9O\xb4Q\xdd\xeba\x15\xa85\xfa\xb3R\x05\x9fI\x01\x97uDX1*\xa0\xea$\xb9BYq$Ne\xa5_t\x14\xd8K\xecǑc\xf1\x9d\xd5\x11X\xa3\xe3{\x1d\x95\xc4Dx\xe9\xa4m\x9c\x7f\x14\xb80M\x00;*\x01kk\xdcq0j\xbd\x9d\xb5NOz\xc8Ώ`;\u0381ʒ(\a\x81V\x16\xf7y\xde 0 || len(pool.ReferencedByVersions) > 0 { - newCrashes := m.countStartupFailures(sandboxes, pool) + newCrashes, err := m.countStartupFailures(sandboxes, pool) + if err != nil { + return err + } if newCrashes > 0 { pool.ConsecutiveCrashCount += int64(newCrashes) pool.LastCrashTime = time.Now() @@ -819,13 +823,27 @@ func (m *Manager) retireStalePending(ctx context.Context, ent *entityserver_v1al // countStartupFailures records each sandbox ID with the counter in the pool // update. UpdatedAt is mutable and cannot identify an already counted failure. // Legacy sandboxes without an outcome retain the old quick-crash heuristic. -func (m *Manager) countStartupFailures(sandboxes []*sandboxWithMeta, pool *compute_v1alpha.SandboxPool) int64 { - count := int64(0) - counted := make(map[entity.Id]bool, len(pool.CountedFailures)) - for _, id := range pool.CountedFailures { - counted[id] = true +func (m *Manager) countStartupFailures(sandboxes []*sandboxWithMeta, pool *compute_v1alpha.SandboxPool) (int64, error) { + var ids []entity.Id + if pool.CountedFailures != "" { + if err := json.Unmarshal([]byte(pool.CountedFailures), &ids); err != nil { + return 0, fmt.Errorf("decoding counted sandbox failures for pool %s: %w", pool.ID, err) + } + } + listed := make(map[entity.Id]bool, len(sandboxes)) + for _, sbm := range sandboxes { + listed[sbm.sandbox.ID] = true + } + retained := make([]entity.Id, 0, len(ids)) + counted := make(map[entity.Id]bool, len(ids)) + for _, id := range ids { + if listed[id] { + retained = append(retained, id) + counted[id] = true + } } + count := int64(0) for _, sbm := range sandboxes { if sbm.sandbox.Status != compute_v1alpha.DEAD || counted[sbm.sandbox.ID] { continue @@ -842,11 +860,17 @@ func (m *Manager) countStartupFailures(sandboxes []*sandboxWithMeta, pool *compu continue } - pool.CountedFailures = append(pool.CountedFailures, sbm.sandbox.ID) + retained = append(retained, sbm.sandbox.ID) + counted[sbm.sandbox.ID] = true count++ } - return count + encoded, err := json.Marshal(retained) + if err != nil { + return 0, fmt.Errorf("encoding counted sandbox failures for pool %s: %w", pool.ID, err) + } + pool.CountedFailures = string(encoded) + return count, nil } // backoffDuration calculates the exponential backoff duration based on consecutive crash count diff --git a/controllers/sandboxpool/manager_test.go b/controllers/sandboxpool/manager_test.go index 9b0462a41..76583ca06 100644 --- a/controllers/sandboxpool/manager_test.go +++ b/controllers/sandboxpool/manager_test.go @@ -2,6 +2,7 @@ package sandboxpool import ( "context" + "encoding/json" "fmt" "testing" "time" @@ -1520,10 +1521,52 @@ func TestCountStartupFailures(t *testing.T) { {sandbox: &compute_v1alpha.Sandbox{ID: "old-legacy", Status: compute_v1alpha.DEAD}, createdAt: now.Add(-6 * time.Minute), updatedAt: now.Add(-time.Minute)}, {sandbox: &compute_v1alpha.Sandbox{ID: "old-failure", Status: compute_v1alpha.DEAD, StartupOutcome: compute_v1alpha.STARTUP_FAILED}, createdAt: now.Add(-10 * time.Minute), updatedAt: now.Add(-2 * time.Minute)}, } - pool := &compute_v1alpha.SandboxPool{LastCrashTime: now.Add(-90 * time.Second), CountedFailures: []entity.Id{"old-failure"}} - assert.Equal(t, int64(3), manager.countStartupFailures(sandboxes, pool), + pool := &compute_v1alpha.SandboxPool{LastCrashTime: now.Add(-90 * time.Second), CountedFailures: `["old-failure"]`} + count, err := manager.countStartupFailures(sandboxes, pool) + require.NoError(t, err) + assert.Equal(t, int64(3), count, "long pre-running failure and quick crashes count, but long-running and previously counted failures do not") - assert.ElementsMatch(t, []entity.Id{"old-failure", "first", "fast-healthy", "legacy"}, pool.CountedFailures) + var ids []entity.Id + require.NoError(t, json.Unmarshal([]byte(pool.CountedFailures), &ids)) + assert.ElementsMatch(t, []entity.Id{"old-failure", "first", "fast-healthy", "legacy"}, ids) +} + +func TestCountedFailuresPrunedAndClearedInStore(t *testing.T) { + ctx := context.Background() + server, cleanup := testutils.NewInMemEntityServer(t) + defer cleanup() + pool := &compute_v1alpha.SandboxPool{ + Service: "web", DesiredInstances: 1, + SandboxSpec: compute_v1alpha.SandboxSpec{Version: "ver-1"}, + } + id, err := server.Client.Create(ctx, "pool", pool) + require.NoError(t, err) + pool.ID = id + sbID, err := server.Client.Create(ctx, "counted-dead", + &compute_v1alpha.Sandbox{Status: compute_v1alpha.DEAD, StartupOutcome: compute_v1alpha.STARTUP_FAILED, Spec: pool.SandboxSpec}, + entityserver.WithLabels(types.LabelSet("service", "web", "pool", id.String()))) + require.NoError(t, err) + ids, err := json.Marshal([]entity.Id{"deleted", sbID}) + require.NoError(t, err) + _, err = server.EAC.Patch(ctx, entity.New(entity.DBId, id, + entity.String(compute_v1alpha.SandboxPoolCountedFailuresId, string(ids))).Attrs(), 0) + require.NoError(t, err) + manager := NewManager(testutils.TestLogger(t), server.EAC) + reconcilePool(t, ctx, server, manager, pool) + updated := getPool(t, ctx, server, id) + assert.JSONEq(t, fmt.Sprintf(`[%q]`, sbID), updated.CountedFailures) + assert.Equal(t, int64(0), updated.ConsecutiveCrashCount, "the retained terminal sandbox is not counted again") + + _, err = server.EAC.Delete(ctx, sbID.String()) + require.NoError(t, err) + reconcilePool(t, ctx, server, manager, pool) + updated = getPool(t, ctx, server, id) + assert.Equal(t, "[]", updated.CountedFailures, "an empty list must replace the stored value") + resp, err := server.EAC.Get(ctx, id.String()) + require.NoError(t, err) + attr, ok := resp.Entity().Entity().Get(compute_v1alpha.SandboxPoolCountedFailuresId) + require.True(t, ok) + assert.Equal(t, "[]", attr.Value.String()) } func TestPartialNodeLossKeepsPoolDesiredDuringCooldown(t *testing.T) { @@ -1689,7 +1732,9 @@ func TestManagerLongStartupFailureBackoff(t *testing.T) { assert.WithinDuration(t, before.Add(backoffDuration(streak)), updated.CooldownUntil, 2*time.Second) assert.Equal(t, int64(0), updated.CurrentInstances) assert.Len(t, listSandboxesForPool(t, ctx, server, pool), int(streak), "no replacement during cooldown") - assert.Len(t, updated.CountedFailures, int(streak)) + var ids []entity.Id + require.NoError(t, json.Unmarshal([]byte(updated.CountedFailures), &ids)) + assert.Len(t, ids, int(streak)) // An exit recorder may rewrite DEAD to STOPPED after the pool // counted it, and cleanup can then restore DEAD with a new UpdatedAt. _, err = server.EAC.Patch(ctx, entity.New(entity.DBId, sbID, @@ -1702,7 +1747,8 @@ func TestManagerLongStartupFailureBackoff(t *testing.T) { reconcilePool(t, ctx, server, manager, pool) updated = getPool(t, ctx, server, id) assert.Equal(t, streak, updated.ConsecutiveCrashCount) - assert.Len(t, updated.CountedFailures, int(streak)) + require.NoError(t, json.Unmarshal([]byte(updated.CountedFailures), &ids)) + assert.Len(t, ids, int(streak)) } // A deployment clears the streak, not the identities already counted. _, err = server.EAC.Patch(ctx, []entity.Attr{ @@ -1714,5 +1760,7 @@ func TestManagerLongStartupFailureBackoff(t *testing.T) { reconcilePool(t, ctx, server, manager, pool) updated := getPool(t, ctx, server, id) assert.Equal(t, int64(0), updated.ConsecutiveCrashCount) - assert.Len(t, updated.CountedFailures, 2) + var ids []entity.Id + require.NoError(t, json.Unmarshal([]byte(updated.CountedFailures), &ids)) + assert.Len(t, ids, 2) } From f24235c8f0612db8194b91422a89fb1f2cf23c92 Mon Sep 17 00:00:00 2001 From: Evan Phoenix Date: Fri, 25 Sep 2026 16:35:08 +0000 Subject: [PATCH 7/8] Use failure watermark instead of persisted sandbox IDs --- api/compute/compute_v1alpha/schema.gen.go | 14 +--- api/compute/schema.yml | 4 -- controllers/sandbox/sandbox.go | 7 +- controllers/sandbox/sandbox_test.go | 5 ++ controllers/sandboxpool/manager.go | 56 +++------------ controllers/sandboxpool/manager_test.go | 86 ++++------------------- 6 files changed, 34 insertions(+), 138 deletions(-) diff --git a/api/compute/compute_v1alpha/schema.gen.go b/api/compute/compute_v1alpha/schema.gen.go index 2522fb2f4..7757e6055 100644 --- a/api/compute/compute_v1alpha/schema.gen.go +++ b/api/compute/compute_v1alpha/schema.gen.go @@ -2034,7 +2034,6 @@ const ( SandboxPoolAppId = entity.Id("dev.miren.compute/sandbox_pool.app") SandboxPoolConsecutiveCrashCountId = entity.Id("dev.miren.compute/sandbox_pool.consecutive_crash_count") SandboxPoolCooldownUntilId = entity.Id("dev.miren.compute/sandbox_pool.cooldown_until") - SandboxPoolCountedFailuresId = entity.Id("dev.miren.compute/sandbox_pool.counted_failures") SandboxPoolCurrentInstancesId = entity.Id("dev.miren.compute/sandbox_pool.current_instances") SandboxPoolDesiredInstancesId = entity.Id("dev.miren.compute/sandbox_pool.desired_instances") SandboxPoolEphemeralId = entity.Id("dev.miren.compute/sandbox_pool.ephemeral") @@ -2052,7 +2051,6 @@ type SandboxPool struct { App entity.Id `cbor:"app,omitempty" json:"app,omitempty"` ConsecutiveCrashCount int64 `cbor:"consecutive_crash_count,omitempty" json:"consecutive_crash_count,omitempty"` CooldownUntil time.Time `cbor:"cooldown_until,omitempty" json:"cooldown_until"` - CountedFailures string `cbor:"counted_failures,omitempty" json:"counted_failures,omitempty"` CurrentInstances int64 `cbor:"current_instances,omitempty" json:"current_instances,omitempty"` DesiredInstances int64 `cbor:"desired_instances,omitempty" json:"desired_instances,omitempty"` Ephemeral bool `cbor:"ephemeral,omitempty" json:"ephemeral,omitempty"` @@ -2076,9 +2074,6 @@ func (o *SandboxPool) Decode(e entity.AttrGetter) { if a, ok := e.Get(SandboxPoolCooldownUntilId); ok && a.Value.Kind() == entity.KindTime { o.CooldownUntil = a.Value.Time() } - if a, ok := e.Get(SandboxPoolCountedFailuresId); ok && a.Value.Kind() == entity.KindString { - o.CountedFailures = a.Value.String() - } if a, ok := e.Get(SandboxPoolCurrentInstancesId); ok && a.Value.Kind() == entity.KindInt64 { o.CurrentInstances = a.Value.Int64() } @@ -2141,9 +2136,6 @@ func (o *SandboxPool) Encode() (attrs []entity.Attr) { if !entity.Empty(o.CooldownUntil) { attrs = append(attrs, entity.Time(SandboxPoolCooldownUntilId, o.CooldownUntil)) } - if !entity.Empty(o.CountedFailures) { - attrs = append(attrs, entity.String(SandboxPoolCountedFailuresId, o.CountedFailures)) - } if !entity.Empty(o.CurrentInstances) { attrs = append(attrs, entity.Int64(SandboxPoolCurrentInstancesId, o.CurrentInstances)) } @@ -2186,9 +2178,6 @@ func (o *SandboxPool) Empty() bool { if !entity.Empty(o.CooldownUntil) { return false } - if !entity.Empty(o.CountedFailures) { - return false - } if !entity.Empty(o.CurrentInstances) { return false } @@ -2226,7 +2215,6 @@ func (o *SandboxPool) InitSchema(sb *schema.SchemaBuilder) { sb.Ref("app", "dev.miren.compute/sandbox_pool.app", schema.Doc("Reference to the app this pool belongs to"), schema.Indexed, schema.Tags("dev.miren.app_ref")) sb.Int64("consecutive_crash_count", "dev.miren.compute/sandbox_pool.consecutive_crash_count", schema.Doc("Number of consecutive sandbox failures, including quick crashes after RUNNING")) sb.Time("cooldown_until", "dev.miren.compute/sandbox_pool.cooldown_until", schema.Doc("Timestamp until which new sandbox creation is paused due to crash loop")) - sb.String("counted_failures", "dev.miren.compute/sandbox_pool.counted_failures", schema.Doc("JSON array of sandbox IDs already included in the failure streak; kept across streak resets")) sb.Int64("current_instances", "dev.miren.compute/sandbox_pool.current_instances", schema.Doc("Current number of sandbox instances (non-STOPPED)")) sb.Int64("desired_instances", "dev.miren.compute/sandbox_pool.desired_instances", schema.Doc("Target number of sandbox instances")) sb.Bool("ephemeral", "dev.miren.compute/sandbox_pool.ephemeral", schema.Doc("True when this pool backs an ephemeral AppVersion. Ephemeral pools never scale beyond 1 instance.")) @@ -2350,5 +2338,5 @@ func init() { (&SandboxPool{}).InitSchema(sb) (&Schedule{}).InitSchema(sb) }) - schema.RegisterEncodedSchema("dev.miren.compute", "v1alpha", []byte("\x1f\x8b\b\x00\x00\x00\x00\x00\x00\xff\xec\\َ\xf4:\x11~\r\x0e\xcba\xdf\x04\xe4\xe7 v\xb1\x1d\x01\xb7\xbcB䎫ӞN\xdb\x19\xdb\xe9\xe9\xe6\x0e\x10\x12B,\xef\xc0?\xc3\x1b\xc25\xf2\x968\x89\x9d8n\xa4s\x93\x9b\x91\xedv}\xb6\xcb\xe5\xaar\xb9&\xaf\x98\xa2\v\x1b9\xb2\r a\xcf\xec˼\x8b\xfe5\xf1\xd0\xfeI/\xe1k\x11\x90\xa2AB\x96'@\\\x1e\xc0\n<\x9d\xb4\x8d%\xfe\xf31\xa4\x96\xb3'\xa8\fD\xed*\x8a\xf6@\xf02\xa5@\x14\x1f\xd8\xcdP\xba\x8a\xa5\\\xe4!h\xfa\xd0\xfeh&Z(\xc3\xc6\xdb\a\x81\xdd6\x1d\x129\xf9Ϸ\x88\xe2\xb00Łu\x14\x97-\xe3\xd2\xd3]O^\xabZ\x13QT\x8c\x02\x95C\xc9\xcep\x0e]̡\x13'\xfbǘ\x96\x1b\x90\n_[\xd7\x01M\xfd>\xa2\xa0<\b\xbd,\xad`\xfb\x05V\x84\xca\xc5]#\x9a^\xf5\xd7\f\xfdr\x9c\xa1\x15\xa3\x12\x11\n\xdc\xe3'\x19\x1aW\xd89\a.f\xc0\x89\xdc\xfc\xfb\xfb\xc8L{ \xd5rA\xd4\xe8\xfb\xdaUo\x879\xb9\x92\x06j0\xf6\xeaɫ\xeb\xe1\x0e\x8c\x99\xdbշ\x97Pĩ\x93\x98\xbd\xd0R9\x87\xac3\\og\xad\x9b\x14\x9c\x90\x98\x98c\x0f\xa68\x9eϢr\x95\xd2(\xe7J\x15z\xbae_\xa4\xa7~\x8dh\x13w<\xe0F\xacX\xe9\xd2\xca9\x9b\x03\x15>\xd0&\xc3\x1bp\\\x15Fa\xdd\xf5\xc3\xccE\x0fȔ&\xa8z#[9#۟\xa6\xc0Y\xb44\xce\xdb\n8_I\x12\xa9`b\xfa\xd0\xf1\xe4Ą\xfc\x1d\xc8\x17\xc6\xcfF\xe5\xfb\r\xfdN\xbeF&\xeaPtP\xc0\x8f\x1b\x1cm\xcbT\x02\x03n\u0600!d\x89*I\xae\xc4J\xd3e\xdcԳ\xfa5r\xcaz$V\x7f,%'\x87N\xfa\xfe\\3j\x9f\x043\x16|\xe1\x86տ\xa5\xd2M\x8a\f\xd5\x04\x0f\xc0aP\xcb\xd1a65\xf5\x98\xbc \xcfs\xd0b\x02\xba\xc9\xf6\x87\xa2\x1e\x06&\xedR\x12\n\xbbXz\xd1\x1d(Hk\xf7M9UT\x1d3\xde\"\x9aƭ\x98\xb3\xf1\x96\x82iXa\xe1\x1c\xb0\x18\x01>\xea\x84\xf3\xf3\x9c\xe9\xa4Fvr,|f\xc0'\xc7N\xadār\xcctFxH\x86\xf6GO\xe0\xfb\xc9\x13\xd8\x10\xdb\xf8A2hNp!\xfd\xb6\x93\x12k\xd8x?PB]\xbe \"G\xb2\xf3(iM8\x9b\xb4\xc9;\xd4y;d\xa0\x14\x92\x9e\xc3w\x93\xe7\x10\xcd\xe29\x10\xacw|\xfe\xaa\x1e\x85bMw\xf1\x8f\xe3Ѷl\x7f\xb7_\x1c!q\x8b\xff\xb6q7\fx\x81\x0f&\x98\xa2\x19\xe2*\xd3=N\x17\x1c\x87JĹ\xec\xbde2T\xf3m\x8aEV\xf3\x13w!\xe1b\\\x1e\xaf\x9e\x7f\xbb\xb7؋O\n\x9emM\xb7\x1d\x0e\x18\x90\x80\x91-\xbc\x8c\x9b\x1ef\x8b\xbe[\x96} \xe6ɫO\xb1\xd3u\x9a\xc5^\xb9\xfa\xa4\xfb1\x16\x8f\xbd\xb8\x17&0\xc5|\x17\xd2\"\xb6\x9c]\t\xb6\xa0\xa7\xbe\xf6\xb0\x18s@\xb8d\xb4\xb1\xae\xd1P\x1d\xfb\xa7\x9b\x0f\x9d \xbf\x87\xb2>\xd8\x1c8[\x19]\xf86OU<7DB\x9f\x83:TS\xd5볧^\r\xe6b\xf7Ɵ\x84>n\xf3̑!\xfc\xaf\\\x9f\xae-Y'+f\xe5\x88M\x1b\xc3\xee\x1auݎHi\xa6\x90\xaa\x9f\x00\x15c\x8a~\x1c\xdeQJh}\xfb(\x1d\xc1\x92\xbc\xadn\xfb\xe7\xe0E\x00|\x93\xb1\x8f\xe4\xbb&Z\xf5H\x16\xee\x03\xe6\xfbi\xb0\xd9kO\xbb\xa9\xa9\xc5o\x18\x03\nf(\xbb4`@\xb8n\x81b\xb5\xef\xf1\xc4bۣv\x12\x12\xefi{\xd4B\xb2\xb6\x85(\x9b:Q\xd8\x1e\x842Y\x9a\x04\xe8x\x02r\xdf'\x96\x06\xe4\x18\x93\xebr\xcc!\x8b1d\xeacG,\xaff\xa3\x01\r\xecX\x9a\xad\t<%'\x9b\x80E\xe9<&\xa8<\x97\xad\x1cb\x80IA6\xa9\xf36\x95\xfb\xf6\xe9\xc0F\xda\x1e\x89\xfc\xfeC\xf4]\xd5\xe2\x14g\xb0a\x0eU\xc8HS\xf1p6i\x97\xc0\xda\xcep/L\xae\xbb\xda9]\xf2\x13\xc2#\x14\xb4\x7f\x9a\xa2\xeeij-\x11\\\xadu\xb1\xc3\xc9-+\xf4r\xee'\x8b\x97-cM\x94;\xef\xfc^\xa9O\xb4Q\xdd\xeba\x15\xa85\xfa\xb3R\x05\x9fI\x01\x97uDX1*\xa0\xea$\xb9BYq$Ne\xa5_t\x14\xd8K\xecǑc\xf1\x9d\xd5\x11X\xa3\xe3{\x1d\x95\xc4Dx\xe9\xa4m\x9c\x7f\x14\xb80M\x00;*\x01kk\xdcq0j\xbd\x9d\xb5NOz\xc8Ώ`;\u0381ʒ(\a\x81V\x16\xf7y\xde\x1b9\xb2\r a\xcf\xec˼\x8b\xfe5\xf1\xd0\xfeI/\xe1k\x11\x90\xa2AB\x96'@\\\x1e\xc0\n<\x9d\xb4\x8d%\xfe\xf31\xa4\x96\xb3'\xa8\fD\xed*\x8a\xf6@\xf02\xa5@\x14\x1f\xd8\xcdP\xba\x8a\xa5\\\xe4!h\xfa\xd0\xfeh&Z(\xc3\xc6\xdb\a\x81\xdd6\x1d\x129\xf9Ϸ\x88\xe2\xb00Łu\x14\x97-\xe3\xd2\xd3]O^\xabZ\x13QT\x8c\x02\x95C\xc9\xcep\x0e]̡\x13'\xfbǘ\x96\x1b\x90\n_[\xd7\x01M\xfd>\xa2\xa0<\b\xbd,\xad`\xfb\x05V\x84\xca\xc5]#\x9a^\xf5\xd7\f\xfdr\x9c\xa1\x15\xa3\x12\x11\n\xdc\xe3'\x19\x1aW\xd89\a.f\xc0\x89\xdc\xfc\xfb\xfb\xc8L{ \xd5rA\xd4\xe8\xfb\xdaUo\x879\xb9\x92\x06j0\xf6\xeaɫ\xeb\xe1\x0e\x8c\x99\xdbշ\x97Pĩ\x93\x98\xbd\xd0R9\x87\xac3\\og\xad\x9b\x14\x9c\x90\x98\x98c\x0f\xa68\x9eϢr\x95\xd2(\xe7J\x15z\xbae_\xa4\xa7~\x8dh\x13w<\xe0F\xacX\xe9\xd2\xca9\x9b\x03\x15>\xd0&\xc3\x1bp\\\x15Fa\xdd\xf5\xc3\xccE\x0fȔ&\xa8z#[9#۟\xa6\xc0Y\xb44\xce\xdb\n8_I\x12\xa9`b\xfa\xd0\xf1\xe4Ą\xfc\x1d\xc8\x17\xc6\xcfF\xe5\xfb\r\xfdN\xbeF&\xeaPtP\xc0\x8f\x1b\x1cm\xcbT\x02\x03n\u0600!d\x89*I\xae\xc4J\xd3e\xdcԳ\xfa5r\xcaz$V\x7f,%'\x87N\xfa\xfe\\3j\x9f\x043\x16|\xe1\x86տ\xa5\xd2M\x8a\f\xd5\x04\x0f\xc0aP\xcb\xd1a65\xf5\x98\xbc \xcfs\xd0b\x02\xba\xc9\xf6\x87\xa2\x1e\x06&\xedR\x12\n\xbbXz\xd1\x1d(Hk\xf7M9UT\x1d3\xde\"\x9aƭ\x98\xb3\xf1\x96\x82iXa\xe1\x1c\xb0\x18\x01>\xea\x84\xf3\xf3\x9c\xe9\xa4Fvr,|f\xc0'\xc7N\xadār\xcctFxH\x86\xf6GO\xe0\xfb\xc9\x13\xd8\x10\xdb\xf8A2hNp!\xfd\xb6\x93\x12k\xd8x?PB]\xbe \"G\xb2\xf3(iM8\x9b\xb4\xc9;\xd4y;d\xa0\x14\x92\x9e\xc3w\x93\xe7\x10\xcd\xe29\x10\xacw|\xfe\xaa\x1e\x85bMw\xf1\x8f\xe3Ѷl\x7f\xb7_\x1c!q\x8b\xff\xb6q7\fx\x81\x0f&\x98\xa2\x19\xe2*\xd3=N\x17\x1c\x87JĹ\xec\xbde2T\xf3m\x8aEV\xf3\x13w!\xe1b\\\x1e\xaf\x9e\x7f\xbb\xb7؋O\n\x9emM\xb7\x1d\x0e\x18\x90\x80\x91-\xbc\x8c\x9b\x1ef\x8b\xbe[\x96} \xe6ɫO\xb1\xd3u\x9a\xc5^\xb9\xfa\xa4\xfb1\x16\x8f\xbd\xb8\x17&0\xc5|\x17\xd2\"\xb6\x9c]\t\xb6\xa0\xa7\xbe\xf6\xb0\x18s@\xb8d\xb4\xb1\xae\xd1P\x1d\xfb\xa7\x9b\x0f\x9d \xbf\x87\xb2>\xd8\x1c8[\x19]\xf86OU<7DB\x9f\x83:TS\xd5볧^\r\xe6b\xf7Ɵ\x84>n\xf3̑!\xfc\xaf\\\x9f\xae-Y'+f\xe5\x88M\x1b\xc3\xee\x1auݎHi\xa6\x90\xaa\x9f\x00\x15c\x8a~\x1c\xdeQJh}\xfb(\x1d\xc1\x92\xbc\xadn\xfb\xe7\xe0E\x00|\x93\xb1\x8f\xe4\xbb&Z\xf5H\x16\xee\x03\xe6\xfbi\xb0\xd9kO\xbb\xa9\xa9\xc5o\x18\x03\nf(\xbb4`@\xb8n\x81b\xb5\xef\xf1\xc4bۣv\x12\x12\xefi{\xd4B\xb2\xb6\x85(\x9b:Q\xd8\x1e\x842Y\x9a\x04\xe8x\x02r\xdf'\x96\x06\xe4\x18\x93\xebr\xcc!\x8b1d\xeacG,\xaff\xa3\x01\r\xecX\x9a\xad\t<%'\x9b\x80E\xe9<&\xa8<\x97\xad\x1cb\x80IA6\xa9\xf36\x95\xfb\xf6\xe9\xc0F\xda\x1e\x89\xfc\xfeC\xf4]\xd5\xe2\x14g\xb0a\x0eU\xc8HS\xf1p6i\x97\xc0\xda\xcep/L\xae\xbb\xda9]\xf2\x13\xc2#\x14\xb4\x7f\x9a\xa2\xeeij-\x11\\\xadu\xb1\xc3\xc9-+\xf4r\xee'\x8b\x97-cM\x94;\xef\xfc^\x9br\x87CG\xdd\xc3*Pk\xf4g\xa5\n>\x93\x02.눰bT@\xd5Ir\x85\xb2\xe2H\x9c\xcaJ\xbf\xe8(\xb0\x97؏#\xc7\xe2;\xab#\xb0F\xc7\xf7:*\x89\x89\xf0\xd2I\xdb8\xff(d\x90G\x80\x1d\xe7@eI\x94%\xa7\x15\x18\xbd\xfe 0 || len(pool.ReferencedByVersions) > 0 { - newCrashes, err := m.countStartupFailures(sandboxes, pool) - if err != nil { - return err - } + newCrashes, latestCrash := m.countStartupFailures(sandboxes, pool) if newCrashes > 0 { pool.ConsecutiveCrashCount += int64(newCrashes) - pool.LastCrashTime = time.Now() + pool.LastCrashTime = latestCrash pool.CooldownUntil = m.calculateBackoff(pool.ConsecutiveCrashCount) m.log.Warn("crash detected, entering cooldown", @@ -820,32 +816,13 @@ func (m *Manager) retireStalePending(ctx context.Context, ent *entityserver_v1al } } -// countStartupFailures records each sandbox ID with the counter in the pool -// update. UpdatedAt is mutable and cannot identify an already counted failure. -// Legacy sandboxes without an outcome retain the old quick-crash heuristic. -func (m *Manager) countStartupFailures(sandboxes []*sandboxWithMeta, pool *compute_v1alpha.SandboxPool) (int64, error) { - var ids []entity.Id - if pool.CountedFailures != "" { - if err := json.Unmarshal([]byte(pool.CountedFailures), &ids); err != nil { - return 0, fmt.Errorf("decoding counted sandbox failures for pool %s: %w", pool.ID, err) - } - } - listed := make(map[entity.Id]bool, len(sandboxes)) - for _, sbm := range sandboxes { - listed[sbm.sandbox.ID] = true - } - retained := make([]entity.Id, 0, len(ids)) - counted := make(map[entity.Id]bool, len(ids)) - for _, id := range ids { - if listed[id] { - retained = append(retained, id) - counted[id] = true - } - } - +// countStartupFailures uses the latest counted sandbox update as a watermark. +// DEAD sandboxes must not be rewritten after counting or they can be counted again. +func (m *Manager) countStartupFailures(sandboxes []*sandboxWithMeta, pool *compute_v1alpha.SandboxPool) (int64, time.Time) { count := int64(0) + latest := pool.LastCrashTime for _, sbm := range sandboxes { - if sbm.sandbox.Status != compute_v1alpha.DEAD || counted[sbm.sandbox.ID] { + if sbm.sandbox.Status != compute_v1alpha.DEAD || !sbm.updatedAt.After(pool.LastCrashTime) { continue } @@ -854,23 +831,12 @@ func (m *Manager) countStartupFailures(sandboxes []*sandboxWithMeta, pool *compu continue } - // Pre-upgrade crashes did not have durable IDs. The old watermark - // protects them across deploys that reset the streak. - if sbm.sandbox.StartupOutcome == "" && !pool.LastCrashTime.IsZero() && !sbm.updatedAt.After(pool.LastCrashTime) { - continue - } - - retained = append(retained, sbm.sandbox.ID) - counted[sbm.sandbox.ID] = true count++ + if sbm.updatedAt.After(latest) { + latest = sbm.updatedAt + } } - - encoded, err := json.Marshal(retained) - if err != nil { - return 0, fmt.Errorf("encoding counted sandbox failures for pool %s: %w", pool.ID, err) - } - pool.CountedFailures = string(encoded) - return count, nil + return count, latest } // backoffDuration calculates the exponential backoff duration based on consecutive crash count diff --git a/controllers/sandboxpool/manager_test.go b/controllers/sandboxpool/manager_test.go index 76583ca06..400e75368 100644 --- a/controllers/sandboxpool/manager_test.go +++ b/controllers/sandboxpool/manager_test.go @@ -2,7 +2,6 @@ package sandboxpool import ( "context" - "encoding/json" "fmt" "testing" "time" @@ -1514,59 +1513,21 @@ func TestCountStartupFailures(t *testing.T) { now := time.Now() manager := &Manager{} sandboxes := []*sandboxWithMeta{ - {sandbox: &compute_v1alpha.Sandbox{ID: "first", Status: compute_v1alpha.DEAD, StartupOutcome: compute_v1alpha.STARTUP_FAILED}, createdAt: now.Add(-6 * time.Minute), updatedAt: now.Add(-time.Minute)}, + {sandbox: &compute_v1alpha.Sandbox{ID: "first", Status: compute_v1alpha.DEAD, StartupOutcome: compute_v1alpha.STARTUP_FAILED}, createdAt: now.Add(-6 * time.Minute), updatedAt: now.Add(-40 * time.Second)}, {sandbox: &compute_v1alpha.Sandbox{ID: "healthy", Status: compute_v1alpha.DEAD, StartupOutcome: compute_v1alpha.STARTUP_RUNNING}, createdAt: now.Add(-time.Hour), updatedAt: now.Add(-30 * time.Second)}, {sandbox: &compute_v1alpha.Sandbox{ID: "fast-healthy", Status: compute_v1alpha.DEAD, StartupOutcome: compute_v1alpha.STARTUP_RUNNING}, createdAt: now.Add(-20 * time.Second), updatedAt: now.Add(-10 * time.Second)}, - {sandbox: &compute_v1alpha.Sandbox{ID: "legacy", Status: compute_v1alpha.DEAD}, createdAt: now.Add(-20 * time.Second), updatedAt: now.Add(-10 * time.Second)}, + {sandbox: &compute_v1alpha.Sandbox{ID: "legacy", Status: compute_v1alpha.DEAD}, createdAt: now.Add(-25 * time.Second), updatedAt: now.Add(-15 * time.Second)}, {sandbox: &compute_v1alpha.Sandbox{ID: "old-legacy", Status: compute_v1alpha.DEAD}, createdAt: now.Add(-6 * time.Minute), updatedAt: now.Add(-time.Minute)}, {sandbox: &compute_v1alpha.Sandbox{ID: "old-failure", Status: compute_v1alpha.DEAD, StartupOutcome: compute_v1alpha.STARTUP_FAILED}, createdAt: now.Add(-10 * time.Minute), updatedAt: now.Add(-2 * time.Minute)}, } - pool := &compute_v1alpha.SandboxPool{LastCrashTime: now.Add(-90 * time.Second), CountedFailures: `["old-failure"]`} - count, err := manager.countStartupFailures(sandboxes, pool) - require.NoError(t, err) + pool := &compute_v1alpha.SandboxPool{LastCrashTime: now.Add(-90 * time.Second)} + count, latest := manager.countStartupFailures(sandboxes, pool) assert.Equal(t, int64(3), count, "long pre-running failure and quick crashes count, but long-running and previously counted failures do not") - var ids []entity.Id - require.NoError(t, json.Unmarshal([]byte(pool.CountedFailures), &ids)) - assert.ElementsMatch(t, []entity.Id{"old-failure", "first", "fast-healthy", "legacy"}, ids) -} - -func TestCountedFailuresPrunedAndClearedInStore(t *testing.T) { - ctx := context.Background() - server, cleanup := testutils.NewInMemEntityServer(t) - defer cleanup() - pool := &compute_v1alpha.SandboxPool{ - Service: "web", DesiredInstances: 1, - SandboxSpec: compute_v1alpha.SandboxSpec{Version: "ver-1"}, - } - id, err := server.Client.Create(ctx, "pool", pool) - require.NoError(t, err) - pool.ID = id - sbID, err := server.Client.Create(ctx, "counted-dead", - &compute_v1alpha.Sandbox{Status: compute_v1alpha.DEAD, StartupOutcome: compute_v1alpha.STARTUP_FAILED, Spec: pool.SandboxSpec}, - entityserver.WithLabels(types.LabelSet("service", "web", "pool", id.String()))) - require.NoError(t, err) - ids, err := json.Marshal([]entity.Id{"deleted", sbID}) - require.NoError(t, err) - _, err = server.EAC.Patch(ctx, entity.New(entity.DBId, id, - entity.String(compute_v1alpha.SandboxPoolCountedFailuresId, string(ids))).Attrs(), 0) - require.NoError(t, err) - manager := NewManager(testutils.TestLogger(t), server.EAC) - reconcilePool(t, ctx, server, manager, pool) - updated := getPool(t, ctx, server, id) - assert.JSONEq(t, fmt.Sprintf(`[%q]`, sbID), updated.CountedFailures) - assert.Equal(t, int64(0), updated.ConsecutiveCrashCount, "the retained terminal sandbox is not counted again") - - _, err = server.EAC.Delete(ctx, sbID.String()) - require.NoError(t, err) - reconcilePool(t, ctx, server, manager, pool) - updated = getPool(t, ctx, server, id) - assert.Equal(t, "[]", updated.CountedFailures, "an empty list must replace the stored value") - resp, err := server.EAC.Get(ctx, id.String()) - require.NoError(t, err) - attr, ok := resp.Entity().Entity().Get(compute_v1alpha.SandboxPoolCountedFailuresId) - require.True(t, ok) - assert.Equal(t, "[]", attr.Value.String()) + assert.Equal(t, now.Add(-10*time.Second), latest, "use the newest counted update, not time.Now or the newest healthy exit") + pool.LastCrashTime = latest + count, _ = manager.countStartupFailures(sandboxes, pool) + assert.Zero(t, count, "reconciliation must not count the same DEAD sandboxes again") } func TestPartialNodeLossKeepsPoolDesiredDuringCooldown(t *testing.T) { @@ -1707,14 +1668,6 @@ func TestManagerLongStartupFailureBackoff(t *testing.T) { manager := NewManager(testutils.TestLogger(t), server.EAC) for streak := int64(1); streak <= 2; streak++ { - if streak == 2 { - // A failure can become DEAD after the pool's List snapshot but - // before its watermark advances. Its older update time must not - // suppress a never-accounted-for sandbox. - _, err = server.EAC.Patch(ctx, entity.New(entity.DBId, id, - (&compute_v1alpha.SandboxPool{LastCrashTime: time.Now().Add(time.Minute)}).Encode).Attrs(), 0) - require.NoError(t, err) - } created := time.Now().Add(-6 * time.Minute) server.Store.NowFunc = func() time.Time { return created } sbID, err := server.Client.Create(ctx, fmt.Sprintf("failed-%d", streak), @@ -1722,35 +1675,26 @@ func TestManagerLongStartupFailureBackoff(t *testing.T) { entityserver.WithLabels(types.LabelSet("service", "web", "pool", id.String()))) require.NoError(t, err) server.Store.NowFunc = nil + diedAt := time.Now().Add(-time.Duration(3-streak) * time.Second).Truncate(time.Millisecond).UTC() + server.Store.NowFunc = func() time.Time { return diedAt } _, err = server.EAC.Patch(ctx, entity.New(entity.DBId, sbID, (&compute_v1alpha.Sandbox{Status: compute_v1alpha.DEAD, StartupOutcome: compute_v1alpha.STARTUP_FAILED}).Encode).Attrs(), 0) require.NoError(t, err) + server.Store.NowFunc = nil before := time.Now() reconcilePool(t, ctx, server, manager, pool) updated := getPool(t, ctx, server, id) assert.Equal(t, streak, updated.ConsecutiveCrashCount) + assert.Equal(t, diedAt, updated.LastCrashTime, "watermark must use the counted event, not reconciliation time") assert.WithinDuration(t, before.Add(backoffDuration(streak)), updated.CooldownUntil, 2*time.Second) assert.Equal(t, int64(0), updated.CurrentInstances) assert.Len(t, listSandboxesForPool(t, ctx, server, pool), int(streak), "no replacement during cooldown") - var ids []entity.Id - require.NoError(t, json.Unmarshal([]byte(updated.CountedFailures), &ids)) - assert.Len(t, ids, int(streak)) - // An exit recorder may rewrite DEAD to STOPPED after the pool - // counted it, and cleanup can then restore DEAD with a new UpdatedAt. - _, err = server.EAC.Patch(ctx, entity.New(entity.DBId, sbID, - (&compute_v1alpha.Sandbox{Status: compute_v1alpha.STOPPED}).Encode).Attrs(), 0) - require.NoError(t, err) - _, err = server.EAC.Patch(ctx, entity.New(entity.DBId, sbID, - (&compute_v1alpha.Sandbox{Status: compute_v1alpha.DEAD}).Encode).Attrs(), 0) - require.NoError(t, err) // The same terminal entity must not increase the streak on another reconcile. reconcilePool(t, ctx, server, manager, pool) updated = getPool(t, ctx, server, id) assert.Equal(t, streak, updated.ConsecutiveCrashCount) - require.NoError(t, json.Unmarshal([]byte(updated.CountedFailures), &ids)) - assert.Len(t, ids, int(streak)) } - // A deployment clears the streak, not the identities already counted. + // A deployment clears the streak but preserves the failure watermark. _, err = server.EAC.Patch(ctx, []entity.Attr{ entity.Ref(entity.DBId, id), entity.Int64(compute_v1alpha.SandboxPoolConsecutiveCrashCountId, 0), @@ -1760,7 +1704,5 @@ func TestManagerLongStartupFailureBackoff(t *testing.T) { reconcilePool(t, ctx, server, manager, pool) updated := getPool(t, ctx, server, id) assert.Equal(t, int64(0), updated.ConsecutiveCrashCount) - var ids []entity.Id - require.NoError(t, json.Unmarshal([]byte(updated.CountedFailures), &ids)) - assert.Len(t, ids, 2) + assert.False(t, updated.LastCrashTime.IsZero()) } From bbf41aa8154326323a04239d0cb7bb728373c673 Mon Sep 17 00:00:00 2001 From: Evan Phoenix Date: Fri, 25 Sep 2026 17:32:57 +0000 Subject: [PATCH 8/8] Avoid rewriting retired sandboxes after failure --- controllers/sandbox/saga_controller.go | 3 +++ controllers/sandbox/saga_controller_test.go | 14 ++++++++++++++ controllers/sandbox/sandbox.go | 3 +++ controllers/sandbox/sandbox_test.go | 16 ++++++++++++++++ 4 files changed, 36 insertions(+) diff --git a/controllers/sandbox/saga_controller.go b/controllers/sandbox/saga_controller.go index ea1327cbd..575b7ded5 100644 --- a/controllers/sandbox/saga_controller.go +++ b/controllers/sandbox/saga_controller.go @@ -95,6 +95,9 @@ func (c *SandboxController) createSandboxViaSaga(ctx context.Context, co *comput if getErr != nil { c.Log.Warn("failed to fetch sandbox after saga failure; leaving startup outcome unchanged", "id", co.ID, "error", getErr) } else { + if current.Status == compute.DEAD { + return fmt.Errorf("saga sandbox creation failed: %w", err) + } revision = meta.GetRevision() if current.StartupOutcome != compute.STARTUP_RUNNING && current.Status != compute.RUNNING { failure.StartupOutcome = compute.STARTUP_FAILED diff --git a/controllers/sandbox/saga_controller_test.go b/controllers/sandbox/saga_controller_test.go index a8ef2947c..895510606 100644 --- a/controllers/sandbox/saga_controller_test.go +++ b/controllers/sandbox/saga_controller_test.go @@ -46,6 +46,20 @@ func TestSagaFailureMarksDeadWhenFinalFetchFails(t *testing.T) { assert.False(t, outcomeSet, "a failed fetch must not derive an outcome from the stale snapshot") } +func TestSagaFailureDoesNotRewriteDead(t *testing.T) { + h := newTestHarness(t) + h.networking.allocateErr = errors.New("no IPs available") + h.entities.sandbox.Status = compute.DEAD + h.entities.sandbox.StartupOutcome = compute.STARTUP_FAILED + c := &SandboxController{ + Log: slog.Default(), ops: sagaFailureOps{SandboxEntityStore: h.entities}, + executor: h.executor, sagaStorage: h.storage, + } + err := c.createSandboxViaSaga(context.Background(), h.entities.sandbox, false) + require.ErrorContains(t, err, "no IPs available") + require.Empty(t, h.entities.patchCalls, "already-DEAD sandbox must retain its failure timestamp") +} + // newSagaControllerForResume wires up only what sagaResumeNeeded reads // (storage + log), so no live containerd client is needed. func newSagaControllerForResume(t *testing.T) *SandboxController { diff --git a/controllers/sandbox/sandbox.go b/controllers/sandbox/sandbox.go index 615693e31..55dc62c60 100644 --- a/controllers/sandbox/sandbox.go +++ b/controllers/sandbox/sandbox.go @@ -3340,6 +3340,9 @@ func (c *SandboxController) retireSandbox(ctx context.Context, id entity.Id) err } var current compute.Sandbox current.Decode(resp.Entity().Entity()) + if current.Status == compute.DEAD { + return nil + } retired := &compute.Sandbox{Status: compute.DEAD} if current.StartupOutcome == "" { switch current.Status { diff --git a/controllers/sandbox/sandbox_test.go b/controllers/sandbox/sandbox_test.go index e68c05ee5..1777e9df6 100644 --- a/controllers/sandbox/sandbox_test.go +++ b/controllers/sandbox/sandbox_test.go @@ -2007,6 +2007,22 @@ func TestRetireSandboxUsesCurrentLifecycle(t *testing.T) { require.Equal(t, compute.STARTUP_RUNNING, got.StartupOutcome) } +func TestRetireSandboxDoesNotRewriteDead(t *testing.T) { + ctx := context.Background() + server, cleanup := entitytestutils.NewInMemEntityServer(t) + defer cleanup() + id, err := server.Client.Create(ctx, "dead", &compute.Sandbox{Status: compute.DEAD, StartupOutcome: compute.STARTUP_FAILED}) + require.NoError(t, err) + before, err := server.EAC.Get(ctx, id.String()) + require.NoError(t, err) + + c := &SandboxController{EAC: server.EAC} + require.NoError(t, c.retireSandbox(ctx, id)) + after, err := server.EAC.Get(ctx, id.String()) + require.NoError(t, err) + require.Equal(t, before.Entity().Revision(), after.Entity().Revision(), "already-DEAD sandbox must retain its failure timestamp") +} + // A sandbox whose command must execute at most once is finished when its // containers vanish. Rebooting it would re-run the command -- for a migration, // not a recoverable mistake.