From 2d52afb606748fcd61572a659438be6322e91aca Mon Sep 17 00:00:00 2001 From: Thanh Nguyen Date: Mon, 24 Aug 2026 12:11:24 +0700 Subject: [PATCH 1/2] fix(substrate): set default TimeoutSeconds in ActorTemplate to prevent spec drift ActorTemplateSpec.ContainerReadyz.TimeoutSeconds has a non-zero +kubebuilder:default (30) but is a non-pointer field, so an unset value here serializes as 0 and permanently disagrees with the apiserver's defaulted stored value, causing reconcileActorTemplate to treat it as spec drift and loop deleting/recreating the ActorTemplate forever. Signed-off-by: Thanh Nguyen --- .../pkg/sandboxbackend/substrate/agent_lifecycle.go | 12 ++++++++++++ .../sandboxbackend/substrate/agent_lifecycle_test.go | 6 ++++++ 2 files changed, 18 insertions(+) diff --git a/go/core/pkg/sandboxbackend/substrate/agent_lifecycle.go b/go/core/pkg/sandboxbackend/substrate/agent_lifecycle.go index 019a3cfbf..216b37366 100644 --- a/go/core/pkg/sandboxbackend/substrate/agent_lifecycle.go +++ b/go/core/pkg/sandboxbackend/substrate/agent_lifecycle.go @@ -98,6 +98,18 @@ func (p *Lifecycle) buildSandboxAgentActorTemplate( Path: "/.well-known/agent-card.json", Port: substrateKagentListenPort, }, + // Must match ActorTemplateSpec's ContainerReadyz.TimeoutSeconds + // +kubebuilder:default=30 (actortemplate_types.go). Leaving this + // unset builds a spec with TimeoutSeconds=0, but the apiserver + // applies the CRD default (30) to whatever gets stored — so every + // freshly-rebuilt "desired" spec permanently disagrees with the + // "existing" one just fetched back from the cluster. + // actorTemplateSpecEqual (apiequality.Semantic.DeepEqual) then + // sees 0 != 30 on every reconcile, reconcileActorTemplate treats + // it as spec drift, and deletes+recreates the ActorTemplate (and + // its golden actor) in an infinite loop. Setting it explicitly + // here keeps "desired" and "existing" in agreement. + TimeoutSeconds: 30, }, }}, WorkerSelector: workerSelectorForPool(wpKey), diff --git a/go/core/pkg/sandboxbackend/substrate/agent_lifecycle_test.go b/go/core/pkg/sandboxbackend/substrate/agent_lifecycle_test.go index 3ff8da8d9..c6f3438b2 100644 --- a/go/core/pkg/sandboxbackend/substrate/agent_lifecycle_test.go +++ b/go/core/pkg/sandboxbackend/substrate/agent_lifecycle_test.go @@ -219,6 +219,12 @@ func TestBuildSandboxAgentActorTemplate(t *testing.T) { require.NotNil(t, c.Readyz.HTTPGet) require.Equal(t, "/.well-known/agent-card.json", c.Readyz.HTTPGet.Path) require.Equal(t, substrateKagentListenPort, c.Readyz.HTTPGet.Port) + // TimeoutSeconds must mirror ActorTemplateSpec's own +kubebuilder:default=30: + // it's a non-pointer field, so leaving it unset here would serialize a desired + // spec with TimeoutSeconds=0 that permanently disagrees with the apiserver's + // defaulted stored value, driving reconcileActorTemplate into a delete+recreate + // loop (see the TimeoutSeconds field comment above for the full mechanism). + require.Equal(t, int32(30), c.Readyz.TimeoutSeconds) names := actorEnvNames(c.Env) require.True(t, names["KAGENT_NAME"], "KAGENT_NAME must be a literal env var") From 8fe625b15016ad039721223c7319b118e458d97e Mon Sep 17 00:00:00 2001 From: Thanh Nguyen Date: Mon, 24 Aug 2026 12:12:15 +0700 Subject: [PATCH 2/2] fix(substrate): mirror OnResume CRD default to prevent infinite recreate loop Same class of bug as ContainerReadyz.TimeoutSeconds: SnapshotsConfig.OnResume is a non-pointer struct with a non-zero +kubebuilder:default (ColdBoot), so leaving it unset in both the SandboxAgent and AgentHarness ActorTemplate builders left the desired spec permanently disagreeing with the apiserver-defaulted stored spec, keeping the delete+recreate loop going even after the TimeoutSeconds fix alone. Signed-off-by: Thanh Nguyen --- .../substrate/agent_lifecycle.go | 11 ++++ .../substrate/agent_lifecycle_test.go | 5 ++ .../substrate/lifecycle_actortemplate.go | 7 +++ .../substrate/lifecycle_actortemplate_test.go | 51 +++++++++++++++++++ 4 files changed, 74 insertions(+) create mode 100644 go/core/pkg/sandboxbackend/substrate/lifecycle_actortemplate_test.go diff --git a/go/core/pkg/sandboxbackend/substrate/agent_lifecycle.go b/go/core/pkg/sandboxbackend/substrate/agent_lifecycle.go index 216b37366..d82bae7b8 100644 --- a/go/core/pkg/sandboxbackend/substrate/agent_lifecycle.go +++ b/go/core/pkg/sandboxbackend/substrate/agent_lifecycle.go @@ -117,6 +117,17 @@ func (p *Lifecycle) buildSandboxAgentActorTemplate( Location: sandboxAgentSnapshotsLocation(sa), OnPause: atev1alpha1.SnapshotScopeFull, OnCommit: atev1alpha1.SnapshotScopeFull, + // Same class of bug as ContainerReadyz.TimeoutSeconds above: + // OnResume is a plain (non-pointer) struct, so Go's encoding/json + // omitempty never actually omits it — "desired" always serializes + // onResume:{}, and the apiserver's structural-schema defaulting then + // fills in fromData: "ColdBoot" (+kubebuilder:default=ColdBoot, + // actortemplate_types.go) on the *stored* object. Leaving this + // unset here left desired.OnResume.FromData="" permanently + // disagreeing with existing's server-defaulted "ColdBoot", which + // alone was enough to keep the delete+recreate loop going even + // after the TimeoutSeconds fix. + OnResume: atev1alpha1.OnResumeConfig{FromData: atev1alpha1.ResumeSourceColdBoot}, }, } applyDurableDirSessionStore(&spec) diff --git a/go/core/pkg/sandboxbackend/substrate/agent_lifecycle_test.go b/go/core/pkg/sandboxbackend/substrate/agent_lifecycle_test.go index c6f3438b2..21efd925f 100644 --- a/go/core/pkg/sandboxbackend/substrate/agent_lifecycle_test.go +++ b/go/core/pkg/sandboxbackend/substrate/agent_lifecycle_test.go @@ -226,6 +226,11 @@ func TestBuildSandboxAgentActorTemplate(t *testing.T) { // loop (see the TimeoutSeconds field comment above for the full mechanism). require.Equal(t, int32(30), c.Readyz.TimeoutSeconds) + // OnResume must mirror SnapshotsConfig's own +kubebuilder:default=ColdBoot for the + // same reason: it's a non-pointer struct, so an unset value here would leave the + // desired spec permanently disagreeing with the apiserver-defaulted stored spec. + require.Equal(t, atev1alpha1.ResumeSourceColdBoot, tmpl.Spec.SnapshotsConfig.OnResume.FromData) + names := actorEnvNames(c.Env) require.True(t, names["KAGENT_NAME"], "KAGENT_NAME must be a literal env var") require.True(t, names["KAGENT_NAMESPACE"], "KAGENT_NAMESPACE must be a literal env var") diff --git a/go/core/pkg/sandboxbackend/substrate/lifecycle_actortemplate.go b/go/core/pkg/sandboxbackend/substrate/lifecycle_actortemplate.go index fc492e5cf..24141491c 100644 --- a/go/core/pkg/sandboxbackend/substrate/lifecycle_actortemplate.go +++ b/go/core/pkg/sandboxbackend/substrate/lifecycle_actortemplate.go @@ -186,6 +186,13 @@ func (p *Lifecycle) buildActorTemplate(ctx context.Context, ah *v1alpha3.AgentHa // re-creates the ActorTemplate every reconcile in a hot loop. OnPause: atev1alpha1.SnapshotScopeFull, OnCommit: atev1alpha1.SnapshotScopeFull, + // OnResume was missing here too: it's a non-pointer struct, so + // encoding/json's omitempty never actually omits it, the apiserver + // defaults its fromData to "ColdBoot" on the stored object + // regardless, and an unset OnResume here left desired at the Go + // zero value ("") forever disagreeing with it — same hot loop via + // a different field. + OnResume: atev1alpha1.OnResumeConfig{FromData: atev1alpha1.ResumeSourceColdBoot}, }, }, } diff --git a/go/core/pkg/sandboxbackend/substrate/lifecycle_actortemplate_test.go b/go/core/pkg/sandboxbackend/substrate/lifecycle_actortemplate_test.go new file mode 100644 index 000000000..b498ac9cb --- /dev/null +++ b/go/core/pkg/sandboxbackend/substrate/lifecycle_actortemplate_test.go @@ -0,0 +1,51 @@ +package substrate + +import ( + "context" + "testing" + + atev1alpha1 "github.com/agent-substrate/substrate/pkg/api/v1alpha1" + "github.com/kagent-dev/kagent/go/api/v1alpha3" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" +) + +// TestBuildActorTemplate exercises ActorTemplate generation for an AgentHarness (the +// non-SandboxAgent path), asserting the SnapshotsConfig mirrors substrate's CRD defaults +// exactly the way buildSandboxAgentActorTemplate does (see agent_lifecycle_test.go's +// TestBuildSandboxAgentActorTemplate for the equivalent SandboxAgent-side coverage). +func TestBuildActorTemplate(t *testing.T) { + t.Parallel() + + const pinnedImage = "registry.example/kagent-dev/kagent/app@sha256:2222222222222222222222222222222222222222222222222222222222222222" + wpKey := types.NamespacedName{Namespace: "kagent", Name: "kagent-default"} + + ah := &v1alpha3.AgentHarness{ + ObjectMeta: metav1.ObjectMeta{Name: "my-harness", Namespace: "kagent"}, + Spec: v1alpha3.AgentHarnessSpec{ + Backend: v1alpha3.AgentHarnessBackendHermes, + Substrate: &v1alpha3.AgentHarnessSubstrateSpec{ + WorkloadImage: pinnedImage, + }, + }, + } + + p := newTestLifecycle(t) + tmpl, err := p.buildActorTemplate(context.Background(), ah, wpKey) + require.NoError(t, err) + + require.Len(t, tmpl.Spec.Containers, 1) + c := tmpl.Spec.Containers[0] + require.Equal(t, pinnedImage, c.Image, "ActorTemplate must use the digest-pinned image") + require.Equal(t, wpKey.Name, tmpl.Spec.WorkerSelector.MatchLabels["kagent.dev/worker-pool"]) + + // SnapshotsConfig must mirror substrate's CRD defaults exactly, or kagent's spec-drift + // check (apiequality.Semantic.DeepEqual in actorTemplateSpecEqual) will treat the + // apiserver-defaulted stored spec as permanently different from the freshly-rebuilt + // desired spec, and reconcileActorTemplate will delete+recreate the ActorTemplate (and + // its golden actor) on every single reconcile. + require.Equal(t, atev1alpha1.SnapshotScopeFull, tmpl.Spec.SnapshotsConfig.OnPause) + require.Equal(t, atev1alpha1.SnapshotScopeFull, tmpl.Spec.SnapshotsConfig.OnCommit) + require.Equal(t, atev1alpha1.ResumeSourceColdBoot, tmpl.Spec.SnapshotsConfig.OnResume.FromData) +}