feat: set service ports when containerPort is set by flpanbin · Pull Request #547 · kubernetes-sigs/agent-sandbox

flpanbin · 2026-04-08T07:18:39Z

related to #154

Populate headless Services with port definitions when the sandbox pod template declares container ports.
Deduplicate ports per protocol, default to TCP when unspecified, and reuse container port names in the Service spec.

services like:

bin % kubectl -n test-abc get svc
NAME                TYPE        CLUSTER-IP   EXTERNAL-IP   PORT(S)   AGE
sandbox-clusterip   ClusterIP   None         <none>        80/TCP    1s

Some details:

I added uniqueness handling for service port names. When duplicate containerPort.Name values appear across containers, the controller now generates a unique service port name (e.g. <name>-<port>-<protocol>) to avoid collisions.
I added a simple opt-in switch via annotation (agents.x-k8s.io/auto-expose-ports: "true"). When unset/false, the controller creates the headless Service without ports; when true, it auto-populates ports from the pod template.

netlify · 2026-04-08T07:18:48Z

✅ Deploy Preview for agent-sandbox canceled.

Name	Link
🔨 Latest commit	`1098b30`
🔍 Latest deploy log	https://app.netlify.com/projects/agent-sandbox/deploys/69e3332b4918520008e0a5c0

aditya-shantanu · 2026-04-08T20:22:48Z

/assign @barney-s

aditya-shantanu · 2026-04-14T04:49:29Z

/ok-to-test

Signed-off-by: bin <bin.pan@daocloud.io>

k8s-ci-robot · 2026-04-18T07:30:55Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: flpanbin
Once this PR has been reviewed and has the lgtm label, please ask for approval from barney-s. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copilot

Pull request overview

This PR adds an opt-in mechanism for the Sandbox controller to populate headless Service.spec.ports based on the containerPort declarations in the Sandbox pod template, addressing Istio’s requirement for Service port declarations.

Changes:

Adds agents.x-k8s.io/auto-expose-ports: "true" annotation to enable auto-populating headless Service ports.
Implements port extraction/deduplication (per <port>/<protocol>) and collision-safe port naming.
Adds unit tests validating Service port generation behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

File	Description
controllers/sandbox_controller.go	Adds annotation gate + port-building helpers and wires them into Service creation.
controllers/sandbox_controller_test.go	Adds tests for Service port population and name collision handling.

Copilot · 2026-04-21T17:04:20Z

+	if shouldAutoExposePorts(sandbox) {
+		ports := buildServicePorts(&sandbox.Spec.PodTemplate.Spec)
+		if len(ports) > 0 {
+			service.Spec.Ports = ports
+		}
+	}


Service port auto-population is only applied on the create path. If the Service already exists (owned by the Sandbox), enabling/removing the agents.x-k8s.io/auto-expose-ports annotation later (or changing container ports) will never update service.Spec.Ports, so the Service can remain stale forever. Consider reconciling Spec.Ports in the resourceOwnedBySandbox path as well (and likely clearing ports when annotation is unset).

Copilot · 2026-04-21T17:04:21Z

+			port := corev1.ServicePort{
+				Port:       containerPort.ContainerPort,
+				Protocol:   protocol,
+				TargetPort: intstr.FromInt32(containerPort.ContainerPort),
+			}
+
+			if containerPort.Name != "" {
+				name := containerPort.Name
+				if _, exists := seenNames[name]; exists {
+					name = uniqueServicePortName(name, containerPort.ContainerPort, protocol)
+				}
+				port.Name = name
+				seenNames[name] = struct{}{}
+			}


buildServicePorts only sets ServicePort.Name when containerPort.Name is non-empty. If a Pod template declares multiple container ports without names (common), this will produce a Service with multiple unnamed ports, which the API server rejects (port names are required when there is more than one Service port). Suggest generating a deterministic name when containerPort.Name is empty (e.g., based on port/protocol) and still enforcing uniqueness.

Copilot · 2026-04-21T17:04:21Z

+const maxServicePortNameLength = 63
+
+func shouldAutoExposePorts(sandbox *sandboxv1alpha1.Sandbox) bool {
+	if sandbox == nil {
+		return false
+	}
+	value, ok := sandbox.Annotations[SandboxExposePortsAnnotation]
+	if !ok {
+		return false
+	}
+	return strings.EqualFold(value, "true")
+}
+
+func uniqueServicePortName(base string, port int32, protocol corev1.Protocol) string {
+	protocolLower := strings.ToLower(string(protocol))
+	suffix := fmt.Sprintf("-%d-%s", port, protocolLower)
+	maxBase := maxServicePortNameLength - len(suffix)
+	if len(base) > maxBase {
+		base = base[:maxBase]
+	}
+	return base + suffix
+}


maxServicePortNameLength is set to 63, but ServicePort.Name is validated as an IANA service name (max 15 chars). On name collisions, uniqueServicePortName can easily exceed that limit (e.g., a 15-char container port name + "-8080-tcp"), causing Service create/update to be rejected. Consider capping to the Service port name limit (15) and truncating the base accordingly (or using a short hash) while keeping the name unique.

Copilot · 2026-04-21T17:04:21Z

+	sandboxLabel             = "agents.x-k8s.io/sandbox-name-hash"
+	SandboxPodNameAnnotation = "agents.x-k8s.io/pod-name"


SandboxPodNameAnnotation is introduced here but the controller code continues to use sandboxv1alpha1.SandboxPodNameAnnotation (see resolvePodName). Having two constants for the same annotation key in different packages is confusing and makes it easy for them to drift. Consider removing this constant or switching to a single source of truth (preferably the API package constant).

Suggested change

sandboxLabel = "agents.x-k8s.io/sandbox-name-hash"

SandboxPodNameAnnotation = "agents.x-k8s.io/pod-name"

sandboxLabel = "agents.x-k8s.io/sandbox-name-hash"

Copilot · 2026-04-21T17:04:22Z

+			name: "returns existing service without changes",
+			initialObjs: []runtime.Object{


Test case name is misleading: this scenario appears to adopt an unowned Service by adding an OwnerReference (and bumps ResourceVersion), so it isn't "without changes". Renaming it to reflect adoption would make failures easier to interpret.

Copilot · 2026-04-21T17:04:22Z

+	if shouldAutoExposePorts(sandbox) {
+		ports := buildServicePorts(&sandbox.Spec.PodTemplate.Spec)
+		if len(ports) > 0 {
+			service.Spec.Ports = ports
+		}
+	}


Current tests cover Service creation with ports, but they don't cover key reconcile edge cases introduced by this feature: (1) toggling the annotation after a Service already exists (owned by the Sandbox) and (2) multiple unnamed container ports, which should still result in a valid Service (requires generated names). Adding unit tests for these cases would help prevent regressions.

k8s-ci-robot requested review from barney-s and justinsb April 8, 2026 07:18

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 8, 2026

flpanbin force-pushed the support-svc-container-port branch from eb72d0e to aa12a0d Compare April 8, 2026 07:51

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 8, 2026

flpanbin mentioned this pull request Apr 8, 2026

feat: set service ports when containerPort is set #156

Closed

flpanbin changed the title ~~set service ports when containerPort is set~~ feat: set service ports when containerPort is set Apr 8, 2026

k8s-ci-robot assigned barney-s Apr 8, 2026

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Apr 14, 2026

set service ports when containerPort is set

1098b30

Signed-off-by: bin <bin.pan@daocloud.io>

flpanbin force-pushed the support-svc-container-port branch from aa12a0d to 1098b30 Compare April 18, 2026 07:30

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 18, 2026

janetkuo requested a review from Copilot April 21, 2026 17:00

Copilot started reviewing on behalf of janetkuo April 21, 2026 17:00 View session

Copilot AI reviewed Apr 21, 2026

View reviewed changes

janetkuo added the action-required: resolve-copilot-comments label Apr 21, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: set service ports when containerPort is set#547

feat: set service ports when containerPort is set#547
flpanbin wants to merge 1 commit intokubernetes-sigs:mainfrom
flpanbin:support-svc-container-port

flpanbin commented Apr 8, 2026

Uh oh!

netlify Bot commented Apr 8, 2026 •

edited

Loading

Uh oh!

aditya-shantanu commented Apr 8, 2026

Uh oh!

aditya-shantanu commented Apr 14, 2026

Uh oh!

k8s-ci-robot commented Apr 18, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 21, 2026

Uh oh!

Copilot AI Apr 21, 2026

Uh oh!

Copilot AI Apr 21, 2026

Uh oh!

Copilot AI Apr 21, 2026

Uh oh!

Copilot AI Apr 21, 2026

Uh oh!

Copilot AI Apr 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

		sandboxLabel = "agents.x-k8s.io/sandbox-name-hash"
		SandboxPodNameAnnotation = "agents.x-k8s.io/pod-name"

		name: "returns existing service without changes",
		initialObjs: []runtime.Object{

Conversation

flpanbin commented Apr 8, 2026

Uh oh!

netlify Bot commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for agent-sandbox canceled.

Uh oh!

aditya-shantanu commented Apr 8, 2026

Uh oh!

aditya-shantanu commented Apr 14, 2026

Uh oh!

k8s-ci-robot commented Apr 18, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

netlify Bot commented Apr 8, 2026 •

edited

Loading