cubeapi/cubemaster: normalize host mount annotations

CubeAPI/src/services/sandboxes.rs

@@ -26,6 +26,21 @@ const RET_CODE_HTTP_OK: i32 = 200;
 const RET_CODE_NOT_FOUND: i32 = 130404;
 const RET_CODE_CONFLICT: i32 = 130409;
 const HOSTDIR_MOUNT_KEY: &str = "host-mount";
+const HOSTDIR_MOUNT_LEGACY_KEY: &str = "hostdir-mount";
+
+fn split_metadata_labels_and_hostdir_annotation(
+    metadata: Option<HashMap<String, String>>,
+    annotations: &mut HashMap<String, String>,
+) -> Option<HashMap<String, String>> {
+    metadata.map(|mut meta| {
+        let host_mount = meta.remove(HOSTDIR_MOUNT_KEY);
+        let legacy_hostdir_mount = meta.remove(HOSTDIR_MOUNT_LEGACY_KEY);
+        if let Some(value) = host_mount.or(legacy_hostdir_mount) {
+            annotations.insert(HOSTDIR_MOUNT_KEY.to_string(), value);
+        }
+        meta
+    })
+}
 
 #[derive(Clone)]
 pub struct SandboxService {
@@ -126,12 +141,7 @@ impl SandboxService {
             ),
         ]);
 
-        let labels = body.metadata.map(|mut meta| {
-            if let Some(value) = meta.remove(HOSTDIR_MOUNT_KEY) {
-                annotations.insert(HOSTDIR_MOUNT_KEY.to_string(), value);
-            }
-            meta
-        });
+        let labels = split_metadata_labels_and_hostdir_annotation(body.metadata, &mut annotations);
 
         let req = CreateSandboxRequest {
             request_id: new_request_id(),
@@ -681,7 +691,10 @@ pub(crate) fn build_cubevs_context(
 mod tests {
     use std::collections::HashMap;
 
-    use super::{build_cubevs_context, filter_by_metadata, from_cubemaster_info};
+    use super::{
+        build_cubevs_context, filter_by_metadata, from_cubemaster_info,
+        split_metadata_labels_and_hostdir_annotation,
+    };
     use crate::cubemaster::SandboxInfo;
     use crate::models::SandboxNetworkConfig;
 
@@ -701,6 +714,46 @@ mod tests {
         assert!(!filter_by_metadata(None, Some("user=alice")));
     }
 
+    #[test]
+    fn split_metadata_moves_host_mount_to_annotation() {
+        let mut annotations = HashMap::new();
+        let labels = split_metadata_labels_and_hostdir_annotation(
+            Some(HashMap::from([
+                ("host-mount".to_string(), "mount-config".to_string()),
+                ("user".to_string(), "alice".to_string()),
+            ])),
+            &mut annotations,
+        )
+        .expect("labels");
+
+        assert_eq!(
+            annotations.get("host-mount").map(String::as_str),
+            Some("mount-config")
+        );
+        assert_eq!(labels.get("user").map(String::as_str), Some("alice"));
+        assert!(!labels.contains_key("host-mount"));
+    }
+
+    #[test]
+    fn split_metadata_accepts_legacy_hostdir_mount_and_prefers_canonical() {
+        let mut annotations = HashMap::new();
+        let labels = split_metadata_labels_and_hostdir_annotation(
+            Some(HashMap::from([
+                ("host-mount".to_string(), "canonical".to_string()),
+                ("hostdir-mount".to_string(), "legacy".to_string()),
+            ])),
+            &mut annotations,
+        )
+        .expect("labels");
+
+        assert_eq!(
+            annotations.get("host-mount").map(String::as_str),
+            Some("canonical")
+        );
+        assert!(!labels.contains_key("host-mount"));
+        assert!(!labels.contains_key("hostdir-mount"));
+    }
+
     #[test]
     fn network_context_prefers_explicit_network_config() {
         let context = build_cubevs_context(

CubeMaster/pkg/service/sandbox/hostdir_mount.go

@@ -16,12 +16,13 @@ import (
 )
 
 const (
-	// AnnotationHostDirMount must match the annotation key that CubeAPI
+	// AnnotationHostDirMount must match the canonical annotation key that CubeAPI
 	// writes when it lifts metadata["host-mount"] onto the sandbox
 	// CreateSandboxRequest; see CubeAPI/src/handlers/sandboxes.rs
 	// (const HOSTDIR_MOUNT_KEY). Keep these in lockstep, otherwise
 	// host-mount requests are silently dropped.
-	AnnotationHostDirMount = "host-mount"
+	AnnotationHostDirMount       = "host-mount"
+	AnnotationHostDirMountLegacy = "hostdir-mount"
 )
 
 type HostDirMountOption struct {
@@ -37,16 +38,21 @@ func injectHostDirMounts(ctx context.Context, req *types.CreateCubeSandboxReq) e
 		log.G(ctx).Infof("[hostdir] no annotations, skip")
 		return nil
 	}
-	raw, ok := req.Annotations[AnnotationHostDirMount]
+	annotationKey := AnnotationHostDirMount
+	raw, ok := req.Annotations[annotationKey]
 	if !ok || strings.TrimSpace(raw) == "" {
-		log.G(ctx).Infof("[hostdir] annotation %q absent or empty, skip", AnnotationHostDirMount)
+		annotationKey = AnnotationHostDirMountLegacy
+		raw, ok = req.Annotations[annotationKey]
+	}
+	if !ok || strings.TrimSpace(raw) == "" {
+		log.G(ctx).Infof("[hostdir] annotation %q/%q absent or empty, skip", AnnotationHostDirMount, AnnotationHostDirMountLegacy)
 		return nil
 	}
-	log.G(ctx).Infof("[hostdir] raw annotation: %s", raw)
+	log.G(ctx).Infof("[hostdir] raw annotation %q: %s", annotationKey, raw)
 
 	var opts []HostDirMountOption
 	if err := json.Unmarshal([]byte(raw), &opts); err != nil {
-		return fmt.Errorf("invalid %q annotation: %w", AnnotationHostDirMount, err)
+		return fmt.Errorf("invalid %q annotation: %w", annotationKey, err)
 	}
 	if len(opts) == 0 {
 		log.G(ctx).Infof("[hostdir] annotation parsed to empty list, skip")
@@ -57,11 +63,11 @@ func injectHostDirMounts(ctx context.Context, req *types.CreateCubeSandboxReq) e
 	for i, o := range opts {
 		if !strings.HasPrefix(o.HostPath, "/") {
 			return fmt.Errorf("%q entry[%d]: hostPath must be an absolute path, got %q",
-				AnnotationHostDirMount, i, o.HostPath)
+				annotationKey, i, o.HostPath)
 		}
 		if !strings.HasPrefix(o.MountPath, "/") {
 			return fmt.Errorf("%q entry[%d]: mountPath must be an absolute path, got %q",
-				AnnotationHostDirMount, i, o.MountPath)
+				annotationKey, i, o.MountPath)
 		}
 	}
 

CubeMaster/pkg/service/sandbox/hostdir_mount_test.go

@@ -0,0 +1,63 @@
+// Copyright (c) 2024 Tencent Inc.
+// SPDX-License-Identifier: Apache-2.0
+//
+
+package sandbox
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"github.com/tencentcloud/CubeSandbox/CubeMaster/pkg/service/sandbox/types"
+)
+
+func TestInjectHostDirMountsAcceptsCanonicalAndLegacyAnnotations(t *testing.T) {
+	raw := `[
+		{"hostPath":"/tmp/rw","mountPath":"/mnt/rw","readOnly":false},
+		{"hostPath":"/tmp/ro","mountPath":"/mnt/ro","readOnly":true}
+	]`
+
+	for _, tt := range []struct {
+		name string
+		key  string
+	}{
+		{name: "canonical", key: AnnotationHostDirMount},
+		{name: "legacy", key: AnnotationHostDirMountLegacy},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			req := &types.CreateCubeSandboxReq{
+				Annotations: map[string]string{tt.key: raw},
+				Containers:  []*types.Container{{Name: "c0"}},
+			}
+
+			require.NoError(t, injectHostDirMounts(context.Background(), req))
+			require.Len(t, req.Volumes, 2)
+			require.Equal(t, "hostdir-0", req.Volumes[0].Name)
+			require.Equal(t, "/tmp/rw", req.Volumes[0].VolumeSource.HostDirVolumeSources.VolumeSources[0].HostPath)
+			require.Equal(t, "hostdir-1", req.Volumes[1].Name)
+			require.Equal(t, "/tmp/ro", req.Volumes[1].VolumeSource.HostDirVolumeSources.VolumeSources[0].HostPath)
+
+			require.Len(t, req.Containers[0].VolumeMounts, 2)
+			require.Equal(t, "/mnt/rw", req.Containers[0].VolumeMounts[0].ContainerPath)
+			require.False(t, req.Containers[0].VolumeMounts[0].Readonly)
+			require.Equal(t, "/mnt/ro", req.Containers[0].VolumeMounts[1].ContainerPath)
+			require.True(t, req.Containers[0].VolumeMounts[1].Readonly)
+		})
+	}
+}
+
+func TestInjectHostDirMountsPrefersCanonicalAnnotation(t *testing.T) {
+	req := &types.CreateCubeSandboxReq{
+		Annotations: map[string]string{
+			AnnotationHostDirMountLegacy: `[{"hostPath":"/legacy","mountPath":"/mnt/legacy"}]`,
+			AnnotationHostDirMount:       `[{"hostPath":"/canonical","mountPath":"/mnt/canonical"}]`,
+		},
+		Containers: []*types.Container{{Name: "c0"}},
+	}
+
+	require.NoError(t, injectHostDirMounts(context.Background(), req))
+	require.Len(t, req.Volumes, 1)
+	require.Equal(t, "/canonical", req.Volumes[0].VolumeSource.HostDirVolumeSources.VolumeSources[0].HostPath)
+	require.Equal(t, "/mnt/canonical", req.Containers[0].VolumeMounts[0].ContainerPath)
+}