Skip to content

refactor(proto): move phase and current_policy_version into status#1565

Open
derekwaynecarr wants to merge 2 commits into
NVIDIA:mainfrom
derekwaynecarr:feat/sandbox-spec-status-split
Open

refactor(proto): move phase and current_policy_version into status#1565
derekwaynecarr wants to merge 2 commits into
NVIDIA:mainfrom
derekwaynecarr:feat/sandbox-spec-status-split

Conversation

@derekwaynecarr
Copy link
Copy Markdown
Collaborator

Summary

Move phase and current_policy_version from SandboxSpec into SandboxStatus to correctly model mutable runtime state. Update all callers in the gateway server, TUI, and Python SDK to read and write these fields through SandboxStatus accessors.

Related Issue

Changes

Testing

  • [x ] mise run pre-commit passes
  • [ x] Unit tests added/updated
  • [ x] E2E tests added/updated (if applicable)

Checklist

  • [ x] Follows Conventional Commits
  • [ x] Commits are signed off (DCO)
  • Architecture docs updated (if applicable)

…atus

Move phase and current_policy_version from SandboxSpec into
SandboxStatus to correctly model mutable runtime state. Update all
callers in the gateway server, TUI, and Python SDK to read and write
these fields through SandboxStatus accessors.

Signed-off-by: Derek Carr <decarr@redhat.com>
@derekwaynecarr derekwaynecarr requested review from a team, maxamillion and mrunalp as code owners May 26, 2026 14:49
@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented May 26, 2026

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@derekwaynecarr
Copy link
Copy Markdown
Collaborator Author

/ok to test 8334767


let cpv = sandbox.current_policy_version();
let mut status = incoming
.status
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the incoming status is None, it looks like we will set sandbox.status to that on line 979 then on line 980 set_phase() is going to create a brand-new SandboxStatus and reset cpv to 0?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there's a chance the k8s driver could send a statusless update

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah @mrunalp found similar, updated patch.

When a driver update arrives without a status payload (e.g. before
Kubernetes populates the status subresource), preserve the stored
phase, conditions, and current policy version instead of resetting
them. Adds a regression test covering the edge case.

Signed-off-by: Derek Carr <decarr@redhat.com>
@derekwaynecarr
Copy link
Copy Markdown
Collaborator Author

/ok to test 122942e

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants