Skip to content

update labels when claiming#201

Open
furykerry wants to merge 1 commit intoopenkruise:masterfrom
furykerry:feat/claim-labeling
Open

update labels when claiming#201
furykerry wants to merge 1 commit intoopenkruise:masterfrom
furykerry:feat/claim-labeling

Conversation

@furykerry
Copy link
Copy Markdown
Member

@furykerry furykerry commented Mar 15, 2026

Ⅰ. Describe what this PR does

Ⅱ. Does this pull request fix one issue?

Ⅲ. Describe how to verify it

Ⅳ. Special notes for reviews

@kruise-bot
Copy link
Copy Markdown

kruise-bot commented Mar 15, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:

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

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 15, 2026 •
edited
Loading

Codecov Report

❌ Patch coverage is 66.66667% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.54%. Comparing base (4d5825c) to head (dadb761).
⚠️ Report is 8 commits behind head on master.

Files with missing lines Patch % Lines
pkg/utils/inplaceupdate/inplace_update.go 0.00% 7 Missing ⚠️
pkg/sandbox-manager/infra/sandboxcr/claim.go 71.42% 2 Missing ⚠️
pkg/servers/e2b/models/extensions.go 87.50% 1 Missing and 1 partial ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #201      +/-   ##
==========================================
+ Coverage   61.80%   62.54%   +0.74%     
==========================================
  Files         104      106       +2     
  Lines        6694     6923     +229     
==========================================
+ Hits         4137     4330     +193     
- Misses       2290     2321      +31     
- Partials      267      272       +5
Flag Coverage Δ
unittests 62.54% <66.66%> (+0.74%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 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.

AiRanthem
AiRanthem reviewed Mar 16, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@AiRanthem AiRanthem left a comment

Choose a reason for hiding this comment

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

/lgtm

@AiRanthem
Copy link
Copy Markdown
Member

AiRanthem commented Mar 16, 2026

The labels should be added to the pod either

@furykerry furykerry force-pushed the feat/claim-labeling branch from 1277918 to 3d67d0b Compare March 20, 2026 01:39
@kruise-bot
Copy link
Copy Markdown

kruise-bot commented Mar 20, 2026

New changes are detected. LGTM label has been removed.

@furykerry furykerry force-pushed the feat/claim-labeling branch 2 times, most recently from 44c1398 to 74d1326 Compare March 20, 2026 02:07
@furykerry furykerry force-pushed the feat/claim-labeling branch from 74d1326 to 53dcbb3 Compare March 28, 2026 08:03
@furykerry furykerry force-pushed the feat/claim-labeling branch 2 times, most recently from 5e0e3c8 to 807817b Compare March 28, 2026 09:32
@kruise-bot kruise-bot added size/XL and removed size/L labels Mar 28, 2026
@furykerry furykerry force-pushed the feat/claim-labeling branch from 807817b to 03095d6 Compare March 29, 2026 14:58
@furykerry
update labels when claiming
dadb761 
Signed-off-by: 守辰 <shouchen.zz@alibaba-inc.com>
@furykerry furykerry force-pushed the feat/claim-labeling branch from 03095d6 to dadb761 Compare March 30, 2026 05:29
@kruise-bot kruise-bot added size/L and removed size/XL labels Mar 30, 2026
zmberg
zmberg reviewed Mar 31, 2026
View reviewed changes
pkg/servers/e2b/create.go
// record the csi mount config to annotation
annotations[models.ExtensionKeyClaimWithCSIMount_MountConfig] = string(csiMountConfigRaw)
sbx.SetAnnotations(annotations)
sbx.SetLabels(labels)
Copy link
Copy Markdown
Member

@zmberg zmberg Mar 31, 2026

Choose a reason for hiding this comment

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

Shouldn't this be spec.template.labels?

zmberg
zmberg reviewed Mar 31, 2026
View reviewed changes
pkg/utils/inplaceupdate/inplace_update.go
originStatus[status.Name] = status.ImageID
}

for k, v := range box.Labels {
Copy link
Copy Markdown
Member

@zmberg zmberg Mar 31, 2026

Choose a reason for hiding this comment

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

box.spec.template.labels?

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

Reviewers

@zmberg zmberg zmberg left review comments

@AiRanthem AiRanthem AiRanthem left review comments

Assignees

@AiRanthem AiRanthem

Labels

size/L

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@furykerry @kruise-bot @AiRanthem @zmberg