refresh info about unattached vols each update retry (#3889)

* reset info about unattached vols each update retry

* add fly deploy test for deploying with new mounts

The FlyDeployHA test already tests deploying with new mounts, but only
runs when the test environment is configured to use multiple regions
(at time of writing the CI pipeline only uses one primary region).
This commit adds a test of adding mounts through deploys that will run
when FlyDeployHA is skipped (and which will be skipped itself if it is
redundant with FlyDeployHA).

* oops, avoid needless out-of-bounds error

* make sure cold and warm machines exlude each other

also consider machines with "replacing" state to be warm

internal/command/deploy/plan.go

@@ -217,25 +217,17 @@ func (md *machineDeployment) updateMachinesWRecovery(ctx context.Context, oldApp
 		pgroup.Go(func() error {
 			eg, ctx := errgroup.WithContext(ctx)
 
-			warmMachines := lo.Filter(machineTuples, func(e machinePairing, i int) bool {
-				if e.oldMachine != nil && e.oldMachine.State == "started" {
+			isWarm := func(e machinePairing, i int) bool {
+				if e.oldMachine != nil && (e.oldMachine.State == "started" || e.oldMachine.State == "replacing") {
 					return true
 				}
-				if e.newMachine != nil && e.newMachine.State == "started" {
+				if e.newMachine != nil && (e.newMachine.State == "started" || e.newMachine.State == "replacing") {
 					return true
 				}
 				return false
-			})
-
-			coldMachines := lo.Filter(machineTuples, func(e machinePairing, i int) bool {
-				if e.oldMachine != nil && e.oldMachine.State != "started" {
-					return true
-				}
-				if e.newMachine != nil && e.newMachine.State != "started" {
-					return true
-				}
-				return false
-			})
+			}
+			warmMachines := lo.Filter(machineTuples, isWarm)
+			coldMachines := lo.Reject(machineTuples, isWarm)
 
 			eg.Go(func() (err error) {
 				poolSize := len(coldMachines)
@@ -300,6 +292,11 @@ func (md *machineDeployment) updateMachinesWRecovery(ctx context.Context, oldApp
 				span.RecordError(updateErr)
 				return fmt.Errorf("failed to get current app state: %w", err)
 			}
+			// we need to refresh information about the state of unattached volumes in the app
+			err = md.setVolumes(ctx)
+			if err != nil {
+				return err
+			}
 			err = md.updateMachinesWRecovery(ctx, currentState, newAppState, sl, updateMachineSettings{
 				pushForward:          false,
 				skipHealthChecks:     settings.skipHealthChecks,

test/preflight/fly_deploy_test.go

@@ -49,6 +49,34 @@ func TestFlyDeployHA(t *testing.T) {
 	f.Fly("deploy")
 }
 
+// This test overlaps partially in functionality with TestFlyDeployHA, but runs
+// when the test environment uses just a single region
+func TestFlyDeploy_AddNewMount(t *testing.T) {
+	f := testlib.NewTestEnvFromEnv(t)
+	if f.SecondaryRegion() != "" {
+		t.Skip()
+	}
+
+	appName := f.CreateRandomAppName()
+
+	f.Fly(
+		"launch --now --org %s --name %s --region %s --image nginx --internal-port 80 --ha=false",
+		f.OrgSlug(), appName, f.PrimaryRegion(),
+	)
+
+	f.WriteFlyToml(`%s
+[mounts]
+	source = "data"
+	destination = "/data"
+	`, f.ReadFile("fly.toml"))
+
+	x := f.FlyAllowExitFailure("deploy")
+	require.Contains(f, x.StdErrString(), `needs volumes with name 'data' to fulfill mounts defined in fly.toml`)
+
+	f.Fly("volume create -a %s -r %s -s 1 data -y", appName, f.PrimaryRegion())
+	f.Fly("deploy")
+}
+
 func TestFlyDeployHAPlacement(t *testing.T) {
 	f := testlib.NewTestEnvFromEnv(t)
 	appName := f.CreateRandomAppName()