Merge 334357a 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 == "."

test/preflight/apps_v2_integration_test.go

@@ -4,6 +4,7 @@
 package preflight
 
 import (
+	"encoding/json"
 	"errors"
 	"fmt"
 	"net/http"
@@ -84,7 +85,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 +110,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 +178,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 +209,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 +253,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 +278,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 +317,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()
 
@@ -347,6 +345,11 @@ web = "nginx -g 'daemon off;'"
 		"web": 1,
 	})
 
+	// json dump `machines` variable as well, to verify that the machine IDs are what we expect
+	machinesJson, _ := json.MarshalIndent(machines, "", "  ")
+	f.Logf("machines variable dump: %s", string(machinesJson))
+	f.Logf("machines before clone: %s", f.Fly("machines list -a %s --json", appName).StdOutString())
+
 	// 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()
 	f.Fly("secrets set 'SOME=MY_SECRET_TEST_STRING' -a %s", appName).AssertSuccessfulExit()
@@ -512,7 +515,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)
 	}

test/preflight/fly_deploy_test.go

@@ -18,7 +18,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
-	//fly "github.com/superfly/fly-go"
+	// fly "github.com/superfly/fly-go"
 
 	"github.com/superfly/flyctl/test/preflight/testlib"
 )
@@ -84,36 +84,11 @@ func TestFlyDeployHAPlacement(t *testing.T) {
 	f := testlib.NewTestEnvFromEnv(t)
 	appName := f.CreateRandomAppName()
 
-	// Create the app without deploying to avoid the Corrosion replication race
 	f.Fly(
-		"launch --org %s --name %s --region %s --image nginx --internal-port 80 --no-deploy",
+		"launch --org %s --name %s --region %s --image nginx --internal-port 80 --ha",
 		f.OrgSlug(), appName, f.PrimaryRegion(),
 	)
 
-	// Retry the deploy command to handle Corrosion replication lag race conditions
-	// The backend may not have replicated the app record to all hosts yet when
-	// creating the second machine for HA, resulting in "sql: no rows in result set" errors
-	var lastError string
-	require.EventuallyWithT(t, func(c *assert.CollectT) {
-		result := f.FlyAllowExitFailure("deploy --buildkit --remote-only")
-		if result.ExitCode() != 0 {
-			stderr := result.StdErrString()
-			lastError = stderr
-			// Only retry if it's the known Corrosion replication lag error
-			if strings.Contains(stderr, "failed to get app: sql: no rows in result set") {
-				t.Logf("Corrosion replication lag detected, retrying... (error: %s)", stderr)
-				assert.Fail(c, "Corrosion replication lag, retrying...")
-			} else {
-				// Log the unexpected error and fail without retrying
-				t.Logf("Deploy failed with unexpected error (will not retry): %s", stderr)
-				assert.Fail(c, fmt.Sprintf("deploy failed with unexpected error: %s", stderr))
-			}
-		} else {
-			// Explicitly assert success so EventuallyWithT knows we passed
-			assert.True(c, true, "deploy succeeded")
-		}
-	}, 30*time.Second, 5*time.Second, "deploy should succeed after Corrosion replication, last error: %s", lastError)
-
 	assertHostDistribution(t, f, appName, 2)
 }
 
@@ -241,35 +216,10 @@ func testDeployNodeAppWithRemoteBuilder(tt *testing.T) {
 	require.NoError(t, err)
 
 	t.Logf("deploy %s", appName)
-	// Retry deploy to handle transient network errors (DNS, WireGuard, buildkit connection issues)
-	// BuildKit deployments can fail with various transient errors during the initial connection
-	var lastError string
-	require.EventuallyWithT(tt, func(c *assert.CollectT) {
-		result := f.FlyAllowExitFailure("deploy --buildkit --remote-only --ha=false")
-		if result.ExitCode() != 0 {
-			stderr := result.StdErrString()
-			lastError = stderr
-			t.Logf("Deploy failed (will retry), error: %s", stderr)
-			assert.Fail(c, "deploy failed, retrying...")
-		} else {
-			assert.True(c, true, "deploy succeeded")
-		}
-	}, 120*time.Second, 10*time.Second, "deploy should succeed after retries, last error: %s", lastError)
+	f.Fly("deploy --remote-only --ha=false")
 
 	t.Logf("deploy %s again", appName)
-	// Retry second deploy as well
-	lastError = ""
-	require.EventuallyWithT(tt, func(c *assert.CollectT) {
-		result := f.FlyAllowExitFailure("deploy --buildkit --remote-only --strategy immediate --ha=false")
-		if result.ExitCode() != 0 {
-			stderr := result.StdErrString()
-			lastError = stderr
-			t.Logf("Deploy failed (will retry), error: %s", stderr)
-			assert.Fail(c, "deploy failed, retrying...")
-		} else {
-			assert.True(c, true, "deploy succeeded")
-		}
-	}, 120*time.Second, 10*time.Second, "deploy should succeed after retries, last error: %s", lastError)
+	f.Fly("deploy --remote-only --strategy immediate")
 
 	body, err := testlib.RunHealthCheck(fmt.Sprintf("https://%s.fly.dev", appName))
 	require.NoError(t, err)
@@ -298,35 +248,10 @@ func testDeployNodeAppWithBuildKitRemoteBuilder(tt *testing.T) {
 	require.NoError(t, err)
 
 	t.Logf("deploy %s with BuildKit", appName)
-	// Retry deploy to handle transient network errors (DNS, WireGuard, buildkit connection issues)
-	// BuildKit deployments can fail with various transient errors during the initial connection
-	var lastError string
-	require.EventuallyWithT(tt, func(c *assert.CollectT) {
-		result := f.FlyAllowExitFailure("deploy --buildkit --remote-only --ha=false")
-		if result.ExitCode() != 0 {
-			stderr := result.StdErrString()
-			lastError = stderr
-			t.Logf("Deploy failed (will retry), error: %s", stderr)
-			assert.Fail(c, "deploy failed, retrying...")
-		} else {
-			assert.True(c, true, "deploy succeeded")
-		}
-	}, 120*time.Second, 10*time.Second, "deploy should succeed after retries, last error: %s", lastError)
+	f.Fly("deploy --buildkit --remote-only --ha=false")
 
 	t.Logf("deploy %s again with BuildKit", appName)
-	// Retry second deploy as well
-	lastError = ""
-	require.EventuallyWithT(tt, func(c *assert.CollectT) {
-		result := f.FlyAllowExitFailure("deploy --buildkit --remote-only --strategy immediate --ha=false")
-		if result.ExitCode() != 0 {
-			stderr := result.StdErrString()
-			lastError = stderr
-			t.Logf("Deploy failed (will retry), error: %s", stderr)
-			assert.Fail(c, "deploy failed, retrying...")
-		} else {
-			assert.True(c, true, "deploy succeeded")
-		}
-	}, 120*time.Second, 10*time.Second, "deploy should succeed after retries, last error: %s", lastError)
+	f.Fly("deploy --buildkit --remote-only --strategy immediate")
 
 	body, err := testlib.RunHealthCheck(fmt.Sprintf("https://%s.fly.dev", appName))
 	require.NoError(t, err)
@@ -461,7 +386,7 @@ func TestDeployManifest(t *testing.T) {
 	appName := f.CreateRandomAppName()
 	f.Fly("launch --org %s --name %s --region %s --image nginx:latest --internal-port 80 --ha=false --strategy rolling", f.OrgSlug(), appName, f.PrimaryRegion())
 
-	var manifestPath = filepath.Join(f.WorkDir(), "manifest.json")
+	manifestPath := filepath.Join(f.WorkDir(), "manifest.json")
 
 	f.Fly("deploy --buildkit --remote-only --export-manifest %s", manifestPath)
 

test/preflight/fly_machine_test.go

@@ -108,7 +108,7 @@ func TestFlyMachineRun_standbyFor(t *testing.T) {
 	s1 = findMachineByID(ml, s1.ID)
 	require.Equal(f, 2, len(ml))
 	// Updating a stopped machine doesn't start it
-	require.Equal(f, "started", s1.State)
+	require.Equal(f, "stopped", s1.State)
 	require.Empty(f, s1.Config.Standbys)
 
 	// Clone and set its standby to the source

test/preflight/testlib/helpers.go

@@ -19,6 +19,7 @@ import (
 	"runtime"
 	"sort"
 	"strings"
+	"sync"
 	"testing"
 	"time"
 
@@ -30,6 +31,16 @@ import (
 
 const defaultRegion = "cdg"
 
+type selectedRegions struct {
+	primary string
+	other   []string
+}
+
+var (
+	regionSelectionOnce sync.Once
+	regionSelection     selectedRegions
+)
+
 type platformRegion struct {
 	Code             string `json:"code"`
 	Name             string `json:"name"`
@@ -92,37 +103,43 @@ func getBestRegions(count int) ([]string, error) {
 }
 
 func primaryRegionFromEnv() string {
-	regions := os.Getenv("FLY_PREFLIGHT_TEST_FLY_REGIONS")
-	if regions == "" {
-		// Try to dynamically select best region
-		best, err := getBestRegions(1)
-		if err == nil && len(best) > 0 {
-			terminal.Warnf("no region set with FLY_PREFLIGHT_TEST_FLY_REGIONS, auto-selected region with best capacity: %s", best[0])
-			return best[0]
-		}
-		// Fall back to hardcoded default
-		terminal.Warnf("no region set with FLY_PREFLIGHT_TEST_FLY_REGIONS, failed to auto-select (%v), using fallback: %s", err, defaultRegion)
-		return defaultRegion
-	}
-	pieces := strings.SplitN(regions, " ", 2)
-	return pieces[0]
+	regions := selectRegionsFromEnv()
+	return regions.primary
 }
 
 func otherRegionsFromEnv() []string {
-	regions := os.Getenv("FLY_PREFLIGHT_TEST_FLY_REGIONS")
-	if regions == "" {
-		// Try to dynamically select best regions (get top 2, skip the first since it's primary)
+	regions := selectRegionsFromEnv()
+	return append([]string(nil), regions.other...)
+}
+
+func selectRegionsFromEnv() selectedRegions {
+	regionSelectionOnce.Do(func() {
+		regions := strings.Fields(os.Getenv("FLY_PREFLIGHT_TEST_FLY_REGIONS"))
+		if len(regions) > 0 {
+			regionSelection.primary = regions[0]
+			if len(regions) > 1 {
+				regionSelection.other = append([]string(nil), regions[1:]...)
+			}
+			return
+		}
+
 		best, err := getBestRegions(2)
-		if err == nil && len(best) > 1 {
-			return best[1:]
+		if err == nil && len(best) > 0 {
+			regionSelection.primary = best[0]
+			if len(best) > 1 {
+				regionSelection.other = append([]string(nil), best[1:]...)
+			}
+			terminal.Debugf("no region set with FLY_PREFLIGHT_TEST_FLY_REGIONS, auto-selected region with best capacity: %s", best[0])
+			return
 		}
-		return nil
-	}
-	pieces := strings.Split(regions, " ")
-	if len(pieces) > 1 {
-		return pieces[1:]
-	} else {
-		return nil
+
+		regionSelection.primary = defaultRegion
+		terminal.Warnf("no region set with FLY_PREFLIGHT_TEST_FLY_REGIONS, failed to auto-select (%v), using fallback: %s", err, defaultRegion)
+	})
+
+	return selectedRegions{
+		primary: regionSelection.primary,
+		other:   append([]string(nil), regionSelection.other...),
 	}
 }