diff --git a/spinifex/handlers/ec2/instance/projection.go b/spinifex/handlers/ec2/instance/projection.go index 33502050c..be2b3964c 100644 --- a/spinifex/handlers/ec2/instance/projection.go +++ b/spinifex/handlers/ec2/instance/projection.go @@ -90,14 +90,21 @@ func ProjectInstance(v *vm.VM, cfg InstanceProjection) (inst *ec2.Instance, stat instanceCopy.IamInstanceProfile = nil } - // Placement survives a stop in AWS, so project it regardless of runtime state - // whenever the instance belongs to a placement group. - if v.PlacementGroupName != "" { - instanceCopy.Placement = &ec2.Placement{ - GroupName: aws.String(v.PlacementGroupName), - AvailabilityZone: aws.String(cfg.AZ), - } + // Every instance has a placement AZ in AWS, group or not, and it survives a + // stop — so project it regardless of runtime state. Copied rather than mutated + // because instanceCopy shares the pointer with the stored instance. + placement := &ec2.Placement{} + if instanceCopy.Placement != nil { + placementCopy := *instanceCopy.Placement + placement = &placementCopy } + if cfg.AZ != "" { + placement.AvailabilityZone = aws.String(cfg.AZ) + } + if v.PlacementGroupName != "" { + placement.GroupName = aws.String(v.PlacementGroupName) + } + instanceCopy.Placement = placement // Echo the consumed capacity reservation so targeted-launch Terraform // converges — without it the instance reports no reservation and the plan diff --git a/spinifex/handlers/ec2/instance/projection_test.go b/spinifex/handlers/ec2/instance/projection_test.go index 0ed919629..9e65a939d 100644 --- a/spinifex/handlers/ec2/instance/projection_test.go +++ b/spinifex/handlers/ec2/instance/projection_test.go @@ -102,6 +102,46 @@ func TestProjectInstance_StoppedRetainsPlacementAndSpot(t *testing.T) { assert.Nil(t, got.CapacityReservationSpecification) } +// TestProjectInstance_ProjectsAZWithoutPlacementGroup is the direct bug fix: an +// ordinary instance is in no placement group, and it used to get no Placement at +// all — so describe-instances reported a null AZ for every normal instance. +func TestProjectInstance_ProjectsAZWithoutPlacementGroup(t *testing.T) { + cfgs := map[string]InstanceProjection{ + "running": runningCfg(), + "stopped": stoppedCfg(80, "stopped"), + } + + for name, cfg := range cfgs { + t.Run(name, func(t *testing.T) { + v := fullVM(vm.StateRunning) + v.PlacementGroupName = "" + + got, _ := ProjectInstance(v, cfg) + + require.NotNil(t, got.Placement) + assert.Equal(t, "us-east-1a", aws.StringValue(got.Placement.AvailabilityZone)) + assert.Nil(t, got.Placement.GroupName) + }) + } +} + +// TestProjectInstance_PlacementRetainsLaunchFields covers a Placement already on +// the stored instance: the projection stamps the AZ onto a copy, leaving the +// launch-time fields intact and the stored instance untouched. +func TestProjectInstance_PlacementRetainsLaunchFields(t *testing.T) { + v := fullVM(vm.StateRunning) + v.PlacementGroupName = "" + v.Instance.Placement = &ec2.Placement{Tenancy: aws.String("default")} + + got, _ := ProjectInstance(v, runningCfg()) + + require.NotNil(t, got.Placement) + assert.Equal(t, "default", aws.StringValue(got.Placement.Tenancy)) + assert.Equal(t, "us-east-1a", aws.StringValue(got.Placement.AvailabilityZone)) + assert.Nil(t, v.Instance.Placement.AvailabilityZone, + "the stored Placement is shared with the VM record, so it must be copied before it is stamped") +} + // TestProjectInstance_EmptyIamArnClearsStaleProfile guards the auto-clear: an // empty ARN on the VM record must wipe any profile left on the stored instance. func TestProjectInstance_EmptyIamArnClearsStaleProfile(t *testing.T) {