From ae29e3c979a34ab658b9b0fdaa9c514070362e1b Mon Sep 17 00:00:00 2001 From: Yibo Zhuang Date: Tue, 1 Sep 2026 13:59:48 -0700 Subject: [PATCH] Worker: preserve configuration of suspended Tart clones (#473) 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 87e5ca8..e493f43 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 07bb958..add866d 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 0000000..2aad25e --- /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 26622ad..9f2c030 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 {