已开启
fix(volume): validate volume args at request entrypoint #173
jing-rui创建于 24 天前
fix(volume): validate volume args at request entrypoint #173
已开启
共 7 个文件变更+189-71
| @@ -91,20 +91,13 @@ build: ## Build all Conch binaries | |||
| 91 | $(GOBUILD) -ldflags "$(VERSION_LDFLAGS)" -o "$(BIN_DIR)/$$cmd" "./cmd/$$cmd"; \ | 91 | $(GOBUILD) -ldflags "$(VERSION_LDFLAGS)" -o "$(BIN_DIR)/$$cmd" "./cmd/$$cmd"; \ |
| 92 | done | 92 | done |
| 93 | 93 | ||
| 94 | -build-offline: | 94 | +static: |
| 95 | - @echo "syncing dependency manifests from image backup..." | 95 | + mkdir -p $(BIN_DIR) |
| 96 | - @cp /go/go.mod.backup ./go.mod | 96 | + CGO_ENABLED=0 GOOS=linux GOARCH=$$(go env GOARCH) \ |
| 97 | - @cp /go/go.sum.backup ./go.sum | 97 | + $(GOBUILD) -mod=readonly -tags "netgo,osusergo" \ |
| 98 | - @echo "building binaries using image cache..." | 98 | + -ldflags '-s -w -extldflags "-static" $(VERSION_LDFLAGS)' \ |
| 99 | - @mkdir -p $(BIN_DIR) | 99 | + -o $(BIN_DIR)/ ./cmd/... |
| 100 | - @for cmd in $(CMDS); do \ | 100 | + |
| 101 | - echo "building static cmd/$$cmd..."; \ | ||
| 102 | - CGO_ENABLED=0 GOOS=linux GOARCH=$$(go env GOARCH) \ | ||
| 103 | - $(GOBUILD) -mod=readonly -tags "netgo,osusergo" \ | ||
| 104 | - -ldflags '-s -w -extldflags "-static" $(VERSION_LDFLAGS)' \ | ||
| 105 | - -o $(BIN_DIR)/$$cmd ./cmd/$$cmd; \ | ||
| 106 | - done | ||
| 107 | - @git checkout go.mod go.sum 2>/dev/null || true | ||
| 108 | 101 | ||
| 109 | build-%: ## Build specific binary (e.g., make build-conchd) | 102 | build-%: ## Build specific binary (e.g., make build-conchd) |
| 110 | @echo "building cmd/$*..." | 103 | @echo "building cmd/$*..." |
| @@ -166,3 +159,4 @@ cleancode: ## Clean code (remove trailing spaces/CR) | |||
| 166 | fi | 159 | fi |
| 167 | @$(CLEANSCRIPT) > /dev/null 2>&1 | 160 | @$(CLEANSCRIPT) > /dev/null 2>&1 |
| 168 | @echo "code cleaning completed" | 161 | @echo "code cleaning completed" |
| 162 | + | ||
| @@ -6,31 +6,32 @@ Volume functionality test for Conch (single-virtiofsd, host-path model). | |||
| 6 | Verifies that a host directory mounted into a sandbox persists across sandbox | 6 | Verifies that a host directory mounted into a sandbox persists across sandbox |
| 7 | lifecycles when the same host path is reused: | 7 | lifecycles when the same host path is reused: |
| 8 | 8 | ||
| 9 | - 1. Ensure host volume dirs exist under ./shared (volume-1 .. volume-10). | 9 | + 1. Ensure host volume dirs exist under ./debug/shared (volume-1 .. volume-100). |
| 10 | - 2. Start a sandbox with ./shared/volume-1 mounted at /workspace, write the | 10 | + 2. Start a sandbox with N host volumes mounted under /workspace, writing the |
| 11 | - current timestamp to /workspace/last.txt, then delete the sandbox. | 11 | + current timestamp to each mount point. |
| 12 | - 3. Start a second sandbox with the SAME host dir mounted at /workspace, | 12 | + 3. Start a second sandbox with the SAME host dirs mounted at the same paths, |
| 13 | - read /workspace/last.txt back to verify data persisted, then delete. | 13 | + read each timestamp back, and print it. |
| 14 | + 4. Wait for confirmation, then delete both sandboxes. | ||
| 14 | 15 | ||
| 15 | Host dirs are created under PWD and left in place for inspection (no tempfile | 16 | Host dirs are created under PWD and left in place for inspection (no tempfile |
| 16 | auto-cleanup), so the persisted last.txt is visible after the run. | 17 | auto-cleanup), so the persisted last.txt is visible after the run. |
| 17 | 18 | ||
| 18 | -Run: python3 tests/volume.py <template_id> | 19 | +Run: CONCH_TEMPLATE_ID=<template_id> python3 examples/volume.py |
| 19 | """ | 20 | """ |
| 20 | 21 | ||
| 22 | +import argparse | ||
| 21 | import os | 23 | import os |
| 22 | -import platform | ||
| 23 | import time | 24 | import time |
| 24 | import sys | 25 | import sys |
| 25 | from conch import Sandbox | 26 | from conch import Sandbox |
| 26 | 27 | ||
| 27 | 28 | ||
| 28 | HOST_BASE = os.path.join(os.getcwd(), "debug/shared") | 29 | HOST_BASE = os.path.join(os.getcwd(), "debug/shared") |
| 29 | -VOLUME_NAMES = [f"volume-{i}" for i in range(1, 11)] # volume-1 .. volume-10 | 30 | +VOLUME_NAMES = [f"volume-{i}" for i in range(1, 100)] # volume-1 .. volume-100 |
| 30 | MOUNT_PATH = "/workspace" | 31 | MOUNT_PATH = "/workspace" |
| 32 | +MAX_MOUNTS = len(VOLUME_NAMES) | ||
| 31 | 33 | ||
| 32 | # Sandbox specification | 34 | # Sandbox specification |
| 33 | -IMAGE_NAME = f"conch/openeuler:volume-{platform.machine()}" | ||
| 34 | VCPU_NUM = 2 | 35 | VCPU_NUM = 2 |
| 35 | VCPU_MAX = 2 | 36 | VCPU_MAX = 2 |
| 36 | RAM_MB = 4096 | 37 | RAM_MB = 4096 |
| @@ -55,7 +56,18 @@ def print_exec(title, ret): | |||
| 55 | print(f"stderr:\n{ret.stderr.strip()}") | 56 | print(f"stderr:\n{ret.stderr.strip()}") |
| 56 | 57 | ||
| 57 | 58 | ||
| 58 | -def run_sandbox_write(tid): | 59 | +def mount_points(count): |
| 60 | + return [MOUNT_PATH if i == 1 else f"{MOUNT_PATH}-{i}" for i in range(1, count + 1)] | ||
| 61 | + | ||
| 62 | + | ||
| 63 | +def volume_mounts(count): | ||
| 64 | + return [ | ||
| 65 | + {"source": os.path.join(HOST_BASE, f"volume-{i}"), "path": path} | ||
| 66 | + for i, path in enumerate(mount_points(count), start=1) | ||
| 67 | + ] | ||
| 68 | + | ||
| 69 | + | ||
| 70 | +def run_sandbox_write(tid, count): | ||
| 59 | t0 = time.perf_counter() | 71 | t0 = time.perf_counter() |
| 60 | 72 | ||
| 61 | box = Sandbox.create( | 73 | box = Sandbox.create( |
| @@ -63,36 +75,35 @@ def run_sandbox_write(tid): | |||
| 63 | vcpu_num=VCPU_NUM, | 75 | vcpu_num=VCPU_NUM, |
| 64 | vcpu_max=VCPU_MAX, | 76 | vcpu_max=VCPU_MAX, |
| 65 | ram_mb=RAM_MB, | 77 | ram_mb=RAM_MB, |
| 66 | - volume_mounts=[{"source": os.path.join(HOST_BASE, "volume-1"), "path": MOUNT_PATH}], | 78 | + volume_mounts=volume_mounts(count), |
| 67 | ) | 79 | ) |
| 68 | - sid = box.sandbox_id | 80 | + ret = box.commands.run(cmd="sh", args=["-c", "df -hT"]) |
| 69 | - try: | 81 | + print_exec("df -hT", ret) |
| 70 | - ret = box.commands.run(cmd="sh", args=["-c", "df -hT"]) | 82 | + print_cost(f'cold-start {VCPU_NUM}-CPU {RAM_MB}MB df -hT', t0) |
| 71 | - print_exec("df -hT", ret) | 83 | + for path in mount_points(count): |
| 72 | - print_cost(f'cold-start {VCPU_NUM}-CPU {RAM_MB}MB df -hT', t0) | 84 | + ret = box.commands.run(cmd="sh", args=["-c", f"date > {path}/last.txt"]) |
| 73 | - ret = box.commands.run(cmd="sh", args=["-c", f"echo $(date) > {MOUNT_PATH}/last.txt"]) | 85 | + print_exec(f"date > {path}/last.txt", ret) |
| 74 | - print_exec(f"echo $(date) > {MOUNT_PATH}/last.txt", ret) | 86 | + return box |
| 75 | - finally: | ||
| 76 | - try: | ||
| 77 | - box.delete() | ||
| 78 | - print(f"Sandbox {sid} deleted.") | ||
| 79 | - except Exception as e: | ||
| 80 | - print(f"Warning: Failed to delete sandbox {sid}: {e}") | ||
| 81 | 87 | ||
| 82 | 88 | ||
| 83 | -def run_sandbox_read(tid): | 89 | +def run_sandbox_read(tid, count): |
| 84 | box = Sandbox.create( | 90 | box = Sandbox.create( |
| 85 | template_id=tid, | 91 | template_id=tid, |
| 86 | vcpu_num=VCPU_NUM, | 92 | vcpu_num=VCPU_NUM, |
| 87 | vcpu_max=VCPU_MAX, | 93 | vcpu_max=VCPU_MAX, |
| 88 | ram_mb=RAM_MB, | 94 | ram_mb=RAM_MB, |
| 89 | - volume_mounts=[{"source": os.path.join(HOST_BASE, "volume-1"), "path": MOUNT_PATH}], | 95 | + volume_mounts=volume_mounts(count), |
| 90 | ) | 96 | ) |
| 91 | - sid = box.sandbox_id | 97 | + for path in mount_points(count): |
| 92 | - try: | 98 | + ret = box.commands.run(cmd="cat", args=[f"{path}/last.txt"]) |
| 93 | - ret = box.commands.run(cmd="cat", args=[f"{MOUNT_PATH}/last.txt"]) | 99 | + date = ret.stdout.strip() or ret.stderr.strip() or "<unavailable>" |
| 94 | - print_exec(f"cat {MOUNT_PATH}/last.txt", ret) | 100 | + print(f"{path}: {date}") |
| 95 | - finally: | 101 | + return box |
| 102 | + | ||
| 103 | + | ||
| 104 | +def delete_sandboxes(sandboxes): | ||
| 105 | + for box in reversed(sandboxes): | ||
| 106 | + sid = box.sandbox_id | ||
| 96 | try: | 107 | try: |
| 97 | box.delete() | 108 | box.delete() |
| 98 | print(f"Sandbox {sid} deleted.") | 109 | print(f"Sandbox {sid} deleted.") |
| @@ -100,21 +111,53 @@ def run_sandbox_read(tid): | |||
| 100 | print(f"Warning: Failed to delete sandbox {sid}: {e}") | 111 | print(f"Warning: Failed to delete sandbox {sid}: {e}") |
| 101 | 112 | ||
| 102 | 113 | ||
| 114 | +def parse_args(argv): | ||
| 115 | + parser = argparse.ArgumentParser( | ||
| 116 | + prog="volume.py", | ||
| 117 | + description=( | ||
| 118 | + "Volume functionality test for Conch (single-virtiofsd, host-path model). " | ||
| 119 | + "Mounts a host dir into a sandbox, writes a timestamp, then verifies the " | ||
| 120 | + "data persists across a second sandbox reusing the same host path." | ||
| 121 | + ), | ||
| 122 | + epilog=( | ||
| 123 | + "Host dirs are created under PWD/debug/shared and left in place for " | ||
| 124 | + "inspection. Set CONCH_TEMPLATE_ID before running, for example: " | ||
| 125 | + "CONCH_TEMPLATE_ID=sha256:abcd1234... python3 examples/volume.py" | ||
| 126 | + ), | ||
| 127 | + formatter_class=argparse.ArgumentDefaultsHelpFormatter, | ||
| 128 | + ) | ||
| 129 | + parser.add_argument( | ||
| 130 | + "-n", | ||
| 131 | + type=int, | ||
| 132 | + default=1, | ||
| 133 | + metavar="N", | ||
| 134 | + help=f"number of volume mount points (1-{MAX_MOUNTS}, default: 1)", | ||
| 135 | + ) | ||
| 136 | + return parser.parse_args(argv) | ||
| 137 | + | ||
| 138 | + | ||
| 103 | def main(): | 139 | def main(): |
| 140 | + args = parse_args(sys.argv[1:]) | ||
| 141 | + template_id = os.environ.get("CONCH_TEMPLATE_ID") | ||
| 142 | + if not template_id: | ||
| 143 | + print("CONCH_TEMPLATE_ID is required") | ||
| 144 | + return | ||
| 145 | + if not 1 <= args.n <= MAX_MOUNTS: | ||
| 146 | + raise SystemExit(f"-n must be between 1 and {MAX_MOUNTS}") | ||
| 104 | ensure_host_volumes() | 147 | ensure_host_volumes() |
| 105 | print(f"Host volume base: {HOST_BASE}") | 148 | print(f"Host volume base: {HOST_BASE}") |
| 149 | + print(f"using template_id: {template_id}") | ||
| 106 | 150 | ||
| 107 | - if len(sys.argv) < 2: | 151 | + sandboxes = [] |
| 108 | - print('missing template_id') | 152 | + try: |
| 109 | - return | 153 | + print("\n=== First sandbox: write to volume ===") |
| 110 | - tid = sys.argv[1] | 154 | + sandboxes.append(run_sandbox_write(template_id, args.n)) |
| 111 | - print(f'using template_id: {tid}') | ||
| 112 | 155 | ||
| 113 | - print("\n=== First sandbox: write to volume ===") | 156 | + print("\n=== Second sandbox: read from volume ===") |
| 114 | - run_sandbox_write(tid) | 157 | + sandboxes.append(run_sandbox_read(template_id, args.n)) |
| 115 | - | 158 | + finally: |
| 116 | - print("\n=== Second sandbox: read from volume ===") | 159 | + if sandboxes: |
| 117 | - run_sandbox_read(tid) | 160 | + delete_sandboxes(sandboxes) |
| 118 | 161 | ||
| 119 | 162 | ||
| 120 | if __name__ == "__main__": | 163 | if __name__ == "__main__": |
| @@ -25,6 +25,7 @@ import ( | |||
| 25 | "github.com/openeuler/Conch/internal/sandbox" | 25 | "github.com/openeuler/Conch/internal/sandbox" |
| 26 | "github.com/openeuler/Conch/internal/sandboxid" | 26 | "github.com/openeuler/Conch/internal/sandboxid" |
| 27 | conchtemplate "github.com/openeuler/Conch/internal/template" | 27 | conchtemplate "github.com/openeuler/Conch/internal/template" |
| 28 | + "github.com/openeuler/Conch/internal/volume" | ||
| 28 | "github.com/openeuler/Conch/pkg/ulog" | 29 | "github.com/openeuler/Conch/pkg/ulog" |
| 29 | ) | 30 | ) |
| 30 | 31 | ||
| @@ -49,6 +50,7 @@ type Service struct { | |||
| 49 | Snapshot SnapshotOps | 50 | Snapshot SnapshotOps |
| 50 | Store state.Store | 51 | Store state.Store |
| 51 | Templates conchtemplate.Store | 52 | Templates conchtemplate.Store |
| 53 | + VolumeManager *volume.Manager | ||
| 52 | SandboxDefaults SandboxDefaults | 54 | SandboxDefaults SandboxDefaults |
| 53 | lifecycleLocks sandboxLifecycleLocks | 55 | lifecycleLocks sandboxLifecycleLocks |
| 54 | } | 56 | } |
| @@ -104,6 +106,29 @@ func (s *Service) SetSandboxDefaults(defaults SandboxDefaults) { | |||
| 104 | s.SandboxDefaults = defaults | 106 | s.SandboxDefaults = defaults |
| 105 | } | 107 | } |
| 106 | 108 | ||
| 109 | +// SetVolumeManager wires the volume manager used to validate create requests. | ||
| 110 | +func (s *Service) SetVolumeManager(vm *volume.Manager) { | ||
| 111 | + if s == nil { | ||
| 112 | + return | ||
| 113 | + } | ||
| 114 | + s.VolumeManager = vm | ||
| 115 | +} | ||
| 116 | + | ||
| 117 | +// validateVolumeMounts rejects invalid mounts before creating sandbox state. | ||
| 118 | +func (s *Service) validateVolumeMounts(ctx context.Context, opts SandboxCreateOptions) error { | ||
| 119 | + if len(opts.VolumeMounts) == 0 { | ||
| 120 | + return nil | ||
| 121 | + } | ||
| 122 | + if s.Templates == nil { | ||
| 123 | + return fmt.Errorf("template store is not configured") | ||
| 124 | + } | ||
| 125 | + entry, err := s.Templates.Get(ctx, opts.TemplateID) | ||
| 126 | + if err != nil { | ||
| 127 | + return err | ||
| 128 | + } | ||
| 129 | + return s.VolumeManager.ValidateMounts(opts.VolumeMounts, entry.BootMode == conchtemplate.BootModeResume) | ||
| 130 | +} | ||
| 131 | + | ||
| 107 | func (s *Service) CreateSandbox(ctx context.Context, opts SandboxCreateOptions) (SandboxCreateResult, error) { | 132 | func (s *Service) CreateSandbox(ctx context.Context, opts SandboxCreateOptions) (SandboxCreateResult, error) { |
| 108 | if s == nil || s.Sandbox == nil { | 133 | if s == nil || s.Sandbox == nil { |
| 109 | return SandboxCreateResult{}, fmt.Errorf("sandbox service is not configured") | 134 | return SandboxCreateResult{}, fmt.Errorf("sandbox service is not configured") |
| @@ -158,6 +183,9 @@ func (s *Service) CreateSandbox(ctx context.Context, opts SandboxCreateOptions) | |||
| 158 | if err := netstack.ValidateSandboxNetworkInputConfig(ctx, opts.Network); err != nil { | 183 | if err := netstack.ValidateSandboxNetworkInputConfig(ctx, opts.Network); err != nil { |
| 159 | return SandboxCreateResult{}, err | 184 | return SandboxCreateResult{}, err |
| 160 | } | 185 | } |
| 186 | + if err := s.validateVolumeMounts(ctx, opts); err != nil { | ||
| 187 | + return SandboxCreateResult{}, err | ||
| 188 | + } | ||
| 161 | agentToken, err := sandbox.GenerateAgentToken() | 189 | agentToken, err := sandbox.GenerateAgentToken() |
| 162 | if err != nil { | 190 | if err != nil { |
| 163 | return SandboxCreateResult{}, err | 191 | return SandboxCreateResult{}, err |
| @@ -158,6 +158,7 @@ func New(cfg *config.Config) (*Daemon, error) { | |||
| 158 | s.runtimeService = conchruntime.New(host.SandboxManager(), host.Client(), store) | 158 | s.runtimeService = conchruntime.New(host.SandboxManager(), host.Client(), store) |
| 159 | s.runtimeService.Snapshot = host.SnapshotServer() | 159 | s.runtimeService.Snapshot = host.SnapshotServer() |
| 160 | s.runtimeService.Templates = host.TemplateStore() | 160 | s.runtimeService.Templates = host.TemplateStore() |
| 161 | + s.runtimeService.SetVolumeManager(s.volumeManager) | ||
| 161 | s.runtimeService.SetSandboxDefaults(runtimeapi.SandboxDefaults{ | 162 | s.runtimeService.SetSandboxDefaults(runtimeapi.SandboxDefaults{ |
| 162 | TemplateID: cfg.Sandbox.DefaultSpec.TemplateID, | 163 | TemplateID: cfg.Sandbox.DefaultSpec.TemplateID, |
| 163 | VMMName: cfg.Sandbox.Backend, | 164 | VMMName: cfg.Sandbox.Backend, |
| @@ -40,6 +40,11 @@ const ( | |||
| 40 | prefillCreateAttempts = 2 | 40 | prefillCreateAttempts = 2 |
| 41 | populateRetryMinDelay = time.Second | 41 | populateRetryMinDelay = time.Second |
| 42 | populateRetryMaxDelay = 30 * time.Second | 42 | populateRetryMaxDelay = 30 * time.Second |
| 43 | + // cleanupWorkers caps concurrent teardown of buffered network slots | ||
| 44 | + // during shutdown. CNI/netns removal is I/O-bound, so a fixed cap | ||
| 45 | + // independent of the host CPU count keeps the host network stack from | ||
| 46 | + // being overwhelmed on pools with many slots. | ||
| 47 | + cleanupWorkers = 16 | ||
| 43 | ) | 48 | ) |
| 44 | 49 | ||
| 45 | var ( | 50 | var ( |
| @@ -188,6 +193,7 @@ func (p *Pool) Start(ctx context.Context) error { | |||
| 188 | // Close stops the population loop, drains and closes the warm queue, and makes | 193 | // Close stops the population loop, drains and closes the warm queue, and makes |
| 189 | // a best-effort attempt to tear down every buffered slot. | 194 | // a best-effort attempt to tear down every buffered slot. |
| 190 | func (p *Pool) Close() { | 195 | func (p *Pool) Close() { |
| 196 | + defer ulog.TraceCost(ulog.TraceStart(), "", "netstack.Pool.Close()") | ||
| 191 | if p == nil { | 197 | if p == nil { |
| 192 | return | 198 | return |
| 193 | } | 199 | } |
| @@ -196,16 +202,45 @@ func (p *Pool) Close() { | |||
| 196 | <-p.populateDone | 202 | <-p.populateDone |
| 197 | } | 203 | } |
| 198 | if p.warmSlots != nil { | 204 | if p.warmSlots != nil { |
| 205 | + // Detach all buffered slots before starting cleanup so the queue is no | ||
| 206 | + // longer usable while teardown runs concurrently. | ||
| 207 | + var slots []*Slot | ||
| 199 | for { | 208 | for { |
| 200 | slot, err := p.warmSlots.Pop() | 209 | slot, err := p.warmSlots.Pop() |
| 201 | if err != nil { | 210 | if err != nil { |
| 202 | break | 211 | break |
| 203 | } | 212 | } |
| 204 | - if err := p.destroyNetworkSlot(context.Background(), slot); err != nil { | 213 | + slots = append(slots, slot) |
| 205 | - ulog.GetLogger().Warn("failed to clean up warm network slot during shutdown", ulog.F("slot_id", slot.ID()), ulog.F("error", err)) | ||
| 206 | - } | ||
| 207 | } | 214 | } |
| 208 | p.warmSlots.Close() | 215 | p.warmSlots.Close() |
| 216 | + | ||
| 217 | + if len(slots) > 0 { | ||
| 218 | + workers := cleanupWorkers | ||
| 219 | + if len(slots) < workers { | ||
| 220 | + workers = len(slots) | ||
| 221 | + } | ||
| 222 | + // Semaphore-bounded goroutines: slots are already materialized in | ||
| 223 | + // a slice, so a channel-based job queue would just re-buffer them. | ||
| 224 | + // The semaphore caps concurrency without that extra hop. | ||
| 225 | + sem := make(chan struct{}, workers) | ||
| 226 | + var wg sync.WaitGroup | ||
| 227 | + for _, slot := range slots { | ||
| 228 | + wg.Add(1) | ||
| 229 | + sem <- struct{}{} | ||
| 230 | + go func(s *Slot) { | ||
| 231 | + defer wg.Done() | ||
| 232 | + defer func() { <-sem }() | ||
| 233 | + if err := p.destroyNetworkSlot(context.Background(), s); err != nil { | ||
| 234 | + ulog.GetLogger().Warn( | ||
| 235 | + "failed to clean up warm network slot during shutdown", | ||
| 236 | + ulog.F("slot_id", s.ID()), | ||
| 237 | + ulog.F("error", err), | ||
| 238 | + ) | ||
| 239 | + } | ||
| 240 | + }(slot) | ||
| 241 | + } | ||
| 242 | + wg.Wait() | ||
| 243 | + } | ||
| 209 | } | 244 | } |
| 210 | if p.cniManager != nil { | 245 | if p.cniManager != nil { |
| 211 | if err := removeHostForwardingRules(p.cniManager.bridgeName, p.hostInterface); err != nil { | 246 | if err := removeHostForwardingRules(p.cniManager.bridgeName, p.hostInterface); err != nil { |
| @@ -407,7 +407,7 @@ func (m *Manager) Create(req CreateRequest) (result CreateResult, err error) { | |||
| 407 | }() | 407 | }() |
| 408 | 408 | ||
| 409 | vmStartSpec := vmStartSpecFromBootSpec(boot.Spec) | 409 | vmStartSpec := vmStartSpecFromBootSpec(boot.Spec) |
| 410 | - volumeDevices, err := m.prepareVolumes(req, boot.Runtime.Resume) | 410 | + volumeDevices, err := m.prepareVolumes(req) |
| 411 | if err != nil { | 411 | if err != nil { |
| 412 | return CreateResult{}, err | 412 | return CreateResult{}, err |
| 413 | } | 413 | } |
| @@ -454,13 +454,10 @@ func (m *Manager) Create(req CreateRequest) (result CreateResult, err error) { | |||
| 454 | return buildSandboxCreateResult(leaseID, req, sbx, boot, runtimeIDs, volumeDevices), nil | 454 | return buildSandboxCreateResult(leaseID, req, sbx, boot, runtimeIDs, volumeDevices), nil |
| 455 | } | 455 | } |
| 456 | 456 | ||
| 457 | -func (m *Manager) prepareVolumes(req CreateRequest, resume bool) ([]volume.Device, error) { | 457 | +func (m *Manager) prepareVolumes(req CreateRequest) ([]volume.Device, error) { |
| 458 | if len(req.VolumeMounts) == 0 { | 458 | if len(req.VolumeMounts) == 0 { |
| 459 | return nil, nil | 459 | return nil, nil |
| 460 | } | 460 | } |
| 461 | - if resume { | ||
| 462 | - return nil, ErrFailedPrecondition.Wrap(fmt.Errorf("sandbox with volumeMounts does not support snapshot startup")) | ||
| 463 | - } | ||
| 464 | if m.volumeManager == nil { | 461 | if m.volumeManager == nil { |
| 465 | return nil, fmt.Errorf("volume manager is not configured") | 462 | return nil, fmt.Errorf("volume manager is not configured") |
| 466 | } | 463 | } |
| @@ -29,35 +29,55 @@ func NewManager(cfg Config) (*Manager, error) { | |||
| 29 | }, nil | 29 | }, nil |
| 30 | } | 30 | } |
| 31 | 31 | ||
| 32 | -func (m *Manager) PrepareSandbox(sandboxID string, mounts []Mount) ([]Device, error) { | 32 | +func (m *Manager) ValidateMounts(mounts []Mount, resumeBoot bool) error { |
| 33 | if len(mounts) == 0 { | 33 | if len(mounts) == 0 { |
| 34 | - return nil, nil | 34 | + return nil |
| 35 | + } | ||
| 36 | + if m == nil { | ||
| 37 | + return fmt.Errorf("volume manager is not configured") | ||
| 38 | + } | ||
| 39 | + if resumeBoot { | ||
| 40 | + return ErrInvalidMount.WrapMessage( | ||
| 41 | + fmt.Errorf("volume_mounts requested on a snapshot-resume (warm) template"), | ||
| 42 | + "volume_mounts are not supported on snapshot-resume (warm) templates; use a cold boot template instead", | ||
| 43 | + ) | ||
| 35 | } | 44 | } |
| 36 | if len(mounts) > m.maxMounts { | 45 | if len(mounts) > m.maxMounts { |
| 37 | - return nil, ErrInvalidMount.Wrap(fmt.Errorf("volumeMounts exceeds limit %d: %d", m.maxMounts, len(mounts))) | 46 | + message := fmt.Sprintf("volume_mounts length %d exceeds configured maximum %d (volume.max_mounts)", len(mounts), m.maxMounts) |
| 47 | + return ErrInvalidMount.WrapMessage(fmt.Errorf("%s", message), message) | ||
| 38 | } | 48 | } |
| 39 | seenTargets := map[string]struct{}{} | 49 | seenTargets := map[string]struct{}{} |
| 40 | for _, mount := range mounts { | 50 | for _, mount := range mounts { |
| 41 | target := filepath.Clean(strings.TrimSpace(mount.Path)) | 51 | target := filepath.Clean(strings.TrimSpace(mount.Path)) |
| 42 | if !filepath.IsAbs(target) { | 52 | if !filepath.IsAbs(target) { |
| 43 | - return nil, ErrInvalidMount.Wrap(fmt.Errorf("volume mount path must be absolute: %s", mount.Path)) | 53 | + return ErrInvalidMount.Wrap(fmt.Errorf("volume mount path must be absolute: %s", mount.Path)) |
| 44 | } | 54 | } |
| 45 | if isBlockedTarget(target) { | 55 | if isBlockedTarget(target) { |
| 46 | - return nil, ErrInvalidMount.Wrap(fmt.Errorf("volume mount path is not allowed: %s", target)) | 56 | + return ErrInvalidMount.Wrap(fmt.Errorf("volume mount path is not allowed: %s", target)) |
| 47 | } | 57 | } |
| 48 | if _, ok := seenTargets[target]; ok { | 58 | if _, ok := seenTargets[target]; ok { |
| 49 | - return nil, ErrInvalidMount.Wrap(fmt.Errorf("duplicate volume mount path: %s", target)) | 59 | + return ErrInvalidMount.Wrap(fmt.Errorf("duplicate volume mount path: %s", target)) |
| 50 | } | 60 | } |
| 51 | seenTargets[target] = struct{}{} | 61 | seenTargets[target] = struct{}{} |
| 52 | 62 | ||
| 53 | source := filepath.Clean(strings.TrimSpace(mount.Source)) | 63 | source := filepath.Clean(strings.TrimSpace(mount.Source)) |
| 54 | if source == "" { | 64 | if source == "" { |
| 55 | - return nil, ErrInvalidMount.Wrap(fmt.Errorf("volume mount source must not be empty")) | 65 | + return ErrInvalidMount.Wrap(fmt.Errorf("volume mount source must not be empty")) |
| 56 | } | 66 | } |
| 57 | if !filepath.IsAbs(source) { | 67 | if !filepath.IsAbs(source) { |
| 58 | - return nil, ErrInvalidMount.Wrap(fmt.Errorf("volume mount source must be absolute: %s", source)) | 68 | + return ErrInvalidMount.Wrap(fmt.Errorf("volume mount source must be absolute: %s", source)) |
| 59 | } | 69 | } |
| 60 | } | 70 | } |
| 71 | + return nil | ||
| 72 | +} | ||
| 73 | + | ||
| 74 | +func (m *Manager) PrepareSandbox(sandboxID string, mounts []Mount) ([]Device, error) { | ||
| 75 | + if err := m.ValidateMounts(mounts, false); err != nil { | ||
| 76 | + return nil, err | ||
| 77 | + } | ||
| 78 | + if len(mounts) == 0 { | ||
| 79 | + return nil, nil | ||
| 80 | + } | ||
| 61 | return m.backend.Prepare(PrepareRequest{ | 81 | return m.backend.Prepare(PrepareRequest{ |
| 62 | SandboxID: sandboxID, | 82 | SandboxID: sandboxID, |
| 63 | Mounts: mounts, | 83 | Mounts: mounts, |