Skip to content

Commit

Permalink
Change restartBuilder to return cleanly when already started
Browse files Browse the repository at this point in the history
Basically, if the builder is in the 'created' state for example, and we
transition into 'started', there's no reason to start it.
  • Loading branch information
billyb2 committed Jul 29, 2024
1 parent c8368e5 commit 4d205c9
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 5 deletions.
14 changes: 11 additions & 3 deletions internal/build/imgsrc/ensure_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func EnsureBuilder(ctx context.Context, org *fly.Organization, region string, re
}

if validateBuilderErr == BuilderMachineNotStarted {
err := restartBuilderMachine(ctx, builderMachine)
err := startBuilder(ctx, builderMachine)
switch {
case errors.Is(err, ShouldReplaceBuilderMachine):
span.AddEvent("recreating builder due to resource reservation error")
Expand Down Expand Up @@ -406,13 +406,21 @@ func createBuilder(ctx context.Context, org *fly.Organization, region, builderNa
return
}

func restartBuilderMachine(ctx context.Context, builderMachine *fly.Machine) error {
func startBuilder(ctx context.Context, builderMachine *fly.Machine) error {
ctx, span := tracing.GetTracer().Start(ctx, "restart_builder_machine")
defer span.End()

flapsClient := flapsutil.ClientFromContext(ctx)

mach.WaitForAnyMachineState(ctx, builderMachine, []string{"started", "stopped"}, 60*time.Second, nil)
state, err := mach.WaitForAnyMachineState(ctx, builderMachine, []string{"started", "stopped"}, 60*time.Second, nil)
if err != nil {
tracing.RecordError(span, err, "error waiting for builder machine to start or stop")
return err
}

if state == "started" {
return nil
}

if err := flapsClient.Restart(ctx, fly.RestartMachineInput{
ID: builderMachine.ID,
Expand Down
4 changes: 2 additions & 2 deletions internal/build/imgsrc/ensure_builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,13 +285,13 @@ func TestRestartBuilderMachine(t *testing.T) {
}

ctx = flapsutil.NewContextWithClient(ctx, &flapsClient)
err := restartBuilderMachine(ctx, &fly.Machine{ID: "bigmachine"})
err := startBuilder(ctx, &fly.Machine{ID: "bigmachine"})
assert.NoError(t, err)
assert.True(t, waitedForStartOrStop)

waitedForStartOrStop = false
couldNotReserveResources = true
err = restartBuilderMachine(ctx, &fly.Machine{ID: "bigmachine"})
err = startBuilder(ctx, &fly.Machine{ID: "bigmachine"})
assert.True(t, waitedForStartOrStop)
assert.Error(t, err)
assert.ErrorIs(t, err, ShouldReplaceBuilderMachine)
Expand Down

0 comments on commit 4d205c9

Please sign in to comment.