Skip to content

feat: propagate podTemplate annotations from Sandbox to Pod#517

Open
noeljackson wants to merge 1 commit intokubernetes-sigs:mainfrom
noeljackson:pr/sandbox-pod-annotation-propagation
Open

feat: propagate podTemplate annotations from Sandbox to Pod#517
noeljackson wants to merge 1 commit intokubernetes-sigs:mainfrom
noeljackson:pr/sandbox-pod-annotation-propagation

Conversation

@noeljackson
Copy link
Copy Markdown
Contributor

Summary

Extend the existing label-sync loop in reconcilePod to also propagate annotations from Sandbox.Spec.PodTemplate.ObjectMeta.Annotations to the running Pod. This is a no-restart, in-place metadata update.

Split out from #459 per maintainer request to keep the annotation propagation discussion separate from the workspaceResources feature.

Why in-place propagation is the right model for Sandbox

justinsb raised a good question in #459 about whether in-place propagation sets a precedent for Deployments/StatefulSets. It doesn't, because the lifecycle models are fundamentally different:

  • Deployments/StatefulSets manage replica sets. PodTemplate changes trigger rolling updates — new Pods replace old ones. That's deliberate: stateless workloads benefit from gradual rollout, canary detection, and rollback.
  • Sandbox is 1:1 — one CR, one Pod. There is no replica set, no rolling update, no rollout strategy. The Pod is the Sandbox. When the Sandbox spec changes, the only sensible thing is to update the Pod in place.

Kubernetes already supports this:

  • Labels and annotations are mutable on Pods (always have been)
  • InPlacePodVerticalScaling (GA path since KEP-1287) makes container resources mutable too

The existing reconcilePod already syncs labels this way. This PR adds annotations to the same loop — no new pattern, just extending existing behavior to another mutable field.

This does not change behavior for Deployments, StatefulSets, or any other workload controller.

Test plan

  • Existing label propagation tests still pass
  • New test: annotations from Sandbox podTemplate are synced to Pod
  • New test: adopted pod gets annotations from Sandbox
  • Pods with existing annotations preserve them (additive merge)

Files

  • controllers/sandbox_controller.go — extend reconcilePod annotation sync
  • controllers/sandbox_controller_test.go — test coverage

When a Sandbox CR's PodTemplate carries annotations, the sandbox
controller now syncs them to the running Pod — the same way it
already syncs labels. This is a no-restart, in-place metadata update.

Sandbox manages a 1:1 relationship (one CR → one Pod), not a replica
set. Unlike Deployments/StatefulSets where podTemplate changes trigger
rolling replacements, Sandbox has no rollout concept — in-place
mutation of mutable Pod fields (labels, annotations, and with
InPlacePodVerticalScaling, resources) is the natural propagation
model for a single-pod controller.

This does not change behavior for Deployments or any other workload
controller. It simply extends the existing label-sync loop in
reconcilePod to also cover annotations.
@netlify
Copy link
Copy Markdown

netlify bot commented Apr 3, 2026

Deploy Preview for agent-sandbox canceled.

Name Link
🔨 Latest commit 9844334
🔍 Latest deploy log https://app.netlify.com/projects/agent-sandbox/deploys/69cfb403d18f1b0008c8bd33

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: noeljackson
Once this PR has been reviewed and has the lgtm label, please assign janetkuo for approval. 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:

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

@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Apr 3, 2026
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

Hi @noeljackson. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 3, 2026
noeljackson added a commit to noeljackson/agent-sandbox that referenced this pull request Apr 3, 2026
@justinsb
Copy link
Copy Markdown
Contributor

justinsb commented Apr 3, 2026

Thanks for opening / splitting out @noeljackson

I think this is a great place to have the discussion about how the apps controllers should behave here, and whether it's OK to deviate for sandbox. The point about Sandbox:Pod being a tighter coupling than Daemonset/Deployment/Statefulset is a good one!

cc @janetkuo @soltysh

@justinsb
Copy link
Copy Markdown
Contributor

justinsb commented Apr 3, 2026

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 3, 2026
@justinsb
Copy link
Copy Markdown
Contributor

justinsb commented Apr 3, 2026

/hold for discussion though (that's why we're splitting it out)

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Apr 3, 2026
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants