fix(sandbox-template): apply min_free_disk on same-name builds by DROWNING2003 · Pull Request #466 · qiniu/qshell

DROWNING2003 · 2026-11-06T10:38:22Z

问题

模板磁盘只在创建时确定：rebuild 接口（POST /templates/{id}，TemplateBuildRequest）没有磁盘字段，min_free_disk_mb 只存在于创建接口（POST /v3/templates，TemplateBuildRequestV3）。

qshell 按 name 命中已有模板后走 rebuild，min_free_disk_mb 永远不生效——构建显示成功，磁盘还是几个月前的容量。E2B 的 Template.build 不受影响，因为它每次都调 POST /v3/templates。

改动

当 min_free_disk_mb > 0 且按 name 命中已有模板时，改走 v3 创建接口以同名提交，由服务端更新该模板并应用磁盘设置。实测模板 ID 不变、alias 不变，磁盘 12559 MiB → 23844 MiB。
命中其他团队的公共模板时直接报错：同样的创建会落在自己团队下，但按 name 解析仍指向公共模板，构建结果不可达。
显式 --template-id / 配置文件 template_id 仍走 rebuild，并保留「磁盘不会改变」的告警。
兼容已废弃的 disk_size_mb：解析该字段仅用于输出迁移提示，避免静默忽略。
同步更新文档与 CHANGELOG。

验证

build #1 (min-free-disk 0)   → diskSizeMB 12559
build #2 同名 (20480)        → diskSizeMB 23844，模板 ID 不变
alias 指向其他团队模板时      → 直接报错，不再静默构建

go test ./iqshell/sandbox/template/...
go build ./...
gofmt -l 无输出

qiniu-prow · 2026-11-06T10:38:32Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: DROWNING2003
Once this PR has been reviewed and has the lgtm label, please assign sxci for approval. 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:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

The template API dropped diskSizeMB in favor of minFreeDiskMb, so a leftover disk_size_mb used to be silently ignored and builds fell back to the team default disk. Parse the retired key only to emit a migration hint.

Rebuild requests carry no disk field, so a template created months ago never picks up min_free_disk_mb. When the name already exists and a positive min-free-disk is set, create through the v3 endpoint instead, matching E2B Template.build; the server keeps the template ID and applies the disk setting.

DROWNING2003 · 2026-11-06T11:22:20Z

/review -claude

Alias lookup can resolve to another team's public template, where a same-name create lands in the caller's team but stays unreachable by name. Verify ownership before re-creating, and correct the docs: the v3 create path updates the same template ID instead of forking a new one.

fennoai

Updated: this review was first written against eabc26e8. While it was being submitted, 73e9a583 ("guard re-create against foreign templates") was pushed, and GitHub attached the review to that commit. The findings below are re-checked against 73e9a583.

73e9a583 resolves the main concerns from the first pass:

Docs, CHANGELOG and comments now say the v3 same-name create keeps the template ID. You also note you confirmed this by testing. That removes the earlier worries about template pile-up, losing the per-template COPY cache, and losing the public flag.
The lookup/recreate branching is simplified to a single decision.
A same-name create against another team's public template now fails early.

go test ./iqshell/sandbox/template/... passes on 73e9a583 and gofmt -s -l is clean.

Reviewed head commit: 73e9a583440eed9e275361c353f21649db63f904

Remaining findings

[P2] An explicit --min-free-disk 0 is still ignored without a warning on name or ID rebuilds (iqshell/sandbox/template/operations/build.go:196)
The recreate path clears templateID before this check, so the old info.MinFreeDiskMB != nil condition never fired there anyway. Narrowing it to > 0 therefore changes only the explicit-0 case. 0 is a documented value ("explicit 0 disables free-space growth"), and docs/sandbox_template_build.md:117 still promises a notice on rebuild. Now --min-free-disk 0 or min_free_disk_mb = 0 against an existing template is dropped with no feedback. You now know the v3 create keeps the ID, so the simplest fix may be to let shouldRecreateForDisk accept any non-nil value, which would apply 0 too. Otherwise, restore the != nil warning.

[P3] The legacy warning names the wrong file and mixes languages (iqshell/sandbox/template/operations/build.go:406)
The message hardcodes qshell.sandbox.toml, but with --config ./prod.toml a different file was read. Use cfg.SourcePath() instead. It is also the only Chinese runtime message in this flow: PrintWarn adds an English Warning: prefix and the other build warnings are English.

[P3] Some docs still say a name hit always rebuilds (see the inline comment on docs/sandbox_template_build.md:50)

Minor: the new CHANGELOG entries sit in the first # 未发布 block. The related min_free_disk_mb addition and the --disk-size / disk_size_mb removal are in the second # 未发布 block below 2.19.13. Entry 1 also calls the key "已废弃" (deprecated), while that block lists it as removed under 不兼容变更 (breaking changes).

fennoai · 2026-11-06T11:32:32Z

+				if err := validateRebuildSourceSelection(info, cliFromImage, cliFromTemplate); err != nil {
+					sbClient.PrintError("%v", err)
+					return
+				}


[Resolved in 73e9a583] ~~[P1] Recreate path depends on unconfirmed server behavior and runs on every build~~

This was written against eabc26e8; it is now anchored to the wrong line because the head moved. 73e9a583 states, from testing, that the v3 same-name create keeps the template ID, and adds an ownership guard. That addresses the 409, template pile-up, COPY-cache and visibility concerns. Nothing left to do here.

fennoai · 2026-11-06T11:32:32Z


 未提供 `template_id` 时，如果配置文件或 CLI 中有 `name`，命令会先按 name 在远端查找模板；命中即进入 rebuild，未命中再创建新模板。按 name 命中的 rebuild 不会把 `template_id` 回写到配置文件，便于同一份配置在多个环境复用。

+例外：配置了 `min_free_disk_mb > 0`（或 `--min-free-disk` 大于 0）且按 name 命中已有模板时，命令不会走 rebuild，而是改用 v3 创建接口以同名提交，由服务端更新该模板并应用磁盘设置（实测模板 ID 不变）。模板磁盘只在创建时确定，rebuild 请求没有磁盘字段，只有创建接口才能应用该设置。若命中的是其他团队的公共模板，命令会直接报错，因为再构建也会被按 name 解析回那个公共模板。显式传入 `--template-id` 或配置文件中存在 `template_id` 时仍走 rebuild，磁盘不会改变，并会打印告警。


[P3] Some docs still say a name hit always rebuilds

The contradiction this comment first raised (new template vs. same ID) is fixed in 73e9a583. A few texts still describe the old behavior, though:

Line 40 of this file says --min-free-disk is "重新构建已有模板时忽略" (ignored when rebuilding an existing template). It now does apply when the name resolves and the value is > 0.

The minimal example in docs/sandbox_template_config.md:126-137 sets min_free_disk_mb = 5120, but still says a name hit rebuilds ("同名模板已存在则 rebuild" / "按 name 命中后 rebuild").

The --help long text at cmd/sandbox_template.go:178 says "命中即对已有模板触发 rebuild" (a name hit triggers a rebuild).

fennoai · 2026-11-06T11:32:32Z

-			if err := validateRebuildSourceSelection(info, cliFromImage, cliFromTemplate); err != nil {
-				sbClient.PrintError("%v", err)
-				return
+			recreate := shouldRecreateForDisk(info, foundByName)


[Resolved in 73e9a583] ~~[P3] Simplify the lookup/recreate branching~~

The branching now makes the decision once and the separate recreate block is gone. Nothing left to do here.

fennoai · 2026-11-06T11:32:32Z

+	if tmpl == nil {
+		return nil
+	}
+	return ownershipError(name, templateID, tmpl.IsOwner)


[P3] The legacy warning names the wrong file and mixes languages

(Because the head moved, this comment landed on ownershipError. The code in question is legacyDiskSizeWarning, at line 406 in 73e9a583.)

The message hardcodes qshell.sandbox.toml, but with --config ./prod.toml a different file was read. Use cfg.SourcePath() instead. It is also the only Chinese runtime message in this flow: PrintWarn adds an English Warning: prefix and the other build warnings are English, so the output mixes languages.

qiniu-prow Bot added the size/M label Nov 6, 2026

This comment was marked as outdated.

Sign in to view

qiniu-prow Bot added size/L and removed size/M labels Nov 6, 2026

fix(sandbox-template): warn when deprecated disk_size_mb is set

f77da86

The template API dropped diskSizeMB in favor of minFreeDiskMb, so a leftover disk_size_mb used to be silently ignored and builds fell back to the team default disk. Parse the retired key only to emit a migration hint.

DROWNING2003 force-pushed the feat/sandbox-template-min-free-disk branch from 75677da to f77da86 Compare November 6, 2026 11:11

qiniu-prow Bot added size/M and removed size/L labels Nov 6, 2026

DROWNING2003 changed the title ~~feat(sandbox-template): support minimum free disk space~~ fix(sandbox-template): apply min_free_disk on same-name builds Nov 6, 2026

qiniu-prow Bot added size/L and removed size/M labels Nov 6, 2026

fennoai Bot reviewed Nov 6, 2026

View reviewed changes

Navigation Menu

Repository navigation

fix(sandbox-template): apply min_free_disk on same-name builds - #466

fix(sandbox-template): apply min_free_disk on same-name builds#466
DROWNING2003 wants to merge 3 commits into
qiniu:masterfrom
DROWNING2003:feat/sandbox-template-min-free-disk

DROWNING2003 commented Nov 6, 2026 •

edited

Loading

Uh oh!

qiniu-prow Bot commented Nov 6, 2026

Uh oh!

This comment was marked as outdated.

Uh oh!

DROWNING2003 commented Nov 6, 2026

Uh oh!

fennoai Bot left a comment •

edited

Loading

Uh oh!

fennoai Bot Nov 6, 2026 •

edited

Loading

Uh oh!

fennoai Bot Nov 6, 2026 •

edited

Loading

Uh oh!

fennoai Bot Nov 6, 2026 •

edited

Loading

Uh oh!

fennoai Bot Nov 6, 2026 •

edited

Loading

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant


		未提供 `template_id` 时，如果配置文件或 CLI 中有 `name`，命令会先按 name 在远端查找模板；命中即进入 rebuild，未命中再创建新模板。按 name 命中的 rebuild 不会把 `template_id` 回写到配置文件，便于同一份配置在多个环境复用。

		例外：配置了 `min_free_disk_mb > 0`（或 `--min-free-disk` 大于 0）且按 name 命中已有模板时，命令不会走 rebuild，而是改用 v3 创建接口以同名提交，由服务端更新该模板并应用磁盘设置（实测模板 ID 不变）。模板磁盘只在创建时确定，rebuild 请求没有磁盘字段，只有创建接口才能应用该设置。若命中的是其他团队的公共模板，命令会直接报错，因为再构建也会被按 name 解析回那个公共模板。显式传入 `--template-id` 或配置文件中存在 `template_id` 时仍走 rebuild，磁盘不会改变，并会打印告警。

Conversation

DROWNING2003 commented Nov 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

问题

改动

验证

Uh oh!

qiniu-prow Bot commented Nov 6, 2026

Uh oh!

This comment was marked as outdated.

Uh oh!

DROWNING2003 commented Nov 6, 2026

Uh oh!

fennoai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Remaining findings

Uh oh!

fennoai Bot Nov 6, 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!

fennoai Bot Nov 6, 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!

fennoai Bot Nov 6, 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!

fennoai Bot Nov 6, 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!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

DROWNING2003 commented Nov 6, 2026 •

edited

Loading

fennoai Bot left a comment •

edited

Loading

fennoai Bot Nov 6, 2026 •

edited

Loading

fennoai Bot Nov 6, 2026 •

edited

Loading

fennoai Bot Nov 6, 2026 •

edited

Loading

fennoai Bot Nov 6, 2026 •

edited

Loading