Add --all flag to fly vol ls (#3486)

* Add --all flag to fly vol ls

Example with and without the `--all` flag:

```
ID                      STATE   NAME    SIZE    REGION  ZONE    ENCRYPTED       ATTACHED VM     CREATED AT
vol_ke628r0000000000    created data    1GB     sea     b820    true            3d8d0000000000  1 year ago

ID                      STATE                   NAME            SIZE    REGION  ZONE    ENCRYPTED       ATTACHED VM     CREATED AT
vol_ke628r0000000000    created                 data            1GB     sea     b820    true            3d8d0000000000  1 year ago
vol_vd3edp0000000000    scheduling_destroy      deleteme        1GB     sea     6d99    true                            30 seconds ago
```

* add preflight test for fly vol ls --all

* try fixing TestPostgres_ImportSuccess test failure with retries

* oops fix go.mod

* more backoff for ssh

* fix bug in `fly ssh console --user`

Previously, `fly ssh console --user username` would fail because the ssh certificate was always generated for the root user. This has been broken for some time, and became more prominent as we started using app specific certs in more places.

Pass through the `--user` flag to the certificate generation.

This also fixes the TestPostgres_ImportSuccess, which relies on the `--user` flag.

go.mod

@@ -17,6 +17,7 @@ require (
 	github.com/buildpacks/pack v0.33.2
 	github.com/cavaliergopher/grab/v3 v3.0.1
 	github.com/cenkalti/backoff v2.2.1+incompatible
+	github.com/cenkalti/backoff/v4 v4.3.0
 	github.com/chzyer/readline v1.5.1
 	github.com/cli/safeexec v1.0.1
 	github.com/docker/docker v25.0.3+incompatible
@@ -132,7 +133,6 @@ require (
 	github.com/beorn7/perks v1.0.1 // indirect
 	github.com/buildpacks/imgutil v0.0.0-20240118145509-e94a1b7de8a9 // indirect
 	github.com/buildpacks/lifecycle v0.18.5 // indirect
-	github.com/cenkalti/backoff/v4 v4.3.0 // indirect
 	github.com/cespare/xxhash/v2 v2.2.0 // indirect
 	github.com/chrismellard/docker-credential-acr-env v0.0.0-20230304212654-82a0ddb27589 // indirect
 	github.com/cloudflare/circl v1.3.7 // indirect

internal/command/ssh/connect.go

@@ -61,7 +61,7 @@ type ConnectParams struct {
 func Connect(p *ConnectParams, addr string) (*ssh.Client, error) {
 	terminal.Debugf("Fetching certificate for %s\n", addr)
 
-	cert, pk, err := singleUseSSHCertificate(p.Ctx, p.Org, p.AppNames)
+	cert, pk, err := singleUseSSHCertificate(p.Ctx, p.Org, p.AppNames, p.Username)
 	if err != nil {
 		return nil, fmt.Errorf("create ssh certificate: %w (if you haven't created a key for your org yet, try `flyctl ssh issue`)", err)
 	}
@@ -100,7 +100,7 @@ func Connect(p *ConnectParams, addr string) (*ssh.Client, error) {
 	return sshClient, nil
 }
 
-func singleUseSSHCertificate(ctx context.Context, org fly.OrganizationImpl, appNames []string) (*fly.IssuedCertificate, ed25519.PrivateKey, error) {
+func singleUseSSHCertificate(ctx context.Context, org fly.OrganizationImpl, appNames []string, user string) (*fly.IssuedCertificate, ed25519.PrivateKey, error) {
 	client := fly.ClientFromContext(ctx)
 	hours := 1
 
@@ -109,7 +109,7 @@ func singleUseSSHCertificate(ctx context.Context, org fly.OrganizationImpl, appN
 		return nil, nil, err
 	}
 
-	icert, err := client.IssueSSHCertificate(ctx, org, []string{DefaultSshUsername, "fly"}, appNames, &hours, pub)
+	icert, err := client.IssueSSHCertificate(ctx, org, []string{user, "fly"}, appNames, &hours, pub)
 	if err != nil {
 		return nil, nil, err
 	}

internal/command/ssh/ssh_terminal.go

@@ -70,7 +70,7 @@ func SSHConnect(p *SSHParams, addr string) error {
 		appNames = append(appNames, p.App)
 	}
 
-	cert, pk, err := singleUseSSHCertificate(p.Ctx, p.Org, appNames)
+	cert, pk, err := singleUseSSHCertificate(p.Ctx, p.Org, appNames, p.Username)
 	if err != nil {
 		return fmt.Errorf("create ssh certificate: %w (if you haven't created a key for your org yet, try `flyctl ssh issue`)", err)
 	}

internal/command/volumes/list.go

@@ -33,6 +33,10 @@ func newList() *cobra.Command {
 	flag.Add(cmd,
 		flag.App(),
 		flag.AppConfig(),
+		flag.Bool{
+			Name:        "all",
+			Description: "Show all volumes including those in destroyed states",
+		},
 	)
 
 	flag.Add(cmd, flag.JSONOutput())
@@ -57,7 +61,12 @@ func runList(ctx context.Context) error {
 		return err
 	}
 
-	volumes, err := flapsClient.GetVolumes(ctx)
+	var volumes []fly.Volume
+	if flag.GetBool(ctx, "all") {
+		volumes, err = flapsClient.GetAllVolumes(ctx)
+	} else {
+		volumes, err = flapsClient.GetVolumes(ctx)
+	}
 	if err != nil {
 		return fmt.Errorf("failed retrieving volumes: %w", err)
 	}

test/preflight/fly_postgres_test.go

@@ -4,9 +4,11 @@
 package preflight
 
 import (
+	"fmt"
 	"testing"
 	"time"
 
+	"github.com/cenkalti/backoff/v4"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	fly "github.com/superfly/fly-go"
@@ -127,6 +129,18 @@ func TestPostgres_ImportSuccess(t *testing.T) {
 		"pg create --org %s --name %s --region %s --initial-cluster-size 1 --vm-size shared-cpu-1x --volume-size 1",
 		f.OrgSlug(), secondAppName, f.PrimaryRegion(),
 	)
+	sshErr := backoff.Retry(func() error {
+		sshWorks := f.FlyAllowExitFailure("ssh console -a %s -u postgres -C \"psql -p 5433 -h /run/postgresql -c 'SELECT 1'\"", firstAppName)
+		if sshWorks.ExitCode() != 0 {
+			return fmt.Errorf("non-zero exit code running fly ssh console")
+		} else {
+			return nil
+		}
+	}, backoff.WithMaxRetries(backoff.NewExponentialBackOff(
+		backoff.WithInitialInterval(100*time.Millisecond),
+		backoff.WithMaxElapsedTime(3*time.Second),
+	), 3))
+	require.NoError(f, sshErr, "failed to connect to first app's postgres over ssh")
 
 	f.Fly(
 		"ssh console -a %s -u postgres -C \"psql -p 5433 -h /run/postgresql -c 'CREATE TABLE app_name (app_name TEXT)'\"",

test/preflight/fly_volume_test.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"github.com/superfly/fly-go"
 	"github.com/superfly/flyctl/test/preflight/testlib"
 )
 
@@ -33,26 +34,53 @@ primary_region = "%s"
 	require.Equal(f, 1, len(ml))
 
 	f.Fly("vol extend -s 4 %s", ml[0].Config.Mounts[0].Volume)
-	require.EventuallyWithT(t, func(c *assert.CollectT) {
+	require.EventuallyWithT(f, func(c *assert.CollectT) {
 		vl := f.VolumeList(appName)
 		require.Equal(c, vl[0].SizeGb, 4)
 	}, 10*time.Second, 2*time.Second)
 
 	f.Fly("vol extend -s +1 %s", ml[0].Config.Mounts[0].Volume)
-	require.EventuallyWithT(t, func(c *assert.CollectT) {
+	require.EventuallyWithT(f, func(c *assert.CollectT) {
 		vl := f.VolumeList(appName)
 		require.Equal(c, vl[0].SizeGb, 5)
 	}, 10*time.Second, 2*time.Second)
 
 	f.Fly("vol extend -s +1gb %s", ml[0].Config.Mounts[0].Volume)
-	require.EventuallyWithT(t, func(c *assert.CollectT) {
+	require.EventuallyWithT(f, func(c *assert.CollectT) {
 		vl := f.VolumeList(appName)
 		require.Equal(c, vl[0].SizeGb, 6)
 	}, 10*time.Second, 2*time.Second)
 
 	f.Fly("vol extend -s 7gb %s", ml[0].Config.Mounts[0].Volume)
-	require.EventuallyWithT(t, func(c *assert.CollectT) {
+	require.EventuallyWithT(f, func(c *assert.CollectT) {
 		vl := f.VolumeList(appName)
 		require.Equal(c, vl[0].SizeGb, 7)
 	}, 10*time.Second, 2*time.Second)
 }
+
+func TestFlyVolumeLs(t *testing.T) {
+	f := testlib.NewTestEnvFromEnv(t)
+	appName := f.CreateRandomAppMachines()
+	v1Res := f.Fly("vol create -s 1 -a %s -r %s --yes --json test_keep", appName, f.PrimaryRegion())
+	var v1 *fly.Volume
+	v1Res.StdOutJSON(&v1)
+	v2Res := f.Fly("vol create -s 1 -a %s -r %s --yes --json test_destroy", appName, f.PrimaryRegion())
+	var v2 *fly.Volume
+	v2Res.StdOutJSON(&v2)
+	f.Fly("vol destroy -y %s", v2.ID)
+	lsRes := f.Fly("vol ls -a %s --json", appName)
+	var ls []*fly.Volume
+	lsRes.StdOutJSON(&ls)
+	require.Len(f, ls, 1)
+	require.Equal(f, v1.ID, ls[0].ID)
+	lsAllRes := f.Fly("vol ls --all -a %s --json", appName)
+	var lsAll []*fly.Volume
+	lsAllRes.StdOutJSON(&lsAll)
+	require.Len(f, lsAll, 2)
+	var lsAllIds []string
+	for _, v := range lsAll {
+		lsAllIds = append(lsAllIds, v.ID)
+	}
+	require.Contains(f, lsAllIds, v1.ID)
+	require.Contains(f, lsAllIds, v2.ID)
+}