diff --git a/docs/architecture/configuration-and-compilation.md b/docs/architecture/configuration-and-compilation.md index 298e7f6620..406e4764da 100644 --- a/docs/architecture/configuration-and-compilation.md +++ b/docs/architecture/configuration-and-compilation.md @@ -115,6 +115,15 @@ reflection and size caches are not state changes and must not be inspected by KRT's reflection-based comparison. Other fields retain their existing equality semantics. +The compilation graph publishes a target only when the revision, its digest, +and the desired ActorTemplate have all been built successfully. Compilation +diagnostics remain separate from runtime preparation failures. When compilation +fails, `desiredRevision` is empty in status and the persisted desired edge is +cleared; an unresolved request is never represented as a runtime digest. The +last successful revision remains available, while abandoned preparations can +be collected and cannot become the latest successful revision through a delayed +completion. + ### WorkerPool sandbox selection For every harness type, the controller resolves `spec.substrate.workerPoolRef` @@ -137,23 +146,33 @@ RuntimeClass. A missing WorkerPool reports `WorkerPoolNotFound`; an unsupported class reports `RevisionInvalid`. Neither produces a desired ActorTemplate. The worker-pool selector is unchanged. WorkerPool updates are tracked through KRT and recompute -the desired revision. Empty and explicit `gvisor` preserve the previous digest -byte-for-byte; `microvm` participates in the digest, so its prepared runtime +the desired revision. Empty and explicit `gvisor` hash the explicit `gvisor` +class; both sandbox classes participate in the digest, so a prepared MicroVM runtime cannot be confused with a gVisor revision. Returning to gVisor restores the original digest. Existing Sessions remain pinned to their revisions. -Unresolved inputs still replace the persisted desired pointer with the requested -identity shown in status, without creating a runtime revision. This releases -abandoned preparations for garbage collection while preserving the current -Agent's last-successful runtime. - Substrate and persistence failures during preparation report `Ready=False` with reason `RuntimePreparationFailed`, rather than remaining silently pending. Status includes a safe error code; a failed precondition also names the expected SandboxConfig and class. Raw backend error messages are not -copied into Kubernetes status. Preparation keeps retrying and clears the failure -after the prerequisite or service recovers; terminal compilation, immutable -template conflict, and golden-snapshot failures are not retried by that poll. +copied into Kubernetes status. The reconciliation queue owns failure retries, +with exponential backoff from one second to thirty seconds and no operational +attempt budget. It clears the failure after the prerequisite or service recovers. +Periodic polling only checks observed templates whose golden snapshots are still +pending. Observation and status updates do not bypass backoff; changed desired +revisions, unresolved inputs, and Agent deletion are reconciled immediately. +Compilation errors, immutable template conflicts, and golden-snapshot failures +stop preparation until the desired inputs change. + +ActorTemplate construction checks the size of each emitted environment value +against Substrate's 32,768-character limit, including the serialized config and +Agent Card. The check counts Unicode code points after JSON encoding, including +its escaping. Oversized values produce `Compatible=False` with reason +`ActorTemplateInvalid` before any Substrate call. A subsequent `InvalidArgument` +from `CreateActorTemplate` is recorded as a terminal `ActorTemplateRejected` +preparation failure. Its backend message is not copied into status because it +can include rejected configuration values. Changing the desired revision allows +preparation to run again. This selection does not by itself guarantee full MicroVM lifecycle or cross-node restore compatibility; those also depend on Substrate, runtime diff --git a/go/core/internal/controller/collections_test.go b/go/core/internal/controller/collections_test.go index 30ac108c97..0a3333fb09 100644 --- a/go/core/internal/controller/collections_test.go +++ b/go/core/internal/controller/collections_test.go @@ -1,18 +1,15 @@ package controller import ( - "context" "testing" "time" atev1alpha1 "github.com/agent-substrate/substrate/pkg/api/v1alpha1" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" kagentv1alpha3 "github.com/kagent-dev/kagent/go/api/v1alpha3" - "github.com/kagent-dev/kagent/go/core/internal/database" v2translator "github.com/kagent-dev/kagent/go/core/internal/translator" "github.com/stretchr/testify/require" "google.golang.org/protobuf/proto" - "istio.io/istio/pkg/kube/controllers" "istio.io/istio/pkg/kube/krt" "istio.io/istio/pkg/kube/krt/krttest" corev1 "k8s.io/api/core/v1" @@ -83,7 +80,7 @@ func TestReconciliationCollectionsCompileAndObserveRevision(t *testing.T) { waitFor(t, func() bool { states := collections.Reconciliations.List() - return len(states) == 1 && states[0].Failure == nil && states[0].DesiredActorTemplate != nil + return len(states) == 1 && states[0].CompilationFailure == nil && states[0].Target != nil }) state := collections.Reconciliations.List()[0] if state.ObservedActorTemplate != nil { @@ -98,7 +95,7 @@ func TestReconciliationCollectionsCompileAndObserveRevision(t *testing.T) { return ready != nil && ready.Status == metav1.ConditionFalse }) - observed := proto.CloneOf(state.DesiredActorTemplate) + observed := proto.CloneOf(state.Target.ActorTemplate) observed.Metadata.Uid = "actor-template-uid" observed.Status = &ateapipb.ActorTemplateStatus{GoldenSnapshotStatus: &ateapipb.GoldenSnapshotStatus{GoldenTag: &ateapipb.ObjectRef{Atespace: "ate-golden", Name: "golden"}}} store := &fakeRuntimeRevisionStore{} @@ -113,59 +110,13 @@ func TestReconciliationCollectionsCompileAndObserveRevision(t *testing.T) { return false } ready := apimeta.FindStatusCondition(updates[0].Status.Conditions, kagentv1alpha3.AgentConditionReady) - return ready != nil && ready.Status == metav1.ConditionTrue && updates[0].Status.LatestSuccessfulRevision == state.RevisionID.String() - }) - - t.Run("deleting revision becomes pending and retries", func(t *testing.T) { - store.pairErr = database.ErrObjectDeleting - require.NoError(t, reconciler.reconcileAgent(t.Context(), state.ResourceName())) - waitFor(t, func() bool { - pending := collections.Reconciliations.GetKey(state.ResourceName()) - updates := collections.AgentStatuses.List() - if pending == nil || pending.ObservedActorTemplate != nil || pending.Failure != nil || len(updates) != 1 { - return false - } - return apimeta.IsStatusConditionFalse(updates[0].Status.Conditions, kagentv1alpha3.AgentConditionReady) - }) - - pollCtx, cancelPoll := context.WithCancel(t.Context()) - queued := make(chan string, 1) - reconciler.agents = controllers.NewQueue("test-deleting-revision", controllers.WithGenericReconciler(func(item any) error { - select { - case queued <- item.(string): - case <-pollCtx.Done(): - } - return nil - })) - go reconciler.agents.Run(pollCtx.Done()) - go reconciler.pollPendingTemplates(pollCtx.Done()) - t.Cleanup(func() { - cancelPoll() - require.NoError(t, reconciler.agents.WaitForClose(time.Second)) - }) - // Retry more than once: the poll must keep working after the KRT event. - for range 2 { - select { - case key := <-queued: - require.Equal(t, state.ResourceName(), key) - require.NoError(t, reconciler.reconcileAgent(t.Context(), key)) - case <-time.After(5 * time.Second): - t.Fatal("pending pair was not requeued while awaiting deletion") - } - } - store.pairErr = nil - require.NoError(t, reconciler.reconcileAgent(t.Context(), state.ResourceName())) - waitFor(t, func() bool { - updates := collections.AgentStatuses.List() - return len(updates) == 1 && true && - apimeta.IsStatusConditionTrue(updates[0].Status.Conditions, kagentv1alpha3.AgentConditionReady) - }) + return ready != nil && ready.Status == metav1.ConditionTrue && updates[0].Status.LatestSuccessfulRevision == state.Target.RevisionID.String() }) modelConfigs.UpdateObject(&kagentv1alpha3.ModelConfig{ObjectMeta: metav1.ObjectMeta{Namespace: "team-a", Name: "model"}, Spec: kagentv1alpha3.ModelConfigSpec{Provider: kagentv1alpha3.ModelProviderOpenAI, Model: "gpt-5.1"}}) waitFor(t, func() bool { states := collections.Reconciliations.List() - return len(states) == 1 && states[0].RevisionID != state.RevisionID && states[0].ObservedActorTemplate == nil + return len(states) == 1 && states[0].Target.RevisionID != state.Target.RevisionID && states[0].ObservedActorTemplate == nil }) } @@ -230,25 +181,25 @@ func TestReconciliationWorkerPoolSandboxClass(t *testing.T) { key := "team-a/assistant" waitFor(t, func() bool { state := reconciliations.GetKey(key) - return state != nil && state.Failure != nil + return state != nil && state.CompilationFailure != nil }) missingPool := &ReconciliationFailure{ Condition: kagentv1alpha3.AgentConditionResolvedRefs, Reason: "WorkerPoolNotFound", Message: `WorkerPool "team-a/selected" not found`, } - require.Equal(t, missingPool, reconciliations.GetKey(key).Failure) - require.Nil(t, reconciliations.GetKey(key).Revision, "missing capacity must fail compilation") + require.Equal(t, missingPool, reconciliations.GetKey(key).CompilationFailure) + require.Nil(t, reconciliations.GetKey(key).Target, "missing capacity must fail compilation") workerPools.UpdateObject(&atev1alpha1.WorkerPool{ ObjectMeta: metav1.ObjectMeta{Namespace: "team-a", Name: "selected"}, }) waitFor(t, func() bool { state := reconciliations.GetKey(key) - return state != nil && state.Failure == nil && state.DesiredActorTemplate != nil && state.Revision.SandboxClass == "" + return state != nil && state.CompilationFailure == nil && state.Target != nil && state.Target.Revision.SandboxClass == "" }) baseline := reconciliations.GetKey(key) - gvisorRevision := baseline.RevisionID + gvisorRevision := baseline.Target.RevisionID require.False(t, gvisorRevision.IsZero()) - gvisorTemplate := proto.CloneOf(baseline.DesiredActorTemplate) + gvisorTemplate := proto.CloneOf(baseline.Target.ActorTemplate) observed := proto.CloneOf(gvisorTemplate) observed.Metadata.Uid = "actor-template-uid" observations.UpdateObject(AgentRuntimeObservation{ @@ -284,14 +235,14 @@ func TestReconciliationWorkerPoolSandboxClass(t *testing.T) { waitFor(t, func() bool { state := reconciliations.GetKey(key) if step.failure != nil { - return state != nil && state.Failure != nil && *state.Failure == *step.failure + return state != nil && state.CompilationFailure != nil && *state.CompilationFailure == *step.failure } - return state != nil && state.Failure == nil && state.DesiredActorTemplate != nil && state.Revision.SandboxClass == step.class + return state != nil && state.CompilationFailure == nil && state.Target != nil && state.Target.Revision.SandboxClass == step.class }) state := reconciliations.GetKey(key) if step.failure != nil { - require.True(t, state.RevisionID.IsZero()) - require.Nil(t, state.DesiredActorTemplate) + require.Nil(t, state.Target) + require.Empty(t, state.desiredRevision()) require.Nil(t, state.ObservedActorTemplate) pairStatus := statusForAgent(*state, 1, gvisorRevision.String()) require.Equal(t, gvisorRevision.String(), pairStatus.LatestSuccessfulRevision) @@ -302,19 +253,19 @@ func TestReconciliationWorkerPoolSandboxClass(t *testing.T) { return } expected := proto.CloneOf(gvisorTemplate) - expected.Metadata.Name = state.DesiredActorTemplate.GetMetadata().GetName() + expected.Metadata.Name = state.Target.ActorTemplate.GetMetadata().GetName() if step.class == atev1alpha1.SandboxClassMicroVM { - require.NotEqual(t, gvisorRevision, state.RevisionID) + require.NotEqual(t, gvisorRevision, state.Target.RevisionID) require.NotEqual(t, gvisorTemplate.GetMetadata().GetName(), expected.Metadata.Name) require.Nil(t, state.ObservedActorTemplate, "a gVisor observation must not satisfy a MicroVM revision") expected.SandboxConfig = &ateapipb.SandboxConfig{SandboxClass: ateapipb.SandboxClass_SANDBOX_CLASS_MICROVM, ConfigName: "microvm"} } else { - require.Equal(t, gvisorRevision, state.RevisionID) + require.Equal(t, gvisorRevision, state.Target.RevisionID) require.Equal(t, gvisorTemplate.GetMetadata().GetName(), expected.Metadata.Name) expected.SandboxConfig = &ateapipb.SandboxConfig{SandboxClass: ateapipb.SandboxClass_SANDBOX_CLASS_GVISOR, ConfigName: "gvisor-default"} } - require.True(t, proto.Equal(expected, state.DesiredActorTemplate), "sandbox selection must preserve the rest of the ActorTemplate") - require.Equal(t, map[string]string{"kagent.dev/worker-pool": "selected"}, state.DesiredActorTemplate.GetWorkerSelector().GetMatchLabels()) + require.True(t, proto.Equal(expected, state.Target.ActorTemplate), "sandbox selection must preserve the rest of the ActorTemplate") + require.Equal(t, map[string]string{"kagent.dev/worker-pool": "selected"}, state.Target.ActorTemplate.GetWorkerSelector().GetMatchLabels()) }) } }) @@ -365,14 +316,14 @@ func TestClaudeReconciliationCompilesActorTemplate(t *testing.T) { ) waitFor(t, func() bool { states := reconciliations.List() - return len(states) == 1 && states[0].Failure == nil && states[0].DesiredActorTemplate != nil + return len(states) == 1 && states[0].CompilationFailure == nil && states[0].Target != nil }) state := reconciliations.List()[0] - if state.Revision == nil || state.Revision.Environment[0].Name != "ANTHROPIC_API_KEY" || state.Revision.Environment[0].Value != v2translator.CredentialPlaceholder { - t.Fatalf("Claude revision environment = %#v", state.Revision) + if state.Target == nil || state.Target.Revision.Environment[0].Name != "ANTHROPIC_API_KEY" || state.Target.Revision.Environment[0].Value != v2translator.CredentialPlaceholder { + t.Fatalf("Claude revision environment = %#v", state.Target.Revision) } - if state.DesiredActorTemplate.GetContainers()[0].GetWakeupProbe().GetHttpGet().GetPort() != 8081 { - t.Fatalf("Claude ActorTemplate readiness = %#v", state.DesiredActorTemplate.GetContainers()[0].GetWakeupProbe()) + if state.Target.ActorTemplate.GetContainers()[0].GetWakeupProbe().GetHttpGet().GetPort() != 8081 { + t.Fatalf("Claude ActorTemplate readiness = %#v", state.Target.ActorTemplate.GetContainers()[0].GetWakeupProbe()) } } @@ -421,14 +372,14 @@ func TestCodexReconciliationCompilesActorTemplate(t *testing.T) { ) waitFor(t, func() bool { states := reconciliations.List() - return len(states) == 1 && states[0].Failure == nil && states[0].DesiredActorTemplate != nil + return len(states) == 1 && states[0].CompilationFailure == nil && states[0].Target != nil }) state := reconciliations.List()[0] - if state.Revision == nil || state.Revision.Environment[0].Name != "OPENAI_API_KEY" || state.Revision.Environment[0].Value != v2translator.CredentialPlaceholder { - t.Fatalf("Codex revision environment = %#v", state.Revision) + if state.Target == nil || state.Target.Revision.Environment[0].Name != "OPENAI_API_KEY" || state.Target.Revision.Environment[0].Value != v2translator.CredentialPlaceholder { + t.Fatalf("Codex revision environment = %#v", state.Target.Revision) } - if state.DesiredActorTemplate.GetContainers()[0].GetWakeupProbe().GetHttpGet().GetPort() != 8081 { - t.Fatalf("Codex ActorTemplate readiness = %#v", state.DesiredActorTemplate.GetContainers()[0].GetWakeupProbe()) + if state.Target.ActorTemplate.GetContainers()[0].GetWakeupProbe().GetHttpGet().GetPort() != 8081 { + t.Fatalf("Codex ActorTemplate readiness = %#v", state.Target.ActorTemplate.GetContainers()[0].GetWakeupProbe()) } } @@ -475,8 +426,8 @@ func TestReconciliationTracksSharedAgentTemplate(t *testing.T) { var initial string waitFor(t, func() bool { for _, state := range reconciliations.List() { - if state.Agent.Name == root.Name && state.Failure == nil { - initial = state.RevisionID.String() + if state.Agent.Name == root.Name && state.CompilationFailure == nil { + initial = state.Target.RevisionID.String() return true } } @@ -488,7 +439,7 @@ func TestReconciliationTracksSharedAgentTemplate(t *testing.T) { waitFor(t, func() bool { for _, state := range reconciliations.List() { if state.Agent.Name == root.Name { - return state.Failure == nil && state.RevisionID.String() != initial + return state.CompilationFailure == nil && state.Target.RevisionID.String() != initial } } return false diff --git a/go/core/internal/controller/equality_test.go b/go/core/internal/controller/equality_test.go index 96ce187d35..813b7f1af2 100644 --- a/go/core/internal/controller/equality_test.go +++ b/go/core/internal/controller/equality_test.go @@ -17,20 +17,21 @@ import ( func equalityTestReconciliation() AgentReconciliation { return AgentReconciliation{ Agent: &kagentv1alpha3.Agent{ObjectMeta: metav1.ObjectMeta{Namespace: "test", Name: "agent"}}, - Revision: &v2translator.Revision{ + Target: &compiledTarget{Revision: v2translator.Revision{ Namespace: "test", AgentName: "agent", AgentCard: &a2apb.AgentCard{Name: "agent"}, }, - RevisionID: v2translator.RevisionID{1}, - Warnings: []string{"warning"}, - DesiredActorTemplate: &ateapipb.ActorTemplate{ - Metadata: &ateapipb.ResourceMetadata{Atespace: "test", Name: "runtime"}, + RevisionID: v2translator.RevisionID{1}, + ActorTemplate: &ateapipb.ActorTemplate{ + Metadata: &ateapipb.ResourceMetadata{Atespace: "test", Name: "runtime"}, + }, }, + Warnings: []string{"warning"}, ObservedActorTemplate: &ateapipb.ActorTemplate{ Metadata: &ateapipb.ResourceMetadata{Atespace: "test", Name: "runtime", Uid: "runtime-uid"}, Status: &ateapipb.ActorTemplateStatus{GoldenSnapshotStatus: &ateapipb.GoldenSnapshotStatus{ErrorMessage: "preparing"}}, }, - Failure: &ReconciliationFailure{Condition: "Ready", Reason: "RuntimePreparationFailed", Message: "retry", Retryable: true}, + PreparationFailure: &ReconciliationFailure{Condition: "Ready", Reason: "RuntimePreparationFailed", Message: "retry", Retryable: true}, } } @@ -43,40 +44,42 @@ func TestAgentReconciliationEquality(t *testing.T) { }{ {name: "identical", change: func(*AgentReconciliation) {}, equal: true}, {name: "protobuf reflection", change: func(r *AgentReconciliation) { - r.Revision.AgentCard.ProtoReflect() - r.DesiredActorTemplate.ProtoReflect() + r.Target.Revision.AgentCard.ProtoReflect() + r.Target.ActorTemplate.ProtoReflect() r.ObservedActorTemplate.ProtoReflect() }, equal: true}, {name: "protobuf caches", change: func(r *AgentReconciliation) { - proto.Size(r.Revision.AgentCard) - proto.Size(r.DesiredActorTemplate) + proto.Size(r.Target.Revision.AgentCard) + proto.Size(r.Target.ActorTemplate) proto.Size(r.ObservedActorTemplate) }, equal: true}, {name: "template source", change: func(r *AgentReconciliation) { r.Agent.Generation++ }}, - {name: "revision identity", change: func(r *AgentReconciliation) { r.RevisionID[0]++ }}, - {name: "revision inputs", change: func(r *AgentReconciliation) { r.Revision.SandboxClass = "microvm" }}, - {name: "agent card", change: func(r *AgentReconciliation) { r.Revision.AgentCard.Name = "changed" }}, - {name: "missing agent card", change: func(r *AgentReconciliation) { r.Revision.AgentCard = nil }}, - {name: "missing revision", change: func(r *AgentReconciliation) { r.Revision = nil }}, + {name: "revision identity", change: func(r *AgentReconciliation) { r.Target.RevisionID[0]++ }}, + {name: "revision inputs", change: func(r *AgentReconciliation) { r.Target.Revision.SandboxClass = "microvm" }}, + {name: "agent card", change: func(r *AgentReconciliation) { r.Target.Revision.AgentCard.Name = "changed" }}, + {name: "missing agent card", change: func(r *AgentReconciliation) { r.Target.Revision.AgentCard = nil }}, + {name: "missing revision", change: func(r *AgentReconciliation) { r.Target = nil }}, {name: "warning", change: func(r *AgentReconciliation) { r.Warnings[0] = "changed" }}, - {name: "desired template", change: func(r *AgentReconciliation) { r.DesiredActorTemplate.Metadata.Name = "changed" }}, + {name: "desired template", change: func(r *AgentReconciliation) { r.Target.ActorTemplate.Metadata.Name = "changed" }}, {name: "observed template identity", change: func(r *AgentReconciliation) { r.ObservedActorTemplate.Metadata.Uid = "changed" }}, {name: "observed template status", change: func(r *AgentReconciliation) { r.ObservedActorTemplate.Status.GoldenSnapshotStatus.ErrorMessage = "changed" }}, - {name: "missing desired template", change: func(r *AgentReconciliation) { r.DesiredActorTemplate = nil }}, {name: "missing observed template", change: func(r *AgentReconciliation) { r.ObservedActorTemplate = nil }}, - {name: "failure", change: func(r *AgentReconciliation) { r.Failure.Message = "changed" }}, - {name: "retryability", change: func(r *AgentReconciliation) { r.Failure.Retryable = false }}, - {name: "failure cleared", change: func(r *AgentReconciliation) { r.Failure = nil }}, + {name: "failure", change: func(r *AgentReconciliation) { r.PreparationFailure.Message = "changed" }}, + {name: "retryability", change: func(r *AgentReconciliation) { r.PreparationFailure.Retryable = false }}, + {name: "failure cleared", change: func(r *AgentReconciliation) { r.PreparationFailure = nil }}, + {name: "compilation failure", change: func(r *AgentReconciliation) { + r.CompilationFailure = &ReconciliationFailure{Reason: "ActorTemplateInvalid"} + }}, } { t.Run(tt.name, func(t *testing.T) { left, right := equalityTestReconciliation(), equalityTestReconciliation() tt.change(&right) require.Equal(t, tt.equal, krt.Equal(left, right)) require.Equal(t, tt.equal, krt.Equal(right, left)) - require.NotNil(t, left.Revision.AgentCard, "comparison must not mutate its inputs") - require.NotNil(t, left.DesiredActorTemplate) + require.NotNil(t, left.Target.Revision.AgentCard, "comparison must not mutate its inputs") + require.NotNil(t, left.Target.ActorTemplate) require.NotNil(t, left.ObservedActorTemplate) }) } @@ -86,7 +89,7 @@ func equalityTestObservation() AgentRuntimeObservation { state := equalityTestReconciliation() return AgentRuntimeObservation{ Namespace: "test", AgentName: "agent", - RevisionID: state.RevisionID, Template: state.ObservedActorTemplate, Failure: state.Failure, + RevisionID: state.Target.RevisionID, Template: state.ObservedActorTemplate, Failure: state.PreparationFailure, } } @@ -130,9 +133,9 @@ func TestPairEqualityDuringProtobufReads(t *testing.T) { wg.Go(func() { <-start for range 10 { - proto.CloneOf(left.DesiredActorTemplate) + proto.CloneOf(left.Target.ActorTemplate) proto.Size(left.ObservedActorTemplate) - proto.Size(left.Revision.AgentCard) + proto.Size(left.Target.Revision.AgentCard) proto.Size(observationLeft.Template) } }) diff --git a/go/core/internal/controller/preparation_test.go b/go/core/internal/controller/preparation_test.go index e3dfb954cc..8b5ddd6b2e 100644 --- a/go/core/internal/controller/preparation_test.go +++ b/go/core/internal/controller/preparation_test.go @@ -3,7 +3,10 @@ package controller import ( "context" "errors" + "strings" + "sync" "testing" + "testing/synctest" "time" atev1alpha1 "github.com/agent-substrate/substrate/pkg/api/v1alpha1" @@ -19,7 +22,6 @@ import ( "google.golang.org/grpc/codes" "google.golang.org/grpc/status" "google.golang.org/protobuf/proto" - "istio.io/istio/pkg/kube/controllers" "istio.io/istio/pkg/kube/krt" "istio.io/istio/pkg/kube/krt/krttest" corev1 "k8s.io/api/core/v1" @@ -46,7 +48,7 @@ func TestUnresolvedPoolReleasesAbandonedRevision(t *testing.T) { store := database.NewClient(pool) collections, harnesses := newPreparationTestCollections(t, "gvisor") initial := collections.Reconciliations.List()[0] - templates := &fakeActorTemplates{template: proto.CloneOf(initial.DesiredActorTemplate)} + templates := &fakeActorTemplates{template: proto.CloneOf(initial.Target.ActorTemplate)} templates.template.Metadata.Uid = "gvisor-uid" templates.template.Status = &ateapipb.ActorTemplateStatus{GoldenSnapshotStatus: &ateapipb.GoldenSnapshotStatus{ GoldenTag: &ateapipb.ObjectRef{Atespace: "ate-golden", Name: "golden"}, @@ -58,7 +60,7 @@ func TestUnresolvedPoolReleasesAbandonedRevision(t *testing.T) { updatedHarness.Spec.Substrate.WorkerPoolRef.Name = "microvm" harnesses.UpdateObject(updatedHarness) waitFor(t, func() bool { - return collections.Reconciliations.GetKey(initial.ResourceName()).RevisionID != initial.RevisionID + return collections.Reconciliations.GetKey(initial.ResourceName()).Target.RevisionID != initial.Target.RevisionID }) preparing := collections.Reconciliations.GetKey(initial.ResourceName()) templates.template = nil @@ -67,7 +69,9 @@ func TestUnresolvedPoolReleasesAbandonedRevision(t *testing.T) { templates.template = proto.CloneOf(templates.template) templates.template.Status = &ateapipb.ActorTemplateStatus{GoldenSnapshotStatus: &ateapipb.GoldenSnapshotStatus{ErrorMessage: "snapshot failed"}} require.NoError(t, reconciler.reconcileAgent(ctx, initial.ResourceName())) - waitFor(t, func() bool { return collections.Reconciliations.GetKey(initial.ResourceName()).Failure != nil }) + waitFor(t, func() bool { + return collections.Reconciliations.GetKey(initial.ResourceName()).PreparationFailure != nil + }) } unreferenced, err := store.ListUnreferencedRuntimeRevisions(ctx) require.NoError(t, err) @@ -78,26 +82,26 @@ func TestUnresolvedPoolReleasesAbandonedRevision(t *testing.T) { harnesses.UpdateObject(updatedHarness) waitFor(t, func() bool { state := collections.Reconciliations.GetKey(initial.ResourceName()) - return state.Failure != nil && state.Failure.Reason == "WorkerPoolNotFound" + return state.CompilationFailure != nil && state.CompilationFailure.Reason == "WorkerPoolNotFound" }) unresolved := collections.Reconciliations.GetKey(initial.ResourceName()) - require.True(t, unresolved.RevisionID.IsZero()) + require.Nil(t, unresolved.Target) for range 2 { require.NoError(t, reconciler.reconcileAgent(ctx, initial.ResourceName())) } require.Empty(t, collections.AgentRuntimeObservations.List()) - require.Nil(t, unresolved.DesiredActorTemplate) - require.Equal(t, preparing.DesiredActorTemplate.GetMetadata().GetName(), templates.template.GetMetadata().GetName(), "unresolved inputs must not create compute") + require.Empty(t, unresolved.desiredRevision()) + require.Equal(t, preparing.Target.ActorTemplate.GetMetadata().GetName(), templates.template.GetMetadata().GetName(), "unresolved inputs must not create compute") // A delayed success for the abandoned preparation must not replace A. - abandoned, err := store.GetRuntimeRevision(ctx, preparing.RevisionID.String()) + abandoned, err := store.GetRuntimeRevision(ctx, preparing.Target.RevisionID.String()) require.NoError(t, err) require.NoError(t, store.RecordRuntimeRevision(ctx, *abandoned, true)) unreferenced, err = store.ListUnreferencedRuntimeRevisions(ctx) require.NoError(t, err) require.Len(t, unreferenced, 1) - require.Equal(t, preparing.RevisionID.String(), unreferenced[0].Revision) - retained, err := store.BeginRuntimeRevisionDeletion(ctx, initial.RevisionID.String()) + require.Equal(t, preparing.Target.RevisionID.String(), unreferenced[0].Revision) + retained, err := store.BeginRuntimeRevisionDeletion(ctx, initial.Target.RevisionID.String()) require.NoError(t, err) require.Nil(t, retained, "last-successful A must remain protected without any sessions") @@ -106,13 +110,13 @@ func TestUnresolvedPoolReleasesAbandonedRevision(t *testing.T) { Agent: &apiv1alpha1.ResourceReference{Namespace: "team-a", Name: "assistant"}, }, "last-good-session") require.NoError(t, err) - require.Equal(t, initial.RevisionID.String(), session.GetPreparedRevision()) + require.Equal(t, initial.Target.RevisionID.String(), session.GetPreparedRevision()) require.NoError(t, NewRuntimeRevisionGC(store, templates).collect(ctx, abandoned.Revision)) require.Nil(t, templates.template) _, err = store.GetRuntimeRevision(ctx, abandoned.Revision) require.ErrorIs(t, err, database.ErrNotFound) - _, err = store.GetRuntimeRevision(ctx, initial.RevisionID.String()) + _, err = store.GetRuntimeRevision(ctx, initial.Target.RevisionID.String()) require.NoError(t, err) updatedHarness = updatedHarness.DeepCopy() @@ -120,7 +124,7 @@ func TestUnresolvedPoolReleasesAbandonedRevision(t *testing.T) { harnesses.UpdateObject(updatedHarness) waitFor(t, func() bool { state := collections.Reconciliations.GetKey(initial.ResourceName()) - return state.Failure == nil && state.RevisionID == preparing.RevisionID + return state.Target != nil && state.PreparationFailure == nil && state.Target.RevisionID == preparing.Target.RevisionID }) require.NoError(t, reconciler.reconcileAgent(ctx, initial.ResourceName()), "restoring valid inputs must prepare the collected revision again") _, err = store.GetRuntimeRevision(ctx, abandoned.Revision) @@ -140,6 +144,7 @@ func TestPreparationErrorsPublishStatusAndRecover(t *testing.T) { {name: "wrong sandbox class", templates: fakeActorTemplates{createErr: status.Error(codes.FailedPrecondition, `SandboxConfig "microvm" has class "gvisor" but sandbox_config.sandbox_class is "microvm"`)}, code: codes.FailedPrecondition}, {name: "sensitive creation error", templates: fakeActorTemplates{createErr: status.Error(codes.FailedPrecondition, "private-backend-detail")}, code: codes.FailedPrecondition}, {name: "get failed", templates: fakeActorTemplates{getErr: status.Error(codes.Unavailable, "private-endpoint")}, code: codes.Unavailable}, + {name: "get invalid argument", templates: fakeActorTemplates{getErr: status.Error(codes.InvalidArgument, "private-endpoint")}, code: codes.InvalidArgument}, {name: "atespace failed", templates: fakeActorTemplates{ensureErr: status.Error(codes.PermissionDenied, "private-credential")}, code: codes.PermissionDenied}, {name: "pair store failed", store: fakeRuntimeRevisionStore{pairErr: errors.New("private-database-connection")}, code: codes.Unknown}, {name: "revision store failed", store: fakeRuntimeRevisionStore{revisionErr: errors.New("private-database-connection")}, code: codes.Unknown}, @@ -154,7 +159,7 @@ func TestPreparationErrorsPublishStatusAndRecover(t *testing.T) { require.Equal(t, test.code, status.Code(err)) waitFor(t, func() bool { state := collections.Reconciliations.GetKey(initial.ResourceName()) - return state.Failure != nil && state.Failure.Retryable + return state.PreparationFailure != nil && state.PreparationFailure.Retryable }) waitFor(t, func() bool { updates := collections.AgentStatuses.List() @@ -178,7 +183,7 @@ func TestPreparationErrorsPublishStatusAndRecover(t *testing.T) { } observation := collections.AgentRuntimeObservations.GetKey(initial.ResourceName()) require.Nil(t, observation.Template) - require.Equal(t, initial.RevisionID, observation.RevisionID) + require.Equal(t, initial.Target.RevisionID, observation.RevisionID) require.Error(t, reconciler.reconcileAgent(t.Context(), initial.ResourceName()), "publishing a failure must not prevent the next retry") test.templates.ensureErr, test.templates.getErr, test.templates.createErr = nil, nil, nil @@ -186,7 +191,7 @@ func TestPreparationErrorsPublishStatusAndRecover(t *testing.T) { require.NoError(t, reconciler.reconcileAgent(t.Context(), initial.ResourceName())) waitFor(t, func() bool { state := collections.Reconciliations.GetKey(initial.ResourceName()) - return state.Failure == nil && state.ObservedActorTemplate != nil + return state.PreparationFailure == nil && state.ObservedActorTemplate != nil }) require.Nil(t, collections.AgentRuntimeObservations.GetKey(initial.ResourceName()).Failure) test.templates.template = proto.CloneOf(test.templates.template) @@ -197,7 +202,7 @@ func TestPreparationErrorsPublishStatusAndRecover(t *testing.T) { waitFor(t, func() bool { harnessStatus := collections.AgentStatuses.List()[0].Status return apimeta.IsStatusConditionTrue(harnessStatus.Conditions, kagentv1alpha3.AgentConditionReady) && - harnessStatus.LatestSuccessfulRevision == initial.RevisionID.String() + harnessStatus.LatestSuccessfulRevision == initial.Target.RevisionID.String() }) }) } @@ -225,59 +230,304 @@ func TestRuntimePreparationFailureSanitizesErrors(t *testing.T) { } } -func TestPreparationFailurePollingAndRevisionIsolation(t *testing.T) { +func TestPreparationRevisionIsolation(t *testing.T) { collections, harnesses := newPreparationTestCollections(t, "microvm") initial := collections.Reconciliations.List()[0] templates := &fakeActorTemplates{createErr: status.Error(codes.FailedPrecondition, `SandboxConfig "microvm" not found`)} reconciler := &Reconciler{collections: collections, templates: templates, store: &fakeRuntimeRevisionStore{}} require.Error(t, reconciler.reconcileAgent(t.Context(), initial.ResourceName())) - waitFor(t, func() bool { return collections.Reconciliations.GetKey(initial.ResourceName()).Failure != nil }) - - pollCtx, cancel := context.WithCancel(t.Context()) - defer cancel() - queued := make(chan string, 1) - reconciler.agents = controllers.NewQueue("test-preparation-failure", controllers.WithGenericReconciler(func(item any) error { - select { - case queued <- item.(string): - case <-pollCtx.Done(): - } - return nil - })) - go reconciler.agents.Run(pollCtx.Done()) - go reconciler.pollPendingTemplates(pollCtx.Done()) - t.Cleanup(func() { - cancel() - require.NoError(t, reconciler.agents.WaitForClose(time.Second)) + waitFor(t, func() bool { + return collections.Reconciliations.GetKey(initial.ResourceName()).PreparationFailure != nil }) - for range 2 { - select { - case key := <-queued: - require.Equal(t, initial.ResourceName(), key) - require.Error(t, reconciler.reconcileAgent(t.Context(), key)) - case <-time.After(5 * time.Second): - t.Fatal("failed preparation was not requeued after its graph event") - } - } - cancel() updatedHarness := harnesses.List()[0].DeepCopy() updatedHarness.Spec.Workload.Args = []string{"changed"} harnesses.UpdateObject(updatedHarness) waitFor(t, func() bool { state := collections.Reconciliations.GetKey(initial.ResourceName()) - return state.RevisionID != initial.RevisionID && state.Failure == nil + return state.Target.RevisionID != initial.Target.RevisionID && state.PreparationFailure == nil }) templates.createErr = nil require.NoError(t, reconciler.reconcileAgent(t.Context(), initial.ResourceName())) observation := collections.AgentRuntimeObservations.GetKey(initial.ResourceName()) - require.NotEqual(t, initial.RevisionID, observation.RevisionID) + require.NotEqual(t, initial.Target.RevisionID, observation.RevisionID) require.Nil(t, observation.Failure, "a new revision must not inherit an older preparation failure") harnesses.DeleteObject("team-a/byo") - waitFor(t, func() bool { return collections.Reconciliations.GetKey(initial.ResourceName()).Failure != nil }) + waitFor(t, func() bool { + return collections.Reconciliations.GetKey(initial.ResourceName()).CompilationFailure != nil + }) require.NoError(t, reconciler.reconcileAgent(t.Context(), initial.ResourceName())) require.Empty(t, collections.AgentRuntimeObservations.List()) } +func TestPreparationQueueAndPollerBackoff(t *testing.T) { + for _, test := range []struct { + name string + templates fakeActorTemplates + store fakeRuntimeRevisionStore + }{ + {name: "missing sandbox config", templates: fakeActorTemplates{createErr: status.Error(codes.FailedPrecondition, "missing config")}}, + {name: "revision being collected", store: fakeRuntimeRevisionStore{pairErr: database.ErrObjectDeleting}}, + {name: "persistence unavailable", store: fakeRuntimeRevisionStore{revisionErr: errors.New("database unavailable")}}, + } { + t.Run(test.name, func(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + collections, _ := newPreparationTestCollections(t, "microvm") + initial := collections.Reconciliations.List()[0] + statusClient := kagentfake.NewSimpleClientset(initial.Agent.DeepCopy()).ApiV1alpha3() + backend := &preparationTestBackend{templates: &test.templates, store: &test.store} + r := newReconciler(collections, backend, backend, statusClient) + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + go r.Run(ctx.Done()) + synctest.Wait() + backend.inspect(func() { + require.Equal(t, 1, test.store.pairCalls, "publishing a failure must not immediately requeue it") + }) + time.Sleep(10 * time.Second) + synctest.Wait() + backend.inspect(func() { + require.Equal(t, 4, test.store.pairCalls, "only exponential backoff may retry failures") + }) + time.Sleep(5 * time.Minute) + synctest.Wait() + var before int + backend.inspect(func() { + require.Greater(t, test.store.pairCalls, 10, "retries must survive the former attempt budget") + before = test.store.pairCalls + }) + time.Sleep(30 * time.Second) + synctest.Wait() + backend.inspect(func() { + require.Equal(t, before+1, test.store.pairCalls, "the poller must not bypass capped backoff") + }) + failed := collections.Reconciliations.GetKey(initial.ResourceName()) + require.Nil(t, failed.CompilationFailure) + require.NotNil(t, failed.Target) + require.NotNil(t, failed.PreparationFailure) + + // Recover without changing any Kubernetes input or manually queuing work. + backend.inspect(func() { + test.templates.createErr = nil + test.store.pairErr, test.store.revisionErr = nil, nil + }) + time.Sleep(30 * time.Second) + synctest.Wait() + require.Nil(t, collections.Reconciliations.GetKey(initial.ResourceName()).PreparationFailure) + backend.inspect(func() { + require.NotNil(t, test.templates.template) + require.False(t, test.store.markedSuccessful) + test.templates.template = proto.CloneOf(test.templates.template) + test.templates.template.Status = &ateapipb.ActorTemplateStatus{GoldenSnapshotStatus: &ateapipb.GoldenSnapshotStatus{ + GoldenTag: &ateapipb.ObjectRef{Atespace: "ate-golden", Name: "golden"}, + }} + }) + time.Sleep(time.Second) + synctest.Wait() + backend.inspect(func() { + require.True(t, test.store.markedSuccessful, "polling must still observe pending golden snapshots") + before = test.store.pairCalls + }) + published, err := statusClient.Agents(initial.Agent.Namespace).Get(t.Context(), initial.Agent.Name, metav1.GetOptions{}) + require.NoError(t, err) + require.True(t, apimeta.IsStatusConditionTrue(published.Status.Conditions, kagentv1alpha3.AgentConditionReady)) + time.Sleep(time.Minute) + synctest.Wait() + backend.inspect(func() { + require.Equal(t, before, test.store.pairCalls, "a ready revision needs no more polling") + }) + cancel() + synctest.Wait() + require.NoError(t, r.agents.WaitForClose(time.Second)) + }) + }) + } +} + +// Waiting for quiescence synchronizes worker writes with test reads, but test +// mutations also need synchronization with the next timer-driven attempt. +type preparationTestBackend struct { + mu sync.Mutex + templates *fakeActorTemplates + store *fakeRuntimeRevisionStore +} + +var _ actorTemplateClient = (*preparationTestBackend)(nil) +var _ runtimeRevisionStore = (*preparationTestBackend)(nil) + +func (p *preparationTestBackend) inspect(f func()) { + p.mu.Lock() + defer p.mu.Unlock() + f() +} + +func (p *preparationTestBackend) EnsureAtespace(ctx context.Context, atespace string) error { + p.mu.Lock() + defer p.mu.Unlock() + return p.templates.EnsureAtespace(ctx, atespace) +} + +func (p *preparationTestBackend) GetActorTemplate(ctx context.Context, atespace, name string) (*ateapipb.ActorTemplate, error) { + p.mu.Lock() + defer p.mu.Unlock() + return p.templates.GetActorTemplate(ctx, atespace, name) +} + +func (p *preparationTestBackend) CreateActorTemplate(ctx context.Context, template *ateapipb.ActorTemplate) (*ateapipb.ActorTemplate, error) { + p.mu.Lock() + defer p.mu.Unlock() + return p.templates.CreateActorTemplate(ctx, template) +} + +func (p *preparationTestBackend) UpsertAgentDefinition(ctx context.Context, definition database.AgentDefinition) error { + p.mu.Lock() + defer p.mu.Unlock() + return p.store.UpsertAgentDefinition(ctx, definition) +} + +func (p *preparationTestBackend) RecordRuntimeRevision(ctx context.Context, revision database.RuntimeRevision, ready bool) error { + p.mu.Lock() + defer p.mu.Unlock() + return p.store.RecordRuntimeRevision(ctx, revision, ready) +} + +func (p *preparationTestBackend) RetireAgentIdentities(ctx context.Context, namespace, name string, except *database.AgentDefinition) error { + p.mu.Lock() + defer p.mu.Unlock() + return p.store.RetireAgentIdentities(ctx, namespace, name, except) +} + +func TestPreparationDesiredChangesBypassBackoff(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + collections, harnesses := newPreparationTestCollections(t, "microvm") + initial := collections.Reconciliations.List()[0] + templates := &fakeActorTemplates{createErr: status.Error(codes.FailedPrecondition, "missing config")} + store := &fakeRuntimeRevisionStore{} + r := newReconciler(collections, templates, store, kagentfake.NewSimpleClientset(initial.Agent.DeepCopy()).ApiV1alpha3()) + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + go r.Run(ctx.Done()) + synctest.Wait() + time.Sleep(10 * time.Second) + synctest.Wait() + require.Equal(t, 4, store.pairCalls) + published, err := r.status.Agents(initial.Agent.Namespace).Get(t.Context(), initial.Agent.Name, metav1.GetOptions{}) + require.NoError(t, err) + agents := collections.Agents.(krt.StaticCollection[*kagentv1alpha3.Agent]) + agents.UpdateObject(published) + synctest.Wait() + require.Equal(t, 4, store.pairCalls, "status feedback must not bypass backoff") + templates.createErr = nil + updated := harnesses.List()[0].DeepCopy() + updated.Spec.Workload.Args = []string{"changed"} + harnesses.UpdateObject(updated) + synctest.Wait() + require.Equal(t, 5, store.pairCalls, "a new desired revision must prepare immediately") + require.NotEqual(t, initial.Target.RevisionID.String(), store.pair.DesiredRevision) + require.Nil(t, collections.Reconciliations.GetKey(initial.ResourceName()).PreparationFailure) + + harnesses.DeleteObject("team-a/byo") + synctest.Wait() + require.Empty(t, store.pair.DesiredRevision, "unresolved inputs must clear the desired runtime immediately") + require.Empty(t, collections.AgentRuntimeObservations.List()) + agents.DeleteObject(initial.ResourceName()) + synctest.Wait() + require.Equal(t, initial.ResourceName(), store.retired, "deletion must retire the Agent immediately") + cancel() + synctest.Wait() + }) +} + +func TestPreparationTerminalFailures(t *testing.T) { + for _, reason := range []string{"ActorTemplateRejected", "ActorTemplateConflict", "ActorTemplateFailed"} { + t.Run(reason, func(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + collections, harnesses := newPreparationTestCollections(t, "microvm") + initial := collections.Reconciliations.List()[0] + templates := &fakeActorTemplates{} + switch reason { + case "ActorTemplateRejected": + templates.createErr = status.Error(codes.InvalidArgument, "private-config-value") + case "ActorTemplateConflict": + templates.template = proto.CloneOf(initial.Target.ActorTemplate) + templates.template.Containers[0].Args = []string{"unexpected"} + case "ActorTemplateFailed": + templates.template = proto.CloneOf(initial.Target.ActorTemplate) + templates.template.Status = &ateapipb.ActorTemplateStatus{GoldenSnapshotStatus: &ateapipb.GoldenSnapshotStatus{ErrorMessage: "snapshot failed"}} + } + store := &fakeRuntimeRevisionStore{} + statusClient := kagentfake.NewSimpleClientset(initial.Agent.DeepCopy()).ApiV1alpha3() + r := newReconciler(collections, templates, store, statusClient) + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + go r.Run(ctx.Done()) + synctest.Wait() + require.Equal(t, 1, store.pairCalls) + state := collections.Reconciliations.GetKey(initial.ResourceName()) + require.Nil(t, state.CompilationFailure) + require.NotNil(t, state.Target) + require.Equal(t, reason, state.PreparationFailure.Reason) + published, err := statusClient.Agents(initial.Agent.Namespace).Get(t.Context(), initial.Agent.Name, metav1.GetOptions{}) + require.NoError(t, err) + ready := apimeta.FindStatusCondition(published.Status.Conditions, kagentv1alpha3.AgentConditionReady) + require.Equal(t, reason, ready.Reason) + require.NotContains(t, ready.Message, "private-config-value") + time.Sleep(5 * time.Minute) + synctest.Wait() + require.Equal(t, 1, store.pairCalls, "neither the queue nor the poller may retry terminal failures") + + store.pairErr = errors.New("database unavailable") + require.ErrorIs(t, r.reconcileAgent(t.Context(), initial.ResourceName()), store.pairErr) + synctest.Wait() + require.Equal(t, state.PreparationFailure, collections.Reconciliations.GetKey(initial.ResourceName()).PreparationFailure, + "a storage failure must preserve the terminal preparation diagnostic") + store.pairErr = nil + templates.getErr = errors.New("Substrate must not be called after a terminal failure") + require.NoError(t, r.reconcileAgent(t.Context(), initial.ResourceName())) + templates.getErr = nil + + templates.createErr, templates.template = nil, nil + updated := harnesses.List()[0].DeepCopy() + updated.Spec.Workload.Args = []string{"changed"} + harnesses.UpdateObject(updated) + synctest.Wait() + require.Equal(t, 4, store.pairCalls) + require.NotNil(t, templates.template, "a changed revision must prepare without the old failure") + require.Nil(t, collections.Reconciliations.GetKey(initial.ResourceName()).PreparationFailure) + cancel() + synctest.Wait() + }) + }) + } +} + +func TestInvalidActorTemplateHasNoCompiledTarget(t *testing.T) { + collections, harnesses := newPreparationTestCollections(t, "microvm") + initial := collections.Reconciliations.List()[0] + updated := harnesses.List()[0].DeepCopy() + updated.Spec.Env = []kagentv1alpha3.RuntimeEnvVar{{Name: "EXTRA", Value: new(strings.Repeat("x", 32769))}} + harnesses.UpdateObject(updated) + waitFor(t, func() bool { + return collections.Reconciliations.GetKey(initial.ResourceName()).CompilationFailure != nil + }) + state := collections.Reconciliations.GetKey(initial.ResourceName()) + require.Equal(t, "ActorTemplateInvalid", state.CompilationFailure.Reason) + require.Nil(t, state.Target, "a revision and digest must not be published when ActorTemplate construction fails") + require.Nil(t, state.PreparationFailure) + store := &fakeRuntimeRevisionStore{} + templates := &fakeActorTemplates{getErr: errors.New("Substrate must not be called for an invalid target")} + r := &Reconciler{collections: collections, templates: templates, store: store} + require.NoError(t, r.reconcileAgent(t.Context(), initial.ResourceName())) + require.Empty(t, store.pair.DesiredRevision) + require.Nil(t, templates.template) + require.Nil(t, store.revision) + agentStatus := statusForAgent(*state, 1, initial.Target.RevisionID.String()) + require.Empty(t, agentStatus.DesiredRevision) + require.Equal(t, initial.Target.RevisionID.String(), agentStatus.LatestSuccessfulRevision) + compatible := apimeta.FindStatusCondition(agentStatus.Conditions, kagentv1alpha3.AgentConditionCompatible) + require.Equal(t, "ActorTemplateInvalid", compatible.Reason) +} + func newPreparationTestCollections(t *testing.T, workerPool string) (Collections, krt.StaticCollection[*kagentv1alpha3.Harness]) { t.Helper() opts := krt.NewOptionsBuilder(t.Context().Done(), "test-preparation", nil) @@ -305,6 +555,7 @@ func newPreparationTestCollections(t *testing.T, workerPool string) (Collections Secrets: krttest.GetMockCollection[*corev1.Secret](mock), WorkerPools: krttest.GetMockCollection[*atev1alpha1.WorkerPool](mock), AgentRuntimeObservations: krt.NewStaticCollection[AgentRuntimeObservation](nil, nil, opts.WithName("AgentRuntimeObservations")...), + ModelConfigStatuses: krttest.GetMockCollection[krt.ObjectWithStatus[*kagentv1alpha3.ModelConfig, kagentv1alpha3.ModelConfigStatus]](mock), } collections.Agents = krt.NewStaticCollection(nil, []*kagentv1alpha3.Agent{testAgent(template, runtimeHarness)}, opts.WithName("Agents")...) collections.Reconciliations = newAgentReconciliations(collections.Agents, v2translator.Collections{ @@ -315,7 +566,7 @@ func newPreparationTestCollections(t *testing.T, workerPool string) (Collections collections.AgentStatuses = newAgentStatuses(collections.Agents, collections.Reconciliations, opts) waitFor(t, func() bool { states := collections.Reconciliations.List() - return len(states) == 1 && states[0].Failure == nil && states[0].DesiredActorTemplate != nil + return len(states) == 1 && states[0].CompilationFailure == nil && states[0].Target != nil }) return collections, harnesses } diff --git a/go/core/internal/controller/reconciler.go b/go/core/internal/controller/reconciler.go index 3738c63311..1b8ef3e33f 100644 --- a/go/core/internal/controller/reconciler.go +++ b/go/core/internal/controller/reconciler.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "math" "reflect" "strings" "time" @@ -32,16 +33,31 @@ import ( ) // AgentReconciliation is the complete desired and observed state for one -// Agent. Failure is data so invalid Agents still produce +// Agent. Compilation failures are data so invalid Agents still produce // status instead of disappearing from the graph. type AgentReconciliation struct { Agent *kagentv1alpha3.Agent - Revision *v2translator.Revision + Target *compiledTarget Warnings []string - RevisionID v2translator.RevisionID - DesiredActorTemplate *ateapipb.ActorTemplate + CompilationFailure *ReconciliationFailure ObservedActorTemplate *ateapipb.ActorTemplate - Failure *ReconciliationFailure + PreparationFailure *ReconciliationFailure +} + +// compiledTarget is published only after compilation, hashing, and ActorTemplate +// construction all succeed. An absent target means there is no desired runtime. +type compiledTarget struct { + Revision v2translator.Revision + RevisionID v2translator.RevisionID + ActorTemplate *ateapipb.ActorTemplate +} + +func (c *compiledTarget) equals(other *compiledTarget) bool { + if c == nil || other == nil { + return c == other + } + return c.RevisionID == other.RevisionID && c.Revision.Equals(other.Revision) && + proto.Equal(c.ActorTemplate, other.ActorTemplate) } func (r AgentReconciliation) ResourceName() string { return r.Agent.Namespace + "/" + r.Agent.Name } @@ -50,28 +66,20 @@ var _ krt.Equaler[AgentReconciliation] = AgentReconciliation{} // Equals keeps KRT from reflecting over protobuf caches that mutate during reads. func (r AgentReconciliation) Equals(other AgentReconciliation) bool { - if (r.Revision == nil) != (other.Revision == nil) || - (r.Revision != nil && !r.Revision.Equals(*other.Revision)) || - !proto.Equal(r.DesiredActorTemplate, other.DesiredActorTemplate) || + if !r.Target.equals(other.Target) || !proto.Equal(r.ObservedActorTemplate, other.ObservedActorTemplate) { return false } - r.Revision, other.Revision = nil, nil - r.DesiredActorTemplate, other.DesiredActorTemplate = nil, nil + r.Target, other.Target = nil, nil r.ObservedActorTemplate, other.ObservedActorTemplate = nil, nil return reflect.DeepEqual(r, other) } func (r AgentReconciliation) desiredRevision() string { - if r.Revision == nil || r.RevisionID.IsZero() { - return requestedRevision(r.Agent) + if r.Target == nil { + return "" } - return r.RevisionID.String() -} - -func (r AgentReconciliation) canPrepare() bool { - return r.Revision != nil && !r.RevisionID.IsZero() && r.DesiredActorTemplate != nil && - (r.Failure == nil || r.Failure.Retryable) + return r.Target.RevisionID.String() } // ReconciliationFailure identifies the condition stage blocked by an Agent. @@ -106,41 +114,41 @@ func newAgentReconciliations( case errors.As(err, &missingPool): reason = "WorkerPoolNotFound" } - state.Failure = &ReconciliationFailure{Condition: condition, Reason: reason, Message: err.Error()} + state.CompilationFailure = &ReconciliationFailure{Condition: condition, Reason: reason, Message: err.Error()} return state } revision := &compilation.Revision - state.Revision = revision state.Warnings = append([]string(nil), compilation.Warnings...) - state.RevisionID, err = revision.Digest() + revisionID, err := revision.Digest() if err != nil { - state.Failure = &ReconciliationFailure{Condition: kagentv1alpha3.AgentConditionCompatible, Reason: "RevisionInvalid", Message: err.Error()} + state.CompilationFailure = &ReconciliationFailure{Condition: kagentv1alpha3.AgentConditionCompatible, Reason: "RevisionInvalid", Message: err.Error()} return state } - state.DesiredActorTemplate, err = substrate.ActorTemplateForRevision(revision, state.RevisionID) + actorTemplate, err := substrate.ActorTemplateForRevision(revision, revisionID) if err != nil { - state.Failure = &ReconciliationFailure{Condition: kagentv1alpha3.AgentConditionCompatible, Reason: "ActorTemplateInvalid", Message: err.Error()} + state.CompilationFailure = &ReconciliationFailure{Condition: kagentv1alpha3.AgentConditionCompatible, Reason: "ActorTemplateInvalid", Message: err.Error()} return state } + state.Target = &compiledTarget{Revision: *revision, RevisionID: revisionID, ActorTemplate: actorTemplate} observed := krt.FetchOne(ctx, agentRuntimeObservations, krt.FilterKey(state.ResourceName())) - if observed == nil || observed.RevisionID != state.RevisionID { + if observed == nil || observed.RevisionID != state.Target.RevisionID { return state } if observed.Failure != nil { - state.Failure = observed.Failure + state.PreparationFailure = observed.Failure return state } state.ObservedActorTemplate = (*observed).Template - if !substrate.ActorTemplateSpecEqual(state.ObservedActorTemplate, state.DesiredActorTemplate) { - state.Failure = &ReconciliationFailure{ + if !substrate.ActorTemplateSpecEqual(state.ObservedActorTemplate, state.Target.ActorTemplate) { + state.PreparationFailure = &ReconciliationFailure{ Condition: kagentv1alpha3.AgentConditionReady, Reason: "ActorTemplateConflict", Message: "existing immutable ActorTemplate differs from the compiled revision", } } else if message := state.ObservedActorTemplate.GetStatus().GetGoldenSnapshotStatus().GetErrorMessage(); message != "" { - state.Failure = &ReconciliationFailure{Condition: kagentv1alpha3.AgentConditionReady, Reason: "ActorTemplateFailed", Message: message} + state.PreparationFailure = &ReconciliationFailure{Condition: kagentv1alpha3.AgentConditionReady, Reason: "ActorTemplateFailed", Message: message} } return state }, opts.WithName("AgentReconciliations")...) @@ -210,6 +218,12 @@ func newReconciler( }) r.agentHandler = collections.Reconciliations.Register(func(event krt.Event[AgentReconciliation]) { + // Observations and status writes must not bypass failure backoff. Only + // a changed desired runtime or Agent identity needs immediate work. + if event.Old != nil && event.New != nil && event.Old.Agent.UID == event.New.Agent.UID && + event.Old.desiredRevision() == event.New.desiredRevision() { + return + } r.agents.Add(krt.GetKey(event.Latest())) }) r.agentStatusHandler = collections.AgentStatuses.Register(func(event krt.Event[krt.ObjectWithStatus[*kagentv1alpha3.Agent, kagentv1alpha3.AgentStatus]]) { @@ -229,11 +243,12 @@ func newReconciler( return r } -// Ten attempts span about 2.5 minutes. Each queue owns its backoff state; -// fresh graph events can enqueue work again after an error budget is exhausted. +// Keep retrying at a capped interval so prerequisites can recover without a +// Kubernetes event. Istio's queue requires a positive attempt limit; MaxInt +// makes that limit unreachable during a controller's lifetime. func newReconciliationQueue(name string, reconcile func(any) error) controllers.Queue { return controllers.NewQueue(name, controllers.WithGenericReconciler(reconcile), - controllers.WithMaxAttempts(10), + controllers.WithMaxAttempts(math.MaxInt), controllers.WithRateLimiter(workqueue.NewTypedItemExponentialFailureRateLimiter[any](time.Second, 30*time.Second)), ) } @@ -263,7 +278,7 @@ func (r *Reconciler) pollPendingTemplates(stop <-chan struct{}) { case <-ticker.C: for _, state := range r.collections.Reconciliations.List() { golden := state.ObservedActorTemplate.GetStatus().GetGoldenSnapshotStatus() - if state.canPrepare() && golden.GetGoldenTag() == nil { + if state.Target != nil && state.PreparationFailure == nil && state.ObservedActorTemplate != nil && golden.GetGoldenTag() == nil { r.agents.Add(state.ResourceName()) } } @@ -281,7 +296,7 @@ func (r *Reconciler) NeedLeaderElection() bool { return true } func (r *Reconciler) reconcileAgent(ctx context.Context, key string) error { state := r.collections.Reconciliations.GetKey(key) if observation := r.collections.AgentRuntimeObservations.GetKey(key); observation != nil && - (state == nil || state.Revision == nil || observation.RevisionID != state.RevisionID) { + (state == nil || state.Target == nil || observation.RevisionID != state.Target.RevisionID) { r.collections.AgentRuntimeObservations.DeleteObject(key) } if state == nil { @@ -300,29 +315,36 @@ func (r *Reconciler) reconcileAgent(ctx context.Context, key string) error { } // Store the desired edge before creating compute so a concurrent collector - // cannot mistake the revision for abandoned state. Unresolved inputs replace - // the old desired edge with their requested identity, not a runtime revision; + // cannot mistake the revision for abandoned state. Unresolved inputs clear + // the old desired edge without inventing a runtime revision; // the upsert preserves the current UID's last-good runtime in either case. if err := r.store.UpsertAgentDefinition(ctx, definition); err != nil { - if errors.Is(err, database.ErrObjectDeleting) { - // A desired digest may be awaiting cleanup from an earlier identity. - // Clearing the observation makes KRT derive a pending Agent, which - // the pending-template poll retries until GC finishes. - r.collections.AgentRuntimeObservations.DeleteObject(key) - return nil - } return r.observePreparationError(*state, fmt.Errorf("store Agent %s: %w", key, err)) } - if !state.canPrepare() { + if state.Target == nil { + return nil + } + if state.PreparationFailure != nil && !state.PreparationFailure.Retryable { return nil } - desiredRef := state.DesiredActorTemplate.GetMetadata() + target := state.Target + desiredRef := target.ActorTemplate.GetMetadata() observed, err := r.templates.GetActorTemplate(ctx, desiredRef.GetAtespace(), desiredRef.GetName()) if status.Code(err) == codes.NotFound { if err := r.templates.EnsureAtespace(ctx, desiredRef.GetAtespace()); err != nil { return r.observePreparationError(*state, fmt.Errorf("ensure Atespace %s: %w", desiredRef.GetAtespace(), err)) } - observed, err = r.templates.CreateActorTemplate(ctx, state.DesiredActorTemplate) + observed, err = r.templates.CreateActorTemplate(ctx, target.ActorTemplate) + if status.Code(err) == codes.InvalidArgument { + // Validation rejects the immutable request, so retries cannot repair + // it. Backend messages may quote configuration; keep status safe. + r.observePreparation(*state, nil, &ReconciliationFailure{ + Condition: kagentv1alpha3.AgentConditionReady, + Reason: "ActorTemplateRejected", + Message: "Substrate rejected the compiled ActorTemplate (InvalidArgument); verify the Agent configuration against Substrate's validation requirements", + }) + return nil + } if status.Code(err) == codes.AlreadyExists { observed, err = r.templates.GetActorTemplate(ctx, desiredRef.GetAtespace(), desiredRef.GetName()) } @@ -330,22 +352,22 @@ func (r *Reconciler) reconcileAgent(ctx context.Context, key string) error { if err != nil { return r.observePreparationError(*state, fmt.Errorf("reconcile ActorTemplate %s/%s: %w", desiredRef.GetAtespace(), desiredRef.GetName(), err)) } - if !substrate.ActorTemplateSpecEqual(observed, state.DesiredActorTemplate) { + if !substrate.ActorTemplateSpecEqual(observed, target.ActorTemplate) { r.observePreparation(*state, observed, nil) return nil } revision := database.RuntimeRevision{ - Revision: state.RevisionID.String(), Namespace: definition.Namespace, + Revision: target.RevisionID.String(), Namespace: definition.Namespace, AgentName: definition.AgentName, AgentUID: definition.AgentUID, - SourceSnapshot: state.Revision.Provenance, AgentCard: state.Revision.AgentCard, - EgressDestinations: state.Revision.EgressDestinations, - Credentials: state.Revision.Credentials, + SourceSnapshot: target.Revision.Provenance, AgentCard: target.Revision.AgentCard, + EgressDestinations: target.Revision.EgressDestinations, + Credentials: target.Revision.Credentials, ActorTemplateAtespace: observed.GetMetadata().GetAtespace(), ActorTemplateName: observed.GetMetadata().GetName(), ActorTemplateUID: observed.GetMetadata().GetUid(), } ready := observed.GetStatus().GetGoldenSnapshotStatus().GetGoldenTag() != nil if err := r.store.RecordRuntimeRevision(ctx, revision, ready); err != nil { - return r.observePreparationError(*state, fmt.Errorf("store runtime revision %s: %w", state.RevisionID, err)) + return r.observePreparationError(*state, fmt.Errorf("store runtime revision %s: %w", target.RevisionID, err)) } // This observation drives Kubernetes Ready status on a separate queue. // Publish it only after session creation can select the persisted revision. @@ -354,9 +376,14 @@ func (r *Reconciler) reconcileAgent(ctx context.Context, key string) error { } func (r *Reconciler) observePreparationError(state AgentReconciliation, err error) error { - if state.canPrepare() { - r.observePreparation(state, nil, runtimePreparationFailure(err, state.DesiredActorTemplate.GetSandboxConfig().GetConfigName(), state.Revision.SandboxClass)) + if state.Target == nil { + return err + } + // Preserve terminal diagnostics if persisting the desired edge fails. + if state.PreparationFailure != nil && !state.PreparationFailure.Retryable { + return err } + r.observePreparation(state, nil, runtimePreparationFailure(err, state.Target.ActorTemplate.GetSandboxConfig().GetConfigName(), state.Target.Revision.SandboxClass)) return err } @@ -384,7 +411,7 @@ func runtimePreparationFailure(err error, configName string, class atev1alpha1.S func (r *Reconciler) observePreparation(state AgentReconciliation, template *ateapipb.ActorTemplate, failure *ReconciliationFailure) { r.collections.AgentRuntimeObservations.ConditionalUpdateObject(AgentRuntimeObservation{ Namespace: state.Agent.Namespace, AgentName: state.Agent.Name, - RevisionID: state.RevisionID, + RevisionID: state.Target.RevisionID, Template: template, Failure: failure, }) diff --git a/go/core/internal/controller/reconciler_test.go b/go/core/internal/controller/reconciler_test.go index 62e5d35bd4..65bd3fecda 100644 --- a/go/core/internal/controller/reconciler_test.go +++ b/go/core/internal/controller/reconciler_test.go @@ -42,8 +42,8 @@ func TestReconcilerPersistsPairInOrder(t *testing.T) { t.Fatal(err) } state := AgentReconciliation{ - Agent: template, - Revision: revision, RevisionID: revisionID, DesiredActorTemplate: desiredActor, + Agent: template, + Target: &compiledTarget{Revision: *revision, RevisionID: revisionID, ActorTemplate: desiredActor}, } reconciliations := krt.NewStaticCollection(nil, []AgentReconciliation{state}, opts.WithName("Reconciliations")...) status := kagentv1alpha3.AgentStatus{ObservedGeneration: 1, Conditions: []metav1.Condition{{Type: kagentv1alpha3.AgentConditionReady, Status: metav1.ConditionFalse}}} @@ -110,15 +110,17 @@ func TestReconcilerPersistsPairInOrder(t *testing.T) { // A fresh reconciler must clean up observations without remembering earlier calls. reconciler = &Reconciler{collections: reconciler.collections, templates: templates, store: store, status: statusClient} - state.DesiredActorTemplate = proto.CloneOf(state.DesiredActorTemplate) - state.DesiredActorTemplate.Metadata.Name = "assistant-next-revision" - state.Revision = &v2translator.Revision{AgentCard: &a2apb.AgentCard{Name: "updated assistant"}} - state.RevisionID, err = state.Revision.Digest() + nextTarget := *state.Target + state.Target = &nextTarget + state.Target.ActorTemplate = proto.CloneOf(state.Target.ActorTemplate) + state.Target.ActorTemplate.Metadata.Name = "assistant-next-revision" + state.Target.Revision = v2translator.Revision{AgentCard: &a2apb.AgentCard{Name: "updated assistant"}} + state.Target.RevisionID, err = state.Target.Revision.Digest() require.NoError(t, err) templates.template = nil reconciliations.UpdateObject(state) require.NoError(t, reconciler.reconcileAgent(t.Context(), state.ResourceName())) - require.Equal(t, state.RevisionID, reconciler.collections.AgentRuntimeObservations.GetKey(state.ResourceName()).RevisionID, "a new revision must replace the previous observation without waiting for GC") + require.Equal(t, state.Target.RevisionID, reconciler.collections.AgentRuntimeObservations.GetKey(state.ResourceName()).RevisionID, "a new revision must replace the previous observation without waiting for GC") require.Len(t, reconciler.collections.AgentRuntimeObservations.List(), 1) reconciliations.DeleteObject(state.ResourceName()) @@ -175,14 +177,14 @@ func TestRuntimeRevisionGCCollectsRetiredRevisions(t *testing.T) { GoldenTag: &ateapipb.ObjectRef{Atespace: "ate-golden", Name: "golden"}, }} state := AgentReconciliation{ - Agent: &kagentv1alpha3.Agent{ObjectMeta: metav1.ObjectMeta{Namespace: "team-a", Name: "assistant", UID: "agent-uid"}}, - Revision: revision, RevisionID: id, DesiredActorTemplate: desired, + Agent: &kagentv1alpha3.Agent{ObjectMeta: metav1.ObjectMeta{Namespace: "team-a", Name: "assistant", UID: "agent-uid"}}, + Target: &compiledTarget{Revision: *revision, RevisionID: id, ActorTemplate: desired}, } states.UpdateObject(state) require.NoError(t, reconciler.reconcileAgent(ctx, state.ResourceName())) // Compile failures preserve the current UID's last successful runtime. - state.Revision = nil + state.Target = nil states.UpdateObject(state) require.NoError(t, reconciler.reconcileAgent(ctx, state.ResourceName())) require.Empty(t, reconciler.collections.AgentRuntimeObservations.List(), "invalid preparation must release its observation before GC") @@ -300,9 +302,11 @@ type fakeRuntimeRevisionStore struct { retired string revisionErr error pairErr error + pairCalls int } func (s *fakeRuntimeRevisionStore) UpsertAgentDefinition(_ context.Context, pair database.AgentDefinition) error { + s.pairCalls++ s.pair = &pair return s.pairErr } @@ -417,13 +421,14 @@ func TestReconciliationQueueRetriesWithBackoff(t *testing.T) { require.EqualValues(t, 2, attempts.Load()) time.Sleep(3 * time.Minute) synctest.Wait() - require.EqualValues(t, 10, attempts.Load(), "persistent failures must exhaust a finite budget") + require.Greater(t, attempts.Load(), int32(10), "persistent failures must outlive the old retry budget") + before := attempts.Load() time.Sleep(time.Minute) synctest.Wait() - require.EqualValues(t, 10, attempts.Load()) + require.Equal(t, before+2, attempts.Load(), "backoff must stay capped at thirty seconds") queue.Add("team-a/assistant/kagent") synctest.Wait() - require.EqualValues(t, 11, attempts.Load(), "new graph events must still enqueue work") + require.Equal(t, before+3, attempts.Load(), "new graph events must still enqueue work") cancel() require.NoError(t, queue.WaitForClose(time.Second)) }) diff --git a/go/core/internal/controller/status.go b/go/core/internal/controller/status.go index aa6a7ee22f..caff9b9e1e 100644 --- a/go/core/internal/controller/status.go +++ b/go/core/internal/controller/status.go @@ -1,14 +1,9 @@ package controller import ( - "crypto/sha256" - "encoding/hex" - "encoding/json" - kagentv1alpha3 "github.com/kagent-dev/kagent/go/api/v1alpha3" "istio.io/istio/pkg/kube/krt" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" ) func newAgentStatuses(agents krt.Collection[*kagentv1alpha3.Agent], states krt.Collection[AgentReconciliation], opts krt.OptionsBuilder) krt.StatusCollection[*kagentv1alpha3.Agent, kagentv1alpha3.AgentStatus] { @@ -29,14 +24,18 @@ func statusForAgent(state AgentReconciliation, generation int64, latestSuccessfu } status.Warnings = append([]string(nil), state.Warnings...) setAgentCondition(&status, generation, kagentv1alpha3.AgentConditionAccepted, metav1.ConditionTrue, "Accepted", "Agent explicitly selects its template and harness") - if state.Failure != nil { - if state.Failure.Condition != kagentv1alpha3.AgentConditionResolvedRefs { + failure := state.CompilationFailure + if failure == nil { + failure = state.PreparationFailure + } + if failure != nil { + if failure.Condition != kagentv1alpha3.AgentConditionResolvedRefs { setAgentCondition(&status, generation, kagentv1alpha3.AgentConditionResolvedRefs, metav1.ConditionTrue, "Resolved", "All runtime references resolved") } - if state.Failure.Condition == kagentv1alpha3.AgentConditionReady { + if failure.Condition == kagentv1alpha3.AgentConditionReady { setAgentCondition(&status, generation, kagentv1alpha3.AgentConditionCompatible, metav1.ConditionTrue, "Compatible", "Resolved configuration is compatible with the Harness") } - setAgentFailure(&status, generation, state.Failure) + setAgentFailure(&status, generation, failure) return status } setAgentCondition(&status, generation, kagentv1alpha3.AgentConditionResolvedRefs, metav1.ConditionTrue, "Resolved", "All runtime references resolved") @@ -45,7 +44,7 @@ func statusForAgent(state AgentReconciliation, generation int64, latestSuccessfu setAgentCondition(&status, generation, kagentv1alpha3.AgentConditionReady, metav1.ConditionFalse, "ActorTemplatePending", "waiting for the ActorTemplate golden snapshot") return status } - status.LatestSuccessfulRevision = state.RevisionID.String() + status.LatestSuccessfulRevision = state.Target.RevisionID.String() setAgentCondition(&status, generation, kagentv1alpha3.AgentConditionReady, metav1.ConditionTrue, "Ready", "ActorTemplate golden snapshot is ready") return status } @@ -70,13 +69,3 @@ func setAgentCondition(status *kagentv1alpha3.AgentStatus, generation int64, con Type: conditionType, Status: conditionStatus, Reason: reason, Message: message, ObservedGeneration: generation, }) } - -func requestedRevision(agent *kagentv1alpha3.Agent) string { - raw, _ := json.Marshal(struct { - UID types.UID - Generation int64 - Spec kagentv1alpha3.AgentSpec - }{agent.UID, agent.Generation, agent.Spec}) - sum := sha256.Sum256(raw) - return hex.EncodeToString(sum[:]) -} diff --git a/go/core/internal/controller/status_test.go b/go/core/internal/controller/status_test.go index d10599084e..bb5024eb8b 100644 --- a/go/core/internal/controller/status_test.go +++ b/go/core/internal/controller/status_test.go @@ -10,10 +10,9 @@ import ( func TestStatusForPairPublishesCompilationWarnings(t *testing.T) { warnings := []string{"partial MCP selection is not enforced"} state := AgentReconciliation{ - Agent: &kagentv1alpha3.Agent{}, - Revision: &v2translator.Revision{}, - Warnings: warnings, - RevisionID: v2translator.RevisionID{1}, + Agent: &kagentv1alpha3.Agent{}, + Warnings: warnings, + Target: &compiledTarget{RevisionID: v2translator.RevisionID{1}}, } status := statusForAgent(state, 1, "") if len(status.Warnings) != 1 || status.Warnings[0] != warnings[0] { diff --git a/go/core/internal/database/models.go b/go/core/internal/database/models.go index 93370f44ef..97423eb004 100644 --- a/go/core/internal/database/models.go +++ b/go/core/internal/database/models.go @@ -51,9 +51,10 @@ type AgentMemorySearchResult struct { // AgentDefinition tracks the desired and last successful runtime for an Agent UID. type AgentDefinition struct { - Namespace string - AgentName string - AgentUID string + Namespace string + AgentName string + AgentUID string + // DesiredRevision is a compiled runtime digest, or empty while inputs are unresolved. DesiredRevision string } diff --git a/go/core/internal/substrate/actor_template.go b/go/core/internal/substrate/actor_template.go index f404069a1c..93825620b9 100644 --- a/go/core/internal/substrate/actor_template.go +++ b/go/core/internal/substrate/actor_template.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "strings" + "unicode/utf8" apia2a "github.com/kagent-dev/kagent/go/api/a2a" @@ -23,6 +24,8 @@ const ( defaultContainerName = "kagent" durableDataVolume = "data" durableDataMount = "/data" + // Matches EnvVar.value's maxLength in Substrate's ateapi.proto. + maxEnvironmentValueRunes = 32768 ) const egressTrustVolume = "egress-trust" @@ -202,6 +205,9 @@ func actorTemplateEnvFromPodEnv(environment []corev1.EnvVar) ([]*ateapipb.EnvVar if _, exists := seen[value.Name]; exists { continue } + if size := utf8.RuneCountInString(value.Value); size > maxEnvironmentValueRunes { + return nil, fmt.Errorf("environment variable %q is %d characters; Substrate supports at most %d", value.Name, size, maxEnvironmentValueRunes) + } seen[value.Name] = struct{}{} result = append(result, &ateapipb.EnvVar{Name: value.Name, Value: value.Value}) } diff --git a/go/core/internal/substrate/actor_template_test.go b/go/core/internal/substrate/actor_template_test.go index 856024639a..35875dfba6 100644 --- a/go/core/internal/substrate/actor_template_test.go +++ b/go/core/internal/substrate/actor_template_test.go @@ -3,6 +3,7 @@ package substrate import ( "encoding/json" "slices" + "strings" "testing" a2atype "github.com/a2aproject/a2a-go/v2/a2a" @@ -167,3 +168,46 @@ func TestActorTemplateSpecEqualIgnoresServerFields(t *testing.T) { t.Fatal("different container image was accepted") } } + +func TestActorTemplateEnvironmentValueSize(t *testing.T) { + const configOverhead = len(`{"instruction":""}`) + for _, test := range []struct { + name string + instruction string + description string + environment []corev1.EnvVar + wantError string + }{ + {name: "ASCII boundary", instruction: strings.Repeat("a", 32768-configOverhead)}, + {name: "ASCII overflow", instruction: strings.Repeat("a", 32769-configOverhead), wantError: `environment variable "KAGENT_CONFIG_JSON" is 32769 characters; Substrate supports at most 32768`}, + {name: "Unicode boundary", instruction: strings.Repeat("日", 32768-configOverhead)}, + {name: "Unicode overflow", instruction: strings.Repeat("日", 32769-configOverhead), wantError: `environment variable "KAGENT_CONFIG_JSON" is 32769 characters; Substrate supports at most 32768`}, + {name: "JSON escaping", instruction: strings.Repeat("<>&", 2000), wantError: `environment variable "KAGENT_CONFIG_JSON" is 36018 characters; Substrate supports at most 32768`}, + {name: "user environment", environment: []corev1.EnvVar{{Name: "EXTRA", Value: strings.Repeat("x", 32769)}}, wantError: `environment variable "EXTRA" is 32769 characters; Substrate supports at most 32768`}, + {name: "agent card", description: strings.Repeat("x", 32769), wantError: `environment variable "KAGENT_AGENT_CARD_JSON"`}, + } { + t.Run(test.name, func(t *testing.T) { + config, err := json.Marshal(struct { + Instruction string `json:"instruction"` + }{test.instruction}) + require.NoError(t, err) + spec := &translator.Revision{ + Namespace: "agents", AgentName: "helper", WorkerPoolName: "default", + ConfigJSON: config, Environment: test.environment, + AgentCard: &a2apb.AgentCard{Name: "helper", Description: test.description, Version: "v1", Capabilities: &a2apb.AgentCapabilities{}, + SupportedInterfaces: []*a2apb.AgentInterface{{Url: "http://127.0.0.1:80", ProtocolBinding: "GRPC", ProtocolVersion: "1.0"}}, + DefaultInputModes: []string{"text"}, DefaultOutputModes: []string{"text"}}, + } + id, err := spec.Digest() + require.NoError(t, err) + template, err := ActorTemplateForRevision(spec, id) + if test.wantError != "" { + require.ErrorContains(t, err, test.wantError) + require.Nil(t, template) + return + } + require.NoError(t, err) + require.NotNil(t, template) + }) + } +} diff --git a/go/core/internal/translator/revision.go b/go/core/internal/translator/revision.go index 68f0bbfb79..9eb6073985 100644 --- a/go/core/internal/translator/revision.go +++ b/go/core/internal/translator/revision.go @@ -84,8 +84,7 @@ func (r *Revision) Digest() (RevisionID, error) { sandboxClass := r.SandboxClass switch sandboxClass { case "", atev1alpha1.SandboxClassGvisor: - // Omit the default class to preserve pre-selection revision digests. - sandboxClass = "" + sandboxClass = atev1alpha1.SandboxClassGvisor case atev1alpha1.SandboxClassMicroVM: default: return RevisionID{}, fmt.Errorf("unsupported sandbox class %q", sandboxClass) @@ -104,7 +103,7 @@ func (r *Revision) Digest() (RevisionID, error) { Provenance json.RawMessage `json:"provenance"` Credentials []egress.Credential `json:"credentials,omitempty"` EgressDestinations []string `json:"egressDestinations"` - SandboxClass atev1alpha1.SandboxClass `json:"sandboxClass,omitempty"` + SandboxClass atev1alpha1.SandboxClass `json:"sandboxClass"` }{ AgentName: r.AgentName, AgentUID: r.AgentUID, Namespace: r.Namespace, Image: r.Image, Command: r.Command, Args: r.Args, Environment: r.Environment, ConfigJSON: r.ConfigJSON, diff --git a/go/core/internal/translator/revision_test.go b/go/core/internal/translator/revision_test.go index 9a07a271cb..4279aefdb3 100644 --- a/go/core/internal/translator/revision_test.go +++ b/go/core/internal/translator/revision_test.go @@ -15,11 +15,13 @@ func TestRevisionDigestIncludesSandboxClass(t *testing.T) { revision := &Revision{Namespace: "agents", AgentName: "helper"} original, err := revision.Digest() require.NoError(t, err) + require.NotEqual(t, "563beefdd1b191baae375aa92e70bcab812a3c4fd2a189cdf74668f1ad45456d", original.String(), + "the default class must participate in the digest instead of preserving the legacy digest") revision.SandboxClass = atev1alpha1.SandboxClassGvisor gvisor, err := revision.Digest() require.NoError(t, err) - require.Equal(t, original, gvisor, "explicit gVisor must preserve the existing default revision") + require.Equal(t, original, gvisor, "empty and explicit gVisor select the same runtime") require.Equal(t, atev1alpha1.SandboxClassGvisor, revision.SandboxClass, "hashing must not mutate the revision") revision.SandboxClass = atev1alpha1.SandboxClassMicroVM