migrate-to-v2: cleanup, make vols work

internal/command/migrate_to_v2/migrate_to_v2.go

@@ -36,9 +36,6 @@ import (
 	"golang.org/x/exp/slices"
 )
 
-// TODO(ali): Remove everything mentioning downtime if it turns out
-//            that we don't need it.
-
 func New() *cobra.Command {
 	return newMigrateToV2()
 }
@@ -150,7 +147,6 @@ type recoveryState struct {
 	machinesCreated        []*api.Machine
 	appLocked              bool
 	scaledToZero           bool
-	nomadVolsReadOnly      bool
 	platformVersion        string
 	onlyPromptToConfigSave bool
 	originalConfig         *appconfig.Config
@@ -292,6 +288,7 @@ func (m *v2PlatformMigrator) rollback(ctx context.Context, tb *render.TextBlock)
 		}
 	}
 	if len(m.createdVolumes) > 0 {
+		tb.Detailf("Removing migration-created volumes")
 		for _, vol := range m.createdVolumes {
 			_, err := m.apiClient.DeleteVolume(ctx, vol.vol.ID)
 			if err != nil {
@@ -306,13 +303,6 @@ func (m *v2PlatformMigrator) rollback(ctx context.Context, tb *render.TextBlock)
 			return err
 		}
 	}
-	if m.recovery.nomadVolsReadOnly {
-		tb.Detailf("Resetting nomad app volumes as read/write")
-		err := m.rollbackVolumesReadOnly(ctx)
-		if err != nil {
-			return err
-		}
-	}
 	if m.recovery.platformVersion != "nomad" {
 
 		tb.Detailf("Setting platform version to 'nomad'")
@@ -472,17 +462,6 @@ func (m *v2PlatformMigrator) Migrate(ctx context.Context) (err error) {
 		return abortedErr
 	}
 
-	tb.Detail("Marking nomad app volumes as read-only")
-
-	if m.usesForkedVolumes {
-		if err = m.markVolumesAsReadOnly(ctx); err != nil {
-			return err
-		}
-		if aborted.Load() {
-			return abortedErr
-		}
-	}
-
 	tb.Detail("Starting machines")
 
 	err = m.createMachines(ctx)
@@ -550,7 +529,7 @@ func (m *v2PlatformMigrator) Migrate(ctx context.Context) (err error) {
 	}
 
 	if !m.requiresDowntime() {
-		tb.Detail("Scaling down to zero nomad VMs now that machines are running.")
+		tb.Detail("Scaling nomad VMs down to zero now that machines are running.")
 
 		err = m.scaleNomadToZero(ctx)
 		if err != nil {
@@ -730,15 +709,15 @@ func (m *v2PlatformMigrator) rollbackDeploy(ctx context.Context) error {
 }
 
 func (m *v2PlatformMigrator) requiresDowntime() bool {
-	return false // m.usesForkedVolumes TODO(ali): Cleanup
+	return m.usesForkedVolumes
 }
 
 func (m *v2PlatformMigrator) determinePrimaryRegion(ctx context.Context) error {
 	if fromFlag := flag.GetString(ctx, "primary-region"); fromFlag != "" {
 		m.appConfig.PrimaryRegion = fromFlag
 		return nil
 	}
-	fmt.Println(m.appConfig.Env)
+
 	if val, ok := m.appConfig.Env["PRIMARY_REGION"]; ok {
 		m.appConfig.PrimaryRegion = val
 		return nil
@@ -801,6 +780,7 @@ func (m *v2PlatformMigrator) ConfirmChanges(ctx context.Context) (bool, error) {
 	}
 	if m.usesForkedVolumes {
 		fmt.Fprintf(m.io.Out, " * Create clones of each volume in use, for the new machines\n")
+		fmt.Fprintf(m.io.Out, "   * These cloned volumes will have the suffix '%s' appended to their names\n", forkedVolSuffix)
 	}
 
 	fmt.Fprintf(m.io.Out, " * Create machines, copying the configuration of each existing VM\n")

internal/command/migrate_to_v2/volumes.go

@@ -9,10 +9,15 @@ import (
 	"github.com/superfly/flyctl/internal/appconfig"
 )
 
+const (
+	forkedVolSuffix = "_machines"
+)
+
 func (m *v2PlatformMigrator) validateVolumes(ctx context.Context) error {
 	if m.isPostgres {
 		return nil
 	}
+	m.usesForkedVolumes = len(m.appConfig.Volumes()) != 0
 	if len(m.appConfig.Volumes()) > 1 {
 		return fmt.Errorf("cannot migrate app %s because it uses multiple [[mounts]], which are not yet supported on Apps V2.\nwatch https://community.fly.io for announcements about multiple volume mounts for Apps V2", m.appFull.Name)
 	}
@@ -76,26 +81,19 @@ func (m *v2PlatformMigrator) nomadVolPath(v *api.Volume) string {
 		return ""
 	}
 
+	// The config has already been patched to use the v2 volume names,
+	// so we have to account for that here
+	name := nomadVolNameToV2VolName(v.Name)
+
 	// TODO(ali): Do process group-specific volumes change the logic here?
 	for _, mount := range m.appConfig.Volumes() {
-		if mount.Source == v.Name {
+		if mount.Source == name {
 			return mount.Destination
 		}
 	}
 	return ""
 }
 
-func (m *v2PlatformMigrator) markVolumesAsReadOnly(ctx context.Context) error {
-	m.recovery.nomadVolsReadOnly = true
-	panic("stub")
-	return nil
-}
-
-func (m *v2PlatformMigrator) rollbackVolumesReadOnly(ctx context.Context) error {
-	panic("stub")
-	return nil
-}
-
 func nomadVolNameToV2VolName(name string) string {
-	return name + "_machines"
+	return name + forkedVolSuffix
 }

test/preflight/apps_v2_integration_test.go

@@ -595,3 +595,54 @@ func TestAppsV2MigrateToV2(t *testing.T) {
 	platformVersion, _ := statusMap["PlatformVersion"].(string)
 	require.Equal(f, "machines", platformVersion)
 }
+
+// This test takes forever. I'm sorry.
+func TestAppsV2MigrateToV2_Volumes(t *testing.T) {
+	var (
+		err     error
+		f       = testlib.NewTestEnvFromEnv(t)
+		appName = f.CreateRandomAppName()
+	)
+	// No spaces or quotes, this is sent unescaped to bash :x
+	successStr := "myvolumehasloaded"
+
+	f.Fly("launch --org %s --name %s --region %s --internal-port 80 --force-nomad --image nginx", f.OrgSlug(), appName, f.PrimaryRegion())
+	f.Fly("vol create -y --app %s -s 2 --region %s vol_test", appName, f.PrimaryRegion())
+	{
+		toml, err := os.ReadFile("fly.toml")
+		if err != nil {
+			f.Fatalf("failed to read fly.toml: %s\n", err)
+		}
+		tomlStr := string(toml) + "\n[[mounts]]\n  source = \"vol_test\"\n  destination = \"/vol\"\n"
+		if err = os.WriteFile("fly.toml", []byte(tomlStr), 0644); err != nil {
+			f.Fatalf("failed to write fly.toml: %s\n", err)
+		}
+	}
+
+	assertHasFlag := func() {
+		output := f.Fly("ssh console -q -C 'cat /vol/flag.txt'")
+		output.AssertSuccessfulExit()
+		outStr := string(output.StdOut().Bytes())
+
+		require.Contains(t, outStr, successStr)
+	}
+
+	f.Fly("deploy --now")
+	f.Fly("ssh console -C \"bash -c 'echo %s > /vol/flag.txt && sync'\"", successStr)
+
+	assertHasFlag()
+
+	time.Sleep(3 * time.Second)
+	f.Fly("migrate-to-v2 --primary-region %s --yes", f.PrimaryRegion())
+	result := f.Fly("status --json")
+
+	var statusMap map[string]any
+	err = json.Unmarshal(result.StdOut().Bytes(), &statusMap)
+	if err != nil {
+		f.Fatalf("failed to parse json: %v [output]: %s\n", err, result.StdOut().String())
+	}
+	platformVersion, _ := statusMap["PlatformVersion"].(string)
+	require.Equal(f, "machines", platformVersion)
+
+	assertHasFlag()
+}