Skip to content

feat(sandboxid): implement core short sandbox ID logic#657

Open
singhsrijan46 wants to merge 2 commits into
openkruise:masterfrom
singhsrijan46:feat/core-short-sandbox-id
Open

feat(sandboxid): implement core short sandbox ID logic#657
singhsrijan46 wants to merge 2 commits into
openkruise:masterfrom
singhsrijan46:feat/core-short-sandbox-id

Conversation

@singhsrijan46

@singhsrijan46 singhsrijan46 commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Ⅰ. Describe what this PR does

This pull request introduces the core sandboxid package and lays the groundwork for supporting short, stable Sandbox IDs as specified in the design spec:

  1. Schema Constant: Added LabelSandboxID (agents.kruise.io/sandbox-id) to api/v1alpha1/sandboxset_types.go.
  2. Short ID Generation & Resolution (sandboxid package):
    • Implemented Base32 lowercase 26-character encoding (GenerateShort) to authoritatively map Sandbox UID to a DNS-safe label.
    • Implemented ID resolution (Resolve) which resolves using the short ID label with a fallback to the deterministically-formatted legacy ID (<namespace>--<name>).
    • Implemented metadata label assignment helper (AssignShort).
  3. Metadata Reset Preservation: Refined resetMetadataForPool in pkg/controller/sandbox/core/recycle.go to explicitly preserve LabelSandboxID during sandbox recycling.
  4. Flaky Test Fixes: Resolved flaky strict timing assertions (assert.Greater vs assert.GreaterOrEqual) in pkg/sandbox-manager/infra/sandboxcr/claim_test.go and clone_test.go where fast mock clocks or immediate rate limit error returns yielded exactly 0s wait times.

Ⅱ. Does this pull request fix one issue?

NONE

Ⅲ. Describe how to verify it

Unit tests have been added and verified:

  1. Core Sandbox ID Tests: go test -v ./pkg/sandbox-manager/sandboxid/...
  2. Recycle Metadata Reset Tests: go test -v ./pkg/controller/sandbox/core/...
  3. Sandbox CR Infra Claim/Clone Tests: go test -v ./pkg/sandbox-manager/infra/sandboxcr/...

Ⅳ. Special notes for reviews

The timing assertions for Wait and WaitReady metrics in sandboxcr were adjusted to assert.GreaterOrEqual to eliminate flakes on fast test runners/environments.

Signed-off-by: Srijan <singhsrijangkp@gmail.com>
@kruise-bot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign furykerry for approval by writing /assign @furykerry in a comment. For more information see:The Kubernetes 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

…and clone tests

Signed-off-by: Srijan <singhsrijangkp@gmail.com>
@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.08%. Comparing base (bc355be) to head (36a4416).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #657      +/-   ##
==========================================
+ Coverage   80.06%   80.08%   +0.01%     
==========================================
  Files         229      230       +1     
  Lines       17862    17895      +33     
==========================================
+ Hits        14302    14332      +30     
- Misses       2975     2978       +3     
  Partials      585      585              
Flag Coverage Δ
unittests 80.08% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants