Skip to content

OLS-3743 Wire Agent.spec.timeouts to HTTP client and sandbox requests #438

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
vimalk78 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

OLS-3743 Wire Agent.spec.timeouts to HTTP client and sandbox requests #438

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 13 additions & 8 deletions controller/agenticrun/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,11 @@ const (
maxResponseSize = 2 << 20 // 2 MiB
runPath = "/v1/agent/run"

ErrMarshalRequest = "failed to marshal request"
ErrCreateHTTPRequest = "failed to create HTTP request"
ErrPost = "POST"
ErrReadResponseBody = "failed to read response body"
ErrMarshalRequest = "failed to marshal request"
ErrCreateHTTPRequest = "failed to create HTTP request"
ErrPost = "POST"
ErrReadResponseBody = "failed to read response body"
defaultHTTPClientTimeout = 5 * time.Minute
)

type agentRunRequest struct {
Expand Down Expand Up @@ -65,7 +66,7 @@ type agentRunResponse struct {

// AgentHTTPClientInterface abstracts HTTP calls to the agent service for testability.
type AgentHTTPClientInterface interface {
Run(ctx context.Context, systemPrompt, query string, outputSchema json.RawMessage, agentCtx *agentContext, extraHeaders http.Header) (*agentRunResponse, error)
Run(ctx context.Context, systemPrompt, query string, outputSchema json.RawMessage, agentCtx *agentContext, extraHeaders http.Header, timeoutMs *int64) (*agentRunResponse, error)
}

// AgentHTTPClient communicates with the agentic-sandbox REST API.
Expand All @@ -74,10 +75,13 @@ type AgentHTTPClient struct {
endpoint string
}

func NewAgentHTTPClient(endpoint string) AgentHTTPClientInterface {
func NewAgentHTTPClient(endpoint string, timeout time.Duration) AgentHTTPClientInterface {
if timeout <= 0 {
timeout = defaultHTTPClientTimeout
}
return &AgentHTTPClient{
httpClient: &http.Client{
Timeout: 5 * time.Minute,
Timeout: timeout,
Transport: &http.Transport{
TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, //nolint:gosec // internal cluster traffic
},
Expand All @@ -86,12 +90,13 @@ func NewAgentHTTPClient(endpoint string) AgentHTTPClientInterface {
}
}

func (c *AgentHTTPClient) Run(ctx context.Context, systemPrompt, query string, outputSchema json.RawMessage, agentCtx *agentContext, extraHeaders http.Header) (*agentRunResponse, error) {
func (c *AgentHTTPClient) Run(ctx context.Context, systemPrompt, query string, outputSchema json.RawMessage, agentCtx *agentContext, extraHeaders http.Header, timeoutMs *int64) (*agentRunResponse, error) {
req := agentRunRequest{
Query: query,
SystemPrompt: systemPrompt,
OutputSchema: outputSchema,
Context: agentCtx,
TimeoutMs: timeoutMs,
}

body, err := json.Marshal(req)
Expand Down
71 changes: 59 additions & 12 deletions controller/agenticrun/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"net/http"
"net/http/httptest"
"testing"
"time"

agenticv1alpha1 "github.com/openshift/lightspeed-agentic-operator/api/v1alpha1"
)
Expand Down Expand Up @@ -35,8 +36,8 @@ func TestAgentHTTPClient_RunSuccess(t *testing.T) {
}))
defer server.Close()

client := NewAgentHTTPClient(server.URL)
resp, err := client.Run(context.Background(), "You are an SRE agent", "check health", nil, nil, nil)
client := NewAgentHTTPClient(server.URL, 0)
resp, err := client.Run(context.Background(), "You are an SRE agent", "check health", nil, nil, nil, nil)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
Expand All @@ -52,16 +53,16 @@ func TestAgentHTTPClient_RunHTTPError(t *testing.T) {
}))
defer server.Close()

client := NewAgentHTTPClient(server.URL)
_, err := client.Run(context.Background(), "", "test", nil, nil, nil)
client := NewAgentHTTPClient(server.URL, 0)
_, err := client.Run(context.Background(), "", "test", nil, nil, nil, nil)
if err == nil {
t.Fatal("expected error for HTTP 500")
}
}

func TestAgentHTTPClient_RunConnectionError(t *testing.T) {
client := NewAgentHTTPClient("http://127.0.0.1:1")
_, err := client.Run(context.Background(), "", "test", nil, nil, nil)
client := NewAgentHTTPClient("http://127.0.0.1:1", 0)
_, err := client.Run(context.Background(), "", "test", nil, nil, nil, nil)
if err == nil {
t.Fatal("expected error for connection failure")
}
Expand Down Expand Up @@ -93,7 +94,7 @@ func TestAgentHTTPClient_RunWithExecutionResult(t *testing.T) {
}))
defer server.Close()

client := NewAgentHTTPClient(server.URL)
client := NewAgentHTTPClient(server.URL, 0)
agentCtx := &agentContext{
TargetNamespaces: []string{"production"},
ExecutionResult: &agentExecutionResult{
Expand All @@ -103,7 +104,7 @@ func TestAgentHTTPClient_RunWithExecutionResult(t *testing.T) {
},
},
}
_, err := client.Run(context.Background(), "", "test", nil, agentCtx, nil)
_, err := client.Run(context.Background(), "", "test", nil, agentCtx, nil, nil)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
Expand All @@ -124,11 +125,11 @@ func TestAgentHTTPClient_RunWithoutExecutionResult(t *testing.T) {
}))
defer server.Close()

client := NewAgentHTTPClient(server.URL)
client := NewAgentHTTPClient(server.URL, 0)
agentCtx := &agentContext{
TargetNamespaces: []string{"production"},
}
_, err := client.Run(context.Background(), "", "test", nil, agentCtx, nil)
_, err := client.Run(context.Background(), "", "test", nil, agentCtx, nil, nil)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
Expand Down Expand Up @@ -158,12 +159,58 @@ func TestAgentHTTPClient_RunWithContext(t *testing.T) {
}))
defer server.Close()

client := NewAgentHTTPClient(server.URL)
client := NewAgentHTTPClient(server.URL, 0)
agentCtx := &agentContext{
TargetNamespaces: []string{"production"},
PreviousAttempts: []agentPreviousAttempt{{Attempt: 1, FailureReason: "timeout"}},
}
_, err := client.Run(context.Background(), "", "test", nil, agentCtx, nil)
_, err := client.Run(context.Background(), "", "test", nil, agentCtx, nil, nil)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
}

func TestAgentHTTPClient_RunTimeoutMsPropagated(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
var req agentRunRequest
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
t.Fatalf("failed to decode request: %v", err)
}
if req.TimeoutMs == nil {
t.Fatal("expected timeout_ms to be set")
}
if *req.TimeoutMs != 600000 {
t.Errorf("timeout_ms = %d, want 600000", *req.TimeoutMs)
}
w.Header().Set("Content-Type", "application/json")
w.Write([]byte(`{"success": true}`))
}))
defer server.Close()

client := NewAgentHTTPClient(server.URL, 10*time.Minute)
timeoutMs := int64(600000)
_, err := client.Run(context.Background(), "", "test", nil, nil, nil, &timeoutMs)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
}

func TestAgentHTTPClient_RunTimeoutMsOmittedWhenNil(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
var req agentRunRequest
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
t.Fatalf("failed to decode request: %v", err)
}
if req.TimeoutMs != nil {
t.Errorf("timeout_ms should be nil, got %d", *req.TimeoutMs)
}
w.Header().Set("Content-Type", "application/json")
w.Write([]byte(`{"success": true}`))
}))
defer server.Close()

client := NewAgentHTTPClient(server.URL, 0)
_, err := client.Run(context.Background(), "", "test", nil, nil, nil, nil)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion controller/agenticrun/reconciler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)}
Expand Down
28 changes: 25 additions & 3 deletions controller/agenticrun/sandbox_agent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (

const (
defaultSandboxTimeout = 5 * time.Minute
httpGracePeriod = 15 * time.Second

ErrAnalysisAgentCall = "analysis agent call"
ErrParseAnalysisResponse = "parse analysis response"
Expand Down Expand Up @@ -61,7 +62,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
Expand Down Expand Up @@ -222,15 +223,36 @@ func (s *SandboxAgentCaller) callWithSandbox(
s.Audit.InjectTraceContext(ctx, run, headers)
}

client := s.ClientFactory(agentURL)
resp, err := client.Run(ctx, "", query, schema, agentCtx, headers)
stepTimeout := timeoutForStep(stepName, step.Agent)
client := s.ClientFactory(agentURL, stepTimeout+httpGracePeriod)
timeoutMs := int64(stepTimeout / time.Millisecond)
resp, err := client.Run(ctx, "", query, schema, agentCtx, headers, &timeoutMs)
if err != nil {
return nil, err
}

return resp.Response, nil
}

func timeoutForStep(stepName string, agent *agenticv1alpha1.Agent) time.Duration {
if agent == nil {
return defaultSandboxTimeout
}
var seconds int32
switch stepName {
case "analysis":
seconds = agent.Spec.Timeouts.AnalysisSeconds
case "execution":
seconds = agent.Spec.Timeouts.ExecutionSeconds
case "verification":
seconds = agent.Spec.Timeouts.VerificationSeconds
}
if seconds > 0 {
return time.Duration(seconds) * time.Second
}
return defaultSandboxTimeout
}

func (s *SandboxAgentCaller) ReleaseSandboxes(ctx context.Context, run *agenticv1alpha1.AgenticRun) error {
log := logf.FromContext(ctx)
var firstErr error
Expand Down
Loading