Skip to content

Conversation

@Pangjiping
Copy link
Collaborator

Summary

  • get kubernetes resource by informer

Testing

  • Not run (explain why)
  • Unit tests
  • Integration tests
  • e2e / manual verification

Breaking Changes

  • None
  • Yes (describe impact and migration path)

Checklist

@Pangjiping Pangjiping requested a review from hittyt February 10, 2026 03:44
@jwx0925
Copy link
Collaborator

jwx0925 commented Feb 13, 2026

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e20519564f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +323 to +325
cached = informer.get(sandbox_id)
if cached:
return cached

Choose a reason for hiding this comment

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

P1 Badge Skip stale cache reads when informer is unsynced

This returns cached data before checking informer sync state, so after any watch/list failure (where _has_synced is flipped to false in WorkloadInformer._run) get_workload can still serve stale objects and never fall back to a direct API read for that sandbox ID. In practice, a sandbox deleted or changed while the informer is degraded can still be reported as existing here, which breaks read-after-delete and status accuracy for the agent-sandbox path.

Useful? React with 👍 / 👎.

Comment on lines +591 to +593
cached = informer.get(sandbox_id)
if cached:
return cached

Choose a reason for hiding this comment

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

P1 Badge Avoid serving cache entries after informer desync

The batchsandbox provider has the same stale-read path: cached objects are returned unconditionally before consulting has_synced, so once the informer loses sync it may continue returning outdated entries for existing cache keys and skip the API fallback. This can surface deleted BatchSandbox resources as still present until a successful resync, causing incorrect 200/404 behavior in production reads.

Useful? React with 👍 / 👎.

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

Labels

component/server feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FEATURE: use informers to listen for Kubernetes object changes.

4 participants