Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion internal/command/deploy/plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -559,7 +559,7 @@ func (md *machineDeployment) updateMachineWChecks(ctx context.Context, oldMachin
}

if !healthcheckResult.machineChecksPassed || !healthcheckResult.smokeChecksPassed {
sl.LogStatus(statuslogger.StatusRunning, fmt.Sprintf("Waiting for machine %s to reach a good state", oldMachine.ID))
sl.LogStatus(statuslogger.StatusRunning, fmt.Sprintf("Waiting for machine %s to reach a good state", machine.ID))
_, err := waitForMachineState(ctx, lm, []string{"stopped", "started", "suspended"}, md.waitTimeout, sl)
if err != nil {
span.RecordError(err)
Expand Down
150 changes: 150 additions & 0 deletions internal/command/deploy/plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,156 @@ func TestUpdateMachines(t *testing.T) {
assert.ErrorAs(t, err, &unrecoverableErr)
}

// TestUpdateMachinesWithNewMachine verifies that deploying with a new machine
// (oldMachine=nil) doesn't panic when health checks need to run. This
// reproduces the nil pointer dereference that occurred when oldMachine.ID was
// accessed in updateMachineWChecks for a newly created machine.
func TestUpdateMachinesWithNewMachine(t *testing.T) {
t.Parallel()

ctx := withQuietIOStreams(context.Background())

// Old state has one machine; new state adds a second machine that doesn't
// exist in the old state, producing a pairing with oldMachine=nil.
oldMachines := []*fly.Machine{
{
ID: "machine1",
State: "started",
HostStatus: fly.HostStatusOk,
Config: &fly.MachineConfig{
Image: "image1",
},
},
}

newMachines := []*fly.Machine{
{
ID: "machine1",
State: "started",
HostStatus: fly.HostStatusOk,
Config: &fly.MachineConfig{
Image: "image2",
},
},
{
ID: "new-machine",
State: "started",
Region: "iad",
HostStatus: fly.HostStatusOk,
Config: &fly.MachineConfig{
Image: "image2",
},
},
}

acquiredLeases := sync.Map{}

flapsClient := &mock.FlapsClient{
AcquireLeaseFunc: func(ctx context.Context, appName, machineID string, ttl *int) (*fly.MachineLease, error) {
if _, ok := acquiredLeases.Load(machineID); ok {
return nil, assert.AnError
}
acquiredLeases.Store(machineID, true)
return &fly.MachineLease{
Data: &fly.MachineLeaseData{
Nonce: machineID + "nonce",
},
}, nil
},
ReleaseLeaseFunc: func(ctx context.Context, appName, machineID, nonce string) error {
acquiredLeases.Delete(machineID)
return nil
},
UpdateFunc: func(ctx context.Context, appName string, builder fly.LaunchMachineInput, nonce string) (out *fly.Machine, err error) {
return &fly.Machine{
ID: builder.ID,
Config: builder.Config,
State: "started",
HostStatus: fly.HostStatusOk,
}, nil
},
LaunchFunc: func(ctx context.Context, appName string, builder fly.LaunchMachineInput) (out *fly.Machine, err error) {
return &fly.Machine{
ID: "created-machine",
Config: builder.Config,
State: "started",
HostStatus: fly.HostStatusOk,
}, nil
},
DestroyFunc: func(ctx context.Context, appName string, input fly.RemoveMachineInput, nonce string) (err error) {
return nil
},
WaitFunc: func(ctx context.Context, appName string, machine *fly.Machine, state string, timeout time.Duration) (err error) {
machine.State = state
return nil
},
ListFunc: func(ctx context.Context, appName, state string) ([]*fly.Machine, error) {
return oldMachines, nil
},
StartFunc: func(ctx context.Context, appName, machineID string, nonce string) (out *fly.MachineStartResponse, err error) {
return &fly.MachineStartResponse{}, nil
},
GetFunc: func(ctx context.Context, appName, machineID string) (*fly.Machine, error) {
for _, m := range newMachines {
if m.ID == machineID {
return m, nil
}
}
// Return the created machine for the newly launched machine
return &fly.Machine{
ID: machineID,
State: "started",
HostStatus: fly.HostStatusOk,
Config: &fly.MachineConfig{Image: "image2"},
}, nil
},
GetProcessesFunc: func(ctx context.Context, appName, machineID string) (fly.MachinePsResponse, error) {
return fly.MachinePsResponse{}, nil
},
RefreshLeaseFunc: func(ctx context.Context, appName, machineID string, ttl *int, nonce string) (*fly.MachineLease, error) {
return &fly.MachineLease{
Status: "success",
Data: &fly.MachineLeaseData{
Nonce: nonce,
},
}, nil
},
}

ctx = flapsutil.NewContextWithClient(ctx, flapsClient)
md := &machineDeployment{
flapsClient: flapsClient,
io: iostreams.FromContext(ctx),
app: &flaps.App{
Name: "myapp",
},
appConfig: &appconfig.Config{AppName: "myapp"},
waitTimeout: 10 * time.Second,
deployRetries: 5,
maxUnavailable: 3,
skipSmokeChecks: true,
}

oldAppState := &AppState{
Machines: oldMachines,
}
newAppState := &AppState{
Machines: newMachines,
}

// skipSmokeChecks=false ensures we enter the code path that previously
// dereferenced oldMachine.ID (which is nil for new machines).
settings := updateMachineSettings{
pushForward: true,
skipHealthChecks: false,
skipSmokeChecks: false,
skipLeaseAcquisition: false,
}

err := md.updateMachinesWRecovery(ctx, oldAppState, newAppState, nil, settings)
assert.NoError(t, err)
}

func TestUpdateOrCreateMachine(t *testing.T) {
t.Parallel()

Expand Down
Loading