From 796b3e14dfd4249da7f7d9e5b481eef105db0d99 Mon Sep 17 00:00:00 2001 From: Jil Kamerling Date: Wed, 24 Sep 2025 10:51:13 +0200 Subject: [PATCH 1/2] feat: PAAL-123 update agent controller; checks for available agent card if a2a else execute a tcp probe; update samples for e2e tests; update CLAUDE.md --- CLAUDE.md | 5 + config/samples/runtime_v1alpha1_agent.yaml | 1 + .../runtime_v1alpha1_agent_template.yaml | 12 +- internal/controller/agent_controller.go | 151 +++- internal/controller/agent_controller_test.go | 653 +++++++++++++++++- internal/equality/equality.go | 6 + internal/equality/equality_test.go | 149 ++++ 7 files changed, 964 insertions(+), 13 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index c7c9a3f..17b77a0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -37,6 +37,11 @@ This folder is hosted as a separate [documentation site](https://docs.agentic-la - **Agent Controller** (`internal/controller/agent_controller.go`): Reconciles Agent resources by: - Creating Kubernetes Deployments for agent workloads - Managing Services for protocol exposure + - **Protocol-aware health checking**: Automatically generates appropriate readiness probes + - A2A agents: HTTP GET with configurable paths (validates agent functionality) + - OpenAI agents: TCP socket probe (validates service availability) + - Priority: A2A > OpenAI > No probe + - No protocols: No readiness probe - Handling framework-specific configurations - **Admission Webhooks** (`internal/webhook/v1alpha1/`): Provides validation and mutation for Agent resources diff --git a/config/samples/runtime_v1alpha1_agent.yaml b/config/samples/runtime_v1alpha1_agent.yaml index 690296d..889985e 100644 --- a/config/samples/runtime_v1alpha1_agent.yaml +++ b/config/samples/runtime_v1alpha1_agent.yaml @@ -10,6 +10,7 @@ spec: image: ghcr.io/agentic-layer/weather-agent:0.3.0 protocols: - type: A2A + path: "/" replicas: 1 env: - name: PORT diff --git a/config/samples/runtime_v1alpha1_agent_template.yaml b/config/samples/runtime_v1alpha1_agent_template.yaml index 5fea972..2fd43f1 100644 --- a/config/samples/runtime_v1alpha1_agent_template.yaml +++ b/config/samples/runtime_v1alpha1_agent_template.yaml @@ -13,13 +13,15 @@ spec: subAgents: - name: summarizer_agent url: "https://agents.example.com/summarizer-agent.json" - tools: - - name: news_fetcher - url: "https://news.mcpservers.org/fetch/mcp" - - name: web_fetch - url: "https://remote.mcpservers.org/fetch/mcp" +# Uncommented for now since health probe needs working mcp tools +# tools: +# - name: news_fetcher +# url: "https://news.mcpservers.org/fetch/mcp" +# - name: web_fetch +# url: "https://remote.mcpservers.org/fetch/mcp" protocols: - type: A2A + path: "/" replicas: 1 env: - name: LOG_LEVEL diff --git a/internal/controller/agent_controller.go b/internal/controller/agent_controller.go index dbf4cfe..b526d19 100644 --- a/internal/controller/agent_controller.go +++ b/internal/controller/agent_controller.go @@ -39,6 +39,8 @@ import ( const ( agentContainerName = "agent" + agentCardEndpoint = "/.well-known/agent-card.json" + a2AProtocol = "A2A" ) // AgentReconciler reconciles a Agent object @@ -356,6 +358,98 @@ func (r *AgentReconciler) mergeEnvironmentVariables(templateEnvVars, userEnvVars return result } +// hasA2AProtocol checks if the agent has A2A protocol configured +func (r *AgentReconciler) hasA2AProtocol(agent *runtimev1alpha1.Agent) bool { + for _, protocol := range agent.Spec.Protocols { + + if protocol.Type == a2AProtocol { + return true + } + } + return false +} + +// hasOpenAIProtocol checks if the agent has OpenAI protocol configured +func (r *AgentReconciler) hasOpenAIProtocol(agent *runtimev1alpha1.Agent) bool { + for _, protocol := range agent.Spec.Protocols { + if protocol.Type == "OpenAI" { + return true + } + } + return false +} + +// getA2AProtocol returns the first A2A protocol configuration found +func (r *AgentReconciler) getA2AProtocol(agent *runtimev1alpha1.Agent) *runtimev1alpha1.AgentProtocol { + for _, protocol := range agent.Spec.Protocols { + if protocol.Type == a2AProtocol { + return &protocol + } + } + return nil +} + +// getOpenAIProtocol returns the first OpenAI protocol configuration found +func (r *AgentReconciler) getOpenAIProtocol(agent *runtimev1alpha1.Agent) *runtimev1alpha1.AgentProtocol { + for _, protocol := range agent.Spec.Protocols { + if protocol.Type == "OpenAI" { + return &protocol + } + } + return nil +} + +// getProtocolPort returns the port from protocol or default if not specified +func (r *AgentReconciler) getProtocolPort(protocol *runtimev1alpha1.AgentProtocol) int32 { + defaultPort := int32(8000) // Default port if none specified + if protocol != nil && protocol.Port != 0 { + return protocol.Port + } + return defaultPort +} + +// getA2AHealthPath returns the A2A health check path based on protocol configuration +func (r *AgentReconciler) getA2AHealthPath(protocol *runtimev1alpha1.AgentProtocol) string { + if protocol != nil && protocol.Path != "" { + // If path is explicitly specified, use it + // Special case: "/" means root path (no prefix) + if protocol.Path == "/" { + return agentCardEndpoint + } + return protocol.Path + agentCardEndpoint + } + // Default for agents without protocol specification or path + return "/a2a" + agentCardEndpoint +} + +// generateReadinessProbe generates appropriate readiness probe based on agent protocols +func (r *AgentReconciler) generateReadinessProbe(agent *runtimev1alpha1.Agent) *corev1.Probe { + // Check if agent has external dependencies (subAgents or tools) + + // Priority: A2A > OpenAI > None + if r.hasA2AProtocol(agent) { + // Use A2A agent card endpoint for health check + a2aProtocol := r.getA2AProtocol(agent) + healthPath := r.getA2AHealthPath(a2aProtocol) + port := r.getProtocolPort(a2aProtocol) + + probe := r.buildA2AReadinessProbe(healthPath, port) + + return probe + } else if r.hasOpenAIProtocol(agent) { + // Use TCP probe for OpenAI-only agents + openaiProtocol := r.getOpenAIProtocol(agent) + port := r.getProtocolPort(openaiProtocol) + + probe := r.buildOpenAIReadinessProbe(port) + + return probe + } + + // No recognized protocols - no readiness probe + return nil +} + // createDeploymentForAgent creates a deployment for the given Agent func (r *AgentReconciler) createDeploymentForAgent(agent *runtimev1alpha1.Agent, deploymentName string) (*appsv1.Deployment, error) { replicas := agent.Spec.Replicas @@ -416,11 +510,12 @@ func (r *AgentReconciler) createDeploymentForAgent(agent *runtimev1alpha1.Agent, Spec: corev1.PodSpec{ Containers: []corev1.Container{ { - Name: agentContainerName, - Image: agentImage, - Ports: containerPorts, - Env: allEnvVars, - EnvFrom: agent.Spec.EnvFrom, + Name: agentContainerName, + Image: agentImage, + Ports: containerPorts, + Env: allEnvVars, + EnvFrom: agent.Spec.EnvFrom, + ReadinessProbe: r.generateReadinessProbe(agent), }, }, }, @@ -524,6 +619,11 @@ func (r *AgentReconciler) needsDeploymentUpdate(existing, desired *appsv1.Deploy return true } + // Check readiness probes + if !r.probesEqual(existingContainer.ReadinessProbe, desiredContainer.ReadinessProbe) { + return true + } + return false } @@ -559,6 +659,7 @@ func (r *AgentReconciler) updateAgentContainer(deployment, desiredDeployment *ap agentContainer.Ports = desiredAgentContainer.Ports agentContainer.Env = desiredAgentContainer.Env agentContainer.EnvFrom = desiredAgentContainer.EnvFrom + agentContainer.ReadinessProbe = desiredAgentContainer.ReadinessProbe return nil } @@ -663,6 +764,11 @@ func (r *AgentReconciler) servicePortsEqual(existing, desired []corev1.ServicePo return true } +// probesEqual compares two readiness probes for equality +func (r *AgentReconciler) probesEqual(existing, desired *corev1.Probe) bool { + return equality.ProbesEqual(existing, desired) +} + // sanitizeAgentName sanitizes the agent name to meet environment variable naming requirements. // Environment variable names should start with a letter (a-z, A-Z) or underscore (_), // and can only contain letters, digits (0-9), and underscores. @@ -709,7 +815,7 @@ func (r *AgentReconciler) sanitizeAgentName(name string) string { func (r *AgentReconciler) buildA2AAgentCardUrl(agent *runtimev1alpha1.Agent) string { // Find the A2A protocol for _, protocol := range agent.Spec.Protocols { - if protocol.Type == "A2A" { + if protocol.Type == a2AProtocol { return fmt.Sprintf("http://%s.%s.svc.cluster.local:%d%s", agent.Name, agent.Namespace, protocol.Port, protocol.Path) } @@ -726,3 +832,36 @@ func (r *AgentReconciler) SetupWithManager(mgr ctrl.Manager) error { Named("agent"). Complete(r) } + +// buildOpenAIReadinessProbe creates TCP-readiness-probe for OpenAI-protocols +func (r *AgentReconciler) buildOpenAIReadinessProbe(port int32) *corev1.Probe { + return &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + TCPSocket: &corev1.TCPSocketAction{ + Port: intstr.FromInt(int(port)), + }, + }, + InitialDelaySeconds: 60, + PeriodSeconds: 10, + TimeoutSeconds: 3, + SuccessThreshold: 1, + FailureThreshold: 3, + } +} + +// buildA2AReadinessProbe creates HTTP-readiness-probe for A2A-protocols. +func (r *AgentReconciler) buildA2AReadinessProbe(healthPath string, port int32) *corev1.Probe { + return &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: healthPath, + Port: intstr.FromInt(int(port)), + }, + }, + InitialDelaySeconds: 60, + PeriodSeconds: 10, + TimeoutSeconds: 3, + SuccessThreshold: 1, + FailureThreshold: 3, + } +} diff --git a/internal/controller/agent_controller_test.go b/internal/controller/agent_controller_test.go index d1100b6..2760b04 100644 --- a/internal/controller/agent_controller_test.go +++ b/internal/controller/agent_controller_test.go @@ -18,15 +18,18 @@ package controller import ( "context" + "fmt" - webhookv1alpha1 "github.com/agentic-layer/agent-runtime-operator/internal/webhook/v1alpha1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + + webhookv1alpha1 "github.com/agentic-layer/agent-runtime-operator/internal/webhook/v1alpha1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/intstr" "sigs.k8s.io/controller-runtime/pkg/reconcile" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -120,6 +123,652 @@ var _ = Describe("Agent Controller", func() { }) }) + Context("When testing health check probe generation", func() { + var reconciler *AgentReconciler + + BeforeEach(func() { + reconciler = &AgentReconciler{ + Client: k8sClient, + Scheme: k8sClient.Scheme(), + } + }) + + It("should generate HTTP probe for A2A agents", func() { + agent := &runtimev1alpha1.Agent{ + Spec: runtimev1alpha1.AgentSpec{ + Protocols: []runtimev1alpha1.AgentProtocol{ + {Type: "A2A", Port: 8000}, + }, + }, + } + + probe := reconciler.generateReadinessProbe(agent) + Expect(probe).NotTo(BeNil()) + Expect(probe.HTTPGet).NotTo(BeNil()) + Expect(probe.HTTPGet.Path).To(Equal("/a2a" + agentCardEndpoint)) + Expect(probe.HTTPGet.Port.IntValue()).To(Equal(8000)) + Expect(probe.InitialDelaySeconds).To(Equal(int32(60))) + }) + + It("should generate TCP probe for OpenAI-only agents", func() { + agent := &runtimev1alpha1.Agent{ + Spec: runtimev1alpha1.AgentSpec{ + Protocols: []runtimev1alpha1.AgentProtocol{ + {Type: "OpenAI", Port: 8000}, + }, + }, + } + + probe := reconciler.generateReadinessProbe(agent) + Expect(probe).NotTo(BeNil()) + Expect(probe.TCPSocket).NotTo(BeNil()) + Expect(probe.TCPSocket.Port.IntValue()).To(Equal(8000)) + Expect(probe.InitialDelaySeconds).To(Equal(int32(60))) + }) + + It("should prioritize A2A probe over OpenAI when both protocols exist", func() { + agent := &runtimev1alpha1.Agent{ + Spec: runtimev1alpha1.AgentSpec{ + Protocols: []runtimev1alpha1.AgentProtocol{ + {Type: "OpenAI", Port: 3000}, + {Type: "A2A", Port: 8000}, + }, + }, + } + + probe := reconciler.generateReadinessProbe(agent) + Expect(probe).NotTo(BeNil()) + Expect(probe.HTTPGet).NotTo(BeNil()) + Expect(probe.HTTPGet.Path).To(Equal("/a2a" + agentCardEndpoint)) + }) + + It("should return nil probe for agents with no recognized protocols", func() { + agent := &runtimev1alpha1.Agent{ + Spec: runtimev1alpha1.AgentSpec{ + Protocols: []runtimev1alpha1.AgentProtocol{}, + }, + } + + probe := reconciler.generateReadinessProbe(agent) + Expect(probe).To(BeNil()) + }) + + It("should correctly detect A2A protocol", func() { + agent := &runtimev1alpha1.Agent{ + Spec: runtimev1alpha1.AgentSpec{ + Protocols: []runtimev1alpha1.AgentProtocol{ + {Type: "A2A", Port: 8000}, + }, + }, + } + + Expect(reconciler.hasA2AProtocol(agent)).To(BeTrue()) + Expect(reconciler.hasOpenAIProtocol(agent)).To(BeFalse()) + }) + + It("should correctly detect OpenAI protocol", func() { + agent := &runtimev1alpha1.Agent{ + Spec: runtimev1alpha1.AgentSpec{ + Protocols: []runtimev1alpha1.AgentProtocol{ + {Type: "OpenAI", Port: 3000}, + }, + }, + } + + Expect(reconciler.hasA2AProtocol(agent)).To(BeFalse()) + Expect(reconciler.hasOpenAIProtocol(agent)).To(BeTrue()) + }) + }) + + Context("When testing protocol-aware health check configuration", func() { + var reconciler *AgentReconciler + + BeforeEach(func() { + reconciler = &AgentReconciler{ + Client: k8sClient, + Scheme: k8sClient.Scheme(), + } + }) + + It("should use custom port from A2A protocol", func() { + agent := &runtimev1alpha1.Agent{ + Spec: runtimev1alpha1.AgentSpec{ + Protocols: []runtimev1alpha1.AgentProtocol{ + {Type: "A2A", Port: 3000}, + }, + }, + } + + probe := reconciler.generateReadinessProbe(agent) + Expect(probe).NotTo(BeNil()) + Expect(probe.HTTPGet).NotTo(BeNil()) + Expect(probe.HTTPGet.Port.IntValue()).To(Equal(3000)) + Expect(probe.HTTPGet.Path).To(Equal("/a2a" + agentCardEndpoint)) + }) + + It("should use custom port from OpenAI protocol", func() { + agent := &runtimev1alpha1.Agent{ + Spec: runtimev1alpha1.AgentSpec{ + Protocols: []runtimev1alpha1.AgentProtocol{ + {Type: "OpenAI", Port: 9000}, + }, + }, + } + + probe := reconciler.generateReadinessProbe(agent) + Expect(probe).NotTo(BeNil()) + Expect(probe.TCPSocket).NotTo(BeNil()) + Expect(probe.TCPSocket.Port.IntValue()).To(Equal(9000)) + }) + + It("should use custom path from A2A protocol", func() { + agent := &runtimev1alpha1.Agent{ + Spec: runtimev1alpha1.AgentSpec{ + Protocols: []runtimev1alpha1.AgentProtocol{ + {Type: "A2A", Path: "/custom"}, + }, + }, + } + + probe := reconciler.generateReadinessProbe(agent) + Expect(probe).NotTo(BeNil()) + Expect(probe.HTTPGet).NotTo(BeNil()) + Expect(probe.HTTPGet.Path).To(Equal(fmt.Sprintf("/custom%s", agentCardEndpoint))) + Expect(probe.HTTPGet.Port.IntValue()).To(Equal(8000)) // Default port + }) + + It("should use both custom path and port from A2A protocol", func() { + agent := &runtimev1alpha1.Agent{ + Spec: runtimev1alpha1.AgentSpec{ + Protocols: []runtimev1alpha1.AgentProtocol{ + {Type: "A2A", Path: "/agent-api", Port: 4000}, + }, + }, + } + + probe := reconciler.generateReadinessProbe(agent) + Expect(probe).NotTo(BeNil()) + Expect(probe.HTTPGet).NotTo(BeNil()) + Expect(probe.HTTPGet.Path).To(Equal(fmt.Sprintf("/agent-api%s", agentCardEndpoint))) + Expect(probe.HTTPGet.Port.IntValue()).To(Equal(4000)) + }) + + It("should use default values when protocol has no port or path specified", func() { + agent := &runtimev1alpha1.Agent{ + Spec: runtimev1alpha1.AgentSpec{ + Protocols: []runtimev1alpha1.AgentProtocol{ + {Type: "A2A"}, // No port or path specified + }, + }, + } + + probe := reconciler.generateReadinessProbe(agent) + Expect(probe).NotTo(BeNil()) + Expect(probe.HTTPGet).NotTo(BeNil()) + Expect(probe.HTTPGet.Path).To(Equal("/a2a" + agentCardEndpoint)) + Expect(probe.HTTPGet.Port.IntValue()).To(Equal(8000)) + }) + + It("should handle multiple protocols with different ports correctly", func() { + agent := &runtimev1alpha1.Agent{ + Spec: runtimev1alpha1.AgentSpec{ + Protocols: []runtimev1alpha1.AgentProtocol{ + {Type: "A2A", Port: 7000}, + {Type: "OpenAI", Port: 8080}, + }, + }, + } + + probe := reconciler.generateReadinessProbe(agent) + Expect(probe).NotTo(BeNil()) + Expect(probe.HTTPGet).NotTo(BeNil()) // A2A takes priority + Expect(probe.HTTPGet.Port.IntValue()).To(Equal(7000)) // A2A port used + }) + + It("should use root path when path is set to '/'", func() { + agent := &runtimev1alpha1.Agent{ + Spec: runtimev1alpha1.AgentSpec{ + Protocols: []runtimev1alpha1.AgentProtocol{ + {Type: "A2A", Path: "/", Port: 8000}, + }, + }, + } + + probe := reconciler.generateReadinessProbe(agent) + Expect(probe).NotTo(BeNil()) + Expect(probe.HTTPGet).NotTo(BeNil()) + Expect(probe.HTTPGet.Path).To(Equal(agentCardEndpoint)) + Expect(probe.HTTPGet.Port.IntValue()).To(Equal(8000)) + }) + }) + + Context("When testing new protocol helper functions", func() { + var reconciler *AgentReconciler + + BeforeEach(func() { + reconciler = &AgentReconciler{ + Client: k8sClient, + Scheme: k8sClient.Scheme(), + } + }) + + It("should get A2A protocol correctly", func() { + agent := &runtimev1alpha1.Agent{ + Spec: runtimev1alpha1.AgentSpec{ + Protocols: []runtimev1alpha1.AgentProtocol{ + {Type: "OpenAI", Port: 3000}, + {Type: "A2A", Port: 4000, Path: "/test"}, + }, + }, + } + + protocol := reconciler.getA2AProtocol(agent) + Expect(protocol).NotTo(BeNil()) + Expect(protocol.Type).To(Equal("A2A")) + Expect(protocol.Port).To(Equal(int32(4000))) + Expect(protocol.Path).To(Equal("/test")) + }) + + It("should get OpenAI protocol correctly", func() { + agent := &runtimev1alpha1.Agent{ + Spec: runtimev1alpha1.AgentSpec{ + Protocols: []runtimev1alpha1.AgentProtocol{ + {Type: "A2A", Port: 4000}, + {Type: "OpenAI", Port: 5000}, + }, + }, + } + + protocol := reconciler.getOpenAIProtocol(agent) + Expect(protocol).NotTo(BeNil()) + Expect(protocol.Type).To(Equal("OpenAI")) + Expect(protocol.Port).To(Equal(int32(5000))) + }) + + It("should return nil when protocol not found", func() { + agent := &runtimev1alpha1.Agent{ + Spec: runtimev1alpha1.AgentSpec{ + Protocols: []runtimev1alpha1.AgentProtocol{ + {Type: "OpenAI"}, + }, + }, + } + + protocol := reconciler.getA2AProtocol(agent) + Expect(protocol).To(BeNil()) + }) + + It("should get protocol port with fallback to default", func() { + protocol := &runtimev1alpha1.AgentProtocol{Type: "A2A", Port: 9000} + port := reconciler.getProtocolPort(protocol) + Expect(port).To(Equal(int32(9000))) + + // Test with zero port (should use default) + protocolNoPort := &runtimev1alpha1.AgentProtocol{Type: "A2A", Port: 0} + portDefault := reconciler.getProtocolPort(protocolNoPort) + Expect(portDefault).To(Equal(int32(8000))) + + // Test with nil protocol (should use default) + portNil := reconciler.getProtocolPort(nil) + Expect(portNil).To(Equal(int32(8000))) + }) + + It("should get A2A health path correctly", func() { + // Test with custom path + protocolWithPath := &runtimev1alpha1.AgentProtocol{Type: "A2A", Path: "/custom-mount"} + path := reconciler.getA2AHealthPath(protocolWithPath) + Expect(path).To(Equal(fmt.Sprintf("/custom-mount%s", agentCardEndpoint))) + + // Test with root path (special "/" value) + protocolRootPath := &runtimev1alpha1.AgentProtocol{Type: "A2A", Path: "/"} + pathRoot := reconciler.getA2AHealthPath(protocolRootPath) + Expect(pathRoot).To(Equal(agentCardEndpoint)) + + // Test with unspecified path field (should use default /a2a) + protocolNoPath := &runtimev1alpha1.AgentProtocol{Type: "A2A"} + pathDefault := reconciler.getA2AHealthPath(protocolNoPath) + Expect(pathDefault).To(Equal("/a2a" + agentCardEndpoint)) + + // Test with nil protocol (should use default) + pathNil := reconciler.getA2AHealthPath(nil) + Expect(pathNil).To(Equal("/a2a" + agentCardEndpoint)) + }) + }) + + Context("When testing probe comparison", func() { + var reconciler *AgentReconciler + + BeforeEach(func() { + reconciler = &AgentReconciler{ + Client: k8sClient, + Scheme: k8sClient.Scheme(), + } + }) + + It("should consider nil probes as equal", func() { + Expect(reconciler.probesEqual(nil, nil)).To(BeTrue()) + }) + + It("should consider nil and non-nil probes as not equal", func() { + probe := &corev1.Probe{InitialDelaySeconds: 10} + Expect(reconciler.probesEqual(nil, probe)).To(BeFalse()) + Expect(reconciler.probesEqual(probe, nil)).To(BeFalse()) + }) + + It("should compare HTTP probes correctly", func() { + probe1 := &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/a2a" + agentCardEndpoint, + Port: intstr.FromInt(8000), + }, + }, + InitialDelaySeconds: 10, + } + probe2 := &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/a2a" + agentCardEndpoint, + Port: intstr.FromInt(8000), + }, + }, + InitialDelaySeconds: 10, + } + + Expect(reconciler.probesEqual(probe1, probe2)).To(BeTrue()) + }) + + It("should detect different HTTP probe paths", func() { + probe1 := &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/a2a" + agentCardEndpoint, + Port: intstr.FromInt(8000), + }, + }, + } + probe2 := &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/health", + Port: intstr.FromInt(8000), + }, + }, + } + + Expect(reconciler.probesEqual(probe1, probe2)).To(BeFalse()) + }) + + It("should compare TCP probes correctly", func() { + probe1 := &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + TCPSocket: &corev1.TCPSocketAction{ + Port: intstr.FromInt(8000), + }, + }, + InitialDelaySeconds: 10, + } + probe2 := &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + TCPSocket: &corev1.TCPSocketAction{ + Port: intstr.FromInt(8000), + }, + }, + InitialDelaySeconds: 10, + } + + Expect(reconciler.probesEqual(probe1, probe2)).To(BeTrue()) + }) + + It("should detect different TCP probe ports", func() { + probe1 := &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + TCPSocket: &corev1.TCPSocketAction{ + Port: intstr.FromInt(8000), + }, + }, + } + probe2 := &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + TCPSocket: &corev1.TCPSocketAction{ + Port: intstr.FromInt(3000), + }, + }, + } + + Expect(reconciler.probesEqual(probe1, probe2)).To(BeFalse()) + }) + + It("should detect mixed probe types as different", func() { + httpProbe := &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/a2a" + agentCardEndpoint, + Port: intstr.FromInt(8000), + }, + }, + } + tcpProbe := &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + TCPSocket: &corev1.TCPSocketAction{ + Port: intstr.FromInt(8000), + }, + }, + } + + Expect(reconciler.probesEqual(httpProbe, tcpProbe)).To(BeFalse()) + }) + }) + + Context("When testing deployment updates with probe changes", func() { + var reconciler *AgentReconciler + + BeforeEach(func() { + reconciler = &AgentReconciler{ + Client: k8sClient, + Scheme: k8sClient.Scheme(), + } + }) + + It("should trigger update when probe changes from HTTP to TCP", func() { + // Create existing deployment with HTTP probe (A2A agent) + existingAgent := &runtimev1alpha1.Agent{ + Spec: runtimev1alpha1.AgentSpec{ + Framework: "google-adk", + Image: "test:v1", + Protocols: []runtimev1alpha1.AgentProtocol{ + {Type: "A2A", Port: 8000}, + }, + }, + } + existingDeployment, err := reconciler.createDeploymentForAgent(existingAgent, "test-agent") + Expect(err).NotTo(HaveOccurred()) + + // Create desired deployment with TCP probe (OpenAI agent) + desiredAgent := &runtimev1alpha1.Agent{ + Spec: runtimev1alpha1.AgentSpec{ + Framework: "google-adk", + Image: "test:v1", + Protocols: []runtimev1alpha1.AgentProtocol{ + {Type: "OpenAI", Port: 8000}, + }, + }, + } + desiredDeployment, err := reconciler.createDeploymentForAgent(desiredAgent, "test-agent") + Expect(err).NotTo(HaveOccurred()) + + // Verify update is needed due to probe change + updateNeeded := reconciler.needsDeploymentUpdate(existingDeployment, desiredDeployment) + Expect(updateNeeded).To(BeTrue()) + + // Verify existing has HTTP probe + existingContainer := reconciler.findAgentContainer(existingDeployment.Spec.Template.Spec.Containers) + Expect(existingContainer.ReadinessProbe).NotTo(BeNil()) + Expect(existingContainer.ReadinessProbe.HTTPGet).NotTo(BeNil()) + + // Verify desired has TCP probe + desiredContainer := reconciler.findAgentContainer(desiredDeployment.Spec.Template.Spec.Containers) + Expect(desiredContainer.ReadinessProbe).NotTo(BeNil()) + Expect(desiredContainer.ReadinessProbe.TCPSocket).NotTo(BeNil()) + }) + + It("should trigger update when probe path changes", func() { + // Create existing deployment with old health endpoint + replicas := int32(1) + existingDeployment := &appsv1.Deployment{ + Spec: appsv1.DeploymentSpec{ + Replicas: &replicas, + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "agent", + Image: "test:v1", + ReadinessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/health", // Old endpoint + Port: intstr.FromInt(8000), + }, + }, + }, + }, + }, + }, + }, + }, + } + + // Create desired deployment with A2A endpoint + agent := &runtimev1alpha1.Agent{ + Spec: runtimev1alpha1.AgentSpec{ + Image: "test:v1", + Protocols: []runtimev1alpha1.AgentProtocol{ + {Type: "A2A", Port: 8000}, + }, + }, + } + desiredDeployment, err := reconciler.createDeploymentForAgent(agent, "test-agent") + Expect(err).NotTo(HaveOccurred()) + + // Verify update is needed due to path change + updateNeeded := reconciler.needsDeploymentUpdate(existingDeployment, desiredDeployment) + Expect(updateNeeded).To(BeTrue()) + + // Verify path difference + existingContainer := reconciler.findAgentContainer(existingDeployment.Spec.Template.Spec.Containers) + desiredContainer := reconciler.findAgentContainer(desiredDeployment.Spec.Template.Spec.Containers) + + Expect(existingContainer.ReadinessProbe.HTTPGet.Path).To(Equal("/health")) + Expect(desiredContainer.ReadinessProbe.HTTPGet.Path).To(Equal("/a2a" + agentCardEndpoint)) + }) + + It("should not trigger update when probes are identical", func() { + // Create two identical A2A agents + agent1 := &runtimev1alpha1.Agent{ + Spec: runtimev1alpha1.AgentSpec{ + Framework: "google-adk", + Image: "test:v1", + Protocols: []runtimev1alpha1.AgentProtocol{ + {Type: "A2A", Port: 8000}, + }, + }, + } + deployment1, err := reconciler.createDeploymentForAgent(agent1, "test-agent") + Expect(err).NotTo(HaveOccurred()) + + agent2 := &runtimev1alpha1.Agent{ + Spec: runtimev1alpha1.AgentSpec{ + Framework: "google-adk", + Image: "test:v1", + Protocols: []runtimev1alpha1.AgentProtocol{ + {Type: "A2A", Port: 8000}, + }, + }, + } + deployment2, err := reconciler.createDeploymentForAgent(agent2, "test-agent") + Expect(err).NotTo(HaveOccurred()) + + // Verify no update needed + updateNeeded := reconciler.needsDeploymentUpdate(deployment1, deployment2) + Expect(updateNeeded).To(BeFalse()) + }) + + It("should trigger update when adding probe to agent without probe", func() { + // Create existing deployment without probe + replicas := int32(1) + existingDeployment := &appsv1.Deployment{ + Spec: appsv1.DeploymentSpec{ + Replicas: &replicas, + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "agent", + Image: "test:v1", + ReadinessProbe: nil, // No probe + }, + }, + }, + }, + }, + } + + // Create desired deployment with A2A probe + agent := &runtimev1alpha1.Agent{ + Spec: runtimev1alpha1.AgentSpec{ + Image: "test:v1", + Protocols: []runtimev1alpha1.AgentProtocol{ + {Type: "A2A", Port: 8000}, + }, + }, + } + desiredDeployment, err := reconciler.createDeploymentForAgent(agent, "test-agent") + Expect(err).NotTo(HaveOccurred()) + + // Verify update is needed (nil -> probe) + updateNeeded := reconciler.needsDeploymentUpdate(existingDeployment, desiredDeployment) + Expect(updateNeeded).To(BeTrue()) + }) + + It("should trigger update when removing probe from agent", func() { + // Create existing deployment with A2A probe + agent := &runtimev1alpha1.Agent{ + Spec: runtimev1alpha1.AgentSpec{ + Image: "test:v1", + Protocols: []runtimev1alpha1.AgentProtocol{ + {Type: "A2A", Port: 8000}, + }, + }, + } + existingDeployment, err := reconciler.createDeploymentForAgent(agent, "test-agent") + Expect(err).NotTo(HaveOccurred()) + + // Create desired deployment without probe + replicas2 := int32(1) + desiredDeployment := &appsv1.Deployment{ + Spec: appsv1.DeploymentSpec{ + Replicas: &replicas2, + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "agent", + Image: "test:v1", + ReadinessProbe: nil, // No probe + }, + }, + }, + }, + }, + } + + // Verify update is needed (probe -> nil) + updateNeeded := reconciler.needsDeploymentUpdate(existingDeployment, desiredDeployment) + Expect(updateNeeded).To(BeTrue()) + }) + }) + Context("When updating Agent resources", func() { const resourceName = "test-update-resource" @@ -1169,7 +1818,7 @@ var _ = Describe("Agent Controller", func() { // Find the A2A_AGENT_CARD_URL variable a2aUrlVar := findEnvVar(templateVars, "A2A_AGENT_CARD_URL") Expect(a2aUrlVar).NotTo(BeNil()) - Expect(a2aUrlVar.Value).To(Equal("http://test-agent.test-namespace.svc.cluster.local:8080/.well-known/agent-card.json")) + Expect(a2aUrlVar.Value).To(Equal(fmt.Sprintf("http://test-agent.test-namespace.svc.cluster.local:8080%s", agentCardEndpoint))) }) It("should not generate A2A_AGENT_CARD_URL when no A2A protocol is present", func() { diff --git a/internal/equality/equality.go b/internal/equality/equality.go index 192f166..80d5a42 100644 --- a/internal/equality/equality.go +++ b/internal/equality/equality.go @@ -36,3 +36,9 @@ func getEnvFromSourceKey(s corev1.EnvFromSource) string { } return "" } + +// ProbesEqual compares two Kubernetes readiness probes for equality. +// This function handles nil cases and uses deep comparison for comprehensive equality checking. +func ProbesEqual(existing, desired *corev1.Probe) bool { + return cmp.Equal(existing, desired) +} diff --git a/internal/equality/equality_test.go b/internal/equality/equality_test.go index 6e0bd5a..ae64ee7 100644 --- a/internal/equality/equality_test.go +++ b/internal/equality/equality_test.go @@ -7,6 +7,7 @@ import ( "github.com/agentic-layer/agent-runtime-operator/internal/equality" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/intstr" ) // A helper function to easily get a pointer to a bool, as required by the 'Optional' field. @@ -220,3 +221,151 @@ func TestEnvFromEqual(t *testing.T) { }) } } + +func TestProbesEqual(t *testing.T) { + testCases := []struct { + name string + a *corev1.Probe + b *corev1.Probe + want bool + }{ + { + name: "should be equal for both nil", + a: nil, + b: nil, + want: true, + }, + { + name: "should not be equal for one nil", + a: nil, + b: &corev1.Probe{InitialDelaySeconds: 10}, + want: false, + }, + { + name: "should not be equal for other nil", + a: &corev1.Probe{InitialDelaySeconds: 10}, + b: nil, + want: false, + }, + { + name: "should be equal for identical HTTP probes", + a: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/health", + Port: intstr.FromInt(8000), + }, + }, + InitialDelaySeconds: 10, + PeriodSeconds: 5, + }, + b: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/health", + Port: intstr.FromInt(8000), + }, + }, + InitialDelaySeconds: 10, + PeriodSeconds: 5, + }, + want: true, + }, + { + name: "should be equal for identical TCP probes", + a: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + TCPSocket: &corev1.TCPSocketAction{ + Port: intstr.FromInt(8000), + }, + }, + InitialDelaySeconds: 10, + }, + b: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + TCPSocket: &corev1.TCPSocketAction{ + Port: intstr.FromInt(8000), + }, + }, + InitialDelaySeconds: 10, + }, + want: true, + }, + { + name: "should not be equal for different HTTP paths", + a: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/health", + Port: intstr.FromInt(8000), + }, + }, + }, + b: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/readiness", + Port: intstr.FromInt(8000), + }, + }, + }, + want: false, + }, + { + name: "should not be equal for different ports", + a: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + TCPSocket: &corev1.TCPSocketAction{ + Port: intstr.FromInt(8000), + }, + }, + }, + b: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + TCPSocket: &corev1.TCPSocketAction{ + Port: intstr.FromInt(3000), + }, + }, + }, + want: false, + }, + { + name: "should not be equal for different probe types", + a: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/health", + Port: intstr.FromInt(8000), + }, + }, + }, + b: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + TCPSocket: &corev1.TCPSocketAction{ + Port: intstr.FromInt(8000), + }, + }, + }, + want: false, + }, + { + name: "should not be equal for different timing settings", + a: &corev1.Probe{ + InitialDelaySeconds: 10, + }, + b: &corev1.Probe{ + InitialDelaySeconds: 15, + }, + want: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + got := equality.ProbesEqual(tc.a, tc.b) + if got != tc.want { + t.Errorf("ProbesEqual() = %v, want %v", got, tc.want) + } + }) + } +} From 4e207014d7d1f6ba9ee1afc4ae2f3f61eb67a8bd Mon Sep 17 00:00:00 2001 From: Jil Kamerling Date: Thu, 2 Oct 2025 13:19:40 +0200 Subject: [PATCH 2/2] feat: PAAL-123 updated default path for readiness probe --- config/samples/runtime_v1alpha1_agent.yaml | 1 - .../runtime_v1alpha1_agent_template.yaml | 1 - internal/controller/agent_controller.go | 7 +---- internal/controller/agent_controller_test.go | 31 +++++++++---------- 4 files changed, 16 insertions(+), 24 deletions(-) diff --git a/config/samples/runtime_v1alpha1_agent.yaml b/config/samples/runtime_v1alpha1_agent.yaml index 889985e..690296d 100644 --- a/config/samples/runtime_v1alpha1_agent.yaml +++ b/config/samples/runtime_v1alpha1_agent.yaml @@ -10,7 +10,6 @@ spec: image: ghcr.io/agentic-layer/weather-agent:0.3.0 protocols: - type: A2A - path: "/" replicas: 1 env: - name: PORT diff --git a/config/samples/runtime_v1alpha1_agent_template.yaml b/config/samples/runtime_v1alpha1_agent_template.yaml index 2fd43f1..c11c5ae 100644 --- a/config/samples/runtime_v1alpha1_agent_template.yaml +++ b/config/samples/runtime_v1alpha1_agent_template.yaml @@ -21,7 +21,6 @@ spec: # url: "https://remote.mcpservers.org/fetch/mcp" protocols: - type: A2A - path: "/" replicas: 1 env: - name: LOG_LEVEL diff --git a/internal/controller/agent_controller.go b/internal/controller/agent_controller.go index b526d19..1bb1869 100644 --- a/internal/controller/agent_controller.go +++ b/internal/controller/agent_controller.go @@ -411,15 +411,10 @@ func (r *AgentReconciler) getProtocolPort(protocol *runtimev1alpha1.AgentProtoco // getA2AHealthPath returns the A2A health check path based on protocol configuration func (r *AgentReconciler) getA2AHealthPath(protocol *runtimev1alpha1.AgentProtocol) string { if protocol != nil && protocol.Path != "" { - // If path is explicitly specified, use it - // Special case: "/" means root path (no prefix) - if protocol.Path == "/" { - return agentCardEndpoint - } return protocol.Path + agentCardEndpoint } // Default for agents without protocol specification or path - return "/a2a" + agentCardEndpoint + return agentCardEndpoint } // generateReadinessProbe generates appropriate readiness probe based on agent protocols diff --git a/internal/controller/agent_controller_test.go b/internal/controller/agent_controller_test.go index 2760b04..2e4f65b 100644 --- a/internal/controller/agent_controller_test.go +++ b/internal/controller/agent_controller_test.go @@ -18,7 +18,6 @@ package controller import ( "context" - "fmt" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -145,7 +144,7 @@ var _ = Describe("Agent Controller", func() { probe := reconciler.generateReadinessProbe(agent) Expect(probe).NotTo(BeNil()) Expect(probe.HTTPGet).NotTo(BeNil()) - Expect(probe.HTTPGet.Path).To(Equal("/a2a" + agentCardEndpoint)) + Expect(probe.HTTPGet.Path).To(Equal(agentCardEndpoint)) Expect(probe.HTTPGet.Port.IntValue()).To(Equal(8000)) Expect(probe.InitialDelaySeconds).To(Equal(int32(60))) }) @@ -179,7 +178,7 @@ var _ = Describe("Agent Controller", func() { probe := reconciler.generateReadinessProbe(agent) Expect(probe).NotTo(BeNil()) Expect(probe.HTTPGet).NotTo(BeNil()) - Expect(probe.HTTPGet.Path).To(Equal("/a2a" + agentCardEndpoint)) + Expect(probe.HTTPGet.Path).To(Equal(agentCardEndpoint)) }) It("should return nil probe for agents with no recognized protocols", func() { @@ -230,11 +229,11 @@ var _ = Describe("Agent Controller", func() { } }) - It("should use custom port from A2A protocol", func() { + It("should use custom port from protocol", func() { agent := &runtimev1alpha1.Agent{ Spec: runtimev1alpha1.AgentSpec{ Protocols: []runtimev1alpha1.AgentProtocol{ - {Type: "A2A", Port: 3000}, + {Type: "A2A", Port: 3000, Path: "/a2a"}, }, }, } @@ -273,7 +272,7 @@ var _ = Describe("Agent Controller", func() { probe := reconciler.generateReadinessProbe(agent) Expect(probe).NotTo(BeNil()) Expect(probe.HTTPGet).NotTo(BeNil()) - Expect(probe.HTTPGet.Path).To(Equal(fmt.Sprintf("/custom%s", agentCardEndpoint))) + Expect(probe.HTTPGet.Path).To(Equal("/custom" + agentCardEndpoint)) Expect(probe.HTTPGet.Port.IntValue()).To(Equal(8000)) // Default port }) @@ -289,7 +288,7 @@ var _ = Describe("Agent Controller", func() { probe := reconciler.generateReadinessProbe(agent) Expect(probe).NotTo(BeNil()) Expect(probe.HTTPGet).NotTo(BeNil()) - Expect(probe.HTTPGet.Path).To(Equal(fmt.Sprintf("/agent-api%s", agentCardEndpoint))) + Expect(probe.HTTPGet.Path).To(Equal("/agent-api" + agentCardEndpoint)) Expect(probe.HTTPGet.Port.IntValue()).To(Equal(4000)) }) @@ -305,7 +304,7 @@ var _ = Describe("Agent Controller", func() { probe := reconciler.generateReadinessProbe(agent) Expect(probe).NotTo(BeNil()) Expect(probe.HTTPGet).NotTo(BeNil()) - Expect(probe.HTTPGet.Path).To(Equal("/a2a" + agentCardEndpoint)) + Expect(probe.HTTPGet.Path).To(Equal(agentCardEndpoint)) Expect(probe.HTTPGet.Port.IntValue()).To(Equal(8000)) }) @@ -337,7 +336,7 @@ var _ = Describe("Agent Controller", func() { probe := reconciler.generateReadinessProbe(agent) Expect(probe).NotTo(BeNil()) Expect(probe.HTTPGet).NotTo(BeNil()) - Expect(probe.HTTPGet.Path).To(Equal(agentCardEndpoint)) + Expect(probe.HTTPGet.Path).To(Equal("/" + agentCardEndpoint)) Expect(probe.HTTPGet.Port.IntValue()).To(Equal(8000)) }) }) @@ -417,21 +416,21 @@ var _ = Describe("Agent Controller", func() { // Test with custom path protocolWithPath := &runtimev1alpha1.AgentProtocol{Type: "A2A", Path: "/custom-mount"} path := reconciler.getA2AHealthPath(protocolWithPath) - Expect(path).To(Equal(fmt.Sprintf("/custom-mount%s", agentCardEndpoint))) + Expect(path).To(Equal("/custom-mount" + agentCardEndpoint)) // Test with root path (special "/" value) - protocolRootPath := &runtimev1alpha1.AgentProtocol{Type: "A2A", Path: "/"} + protocolRootPath := &runtimev1alpha1.AgentProtocol{Type: "A2A", Path: "/a2a"} pathRoot := reconciler.getA2AHealthPath(protocolRootPath) - Expect(pathRoot).To(Equal(agentCardEndpoint)) + Expect(pathRoot).To(Equal("/a2a" + agentCardEndpoint)) // Test with unspecified path field (should use default /a2a) protocolNoPath := &runtimev1alpha1.AgentProtocol{Type: "A2A"} pathDefault := reconciler.getA2AHealthPath(protocolNoPath) - Expect(pathDefault).To(Equal("/a2a" + agentCardEndpoint)) + Expect(pathDefault).To(Equal(agentCardEndpoint)) // Test with nil protocol (should use default) pathNil := reconciler.getA2AHealthPath(nil) - Expect(pathNil).To(Equal("/a2a" + agentCardEndpoint)) + Expect(pathNil).To(Equal(agentCardEndpoint)) }) }) @@ -660,7 +659,7 @@ var _ = Describe("Agent Controller", func() { desiredContainer := reconciler.findAgentContainer(desiredDeployment.Spec.Template.Spec.Containers) Expect(existingContainer.ReadinessProbe.HTTPGet.Path).To(Equal("/health")) - Expect(desiredContainer.ReadinessProbe.HTTPGet.Path).To(Equal("/a2a" + agentCardEndpoint)) + Expect(desiredContainer.ReadinessProbe.HTTPGet.Path).To(Equal(agentCardEndpoint)) }) It("should not trigger update when probes are identical", func() { @@ -1818,7 +1817,7 @@ var _ = Describe("Agent Controller", func() { // Find the A2A_AGENT_CARD_URL variable a2aUrlVar := findEnvVar(templateVars, "A2A_AGENT_CARD_URL") Expect(a2aUrlVar).NotTo(BeNil()) - Expect(a2aUrlVar.Value).To(Equal(fmt.Sprintf("http://test-agent.test-namespace.svc.cluster.local:8080%s", agentCardEndpoint))) + Expect(a2aUrlVar.Value).To(Equal("http://test-agent.test-namespace.svc.cluster.local:8080" + agentCardEndpoint)) }) It("should not generate A2A_AGENT_CARD_URL when no A2A protocol is present", func() {