feat: Update sandbox status with phase by barney-s · Pull Request #121 · kubernetes-sigs/agent-sandbox

barney-s · 2025-10-27T22:05:50Z

Adds a 'Phase' field to the 'SandboxStatus' to provide a more explicit status representation of the sandbox.

The phase can be one of 'Pending', 'Running', 'Paused', 'Terminating', or 'Failed'.

This addresses issue #119.

netlify · 2025-10-27T22:05:56Z

✅ Deploy Preview for agent-sandbox canceled.

Name	Link
🔨 Latest commit	`e8124f2`
🔍 Latest deploy log	https://app.netlify.com/projects/agent-sandbox/deploys/698e69505ad63b0008b198c8

controllers/sandbox_controller.go

hzxuzhonghu · 2025-11-21T07:15:58Z

@justinsb can you approve, we want to use this explicit status

api/v1alpha1/sandbox_types.go

Adds a 'Phase' field to the 'SandboxStatus' to provide a more explicit status representation of the sandbox. The phase can be one of 'Pending', 'Running', 'Paused', 'Terminating', or 'Failed'. This addresses issue kubernetes-sigs#119.

igooch

lgtm

vicentefb · 2026-02-13T18:20:29Z

controllers/sandbox_controller.go

 	// If the sandbox is being deleted, do nothing
 	if !sandbox.ObjectMeta.DeletionTimestamp.IsZero() {
 		log.Info("Sandbox is being deleted")
+		//sandbox.Status.Phase = sandboxv1alpha1.SandboxPhaseTerminating


nit: is this comment intended ?

vicentefb · 2026-02-13T18:21:01Z

/lgtm

janetkuo

There's a violation of K8s API convention.

janetkuo · 2026-02-13T22:06:51Z

api/v1alpha1/sandbox_types.go

+	// Terminating: The Sandbox is terminating.
+	//
+	// +optional
+	Phase SandboxPhase `json:"phase,omitempty"`


Using phase is a violation of K8s API convention. From https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#typical-status-properties:

The pattern of using phase is deprecated. Newer API types should use conditions instead. Phase was essentially a state-machine enumeration field, that contradicted system-design principles and hampered evolution, since adding new enum values breaks backward compatibility. Rather than encouraging clients to infer implicit properties from phases, we prefer to explicitly expose the individual conditions that clients need to monitor. Conditions also have the benefit that it is possible to create some conditions with uniform meaning across all resource types, while still exposing others that are unique to specific resource types. See #7856 for more details and discussion.

Clients need to monitor the individual conditions that the controller exposes.

Do you propose one condition per phase with true/false ? Or a single condition that is always true but reason is the phase.

That would be just a phase with a different field name. We need to avoid the anti-pattern of using enum, i.e. clients must know all possible reasons to understand the state. Also, condition status needs to be true/false/unknown.
Conditions should describe what's true about the sandbox at a given time, and they can have overlapping states.

For example, we should have these conditions (some from standard conditions): Initialized, Ready, Expired, Terminating.

Paused/Failed should be represented by Ready by setting it to false with appropriate Reason and Message.

As part of the Suspend and resume operation, we want to include very clear condition on when the Sandbox is being Suspended.
We can have a suspended condition with status True or False.

More details on the lifecycle of the condition during a suspend and resume operation: Design.

janetkuo · 2026-02-13T22:11:41Z

controllers/sandbox_controller.go

 		if err := r.Delete(ctx, sandbox); err != nil && !k8serrors.IsNotFound(err) {
 			allErrors = errors.Join(allErrors, fmt.Errorf("failed to delete sandbox: %w", err))
 		} else {
+			sandbox.Status.Phase = sandboxv1alpha1.SandboxPhaseTerminating


This only changes the in-memory sandbox. Will this sandbox status be updated?

vicentefb · 2026-02-17T17:18:48Z

/assign @janetkuo

janetkuo · 2026-02-17T17:50:17Z

api/v1alpha1/sandbox_types.go

+	// Terminating: The Sandbox is terminating.
+	//
+	// +optional
+	Phase SandboxPhase `json:"phase,omitempty"`


That would be just a phase with a different field name. We need to avoid the anti-pattern of using enum, i.e. clients must know all possible reasons to understand the state. Also, condition status needs to be true/false/unknown.
Conditions should describe what's true about the sandbox at a given time, and they can have overlapping states.

For example, we should have these conditions (some from standard conditions): Initialized, Ready, Expired, Terminating.

Paused/Failed should be represented by Ready by setting it to false with appropriate Reason and Message.

k8s-ci-robot · 2026-02-17T17:58:26Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: barney-s, hzxuzhonghu, igooch
Once this PR has been reviewed and has the lgtm label, please ask for approval from janetkuo. 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

k8s-ci-robot · 2026-03-19T19:08:31Z

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.

k8s-ci-robot · 2026-03-19T19:46:23Z

@barney-s: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
presubmit-agent-sandbox-e2e-test	`e8124f2`	link	true	`/test presubmit-agent-sandbox-e2e-test`
presubmit-agent-sandbox-lint-api	`e8124f2`	link	true	`/test presubmit-agent-sandbox-lint-api`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

aditya-shantanu · 2026-03-31T23:06:57Z

/lgtm

igooch · 2026-03-31T23:50:10Z

Can this be closed in favor of #422?

aditya-shantanu · 2026-04-01T02:43:45Z

+1. #422 is the latest one.

We should close this one.
This is an old inactive pr.

/close

k8s-ci-robot · 2026-04-01T02:43:52Z

@aditya-shantanu: Closed this PR.

Details

In response to this:

+1. #422 is the latest one.

We should close this one.
This is an old inactive pr.

/close

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 added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Oct 27, 2025

k8s-ci-robot requested review from justinsb and soltysh October 27, 2025 22:05

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Oct 27, 2025

barney-s force-pushed the issue-119-fixes branch from c175169 to f1024c1 Compare October 27, 2025 22:36

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 27, 2025

barney-s force-pushed the issue-119-fixes branch 2 times, most recently from 7f541c2 to 84ed9ec Compare October 27, 2025 23:04

LiZhenCheng9527 reviewed Oct 28, 2025

View reviewed changes

controllers/sandbox_controller.go Show resolved Hide resolved

barney-s requested a review from LiZhenCheng9527 October 28, 2025 05:27

barney-s force-pushed the issue-119-fixes branch from 84ed9ec to cb934d6 Compare October 28, 2025 09:10

k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 28, 2025

barney-s force-pushed the issue-119-fixes branch from cb934d6 to 5429888 Compare October 28, 2025 10:06

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 28, 2025

barney-s force-pushed the issue-119-fixes branch from 5429888 to 75760ac Compare October 28, 2025 10:52

k8s-ci-robot added the do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. label Oct 28, 2025

barney-s force-pushed the issue-119-fixes branch from 75760ac to 7b3914d Compare October 28, 2025 12:31

k8s-ci-robot removed the do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. label Oct 28, 2025

barney-s force-pushed the issue-119-fixes branch 3 times, most recently from 50a76b7 to b3dd8d6 Compare October 28, 2025 15:27

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 28, 2025

barney-s force-pushed the issue-119-fixes branch from b3dd8d6 to ddd4ea8 Compare October 28, 2025 17:01

k8s-ci-robot added do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 28, 2025

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 26, 2025

igooch reviewed Feb 10, 2026

View reviewed changes

api/v1alpha1/sandbox_types.go Show resolved Hide resolved

feat: Update sandbox status with phase

e8124f2

Adds a 'Phase' field to the 'SandboxStatus' to provide a more explicit status representation of the sandbox. The phase can be one of 'Pending', 'Running', 'Paused', 'Terminating', or 'Failed'. This addresses issue kubernetes-sigs#119.

barney-s force-pushed the issue-119-fixes branch from 84ed9ec to e8124f2 Compare February 12, 2026 23:59

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

igooch approved these changes Feb 13, 2026

View reviewed changes

vicentefb reviewed Feb 13, 2026

View reviewed changes

k8s-ci-robot assigned vicentefb Feb 13, 2026

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 13, 2026

janetkuo requested changes Feb 13, 2026

View reviewed changes

k8s-ci-robot assigned janetkuo Feb 17, 2026

janetkuo requested changes Feb 17, 2026

View reviewed changes

janetkuo mentioned this pull request Feb 17, 2026

feat: Implement ProgressDeadlineSeconds for Sandbox Resource #307

Open

vicentefb mentioned this pull request Feb 18, 2026

Add status method to Sandbox Client and also add unit tests associated with it. #280

Closed

janetkuo removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 2, 2026

SHRUTI6991 mentioned this pull request Mar 16, 2026

Implement Sandbox status via conditions #422

Open

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

k8s-ci-robot assigned aditya-shantanu Mar 31, 2026

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 31, 2026

k8s-ci-robot closed this Apr 1, 2026

Conversation

barney-s commented Oct 27, 2025

Uh oh!

netlify bot commented Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for agent-sandbox canceled.

Uh oh!

Uh oh!

hzxuzhonghu commented Nov 21, 2025

Uh oh!

Uh oh!

igooch left a comment

Choose a reason for hiding this comment

Uh oh!

vicentefb Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

vicentefb commented Feb 13, 2026

Uh oh!

janetkuo left a comment

Choose a reason for hiding this comment

Uh oh!

janetkuo Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

barney-s Feb 14, 2026

Choose a reason for hiding this comment

Uh oh!

janetkuo Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

SHRUTI6991 Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

janetkuo Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

vicentefb commented Feb 17, 2026

Uh oh!

janetkuo Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Feb 17, 2026

Uh oh!

k8s-ci-robot commented Mar 19, 2026

Uh oh!

k8s-ci-robot commented Mar 19, 2026

Uh oh!

aditya-shantanu commented Mar 31, 2026

Uh oh!

igooch commented Mar 31, 2026

Uh oh!

aditya-shantanu commented Apr 1, 2026

Uh oh!

k8s-ci-robot commented Apr 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

netlify bot commented Oct 27, 2025 •

edited

Loading

SHRUTI6991 Feb 25, 2026 •

edited

Loading