diff --git a/CubeAPI/src/services/sandboxes.rs b/CubeAPI/src/services/sandboxes.rs index de909563..ae56689b 100644 --- a/CubeAPI/src/services/sandboxes.rs +++ b/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>, + annotations: &mut HashMap, +) -> Option> { + 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( diff --git a/CubeMaster/pkg/service/sandbox/hostdir_mount.go b/CubeMaster/pkg/service/sandbox/hostdir_mount.go index 1a4e552c..6e73dfd8 100644 --- a/CubeMaster/pkg/service/sandbox/hostdir_mount.go +++ b/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) } } diff --git a/CubeMaster/pkg/service/sandbox/hostdir_mount_test.go b/CubeMaster/pkg/service/sandbox/hostdir_mount_test.go new file mode 100644 index 00000000..f5bbc4f5 --- /dev/null +++ b/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) +}