diff --git a/controller/agenticrun/client.go b/controller/agenticrun/client.go index f00dd138..c8a9cf8e 100644 --- a/controller/agenticrun/client.go +++ b/controller/agenticrun/client.go @@ -79,10 +79,10 @@ type AgentHTTPClient struct { endpoint string } -func NewAgentHTTPClient(endpoint string) AgentHTTPClientInterface { +func NewAgentHTTPClient(endpoint string, timeout time.Duration) AgentHTTPClientInterface { return &AgentHTTPClient{ httpClient: &http.Client{ - Timeout: 5 * time.Minute, + Timeout: timeout, Transport: &http.Transport{ TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, //nolint:gosec // internal cluster traffic }, diff --git a/controller/agenticrun/client_test.go b/controller/agenticrun/client_test.go index 9f6b584d..01d21f60 100644 --- a/controller/agenticrun/client_test.go +++ b/controller/agenticrun/client_test.go @@ -6,6 +6,7 @@ import ( "net/http" "net/http/httptest" "testing" + "time" agenticv1alpha1 "github.com/openshift/lightspeed-agentic-operator/api/v1alpha1" ) @@ -35,7 +36,7 @@ func TestAgentHTTPClient_RunSuccess(t *testing.T) { })) defer server.Close() - client := NewAgentHTTPClient(server.URL) + client := NewAgentHTTPClient(server.URL, 5*time.Minute) resp, err := client.Run(context.Background(), "You are an SRE agent", "check health", nil, nil, nil) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -52,7 +53,7 @@ func TestAgentHTTPClient_RunHTTPError(t *testing.T) { })) defer server.Close() - client := NewAgentHTTPClient(server.URL) + client := NewAgentHTTPClient(server.URL, 5*time.Minute) _, err := client.Run(context.Background(), "", "test", nil, nil, nil) if err == nil { t.Fatal("expected error for HTTP 500") @@ -60,7 +61,7 @@ func TestAgentHTTPClient_RunHTTPError(t *testing.T) { } func TestAgentHTTPClient_RunConnectionError(t *testing.T) { - client := NewAgentHTTPClient("http://127.0.0.1:1") + client := NewAgentHTTPClient("http://127.0.0.1:1", 5*time.Minute) _, err := client.Run(context.Background(), "", "test", nil, nil, nil) if err == nil { t.Fatal("expected error for connection failure") @@ -100,7 +101,7 @@ func TestAgentHTTPClient_RunWithExecutionResult(t *testing.T) { })) defer server.Close() - client := NewAgentHTTPClient(server.URL) + client := NewAgentHTTPClient(server.URL, 5*time.Minute) agentCtx := &agentContext{ TargetNamespaces: []string{"production"}, ExecutionResult: &agentExecutionResult{ @@ -135,7 +136,7 @@ func TestAgentHTTPClient_RunWithoutExecutionResult(t *testing.T) { })) defer server.Close() - client := NewAgentHTTPClient(server.URL) + client := NewAgentHTTPClient(server.URL, 5*time.Minute) agentCtx := &agentContext{ TargetNamespaces: []string{"production"}, } @@ -169,7 +170,7 @@ func TestAgentHTTPClient_RunWithContext(t *testing.T) { })) defer server.Close() - client := NewAgentHTTPClient(server.URL) + client := NewAgentHTTPClient(server.URL, 5*time.Minute) agentCtx := &agentContext{ TargetNamespaces: []string{"production"}, PreviousAttempts: []agentPreviousAttempt{{Attempt: 1, FailureReason: "timeout"}}, diff --git a/controller/agenticrun/reconciler_test.go b/controller/agenticrun/reconciler_test.go index 0e272364..c18e5d09 100644 --- a/controller/agenticrun/reconciler_test.go +++ b/controller/agenticrun/reconciler_test.go @@ -262,7 +262,7 @@ func newMockSandboxAgent(analysisJSON, executionJSON, verificationJSON string) ( caller := &SandboxAgentCaller{ Sandbox: sandbox, K8sClient: fc, - ClientFactory: func(_ string) AgentHTTPClientInterface { + ClientFactory: func(_ string, _ time.Duration) AgentHTTPClientInterface { resp := responses[callCount%len(responses)] callCount++ httpClient.response = &agentRunResponse{Response: json.RawMessage(resp)} diff --git a/controller/agenticrun/revision_test.go b/controller/agenticrun/revision_test.go index 2d0c6e9a..827a1804 100644 --- a/controller/agenticrun/revision_test.go +++ b/controller/agenticrun/revision_test.go @@ -92,6 +92,9 @@ func TestBuildAnalysisQuery_FullAgenticRun(t *testing.T) { if !strings.Contains(result, "Verification plan") { t.Error("full run should mention verification plan") } + if !strings.Contains(result, "read-only cluster access") { + t.Error("full run should constrain verification to read-only commands") + } if !strings.Contains(result, "Fix the crash") { t.Error("should contain the request text") } diff --git a/controller/agenticrun/sandbox_agent.go b/controller/agenticrun/sandbox_agent.go index 72890f5f..f99323c6 100644 --- a/controller/agenticrun/sandbox_agent.go +++ b/controller/agenticrun/sandbox_agent.go @@ -17,6 +17,10 @@ import ( const ( defaultSandboxTimeout = 5 * time.Minute + analysisStepTimeout = 10 * time.Minute + executionStepTimeout = 10 * time.Minute + verificationStepTimeout = 30 * time.Minute + ErrAnalysisAgentCall = "analysis agent call" ErrParseAnalysisResponse = "parse analysis response" ErrExecutionAgentCall = "execution agent call" @@ -62,7 +66,7 @@ type SandboxLifecycle interface { type SandboxAgentCaller struct { Sandbox SandboxLifecycle K8sClient client.Client - ClientFactory func(endpoint string) AgentHTTPClientInterface + ClientFactory func(endpoint string, timeout time.Duration) AgentHTTPClientInterface Namespace string Timeout time.Duration Audit AuditLogger @@ -188,6 +192,19 @@ func (s *SandboxAgentCaller) Escalate(ctx context.Context, run *agenticv1alpha1. }, nil } +func stepTimeout(step string) time.Duration { + switch step { + case "analysis", "escalation": + return analysisStepTimeout + case "execution": + return executionStepTimeout + case "verification": + return verificationStepTimeout + default: + return analysisStepTimeout + } +} + func (s *SandboxAgentCaller) callWithSandbox( ctx context.Context, run *agenticv1alpha1.AgenticRun, @@ -227,7 +244,7 @@ func (s *SandboxAgentCaller) callWithSandbox( s.Audit.InjectTraceContext(ctx, run, headers) } - client := s.ClientFactory(agentURL) + client := s.ClientFactory(agentURL, stepTimeout(stepName)-2*time.Minute) resp, err := client.Run(ctx, "", query, schema, agentCtx, headers) if err != nil { return nil, err diff --git a/controller/agenticrun/sandbox_agent_test.go b/controller/agenticrun/sandbox_agent_test.go index 4a454988..1ee09085 100644 --- a/controller/agenticrun/sandbox_agent_test.go +++ b/controller/agenticrun/sandbox_agent_test.go @@ -61,7 +61,7 @@ func newTestSandboxAgentCaller(sandbox *mockSandboxProvider, httpClient *mockHTT return &SandboxAgentCaller{ Sandbox: sandbox, K8sClient: fc, - ClientFactory: func(_ string) AgentHTTPClientInterface { return httpClient }, + ClientFactory: func(_ string, _ time.Duration) AgentHTTPClientInterface { return httpClient }, Namespace: "test-ns", Timeout: 5 * time.Minute, } @@ -76,7 +76,7 @@ func newTestSandboxAgentCallerWithAgenticRun(sandbox *mockSandboxProvider, httpC return &SandboxAgentCaller{ Sandbox: sandbox, K8sClient: fc, - ClientFactory: func(_ string) AgentHTTPClientInterface { return httpClient }, + ClientFactory: func(_ string, _ time.Duration) AgentHTTPClientInterface { return httpClient }, Namespace: "test-ns", Timeout: 5 * time.Minute, } @@ -540,6 +540,12 @@ func TestSandboxAgentCaller_VerificationQueryFraming(t *testing.T) { if strings.Contains(httpClient.lastQuery, "Pod crashing with OOMKilled") { t.Error("verification query should NOT contain the original request") } + if !strings.Contains(httpClient.lastQuery, "Convergence-dependent checks") { + t.Error("verification query should contain convergence retry guidance") + } + if !strings.Contains(httpClient.lastQuery, "wait and retry before reporting failure") { + t.Error("verification query should instruct agent to retry convergence checks") + } } func TestSandboxAgentCaller_ExecutionQueryNilOption(t *testing.T) { diff --git a/controller/agenticrun/templates/analysis_query.tmpl b/controller/agenticrun/templates/analysis_query.tmpl index 06c35de3..fbe8886f 100644 --- a/controller/agenticrun/templates/analysis_query.tmpl +++ b/controller/agenticrun/templates/analysis_query.tmpl @@ -1,7 +1,15 @@ -You are an analysis agent for OpenShift clusters. Diagnose the problem. Determine the root cause. Produce a remediation plan. A human will review and approve this plan before execution. Do NOT run commands that change the cluster state. You can only read. Write remediation commands for an execution agent to run after human approval. +You are an analysis agent. Diagnose the problem. Determine the root cause. Produce a remediation plan. A human will review and approve this plan before execution. Do NOT run commands that change the cluster state. You can only read. Write remediation commands for an execution agent to run after human approval. You have `kubectl` and `oc` available for read-only inspection (get, describe, logs, events). Use them to inspect the cluster BEFORE you diagnose. Do not guess from local files. +## Before diagnosing — determine the failure scope + +1. **Cross-service correlation**: Check whether other workloads in the affected namespace(s) show the same or similar errors. Compare event and log timestamps across services to identify correlated failure onset. Also check recent cluster events for infrastructure changes (node conditions, certificate renewals, network policy updates, operator upgrades). If multiple services fail with the same error class at the same time, the root cause is likely a shared dependency. +2. **App vs infrastructure errors**: Distinguish application logic errors from infrastructure errors. Application: NPE, division by zero, assertion failures, panic. Infrastructure: connection refused, pool exhausted, timeout, OOM, disk pressure, certificate expired. Infrastructure errors require infrastructure-level fixes, not application rollbacks. +3. **Trace the dependency graph**: Inspect environment variables, ConfigMaps, and connection strings to discover shared backends — these may reside in other namespaces. If affected workloads share a backend (database, cache, message bus, external API), check the backend's health first. + +Match your remediation and verification to the scope. Infrastructure problems need infrastructure-level fixes and checks. Application bugs need application-specific fixes and checks. + When more than one solution exists, propose multiple remediation options. For each option: - **Diagnose** the root cause with confidence level. @@ -40,7 +48,7 @@ When more than one solution exists, propose multiple remediation options. For ea {{- end}} {{- if .HasVerification}} -- **Verification plan** — checks to confirm the fix worked. +- **Verification plan** — checks to confirm the fix worked. The verification agent has **read-only cluster access** (get, list, watch only). Do NOT propose `exec`, `port-forward`, `cp`, `attach`, or `proxy` commands. These require escalated permissions the agent lacks. Use `oc get`, `oc describe`, `oc logs`, `oc get events`, or JSONPath queries. {{- end}} - **Risk assessment** and reversibility. diff --git a/controller/agenticrun/templates/execution_query.tmpl b/controller/agenticrun/templates/execution_query.tmpl index 2da75314..69921a7f 100644 --- a/controller/agenticrun/templates/execution_query.tmpl +++ b/controller/agenticrun/templates/execution_query.tmpl @@ -1,4 +1,4 @@ -You are an execution agent for OpenShift clusters. Execute the approved remediation option below. Do not re-analyze the problem. Do not propose alternative solutions. +You are an execution agent. Execute the approved remediation option below. Do not re-analyze the problem. Do not propose alternative solutions. The approved option contains a concrete remediation script — an ordered list of exact bash commands. Follow these rules: diff --git a/controller/agenticrun/templates/verification_query.tmpl b/controller/agenticrun/templates/verification_query.tmpl index 35854867..a74778cd 100644 --- a/controller/agenticrun/templates/verification_query.tmpl +++ b/controller/agenticrun/templates/verification_query.tmpl @@ -1,6 +1,21 @@ -You are a verification agent for OpenShift clusters. Verify that the executed remediation was applied correctly. Verify that the issue is resolved. Do not execute any additional changes. Only verify. +You are a verification agent. Verify that the issue is resolved. Do not execute changes — only verify. -Run the verification checks from the approved option's verification plan. Compare the current cluster state against the expected outcomes. Report each check as Passed or Failed with evidence. +Run each check from the approved option's verification plan in order. Report every check as Passed or Failed with evidence. + +**Fix syntax errors only.** If a command fails due to a syntax error (malformed flag, wrong argument order), fix the syntax and retry. Do not change the intent, target, or resource of the command. Do not add checks beyond the verification plan. + +### Convergence-dependent checks + +Some checks depend on cluster state that converges over time after a remediation. For example: alerts clearing, pods becoming ready, or metrics dropping below a threshold. + +If a convergence-dependent check fails on the first attempt, wait and retry before reporting failure. Be patient — cluster state often takes minutes to converge after a remediation. Use these guidelines: + +- **Alerts** (for example, alert stopped firing): wait 60 seconds between retries, up to 10 retries (~10 minutes). Alerts often have a `for` duration before they clear. +- **Pod readiness / rollout**: wait 15–30 seconds between retries, up to 10 retries (~5 minutes). Image pulls, init containers, and readiness probes all add latency. +- **Metrics** (for example, error rate below threshold): wait 60 seconds between retries, up to 10 retries (~10 minutes). Metrics windows need time to reflect the new state. +- **Instant state checks** (for example, image tag or config value): do not retry. These reflect immediately. + +Do not give up early. Use the full retry budget before reporting a check as Failed. If a check passes on a later retry, report it as Passed. If the check still fails after all retries, report Failed with the last observed value. ## Approved Option