Merge 4c8f735 into c12bb49

Author: dangra

internal/command/deploy/machines_launchinput.go

@@ -2,6 +2,7 @@ package deploy
 
 import (
 	"fmt"
+	"slices"
 	"strconv"
 	"strings"
 
@@ -297,18 +298,14 @@ func skipLaunch(origMachineRaw *fly.Machine, mConfig *fly.MachineConfig) bool {
 	}
 
 	switch {
-	case state == fly.MachineStateStarted:
+	case slices.Contains([]string{fly.MachineStateStarted, "starting", "failed"}, state):
 		return false
 	case len(mConfig.Standbys) > 0:
 		return true
-	case state == fly.MachineStateStopped, state == fly.MachineStateSuspended:
-		for _, s := range mConfig.Services {
-			if (s.Autostop != nil && *s.Autostop != fly.MachineAutostopOff) || (s.Autostart != nil && *s.Autostart) {
-				return true
-			}
-		}
+	case origMachineRaw == nil:
+		return false
 	}
-	return false
+	return true
 }
 
 // updateContainerImage sets container.Image = mConfig.Image in any container where image == "."

internal/command/deploy/machines_launchinput_test.go

@@ -15,7 +15,7 @@ import (
 )
 
 func makeTerminalLoggerQuiet(tb testing.TB) {
-	var originalLogger = terminal.DefaultLogger
+	originalLogger := terminal.DefaultLogger
 	terminal.DefaultLogger = logger.New(os.Stdout, logger.Error, true)
 
 	tb.Cleanup(func() {
@@ -85,6 +85,7 @@ func testLaunchInputForBasic(t *testing.T) {
 		Region:     li.Region,
 		Config:     helpers.Clone(li.Config),
 		HostStatus: fly.HostStatusOk,
+		State:      fly.MachineStateStarted,
 	}
 	// also must preserve any user's added metadata except for known fly metadata keys
 	origMachineRaw.Config.Metadata["user-added-me"] = "keep it"
@@ -104,6 +105,7 @@ func testLaunchInputForBasic(t *testing.T) {
 		Region:     li.Region,
 		Config:     helpers.Clone(li.Config),
 		HostStatus: fly.HostStatusOk,
+		State:      fly.MachineStateStarted,
 	}
 	want.Config.Image = "super/globe"
 	want.Config.Env["NOT_SET_ON_RESTART_ONLY"] = "true"
@@ -253,7 +255,6 @@ func testLaunchInputForOnMounts(t *testing.T) {
 	assert.Equal(t, "ab1234567890", li.ID)
 	assert.True(t, li.RequiresReplacement)
 	assert.Empty(t, li.Config.Mounts)
-
 }
 
 // test mounts with auto volume resize

internal/command/deploy/machines_test.go

@@ -279,6 +279,7 @@ func Test_resolveUpdatedMachineConfig_Mounts(t *testing.T) {
 	}, li)
 
 	origMachine := &fly.Machine{
+		State:      fly.MachineStateStarted,
 		HostStatus: fly.HostStatusOk,
 		Config: &fly.MachineConfig{
 			Mounts: []fly.MachineMount{{
@@ -329,6 +330,7 @@ func Test_resolveUpdatedMachineConfig_restartOnly(t *testing.T) {
 	md.img = "SHOULD-NOT-USE-THIS-TAG"
 
 	origMachine := &fly.Machine{
+		State:      fly.MachineStateStarted,
 		HostStatus: fly.HostStatusOk,
 		ID:         "OrigID",
 		Config: &fly.MachineConfig{
@@ -371,6 +373,7 @@ func Test_resolveUpdatedMachineConfig_restartOnlyProcessGroup(t *testing.T) {
 	md.img = "SHOULD-NOT-USE-THIS-TAG"
 
 	origMachine := &fly.Machine{
+		State:      fly.MachineStateStarted,
 		HostStatus: fly.HostStatusOk,
 		ID:         "OrigID",
 		Config: &fly.MachineConfig{

internal/command/machine/update.go

@@ -16,7 +16,6 @@ import (
 	"github.com/superfly/flyctl/internal/flag"
 	"github.com/superfly/flyctl/internal/flyerr"
 	mach "github.com/superfly/flyctl/internal/machine"
-	"github.com/superfly/flyctl/internal/watch"
 )
 
 func newUpdate() *cobra.Command {
@@ -79,9 +78,7 @@ func newUpdate() *cobra.Command {
 
 func runUpdate(ctx context.Context) (err error) {
 	var (
-		io       = iostreams.FromContext(ctx)
-		colorize = io.ColorScheme()
-
+		io               = iostreams.FromContext(ctx)
 		autoConfirm      = flag.GetBool(ctx, "yes")
 		skipHealthChecks = flag.GetBool(ctx, "skip-health-checks")
 		skipStart        = flag.GetBool(ctx, "skip-start")
@@ -169,20 +166,10 @@ func runUpdate(ctx context.Context) (err error) {
 				Descript: timeoutErr.Description(),
 				Suggest:  "Try increasing the --wait-timeout",
 			}
-
 		}
 		return err
 	}
 
-	if !(input.SkipLaunch || flag.GetDetach(ctx)) {
-		fmt.Fprintln(io.Out, colorize.Green("==> "+"Monitoring health checks"))
-
-		if err := watch.MachinesChecks(ctx, appName, []*fly.Machine{machine}); err != nil {
-			return err
-		}
-		fmt.Fprintln(io.Out)
-	}
-
 	fmt.Fprintf(io.Out, "\nMonitor machine status here:\nhttps://fly.io/apps/%s/machines/%s\n", appName, machine.ID)
 
 	return nil

internal/machine/update.go

@@ -85,8 +85,25 @@ func Update(ctx context.Context, appName string, m *fly.Machine, input *fly.Laun
 		return fmt.Errorf("could not update machine %s: %w", m.ID, err)
 	}
 
-	waitForAction := "start"
-	if input.SkipLaunch || m.Config.Schedule != "" || m.State != fly.MachineStateStarted {
+	waitForAction := "none"
+	switch m.State {
+	case fly.MachineStateStarted, "failed", "starting":
+		waitForAction = "start"
+	case fly.MachineStateStopped, fly.MachineStateSuspended, "stopping", "suspending":
+		waitForAction = "stop"
+	default:
+		// look for past machine events
+		for _, ev := range m.Events {
+			if ev.Type == "start" || ev.Status == "started" || ev.Status == "starting" {
+				waitForAction = "start"
+				break
+			} else if ev.Status == "stopped" || ev.Status == "stopping" || ev.Status == "suspended" || ev.Status == "suspending" {
+				waitForAction = "stop"
+				break
+			}
+		}
+	}
+	if input.SkipLaunch {
 		waitForAction = "stop"
 	}
 
@@ -99,11 +116,9 @@ func Update(ctx context.Context, appName string, m *fly.Machine, input *fly.Laun
 		return err
 	}
 
-	if !input.SkipLaunch {
-		if !input.SkipHealthChecks {
-			if err := watch.MachinesChecks(ctx, appName, []*fly.Machine{updatedMachine}); err != nil {
-				return fmt.Errorf("failed to wait for health checks to pass: %w", err)
-			}
+	if waitForAction == "start" && !input.SkipHealthChecks {
+		if err := watch.MachinesChecks(ctx, appName, []*fly.Machine{updatedMachine}); err != nil {
+			return fmt.Errorf("failed to wait for health checks to pass: %w", err)
 		}
 	}
 

internal/machine/wait.go

@@ -27,6 +27,8 @@ func WaitForStartOrStop(ctx context.Context, appName string, machine *fly.Machin
 		waitOnAction = "started"
 	case "stop":
 		waitOnAction = "stopped"
+	case "none":
+		return nil
 	default:
 		return invalidAction
 	}

test/preflight/apps_v2_integration_test.go

@@ -84,7 +84,7 @@ func TestAppsV2Example(t *testing.T) {
 ENV BUILT_BY_DOCKERFILE=true
 `
 	dockerfilePath := filepath.Join(f.WorkDir(), "Dockerfile")
-	err := os.WriteFile(dockerfilePath, []byte(dockerfileContent), 0644)
+	err := os.WriteFile(dockerfilePath, []byte(dockerfileContent), 0o644)
 	if err != nil {
 		f.Fatalf("failed to write dockerfile at %s error: %v", dockerfilePath, err)
 	}
@@ -109,7 +109,7 @@ func TestAppsV2ConfigChanges(t *testing.T) {
 	newConfigFile := strings.Replace(string(configFileBytes), `FOO = 'BAR'`, `BAR = "QUX"`, 1)
 	require.Contains(f, newConfigFile, `BAR = "QUX"`)
 
-	err = os.WriteFile(configFilePath, []byte(newConfigFile), 0666)
+	err = os.WriteFile(configFilePath, []byte(newConfigFile), 0o666)
 	require.NoError(t, err)
 
 	f.Fly("deploy --buildkit --remote-only --detach")
@@ -177,7 +177,7 @@ func TestAppsV2Config_ParseExperimental(t *testing.T) {
 	  auto_rollback = true
 	`
 
-	err := os.WriteFile(configFilePath, []byte(config), 0644)
+	err := os.WriteFile(configFilePath, []byte(config), 0o644)
 	require.NoError(t, err, "error trying to write %s", configFilePath)
 
 	result := f.Fly("launch --no-deploy --ha=false --name %s --region ord --copy-config --org %s", appName, f.OrgSlug())
@@ -208,7 +208,7 @@ func TestAppsV2Config_ProcessGroups(t *testing.T) {
 
 	deployToml := func(toml string) *testlib.FlyctlResult {
 		toml = "app = \"" + appName + "\"\n" + toml
-		err := os.WriteFile(configFilePath, []byte(toml), 0666)
+		err := os.WriteFile(configFilePath, []byte(toml), 0o666)
 		require.NoError(t, err, "error trying to write %s", configFilePath)
 		cmd := f.Fly("deploy --buildkit --remote-only --detach --now --image nginx --ha=false")
 		cmd.AssertSuccessfulExit()
@@ -252,10 +252,8 @@ func TestAppsV2Config_ProcessGroups(t *testing.T) {
 
 	deployOut := deployToml(`
 [[services]]
-  http_checks = []
   internal_port = 8080
   protocol = "tcp"
-  script_checks = []
 
 		[[services.ports]]
 		port = 80
@@ -279,10 +277,8 @@ bar_web = "bash -c 'while true; do sleep 10; done'"
 
 [[services]]
   processes = ["web"] # this service only applies to the web process
-  http_checks = []
   internal_port = 8080
   protocol = "tcp"
-  script_checks = []
 
 		[[services.ports]]
 		port = 80
@@ -320,6 +316,7 @@ bar_web = "bash -c 'while true; do sleep 10; done'"
 	if len(f.OtherRegions()) > 0 {
 		secondaryRegion = f.OtherRegions()[0]
 	}
+
 	f.Fly("m clone %s --region %s", barWebMachId, secondaryRegion)
 	f.Fly("machine update %s -m ABCD=EFGH -y", webMachId).AssertSuccessfulExit()
 
@@ -349,6 +346,13 @@ web = "nginx -g 'daemon off;'"
 
 	// Step 5: Set secrets, to ensure that machine data is kept during a 'restartOnly' deploy.
 	f.Fly("machine update %s -m CUSTOM=META -y", webMachId).AssertSuccessfulExit()
+	// XXX: this sucks but until update waits for the machine to reach a terminal state
+	// before returning, we have to wait an arbitrary amount of time here to ensure
+	// that the machine is ready for another update.
+	// TODO: Replace with an eventually loop
+	time.Sleep(10 * time.Second)
+
+	f.Logf("machines before update: %s", f.Fly("m status -d -a %s %s", appName, webMachId).StdOutString())
 	f.Fly("secrets set 'SOME=MY_SECRET_TEST_STRING' -a %s", appName).AssertSuccessfulExit()
 
 	machines = f.MachinesList(appName)
@@ -512,7 +516,7 @@ func TestImageLabel(t *testing.T) {
 ENV BUILT_BY_DOCKERFILE=true
 `
 	dockerfilePath := filepath.Join(f.WorkDir(), "Dockerfile")
-	err := os.WriteFile(dockerfilePath, []byte(dockerfileContent), 0644)
+	err := os.WriteFile(dockerfilePath, []byte(dockerfileContent), 0o644)
 	if err != nil {
 		f.Fatalf("failed to write dockerfile at %s error: %v", dockerfilePath, err)
 	}