cubeapi/cubemaster: normalize host mount annotations

[bigjar]

Summary

• keep host-mount as the canonical annotation used between CubeAPI and CubeMaster
• accept legacy hostdir-mount metadata/annotations for compatibility
• add focused unit tests for CubeAPI metadata lifting and CubeMaster hostdir injection

Tests

• go test ./pkg/service/sandbox -run HostDirMount -count=1
• cargo test --manifest-path CubeAPI/Cargo.toml split_metadata -- --nocapture (blocked locally: crates.io timed out while downloading utoipa)

Notes

• This PR is based on TencentCloud/CubeSandbox@master.
• Per CONTRIBUTING.md, AI-generated commits do not add Signed-off-by lines. The human contributor should add DCO sign-offs before merge if required by CI.

[chenhengqi]

/cubebot review this pr.

[github-actions]

Claude finished @chenhengqi's task in 1m 38s —— View job

---

PR Review: Normalize host mount annotations

• Read CLAUDE.md and repo context
• Review CubeAPI changes (CubeAPI/src/services/sandboxes.rs)
• Review CubeMaster changes (CubeMaster/pkg/service/sandbox/hostdir_mount.go)
• Review new test file (CubeMaster/pkg/service/sandbox/hostdir_mount_test.go)
• Provide comprehensive review feedback

---

Overall Assessment

This is a clean, well-scoped PR that adds backward compatibility for the legacy hostdir-mount annotation key while establishing host-mount as the canonical key across CubeAPI and CubeMaster. The implementation is correct, the priority logic (canonical preferred over legacy) is consistent on both sides, and the tests cover the key scenarios.

What works well

• Consistent priority semantics: Both CubeAPI and CubeMaster prefer the canonical host-mount key over the legacy hostdir-mount key. On the CubeAPI side, host_mount.or(legacy_hostdir_mount) ensures canonical wins; on the CubeMaster side, the canonical key is checked first with fallback to legacy.
• Clean extraction: The split_metadata_labels_and_hostdir_annotation function is a good refactor — it replaces the inline closure and makes the metadata-lifting logic testable.
• Both keys removed from labels: The CubeAPI side removes both host-mount and hostdir-mount from metadata labels even when only one is present. This prevents either key from leaking into labels regardless of which the client sent.
• Good test coverage: Tests on both sides verify canonical-only, legacy-only, and both-keys-present scenarios.

Issues and suggestions

1. Stale comment: wrong file path reference (pre-existing, but worth fixing)

CubeMaster/pkg/service/sandbox/hostdir_mount.go:21 — The comment references CubeAPI/src/handlers/sandboxes.rs, but the actual file is CubeAPI/src/services/sandboxes.rs. This PR already modifies this comment block (added "canonical"), so it would be a good opportunity to fix the path:

// CreateSandboxRequest; see CubeAPI/src/handlers/sandboxes.rs

should be:

// CreateSandboxRequest; see CubeAPI/src/services/sandboxes.rs

Fix this →

2. Missing test: None metadata and empty-metadata edge cases (CubeAPI)

The two new Rust tests cover the canonical and legacy key paths, but there's no test for:

• split_metadata_labels_and_hostdir_annotation(None, &mut annotations) — should return None and leave annotations untouched
• split_metadata_labels_and_hostdir_annotation(Some(HashMap::new()), &mut annotations) — should return Some(empty) without touching annotations

These are low-risk edge cases since the function handles them correctly, but a brief test would document the expected behavior.

3. Missing test: no-annotation and empty-annotation cases (CubeMaster)

The Go test file covers the happy paths (canonical key, legacy key, both keys), but doesn't test:

• injectHostDirMounts with nil annotations — early return path
• injectHostDirMounts with neither key present — also early return
• injectHostDirMounts with empty/whitespace-only values — the TrimSpace guard

Again, low risk but would improve coverage of the fallback logic this PR introduces.

4. Minor: CubeMaster fallback log message could be more helpful

CubeMaster/pkg/service/sandbox/hostdir_mount.go:48 — When both keys are absent, the log says:

log.G(ctx).Infof("[hostdir] annotation %q/%q absent or empty, skip", AnnotationHostDirMount, AnnotationHostDirMountLegacy)

This is fine, but consider logging at debug level instead of info — this will fire on every sandbox creation that doesn't use host mounts, which could be noisy.

5. Nit: CubeAPI constant naming

The constant HOSTDIR_MOUNT_KEY has "hostdir" in its name but maps to the value "host-mount". Now that the legacy HOSTDIR_MOUNT_LEGACY_KEY maps to "hostdir-mount", the naming is slightly confusing — HOSTDIR_MOUNT_KEY sounds like it should be the one with "hostdir-mount". Renaming to something like HOST_MOUNT_KEY / HOST_MOUNT_LEGACY_KEY would be clearer, though this is purely cosmetic and could be done as a follow-up.

Summary

The core logic is correct and well-tested. The main actionable item is fixing the stale file path in the comment (issue #1). The rest are suggestions for improved test coverage and minor polish.

[chenhengqi]

What's the purpose of this change? Where does this legacy annotation come from?

[fslongjin]

@bigjar ping~

[fslongjin]

@bigjar ping~