\\Better handling for Postgres image versions and features on Stolon clusters (#3803)

* Check whether a cluster is Flex before allowing backup-related commands.

* Emit better errors when image versions are malformed and likely indicate an update.

* Add missing nil pointer check.

Author: ndarilek

internal/command/postgres/backup.go

@@ -86,15 +86,6 @@ func runBackupRestore(ctx context.Context) error {
 		destAppName = flag.FirstArg(ctx)
 	)
 
-	enabled, err := isBackupEnabled(ctx, appName)
-	if err != nil {
-		return err
-	}
-
-	if !enabled {
-		return fmt.Errorf("backups are not enabled. Run `fly pg backup enable -a %s` to enable them", appName)
-	}
-
 	flapsClient, err := flapsutil.NewClientWithOptions(ctx, flaps.NewClientOpts{
 		AppName: appName,
 	})
@@ -111,17 +102,30 @@ func runBackupRestore(ctx context.Context) error {
 		return fmt.Errorf("No active machines")
 	}
 
-	// Ensure the the app has the required flex version.
-	if err := hasRequiredFlexVersionOnMachines(appName, machines, backupVersion); err != nil {
+	// Resolve the leader
+	leader, err := pickLeader(ctx, machines)
+	if err != nil {
 		return err
 	}
 
-	// Resolve the leader
-	leader, err := pickLeader(ctx, machines)
+	if !IsFlex(leader) {
+		return fmt.Errorf("backups are only supported on Flexclusters")
+	}
+
+	enabled, err := isBackupEnabled(ctx, appName)
 	if err != nil {
 		return err
 	}
 
+	if !enabled {
+		return fmt.Errorf("backups are not enabled. Run `fly pg backup enable -a %s` to enable them", appName)
+	}
+
+	// Ensure the the app has the required flex version.
+	if err := hasRequiredFlexVersionOnMachines(appName, machines, backupVersion); err != nil {
+		return err
+	}
+
 	// TODO - Use this to create new Tigris access keys. However, if we can't yet revoke
 	// access keys after the restore process completes, we should understand the implications of
 	// creating potentially many access keys.
@@ -212,15 +216,6 @@ func runBackupCreate(ctx context.Context) error {
 		appName = appconfig.NameFromContext(ctx)
 	)
 
-	enabled, err := isBackupEnabled(ctx, appName)
-	if err != nil {
-		return err
-	}
-
-	if !enabled {
-		return fmt.Errorf("backups are not enabled. Run `fly pg backup enable -a %s` to enable them", appName)
-	}
-
 	flapsClient, err := flapsutil.NewClientWithOptions(ctx, flaps.NewClientOpts{
 		AppName: appName,
 	})
@@ -237,16 +232,29 @@ func runBackupCreate(ctx context.Context) error {
 		return fmt.Errorf("No active machines")
 	}
 
-	if err := hasRequiredFlexVersionOnMachines(appName, machines, backupVersion); err != nil {
+	// Ensure the backup is issued against the primary.
+	leader, err := pickLeader(ctx, machines)
+	if err != nil {
 		return err
 	}
 
-	// Ensure the backup is issued against the primary.
-	leader, err := pickLeader(ctx, machines)
+	if !IsFlex(leader) {
+		return fmt.Errorf("backups are only supported on Flexclusters")
+	}
+
+	enabled, err := isBackupEnabled(ctx, appName)
 	if err != nil {
 		return err
 	}
 
+	if !enabled {
+		return fmt.Errorf("backups are not enabled. Run `fly pg backup enable -a %s` to enable them", appName)
+	}
+
+	if err := hasRequiredFlexVersionOnMachines(appName, machines, backupVersion); err != nil {
+		return err
+	}
+
 	if !hasRequiredMemoryForBackup(*leader) {
 		return fmt.Errorf("backup creation requires at least 512MB of memory. Use `fly m update %s --vm-memory 512` to scale up.", leader.ID)
 	}
@@ -330,12 +338,16 @@ func runBackupEnable(ctx context.Context) error {
 		return fmt.Errorf("No active machines")
 	}
 
-	if err := hasRequiredFlexVersionOnMachines(appName, machines, backupVersion); err != nil {
+	leader, err := pickLeader(ctx, machines)
+	if err != nil {
 		return err
 	}
 
-	leader, err := pickLeader(ctx, machines)
-	if err != nil {
+	if !IsFlex(leader) {
+		return fmt.Errorf("backups are only supported on Flexclusters")
+	}
+
+	if err := hasRequiredFlexVersionOnMachines(appName, machines, backupVersion); err != nil {
 		return err
 	}
 
@@ -397,23 +409,14 @@ func runBackupList(ctx context.Context) error {
 		appName = appconfig.NameFromContext(ctx)
 	)
 
-	enabled, err := isBackupEnabled(ctx, appName)
-	if err != nil {
-		return err
-	}
-
-	if !enabled {
-		return fmt.Errorf("backups are not enabled. Run `fly pg backup enable -a %s` to enable them", appName)
-	}
-
 	flapsClient, err := flapsutil.NewClientWithOptions(ctx, flaps.NewClientOpts{
 		AppName: appName,
 	})
 	if err != nil {
 		return fmt.Errorf("list of machines could not be retrieved: %w", err)
 	}
 
-	machines, err := flapsClient.List(ctx, "started")
+	machines, err := flapsClient.ListActive(ctx)
 	if err != nil {
 		return err
 	}
@@ -422,11 +425,24 @@ func runBackupList(ctx context.Context) error {
 		return fmt.Errorf("No active machines")
 	}
 
-	if err = hasRequiredFlexVersionOnMachines(appName, machines, backupVersion); err != nil {
+	machine := machines[0]
+
+	if !IsFlex(machine) {
+		return fmt.Errorf("backups are only supported on Flexclusters")
+	}
+
+	enabled, err := isBackupEnabled(ctx, appName)
+	if err != nil {
 		return err
 	}
 
-	machine := machines[0]
+	if !enabled {
+		return fmt.Errorf("backups are not enabled. Run `fly pg backup enable -a %s` to enable them", appName)
+	}
+
+	if err = hasRequiredFlexVersionOnMachines(appName, machines, backupVersion); err != nil {
+		return err
+	}
 
 	return ExecOnMachine(ctx, flapsClient, machine.ID, "flexctl backup list")
 }
@@ -540,15 +556,6 @@ func runBackupConfigShow(ctx context.Context) error {
 		appName = appconfig.NameFromContext(ctx)
 	)
 
-	enabled, err := isBackupEnabled(ctx, appName)
-	if err != nil {
-		return err
-	}
-
-	if !enabled {
-		return fmt.Errorf("backups are not enabled. Run `fly pg backup enable -a %s` to enable them", appName)
-	}
-
 	flapsClient, err := flapsutil.NewClientWithOptions(ctx, flaps.NewClientOpts{
 		AppName: appName,
 	})
@@ -565,6 +572,19 @@ func runBackupConfigShow(ctx context.Context) error {
 		return fmt.Errorf("No active machines")
 	}
 
+	if !IsFlex(machines[0]) {
+		return fmt.Errorf("backups are only supported on Flexclusters")
+	}
+
+	enabled, err := isBackupEnabled(ctx, appName)
+	if err != nil {
+		return err
+	}
+
+	if !enabled {
+		return fmt.Errorf("backups are not enabled. Run `fly pg backup enable -a %s` to enable them", appName)
+	}
+
 	// Ensure the the app has the required flex version.
 	if err := hasRequiredFlexVersionOnMachines(appName, machines, backupConfigVersion); err != nil {
 		return err
@@ -594,6 +614,10 @@ func runBackupConfigUpdate(ctx context.Context) error {
 		return fmt.Errorf("No active machines")
 	}
 
+	if !IsFlex(machines[0]) {
+		return fmt.Errorf("backups are only supported on Flexclusters")
+	}
+
 	// Ensure the the app has the required flex version.
 	if err := hasRequiredFlexVersionOnMachines(appName, machines, backupConfigVersion); err != nil {
 		return err

internal/command/postgres/postgres.go

@@ -126,7 +126,7 @@ func hasRequiredFlexVersionOnMachines(appName string, machines []*fly.Machine, f
 	}
 
 	err := hasRequiredVersionOnMachines(appName, machines, "", flexVersion, "")
-	if strings.Contains(err.Error(), "Malformed version") {
+	if err != nil && strings.Contains(err.Error(), "Malformed version") {
 		return fmt.Errorf("This image is not compatible with this feature.")
 	}
 	return err