diff --git a/spinifex/daemon/daemon_system_instance.go b/spinifex/daemon/daemon_system_instance.go index d8a65b4dc..641727169 100644 --- a/spinifex/daemon/daemon_system_instance.go +++ b/spinifex/daemon/daemon_system_instance.go @@ -8,6 +8,7 @@ import ( "log/slog" "os" "path/filepath" + "slices" "strings" "time" @@ -383,13 +384,9 @@ func (d *Daemon) LaunchSystemInstance(input *handlers_elbv2.SystemInstanceInput) } } - // Store extra hostfwd ports (filled in by StartInstance with actual host ports) - if len(input.HostfwdPorts) > 0 { - instance.ExtraHostfwd = make(map[int]int, len(input.HostfwdPorts)) - for _, port := range input.HostfwdPorts { - instance.ExtraHostfwd[port] = 0 // host port assigned in StartInstance - } - } + // The requested guest ports. The host ports they land on are observed by + // the launch and reported back through HostfwdPortMap. + instance.HostfwdPorts = slices.Clone(input.HostfwdPorts) // Direct-boot (microvm) path: build the vm.Config directly with microvm // machine settings and fw_cfg blobs for network, lb-agent env, and CA cert. @@ -423,7 +420,7 @@ func (d *Daemon) LaunchSystemInstance(input *handlers_elbv2.SystemInstanceInput) InstanceID: instance.ID, PrivateIP: privateIP, PublicIP: publicIP, - HostfwdMap: instance.ExtraHostfwd, + HostfwdMap: instance.HostfwdPortMap, }, nil } diff --git a/spinifex/handlers/ec2/instance/service_impl.go b/spinifex/handlers/ec2/instance/service_impl.go index c72ac227d..138ff03d4 100644 --- a/spinifex/handlers/ec2/instance/service_impl.go +++ b/spinifex/handlers/ec2/instance/service_impl.go @@ -2129,8 +2129,6 @@ func (s *InstanceServiceImpl) ModifyInstanceAttribute(ctx context.Context, input v.InstanceType = newType v.Config.InstanceType = newType v.Instance.InstanceType = aws.String(newType) - // Clear StateReason — resolves capacity-unavailable state from instance-type-missing bug. - v.Instance.StateReason = nil } if input.UserData != nil && input.UserData.Value != nil { diff --git a/spinifex/handlers/ec2/instance/service_impl_test.go b/spinifex/handlers/ec2/instance/service_impl_test.go index 380292981..4c2284eca 100644 --- a/spinifex/handlers/ec2/instance/service_impl_test.go +++ b/spinifex/handlers/ec2/instance/service_impl_test.go @@ -1357,7 +1357,9 @@ func TestModifyInstanceAttribute_ChangeUserData(t *testing.T) { assert.Equal(t, "IyEvYmluL2Jhc2g=", *updated.RunInstancesInput.UserData) } -func TestModifyInstanceAttribute_ClearsStateReason(t *testing.T) { +// StateReason is observed state, so the API must not touch it. The node clears +// it when it starts the instance; until then the reason for the stop stands. +func TestModifyInstanceAttribute_LeavesStateReasonToTheNode(t *testing.T) { id := "i-recovery" store := &vmmock.StateStore{Stopped: map[string]*vm.VM{ id: { @@ -1386,7 +1388,9 @@ func TestModifyInstanceAttribute_ClearsStateReason(t *testing.T) { updated := store.Stopped[id] require.NotNil(t, updated) - assert.Nil(t, updated.Instance.StateReason) + assert.Equal(t, "t3.micro", updated.InstanceType, "the type change itself must still apply") + require.NotNil(t, updated.Instance.StateReason, "the API must not clear observed state") + assert.Equal(t, "Server.InsufficientInstanceCapacity", *updated.Instance.StateReason.Code) } func TestModifyInstanceAttribute_DisableApiTermination_Running(t *testing.T) { diff --git a/spinifex/vm/desired_state_test.go b/spinifex/vm/desired_state_test.go index 60df56936..5e80172de 100644 --- a/spinifex/vm/desired_state_test.go +++ b/spinifex/vm/desired_state_test.go @@ -1,10 +1,11 @@ -package vm +package vm_test import ( "encoding/json" "testing" "github.com/mulgadc/spinifex/spinifex/types" + "github.com/mulgadc/spinifex/spinifex/vm" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -13,49 +14,49 @@ import ( // command it was stamped with. Losing that on upgrade would relaunch an // instance the operator deliberately stopped. func TestVMUnmarshal_LegacyOperatorStopFolds(t *testing.T) { - var v VM + var v vm.VM require.NoError(t, json.Unmarshal([]byte( `{"id":"i-1","status":"stopped","attributes":{"stop_instance":true}}`), &v)) - assert.Equal(t, DesiredStopped, v.DesiredState) + assert.Equal(t, vm.DesiredStopped, v.DesiredState) assert.Nil(t, v.LegacyAttributes, "the legacy field is read once and dropped") } // A drain stop never stamped StopInstance, so a legacy drain-stopped record // must stay desired-running and be relaunched. func TestVMUnmarshal_LegacyDrainStopStaysRunning(t *testing.T) { - var v VM + var v vm.VM require.NoError(t, json.Unmarshal([]byte( `{"id":"i-1","status":"stopped","attributes":{"stop_instance":false}}`), &v)) - assert.Equal(t, DesiredRunning, v.DesiredState) + assert.Equal(t, vm.DesiredRunning, v.DesiredState) } // Only StopInstance folds. TerminateInstance was a launch-race latch, not a // persisted intent, and a terminated record is already settled by Status. func TestVMUnmarshal_LegacyTerminateDoesNotFold(t *testing.T) { - var v VM + var v vm.VM require.NoError(t, json.Unmarshal([]byte( `{"id":"i-1","status":"terminated","attributes":{"delete_instance":true}}`), &v)) - assert.Equal(t, DesiredRunning, v.DesiredState) + assert.Equal(t, vm.DesiredRunning, v.DesiredState) assert.Nil(t, v.DeletionTimestamp) } // A record carrying both must take the new field: the legacy one is only a // fallback for records written before it existed. func TestVMUnmarshal_ExplicitDesiredStateWins(t *testing.T) { - var v VM + var v vm.VM require.NoError(t, json.Unmarshal([]byte( `{"id":"i-1","desired_state":"stopped","attributes":{"stop_instance":false}}`), &v)) - assert.Equal(t, DesiredStopped, v.DesiredState) + assert.Equal(t, vm.DesiredStopped, v.DesiredState) } // The point of the change is that the command stops being persisted at all. // Round-tripping a legacy record must emit desired_state and no attributes. func TestVMMarshal_LegacyAttributesNotWrittenBack(t *testing.T) { - var v VM + var v vm.VM require.NoError(t, json.Unmarshal([]byte( `{"id":"i-1","status":"stopped","attributes":{"stop_instance":true}}`), &v)) @@ -68,11 +69,49 @@ func TestVMMarshal_LegacyAttributesNotWrittenBack(t *testing.T) { assert.Contains(t, raw, "desired_state") } +// A legacy record held the requested guest port and the host port the launch +// bound in one map. Both halves must survive the split. +func TestVMUnmarshal_LegacyHostfwdSplits(t *testing.T) { + var v vm.VM + require.NoError(t, json.Unmarshal([]byte( + `{"id":"i-1","extra_hostfwd":{"8443":19443,"8080":18080}}`), &v)) + + assert.Equal(t, []int{8080, 8443}, v.HostfwdPorts, "requested ports are sorted so a launch is deterministic") + assert.Equal(t, map[int]int{8080: 18080, 8443: 19443}, v.HostfwdPortMap) + assert.Nil(t, v.LegacyExtraHostfwd, "the legacy field is read once and dropped") +} + +// A record written before the VM ever launched carries zeroes. A zero is "not +// bound yet", not a host port, so it must not become a mapping. +func TestVMUnmarshal_LegacyHostfwdUnboundPortsAreNotMapped(t *testing.T) { + var v vm.VM + require.NoError(t, json.Unmarshal([]byte( + `{"id":"i-1","extra_hostfwd":{"8080":0,"8443":19443}}`), &v)) + + assert.Equal(t, []int{8080, 8443}, v.HostfwdPorts) + assert.Equal(t, map[int]int{8443: 19443}, v.HostfwdPortMap) +} + +func TestVMMarshal_LegacyHostfwdNotWrittenBack(t *testing.T) { + var v vm.VM + require.NoError(t, json.Unmarshal([]byte( + `{"id":"i-1","extra_hostfwd":{"8080":18080}}`), &v)) + + out, err := json.Marshal(&v) + require.NoError(t, err) + + var raw map[string]json.RawMessage + require.NoError(t, json.Unmarshal(out, &raw)) + assert.NotContains(t, raw, "extra_hostfwd") + assert.Contains(t, raw, "hostfwd_ports") + assert.Contains(t, raw, "hostfwd_port_map") +} + // The other twelve command booleans had nowhere to go because nothing read them // back. A record must carry none of them, so they are gone rather than unread. func TestVMMarshal_CommandBooleansAreNotPersisted(t *testing.T) { legacy, err := json.Marshal(map[string]any{ - "id": "i-1", "status": StateRunning, + "id": "i-1", "status": vm.StateRunning, "attributes": types.EC2CommandAttributes{ AttachVolume: true, DetachVolume: true, @@ -90,7 +129,7 @@ func TestVMMarshal_CommandBooleansAreNotPersisted(t *testing.T) { }) require.NoError(t, err) - var v VM + var v vm.VM require.NoError(t, json.Unmarshal(legacy, &v)) require.Nil(t, v.LegacyAttributes) diff --git a/spinifex/vm/lifecycle.go b/spinifex/vm/lifecycle.go index 1c5d97c1e..9b5e1cc0a 100644 --- a/spinifex/vm/lifecycle.go +++ b/spinifex/vm/lifecycle.go @@ -206,6 +206,15 @@ func (m *Manager) launch(ctx context.Context, instance *VM) (err error) { return nil } + // A reason left by the stop, or by a restore that found this node could not + // schedule the instance, describes a state the instance is now leaving. + // Clearing it here keeps the node the only writer of the field. + m.Inspect(instance, func(v *VM) { + if v.Instance != nil { + v.Instance.StateReason = nil + } + }) + pid, _ := utils.ReadPidFile(instance.ID) if pid > 0 { process, err := os.FindProcess(pid) @@ -712,27 +721,32 @@ func (m *Manager) appendDevHostfwdNIC(instance *VM) { var nb strings.Builder fmt.Fprintf(&nb, "user,id=dev0,hostfwd=tcp:%s:%s-:22", bindIP, sshDebugPort) - if instance.ExtraHostfwd != nil { - for guestPort := range instance.ExtraHostfwd { - fwdAddr, fwdErr := findFreePort() - if fwdErr != nil { - slog.Warn("DEV_NETWORKING: failed to find free port for extra hostfwd", "guestPort", guestPort, "err", fwdErr) - continue - } - _, hostPort, splitErr := net.SplitHostPort(fwdAddr) - if splitErr != nil { - slog.Warn("DEV_NETWORKING: failed to parse extra hostfwd address", "fwdAddr", fwdAddr, "err", splitErr) - continue - } - hostPortInt, convErr := strconv.Atoi(hostPort) - if convErr != nil { - slog.Warn("DEV_NETWORKING: failed to convert extra hostfwd port", "hostPort", hostPort, "err", convErr) - continue - } - fmt.Fprintf(&nb, ",hostfwd=tcp:%s:%s-:%d", bindIP, hostPort, guestPort) - instance.ExtraHostfwd[guestPort] = hostPortInt - slog.Info("DEV_NETWORKING: extra hostfwd", "guestPort", guestPort, "hostPort", hostPort, "instanceId", instance.ID) + // The map is what this launch observed, so it is rebuilt rather than + // updated: a port that bound on a previous launch but not on this one must + // not still be advertised. + instance.HostfwdPortMap = nil + for _, guestPort := range instance.HostfwdPorts { + fwdAddr, fwdErr := findFreePort() + if fwdErr != nil { + slog.Warn("DEV_NETWORKING: failed to find free port for extra hostfwd", "guestPort", guestPort, "err", fwdErr) + continue } + _, hostPort, splitErr := net.SplitHostPort(fwdAddr) + if splitErr != nil { + slog.Warn("DEV_NETWORKING: failed to parse extra hostfwd address", "fwdAddr", fwdAddr, "err", splitErr) + continue + } + hostPortInt, convErr := strconv.Atoi(hostPort) + if convErr != nil { + slog.Warn("DEV_NETWORKING: failed to convert extra hostfwd port", "hostPort", hostPort, "err", convErr) + continue + } + fmt.Fprintf(&nb, ",hostfwd=tcp:%s:%s-:%d", bindIP, hostPort, guestPort) + if instance.HostfwdPortMap == nil { + instance.HostfwdPortMap = make(map[int]int, len(instance.HostfwdPorts)) + } + instance.HostfwdPortMap[guestPort] = hostPortInt + slog.Info("DEV_NETWORKING: extra hostfwd", "guestPort", guestPort, "hostPort", hostPort, "instanceId", instance.ID) } instance.Config.NetDevs = append(instance.Config.NetDevs, NetDev{Value: nb.String()}) diff --git a/spinifex/vm/lifecycle_test.go b/spinifex/vm/lifecycle_test.go index bba345852..9c3a2c2f0 100644 --- a/spinifex/vm/lifecycle_test.go +++ b/spinifex/vm/lifecycle_test.go @@ -650,8 +650,8 @@ func stubFindFreePort(t *testing.T, results ...struct { return &idx } -func newDevNICInstance(id string, extra map[int]int) *VM { - return &VM{ID: id, ExtraHostfwd: extra} +func newDevNICInstance(id string, guestPorts ...int) *VM { + return &VM{ID: id, HostfwdPorts: guestPorts} } func TestAppendDevHostfwdNIC_SSHPortFails_NICSkipped(t *testing.T) { @@ -662,7 +662,7 @@ func TestAppendDevHostfwdNIC_SSHPortFails_NICSkipped(t *testing.T) { m := NewManager() m.SetDeps(Deps{BindHost: "10.0.0.1"}) - instance := newDevNICInstance("i-ssh-fail", nil) + instance := newDevNICInstance("i-ssh-fail") m.appendDevHostfwdNIC(instance) @@ -678,7 +678,7 @@ func TestAppendDevHostfwdNIC_SSHAddrUnparsable_NICSkipped(t *testing.T) { }{addr: "nocolon", err: nil}) m := NewManager() - instance := newDevNICInstance("i-ssh-bad-addr", nil) + instance := newDevNICInstance("i-ssh-bad-addr") m.appendDevHostfwdNIC(instance) @@ -687,7 +687,7 @@ func TestAppendDevHostfwdNIC_SSHAddrUnparsable_NICSkipped(t *testing.T) { assert.Empty(t, instance.Config.Devices) } -func TestAppendDevHostfwdNIC_ExtraHostfwdNil_ShortCircuit(t *testing.T) { +func TestAppendDevHostfwdNIC_NoExtraPorts_ShortCircuit(t *testing.T) { calls := stubFindFreePort(t, struct { addr string err error @@ -695,7 +695,7 @@ func TestAppendDevHostfwdNIC_ExtraHostfwdNil_ShortCircuit(t *testing.T) { m := NewManager() m.SetDeps(Deps{BindHost: "0.0.0.0"}) - instance := newDevNICInstance("i-no-extra", nil) + instance := newDevNICInstance("i-no-extra") m.appendDevHostfwdNIC(instance) @@ -719,14 +719,14 @@ func TestAppendDevHostfwdNIC_ExtraPortFails_WarningContinues(t *testing.T) { ) m := NewManager() - instance := newDevNICInstance("i-extra-fail", map[int]int{8080: 0}) + instance := newDevNICInstance("i-extra-fail", 8080) m.appendDevHostfwdNIC(instance) assert.Equal(t, 2, *calls) require.Len(t, instance.Config.NetDevs, 1) assert.Equal(t, "user,id=dev0,hostfwd=tcp:127.0.0.1:2222-:22", instance.Config.NetDevs[0].Value, "failed extra entry must not appear in netdev value") - assert.Equal(t, 0, instance.ExtraHostfwd[8080], "failed entry must leave the map value at its zero") + assert.NotContains(t, instance.HostfwdPortMap, 8080, "a port that never bound must be absent, not mapped to zero") require.Len(t, instance.Config.Devices, 1) } @@ -747,24 +747,18 @@ func TestAppendDevHostfwdNIC_ExtraAddrUnparsable_EntrySkipped(t *testing.T) { ) m := NewManager() - instance := newDevNICInstance("i-extra-split", map[int]int{8080: 0, 8443: 0}) + instance := newDevNICInstance("i-extra-split", 8080, 8443) m.appendDevHostfwdNIC(instance) assert.Equal(t, 3, *calls) require.Len(t, instance.Config.NetDevs, 1) - var goodGuest, badGuest int - for g := range instance.ExtraHostfwd { - if instance.ExtraHostfwd[g] == 9090 { - goodGuest = g - } else { - badGuest = g - } - } - require.NotZero(t, goodGuest, "exactly one entry must have been populated") - assert.Equal(t, 0, instance.ExtraHostfwd[badGuest], "skipped entry must remain at zero") - assert.Contains(t, instance.Config.NetDevs[0].Value, fmt.Sprintf("hostfwd=tcp:127.0.0.1:9090-:%d", goodGuest)) + // Ports are visited in the order requested, so the unparsable address is + // consumed by 8080 and 8443 gets the good one. + assert.NotContains(t, instance.HostfwdPortMap, 8080, "skipped entry must be absent") + assert.Equal(t, map[int]int{8443: 9090}, instance.HostfwdPortMap) + assert.Contains(t, instance.Config.NetDevs[0].Value, "hostfwd=tcp:127.0.0.1:9090-:8443") assert.NotContains(t, instance.Config.NetDevs[0].Value, "garbage") } @@ -781,17 +775,17 @@ func TestAppendDevHostfwdNIC_ExtraPortAtoiFails_EntrySkipped(t *testing.T) { ) m := NewManager() - instance := newDevNICInstance("i-extra-atoi", map[int]int{8080: 0}) + instance := newDevNICInstance("i-extra-atoi", 8080) m.appendDevHostfwdNIC(instance) assert.Equal(t, 2, *calls) require.Len(t, instance.Config.NetDevs, 1) assert.Equal(t, "user,id=dev0,hostfwd=tcp:127.0.0.1:2222-:22", instance.Config.NetDevs[0].Value, "non-numeric extra port must not be appended to netdev value") - assert.Equal(t, 0, instance.ExtraHostfwd[8080], "skipped entry must remain at zero") + assert.NotContains(t, instance.HostfwdPortMap, 8080, "skipped entry must be absent") } -func TestAppendDevHostfwdNIC_ExtraHostfwd_HappyPath(t *testing.T) { +func TestAppendDevHostfwdNIC_ExtraPorts_HappyPath(t *testing.T) { stubFindFreePort(t, struct { addr string @@ -804,11 +798,11 @@ func TestAppendDevHostfwdNIC_ExtraHostfwd_HappyPath(t *testing.T) { ) m := NewManager() - instance := newDevNICInstance("i-extra-ok", map[int]int{8080: 0}) + instance := newDevNICInstance("i-extra-ok", 8080) m.appendDevHostfwdNIC(instance) - assert.Equal(t, 18080, instance.ExtraHostfwd[8080]) + assert.Equal(t, map[int]int{8080: 18080}, instance.HostfwdPortMap) require.Len(t, instance.Config.NetDevs, 1) assert.Equal(t, "user,id=dev0,hostfwd=tcp:127.0.0.1:2222-:22,hostfwd=tcp:127.0.0.1:18080-:8080", instance.Config.NetDevs[0].Value) require.Len(t, instance.Config.Devices, 1) diff --git a/spinifex/vm/state_reason_test.go b/spinifex/vm/state_reason_test.go new file mode 100644 index 000000000..f501bbca1 --- /dev/null +++ b/spinifex/vm/state_reason_test.go @@ -0,0 +1,73 @@ +// Drives Manager.launch through the unexported relaunchTestManager harness, +// the only seam that reaches the launch path without a real QEMU process. +// +//test:in-package +package vm + +import ( + "context" + "errors" + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func unschedulableReason() *ec2.StateReason { + return &ec2.StateReason{ + Code: aws.String("Server.InsufficientInstanceCapacity"), + Message: aws.String("instance type m7i.small is not available on this node"), + } +} + +// The node writes StateReason when it stops or fails an instance, so the node +// is also what clears it. A launch clears before it can fail, so a failed +// launch that stamps a fresh reason does not have to fight a stale one. +func TestLaunch_ClearsStaleStateReason(t *testing.T) { + m, mounter, _, _ := relaunchTestManager(t) + + instance := &VM{ + ID: "i-stale-reason", + Status: StateStopped, + InstanceType: "m7i.small", + Instance: &ec2.Instance{StateReason: unschedulableReason()}, + } + m.Insert(instance) + mounter.behavior[instance.ID] = func(*VM) error { return errors.New("mount failed") } + + require.Error(t, m.Run(context.Background(), instance), "the launch is expected to fail at Mount") + assert.Nil(t, instance.Instance.StateReason, + "the reason describes the state the instance is leaving and must not outlive the launch attempt") +} + +// A launch that never starts must leave the reason alone: it still describes +// the state the instance is actually in. +func TestLaunch_AbortedByTerminate_KeepsStateReason(t *testing.T) { + m, _, _, _ := relaunchTestManager(t) + + instance := &VM{ + ID: "i-terminating", + Status: StateShuttingDown, + InstanceType: "m7i.small", + Instance: &ec2.Instance{StateReason: unschedulableReason()}, + } + m.Insert(instance) + + require.NoError(t, m.Run(context.Background(), instance), "a raced terminate aborts the launch quietly") + require.NotNil(t, instance.Instance.StateReason) + assert.Equal(t, "Server.InsufficientInstanceCapacity", *instance.Instance.StateReason.Code) +} + +// markUnschedulable is the writer this clear is the counterpart of; a record +// with no ec2.Instance must not panic either side. +func TestLaunch_NoEC2Instance_NoPanic(t *testing.T) { + m, mounter, _, _ := relaunchTestManager(t) + + instance := &VM{ID: "i-bare", Status: StateStopped, InstanceType: "m7i.small"} + m.Insert(instance) + mounter.behavior[instance.ID] = func(*VM) error { return errors.New("mount failed") } + + require.Error(t, m.Run(context.Background(), instance)) +} diff --git a/spinifex/vm/vm.go b/spinifex/vm/vm.go index 64901a9a7..d75e23229 100644 --- a/spinifex/vm/vm.go +++ b/spinifex/vm/vm.go @@ -6,6 +6,7 @@ import ( "log/slog" "os" "os/exec" + "slices" "strconv" "strings" "sync" @@ -147,10 +148,20 @@ type VM struct { PlacementGroupName string `json:"placement_group_name,omitempty"` PlacementGroupNode string `json:"placement_group_node,omitempty"` - // ExtraHostfwdPorts lists additional guest ports to forward from the host - // via the QEMU user-mode dev NIC. Used by ALB VMs to expose HTTP ports. - // Maps guest port → host port (host port filled in by StartInstance). - ExtraHostfwd map[int]int `json:"extra_hostfwd,omitempty"` + // HostfwdPorts lists the guest ports to forward from the host via the QEMU + // user-mode dev NIC. Requested at launch; used by ALB VMs to expose HTTP + // ports. + HostfwdPorts []int `json:"hostfwd_ports,omitempty"` + + // HostfwdPortMap records which host port the launch actually bound for each + // guest port in HostfwdPorts. Observed rather than requested, so it is + // rebuilt on every launch and a port that could not be bound is absent. + HostfwdPortMap map[int]int `json:"hostfwd_port_map,omitempty"` + + // LegacyExtraHostfwd is the single guest→host map records used before the + // request and the observation were separated. Read on decode to recover + // both halves, then dropped; never written. + LegacyExtraHostfwd map[int]int `json:"extra_hostfwd,omitempty"` // ManagedBy identifies the Spinifex platform component that owns this // VM (e.g. "elbv2"). Empty for customer-launched instances. The UI @@ -208,9 +219,10 @@ type VM struct { SpotInstanceRequestId string `json:"spot_instance_request_id,omitempty"` } -// UnmarshalJSON folds a legacy record's operator-stop signal into DesiredState. -// Done on decode so every read path inherits it and none has to remember, and -// so the legacy field is dropped rather than written back out. +// UnmarshalJSON folds the fields of legacy records that mixed a request with an +// observation into the pair that separates them. Done on decode so every read +// path inherits it and none has to remember, and so the legacy fields are +// dropped rather than written back out. func (v *VM) UnmarshalJSON(data []byte) error { type alias VM if err := json.Unmarshal(data, (*alias)(v)); err != nil { @@ -220,6 +232,21 @@ func (v *VM) UnmarshalJSON(data []byte) error { v.DesiredState = DesiredStopped } v.LegacyAttributes = nil + + if len(v.HostfwdPorts) == 0 && len(v.LegacyExtraHostfwd) > 0 { + v.HostfwdPorts = make([]int, 0, len(v.LegacyExtraHostfwd)) + for guest, host := range v.LegacyExtraHostfwd { + v.HostfwdPorts = append(v.HostfwdPorts, guest) + if host != 0 { + if v.HostfwdPortMap == nil { + v.HostfwdPortMap = make(map[int]int, len(v.LegacyExtraHostfwd)) + } + v.HostfwdPortMap[guest] = host + } + } + slices.Sort(v.HostfwdPorts) + } + v.LegacyExtraHostfwd = nil return nil }