From 2f3093b212433292cff3e6aabe8a5974ff734f61 Mon Sep 17 00:00:00 2001 From: Yibo Zhuang Date: Mon, 31 Aug 2026 18:34:11 -0700 Subject: [PATCH] Worker: preserve configuration of suspended Tart clones Read the cloned VM state before applying resource overrides or randomizing its MAC address and serial. Suspended clones must retain the configuration used to save their state. Cover suspended and stopped clones through the command runner, including assigned resource precedence, optional image FQN lookup, and metadata command and JSON errors. --- internal/worker/vmmanager/tart/cmd.go | 20 ++ internal/worker/vmmanager/tart/tart.go | 9 + internal/worker/vmmanager/tart/tart_test.go | 215 ++++++++++++++++++++ internal/worker/vmmanager/vmmanager.go | 9 +- 4 files changed, 249 insertions(+), 4 deletions(-) create mode 100644 internal/worker/vmmanager/tart/tart_test.go diff --git a/internal/worker/vmmanager/tart/cmd.go b/internal/worker/vmmanager/tart/cmd.go index 87e5ca82..e493f434 100644 --- a/internal/worker/vmmanager/tart/cmd.go +++ b/internal/worker/vmmanager/tart/cmd.go @@ -2,6 +2,7 @@ package tart import ( "context" + "encoding/json" "github.com/cirruslabs/orchard/internal/worker/vmmanager" "github.com/cirruslabs/orchard/internal/worker/vmmanager/base" @@ -17,3 +18,22 @@ func Tart(ctx context.Context, logger *zap.SugaredLogger, args ...string) (strin func List(ctx context.Context, logger *zap.SugaredLogger) ([]vmmanager.VMInfo, error) { return base.List(ctx, logger, tartCommandName) } + +func Info(ctx context.Context, logger *zap.SugaredLogger, name string) (*vmmanager.VMInfo, error) { + output, _, err := Tart(ctx, logger, "get", name, "--format", "json") + if err != nil { + return nil, err + } + + info := &vmmanager.VMInfo{ + Name: name, + Source: "local", + State: "", + Running: false, + } + if err := json.Unmarshal([]byte(output), info); err != nil { + return nil, err + } + + return info, nil +} diff --git a/internal/worker/vmmanager/tart/tart.go b/internal/worker/vmmanager/tart/tart.go index 15d0c31c..718be8a9 100644 --- a/internal/worker/vmmanager/tart/tart.go +++ b/internal/worker/vmmanager/tart/tart.go @@ -153,6 +153,15 @@ func (vm *VM) cloneAndConfigure(ctx context.Context) error { vm.imageFQN.Store(&fqn) } + // A suspended VM must resume with the configuration used to save its state. + info, err := Info(ctx, vm.logger, vm.id()) + if err != nil { + return err + } + if info.State == "suspended" { + return nil + } + // Set memory vm.SetStatusMessage("configuring VM...") diff --git a/internal/worker/vmmanager/tart/tart_test.go b/internal/worker/vmmanager/tart/tart_test.go new file mode 100644 index 00000000..2aad25e3 --- /dev/null +++ b/internal/worker/vmmanager/tart/tart_test.go @@ -0,0 +1,215 @@ +//nolint:exhaustruct_v5,goconst // Fixtures omit unrelated fields and keep expected command strings explicit. +package tart //nolint:testpackage // Exercise clone and configuration through the real command runner. + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/cirruslabs/orchard/internal/worker/ondiskname" + "github.com/cirruslabs/orchard/internal/worker/vmmanager/base" + v1 "github.com/cirruslabs/orchard/pkg/resource/v1" + "github.com/stretchr/testify/require" + "go.uber.org/zap" +) + +func TestCloneAndConfigurePreservesSuspendedVM(t *testing.T) { + for _, failedCommand := range []string{"", "fqn"} { + t.Run("failed command="+failedCommand, func(t *testing.T) { + commandLog := installCloneFakeTart(t, + `{"OS":"darwin","CPU":4,"Memory":8192,"Disk":50,"Running":false,"State":"suspended"}`, + failedCommand) + vm := newCloneTestVM(v1.VM{ + Name: "test-vm", + UID: "00112233-4455-6677-8899-aabbccddeeff", + Image: "source-image", + CPU: 2, + AssignedCPU: 6, + Memory: 4096, + AssignedMemory: 12288, + DiskSize: 100, + RandomSerial: true, + }) + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) + defer cancel() + + require.NoError(t, vm.cloneAndConfigure(ctx)) + requireCloneCommands(t, commandLog, []string{ + "clone source-image " + vm.id(), + "fqn source-image", + "get " + vm.id() + " --format json", + }) + require.False(t, vm.ConditionsSet().ContainsOne(v1.ConditionTypeCloning)) + if failedCommand == "fqn" { + require.Nil(t, vm.ImageFQN()) + } else { + require.NotNil(t, vm.ImageFQN()) + require.Equal(t, "registry.example/source@sha256:abc", *vm.ImageFQN()) + } + }) + } +} + +func TestCloneAndConfigureConfiguresStoppedVM(t *testing.T) { + tests := []struct { + name string + resource v1.VM + setArgs []string + }{ + { + name: "requested resources", + resource: v1.VM{ + CPU: 2, + Memory: 4096, + DiskSize: 100, + RandomSerial: true, + }, + setArgs: []string{"--memory 4096", "--cpu 2", "--disk-size 100", "--random-mac", "--random-serial"}, + }, + { + name: "assigned resources override requested resources", + resource: v1.VM{ + CPU: 2, + AssignedCPU: 6, + Memory: 4096, + AssignedMemory: 12288, + }, + setArgs: []string{"--memory 12288", "--cpu 6", "--random-mac"}, + }, + { + name: "image resource defaults", + setArgs: []string{"--random-mac"}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + commandLog := installCloneFakeTart(t, `{"Running":false,"State":"stopped"}`, "") + test.resource.Name = "test-vm" + test.resource.UID = "00112233-4455-6677-8899-aabbccddeeff" + test.resource.Image = "source-image" + vm := newCloneTestVM(test.resource) + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) + defer cancel() + + require.NoError(t, vm.cloneAndConfigure(ctx)) + commands := []string{ + "clone source-image " + vm.id(), + "fqn source-image", + "get " + vm.id() + " --format json", + } + for _, args := range test.setArgs { + commands = append(commands, "set "+args+" "+vm.id()) + } + requireCloneCommands(t, commandLog, commands) + }) + } +} + +func TestCloneAndConfigureStopsOnMetadataError(t *testing.T) { + tests := []struct { + name string + output string + failedCommand string + wantError string + }{ + { + name: "get command fails", + failedCommand: "get", + wantError: "injected get failure", + }, + { + name: "invalid JSON", + output: `{"State":`, + wantError: "unexpected end of JSON input", + }, + { + name: "invalid state type", + output: `{"State":123}`, + wantError: "cannot unmarshal number", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + commandLog := installCloneFakeTart(t, test.output, test.failedCommand) + vm := newCloneTestVM(v1.VM{ + Name: "test-vm", + UID: "00112233-4455-6677-8899-aabbccddeeff", + Image: "source-image", + Memory: 4096, + }) + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) + defer cancel() + + err := vm.cloneAndConfigure(ctx) + require.ErrorContains(t, err, test.wantError) + if test.name == "invalid JSON" { + var syntaxError *json.SyntaxError + require.ErrorAs(t, err, &syntaxError) + } + if test.name == "invalid state type" { + var typeError *json.UnmarshalTypeError + require.ErrorAs(t, err, &typeError) + } + requireCloneCommands(t, commandLog, []string{ + "clone source-image " + vm.id(), + "fqn source-image", + "get " + vm.id() + " --format json", + }) + }) + } +} + +func newCloneTestVM(resource v1.VM) *VM { + logger := zap.NewNop().Sugar() + + return &VM{ + onDiskName: ondiskname.NewFromResource(resource), + resource: resource, + logger: logger, + VM: base.NewVM(logger), + } +} + +func requireCloneCommands(t *testing.T, commandLog string, commands []string) { + t.Helper() + + logged, err := os.ReadFile(commandLog) //nolint:gosec // The command log is created in t.TempDir. + require.NoError(t, err) + require.Equal(t, commands, strings.Split(strings.TrimSpace(string(logged)), "\n")) +} + +func installCloneFakeTart(t *testing.T, infoOutput string, failedCommand string) string { + t.Helper() + + dir := t.TempDir() + commandLog := filepath.Join(dir, "commands.log") + script := `#!/bin/sh +set -eu +printf '%s\n' "$*" >> "$ORCHARD_TEST_TART_COMMAND_LOG" +if [ "$1" = "$ORCHARD_TEST_TART_FAILED_COMMAND" ]; then + printf 'injected %s failure\n' "$1" >&2 + exit 1 +fi +case "$1" in + clone|set) ;; + fqn) printf 'registry.example/source@sha256:abc\n' ;; + get) printf '%s\n' "$ORCHARD_TEST_TART_INFO" ;; + *) printf 'unexpected command: %s\n' "$*" >&2; exit 1 ;; +esac +` + commandPath := filepath.Join(dir, tartCommandName) + require.NoError(t, os.WriteFile(commandPath, []byte(script), 0o600)) + require.NoError(t, os.Chmod(commandPath, 0o700)) //nolint:gosec // The fake Tart command must be executable. + t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) + t.Setenv("ORCHARD_TEST_TART_COMMAND_LOG", commandLog) + t.Setenv("ORCHARD_TEST_TART_INFO", infoOutput) + t.Setenv("ORCHARD_TEST_TART_FAILED_COMMAND", failedCommand) + + return commandLog +} diff --git a/internal/worker/vmmanager/vmmanager.go b/internal/worker/vmmanager/vmmanager.go index 26622adc..9f2c030c 100644 --- a/internal/worker/vmmanager/vmmanager.go +++ b/internal/worker/vmmanager/vmmanager.go @@ -26,11 +26,12 @@ type VM interface { Delete() error } +//nolint:tagliatelle // The Tart and Vetu CLI JSON fields are capitalized. type VMInfo struct { - Name string - Source string - State string - Running bool + Name string `json:"Name"` + Source string `json:"Source"` + State string `json:"State"` + Running bool `json:"Running"` } type VMManager struct {