Repository: supabase/cli
Pull request #1278 (2023-07-04 03:59:30)

fix: wait for containers to be ready after db reset
## What kind of change does this PR introduce?

Bug fix https://github.com/supabase/cli/issues/1277

## What is the new behavior?

Wait for storage migrations to finish running before applying user migrations.

## Additional context

Add any other context or screenshots.



Patch:
diff --git a/internal/db/branch/switch_/switch_.go b/internal/db/branch/switch_/switch_.go
index 73e98022..111cf394 100644
--- a/internal/db/branch/switch_/switch_.go
+++ b/internal/db/branch/switch_/switch_.go
@@ -66,7 +66,11 @@ func switchDatabase(ctx context.Context, source, target string, options ...func(
 	if err := reset.DisconnectClients(ctx, conn); err != nil {
 		return err
 	}
-	defer reset.RestartDatabase(context.Background(), os.Stderr)
+	defer func() {
+		if err := reset.RestartDatabase(context.Background(), os.Stderr); err != nil {
+			fmt.Fprintln(os.Stderr, "Failed to restart database:", err)
+		}
+	}()
 	backup := "ALTER DATABASE postgres RENAME TO " + source + ";"
 	if _, err := conn.Exec(ctx, backup); err != nil {
 		return err
diff --git a/internal/db/reset/reset.go b/internal/db/reset/reset.go
index 63226e35..c3f3d4d1 100644
--- a/internal/db/reset/reset.go
+++ b/internal/db/reset/reset.go
@@ -10,7 +10,9 @@ import (
 	"strings"
 	"time"
 
+	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/container"
+	"github.com/docker/docker/pkg/stdcopy"
 	"github.com/jackc/pgconn"
 	"github.com/jackc/pgerrcode"
 	"github.com/jackc/pgx/v4"
@@ -28,6 +30,8 @@ const (
 )
 
 var (
+	ErrUnhealthy  = errors.New("service not healthy")
+	ErrDatabase   = errors.New("database is not healthy")
 	healthTimeout = 5 * time.Second
 	//go:embed templates/drop.sql
 	dropObjects string
@@ -61,13 +65,15 @@ func Run(ctx context.Context, config pgconn.Config, fsys afero.Fs, options ...fu
 
 func resetDatabase(ctx context.Context, fsys afero.Fs, options ...func(*pgx.ConnConfig)) error {
 	fmt.Fprintln(os.Stderr, "Resetting local database...")
-	if err := RecreateDatabase(ctx, options...); err != nil {
+	if err := recreateDatabase(ctx, options...); err != nil {
 		return err
 	}
 	if err := initDatabase(ctx, options...); err != nil {
 		return err
 	}
-	RestartDatabase(context.Background(), os.Stderr)
+	if err := RestartDatabase(ctx, os.Stderr); err != nil {
+		return err
+	}
 	conn, err := utils.ConnectLocalPostgres(ctx, pgconn.Config{}, options...)
 	if err != nil {
 		return err
@@ -93,7 +99,7 @@ func InitialiseDatabase(ctx context.Context, conn *pgx.Conn, fsys afero.Fs) erro
 }
 
 // Recreate postgres database by connecting to template1
-func RecreateDatabase(ctx context.Context, options ...func(*pgx.ConnConfig)) error {
+func recreateDatabase(ctx context.Context, options ...func(*pgx.ConnConfig)) error {
 	conn, err := utils.ConnectLocalPostgres(ctx, pgconn.Config{User: "supabase_admin", Database: "template1"}, options...)
 	if err != nil {
 		return err
@@ -138,34 +144,34 @@ func DisconnectClients(ctx context.Context, conn *pgx.Conn) error {
 	return err
 }
 
-func RestartDatabase(ctx context.Context, w io.Writer) {
+func RestartDatabase(ctx context.Context, w io.Writer) error {
 	fmt.Fprintln(w, "Restarting containers...")
 	// Some extensions must be manually restarted after pg_terminate_backend
 	// Ref: https://github.com/citusdata/pg_cron/issues/99
 	if err := utils.Docker.ContainerRestart(ctx, utils.DbId, container.StopOptions{}); err != nil {
-		fmt.Fprintln(w, "Failed to restart database:", err)
-		return
+		return err
 	}
 	if !WaitForHealthyService(ctx, utils.DbId, healthTimeout) {
-		fmt.Fprintln(w, "Database is not healthy.")
-		return
+		return ErrDatabase
 	}
 	// TODO: update storage-api to handle postgres restarts
 	if err := utils.Docker.ContainerRestart(ctx, utils.StorageId, container.StopOptions{}); err != nil {
-		fmt.Fprintln(w, "Failed to restart storage-api:", err)
+		return fmt.Errorf("failed to restart storage-api: %w", err)
 	}
 	// Reload PostgREST schema cache.
 	if err := utils.Docker.ContainerKill(ctx, utils.RestId, "SIGUSR1"); err != nil {
-		fmt.Fprintln(w, "Error reloading PostgREST schema cache:", err)
+		return fmt.Errorf("failed to reload PostgREST schema cache: %w", err)
 	}
 	// TODO: update gotrue to handle postgres restarts
 	if err := utils.Docker.ContainerRestart(ctx, utils.GotrueId, container.StopOptions{}); err != nil {
-		fmt.Fprintln(w, "Failed to restart gotrue:", err)
+		return fmt.Errorf("failed to restart gotrue: %w", err)
 	}
 	// TODO: update realtime to handle postgres restarts
 	if err := utils.Docker.ContainerRestart(ctx, utils.RealtimeId, container.StopOptions{}); err != nil {
-		fmt.Fprintln(w, "Failed to restart realtime:", err)
+		return fmt.Errorf("failed to restart realtime: %w", err)
 	}
+	// Wait for services with internal schema migrations
+	return WaitForServiceReady(ctx, []string{utils.StorageId, utils.GotrueId})
 }
 
 func RetryEverySecond(ctx context.Context, callback func() bool, timeout time.Duration) bool {
@@ -188,6 +194,39 @@ func WaitForHealthyService(ctx context.Context, container string, timeout time.D
 	return RetryEverySecond(ctx, probe, timeout)
 }
 
+func WaitForServiceReady(ctx context.Context, started []string) error {
+	probe := func() bool {
+		var unhealthy []string
+		for _, container := range started {
+			if !status.IsServiceReady(ctx, container) {
+				unhealthy = append(unhealthy, container)
+			}
+		}
+		started = unhealthy
+		return len(started) == 0
+	}
+	if !RetryEverySecond(ctx, probe, 30*time.Second) {
+		// Print container logs for easier debugging
+		for _, container := range started {
+			logs, err := utils.Docker.ContainerLogs(ctx, container, types.ContainerLogsOptions{
+				ShowStdout: true,
+				ShowStderr: true,
+			})
+			if err != nil {
+				fmt.Fprintln(os.Stderr, err)
+				continue
+			}
+			fmt.Fprintln(os.Stderr, container, "container logs:")
+			if _, err := stdcopy.StdCopy(os.Stderr, os.Stderr, logs); err != nil {
+				fmt.Fprintln(os.Stderr, err)
+			}
+			logs.Close()
+		}
+		return fmt.Errorf("%w: %v", ErrUnhealthy, started)
+	}
+	return nil
+}
+
 func resetRemote(ctx context.Context, config pgconn.Config, fsys afero.Fs, options ...func(*pgx.ConnConfig)) error {
 	conn, err := utils.ConnectRemotePostgres(ctx, config, options...)
 	if err != nil {
diff --git a/internal/db/start/start.go b/internal/db/start/start.go
index 38f52c83..01aa66db 100644
--- a/internal/db/start/start.go
+++ b/internal/db/start/start.go
@@ -101,7 +101,7 @@ func StartDatabase(ctx context.Context, fsys afero.Fs, w io.Writer, options ...f
 		return err
 	}
 	if !reset.WaitForHealthyService(ctx, utils.DbId, 20*time.Second) {
-		fmt.Fprintln(os.Stderr, "Database is not healthy.")
+		return reset.ErrDatabase
 	}
 	// Initialise if we are on PG14 and there's no existing db volume
 	if noBackupVolume {
diff --git a/internal/start/start.go b/internal/start/start.go
index 0625a3a8..54f7c6aa 100644
--- a/internal/start/start.go
+++ b/internal/start/start.go
@@ -13,9 +13,7 @@ import (
 	"text/template"
 	"time"
 
-	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/container"
-	"github.com/docker/docker/pkg/stdcopy"
 	"github.com/docker/go-connections/nat"
 	"github.com/jackc/pgconn"
 	"github.com/jackc/pgx/v4"
@@ -28,8 +26,6 @@ import (
 	"github.com/supabase/cli/internal/utils"
 )
 
-var errUnhealthy = errors.New("service not healthy")
-
 func Run(ctx context.Context, fsys afero.Fs, excludedContainers []string, ignoreHealthCheck bool, projectRef, dbUrl string) error {
 	// Sanity checks.
 	{
@@ -77,7 +73,7 @@ func Run(ctx context.Context, fsys afero.Fs, excludedContainers []string, ignore
 		}
 		return run(p, ctx, fsys, excludedContainers, dbConfig)
 	}); err != nil {
-		if ignoreHealthCheck && errors.Is(err, errUnhealthy) {
+		if ignoreHealthCheck && errors.Is(err, reset.ErrUnhealthy) {
 			fmt.Fprintln(os.Stderr, err)
 		} else {
 			utils.DockerRemoveAll(context.Background())
@@ -199,7 +195,7 @@ EOF
 		); err != nil {
 			return err
 		}
-		if err := waitForServiceReady(ctx, []string{utils.VectorId}); err != nil {
+		if err := reset.WaitForServiceReady(ctx, []string{utils.VectorId}); err != nil {
 			return err
 		}
 	}
@@ -260,7 +256,7 @@ EOF
 		); err != nil {
 			return err
 		}
-		if err := waitForServiceReady(ctx, []string{utils.LogflareId}); err != nil {
+		if err := reset.WaitForServiceReady(ctx, []string{utils.LogflareId}); err != nil {
 			return err
 		}
 	}
@@ -652,7 +648,7 @@ EOF
 		started = append(started, utils.StudioId)
 	}
 
-	return waitForServiceReady(ctx, started)
+	return reset.WaitForServiceReady(ctx, started)
 }
 
 func isContainerExcluded(imageName string, excluded map[string]bool) bool {
@@ -670,36 +666,3 @@ func ExcludableContainers() []string {
 	}
 	return names
 }
-
-func waitForServiceReady(ctx context.Context, started []string) error {
-	probe := func() bool {
-		var unhealthy []string
-		for _, container := range started {
-			if !status.IsServiceReady(ctx, container) {
-				unhealthy = append(unhealthy, container)
-			}
-		}
-		started = unhealthy
-		return len(started) == 0
-	}
-	if !reset.RetryEverySecond(ctx, probe, 30*time.Second) {
-		// Print container logs for easier debugging
-		for _, container := range started {
-			logs, err := utils.Docker.ContainerLogs(ctx, container, types.ContainerLogsOptions{
-				ShowStdout: true,
-				ShowStderr: true,
-			})
-			if err != nil {
-				fmt.Fprintln(os.Stderr, err)
-				continue
-			}
-			fmt.Fprintln(os.Stderr, container, "container logs:")
-			if _, err := stdcopy.StdCopy(os.Stderr, os.Stderr, logs); err != nil {
-				fmt.Fprintln(os.Stderr, err)
-			}
-			logs.Close()
-		}
-		return fmt.Errorf("%w: %v", errUnhealthy, started)
-	}
-	return nil
-}
diff --git a/internal/utils/deno.go b/internal/utils/deno.go
index 4311e598..2970b0dc 100644
--- a/internal/utils/deno.go
+++ b/internal/utils/deno.go
@@ -68,24 +68,14 @@ func InstallOrUpgradeDeno(ctx context.Context, fsys afero.Fs) error {
 	}
 
 	// 1. Determine OS triple
-	var assetFilename string
+	assetFilename, err := getDenoAssetFileName()
+	if err != nil {
+		return err
+	}
 	assetRepo := "denoland/deno"
-	{
-		if runtime.GOOS == "darwin" && runtime.GOARCH == "amd64" {
-			assetFilename = "deno-x86_64-apple-darwin.zip"
-		} else if runtime.GOOS == "darwin" && runtime.GOARCH == "arm64" {
-			assetFilename = "deno-aarch64-apple-darwin.zip"
-		} else if runtime.GOOS == "linux" && runtime.GOARCH == "amd64" {
-			assetFilename = "deno-x86_64-unknown-linux-gnu.zip"
-		} else if runtime.GOOS == "linux" && runtime.GOARCH == "arm64" {
-			// TODO: version pin to official release once available https://github.com/denoland/deno/issues/1846
-			assetRepo = "LukeChannings/deno-arm64"
-			assetFilename = "deno-linux-arm64.zip"
-		} else if runtime.GOOS == "windows" && runtime.GOARCH == "amd64" {
-			assetFilename = "deno-x86_64-pc-windows-msvc.zip"
-		} else {
-			return errors.New("Platform " + runtime.GOOS + "/" + runtime.GOARCH + " is currently unsupported for Functions.")
-		}
+	if runtime.GOOS == "linux" && runtime.GOARCH == "arm64" {
+		// TODO: version pin to official release once available https://github.com/denoland/deno/issues/1846
+		assetRepo = "LukeChannings/deno-arm64"
 	}
 
 	// 2. Download & install Deno binary.
diff --git a/internal/utils/deno_darwin.go b/internal/utils/deno_darwin.go
new file mode 100644
index 00000000..cfe3d5cf
--- /dev/null
+++ b/internal/utils/deno_darwin.go
@@ -0,0 +1,28 @@
+//go:build darwin
+
+package utils
+
+import (
+	"fmt"
+	"syscall"
+)
+
+func getDenoAssetFileName() (string, error) {
+	// Simple runtime.GOARCH detection doesn't work if the CLI is
+	// running under Rosetta:
+	// https://github.com/supabase/cli/issues/1266. So as a workaround
+	// we use Apple Silicon detection:
+	// https://www.yellowduck.be/posts/detecting-apple-silicon-via-go.
+	_, err := syscall.Sysctl("sysctl.proc_translated")
+	if err != nil {
+		if err.Error() == "no such file or directory" {
+			// Running on Intel Mac.
+			return "deno-x86_64-apple-darwin.zip", nil
+		} else {
+			return "", fmt.Errorf("failed to determine OS triple: %w", err)
+		}
+	} else {
+		// Running on Apple Silicon.
+		return "deno-aarch64-apple-darwin.zip", nil
+	}
+}
diff --git a/internal/utils/deno_generic.go b/internal/utils/deno_generic.go
new file mode 100644
index 00000000..a9e6f15b
--- /dev/null
+++ b/internal/utils/deno_generic.go
@@ -0,0 +1,21 @@
+//go:build !darwin
+
+package utils
+
+import (
+	"errors"
+	"runtime"
+)
+
+func getDenoAssetFileName() (string, error) {
+	if runtime.GOOS == "linux" && runtime.GOARCH == "amd64" {
+		return "deno-x86_64-unknown-linux-gnu.zip", nil
+	} else if runtime.GOOS == "linux" && runtime.GOARCH == "arm64" {
+		// TODO: version pin to official release once available https://github.com/denoland/deno/issues/1846
+		return "deno-linux-arm64.zip", nil
+	} else if runtime.GOOS == "windows" && runtime.GOARCH == "amd64" {
+		return "deno-x86_64-pc-windows-msvc.zip", nil
+	} else {
+		return "", errors.New("Platform " + runtime.GOOS + "/" + runtime.GOARCH + " is currently unsupported for Functions.")
+	}
+}


Test patch:
diff --git a/internal/db/reset/reset_test.go b/internal/db/reset/reset_test.go
index 798c07dd..fe9afafc 100644
--- a/internal/db/reset/reset_test.go
+++ b/internal/db/reset/reset_test.go
@@ -1,7 +1,6 @@
 package reset
 
 import (
-	"bytes"
 	"context"
 	"fmt"
 	"io"
@@ -12,6 +11,7 @@ import (
 	"time"
 
 	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/errdefs"
 	"github.com/jackc/pgconn"
 	"github.com/jackc/pgerrcode"
 	"github.com/spf13/afero"
@@ -179,12 +179,12 @@ func TestRecreateDatabase(t *testing.T) {
 			Query("CREATE DATABASE postgres WITH OWNER postgres").
 			Reply("CREATE DATABASE")
 		// Run test
-		assert.NoError(t, RecreateDatabase(context.Background(), conn.Intercept))
+		assert.NoError(t, recreateDatabase(context.Background(), conn.Intercept))
 	})
 
 	t.Run("throws error on invalid port", func(t *testing.T) {
 		utils.Config.Db.Port = 0
-		assert.ErrorContains(t, RecreateDatabase(context.Background()), "invalid port")
+		assert.ErrorContains(t, recreateDatabase(context.Background()), "invalid port")
 	})
 
 	t.Run("continues on disconnecting missing database", func(t *testing.T) {
@@ -197,7 +197,7 @@ func TestRecreateDatabase(t *testing.T) {
 			Query(fmt.Sprintf(utils.TerminateDbSqlFmt, "postgres")).
 			ReplyError(pgerrcode.UndefinedTable, `relation "pg_stat_activity" does not exist`)
 		// Run test
-		err := RecreateDatabase(context.Background(), conn.Intercept)
+		err := recreateDatabase(context.Background(), conn.Intercept)
 		// Check error
 		assert.ErrorContains(t, err, `ERROR: relation "pg_stat_activity" does not exist (SQLSTATE 42P01)`)
 	})
@@ -210,7 +210,7 @@ func TestRecreateDatabase(t *testing.T) {
 		conn.Query("ALTER DATABASE postgres ALLOW_CONNECTIONS false;").
 			ReplyError(pgerrcode.InvalidParameterValue, `cannot disallow connections for current database`)
 		// Run test
-		err := RecreateDatabase(context.Background(), conn.Intercept)
+		err := recreateDatabase(context.Background(), conn.Intercept)
 		// Check error
 		assert.ErrorContains(t, err, "ERROR: cannot disallow connections for current database (SQLSTATE 22023)")
 	})
@@ -228,7 +228,7 @@ func TestRecreateDatabase(t *testing.T) {
 			ReplyError(pgerrcode.ObjectInUse, `database "postgres" is used by an active logical replication slot`).
 			Query("CREATE DATABASE postgres WITH OWNER postgres")
 		// Run test
-		err := RecreateDatabase(context.Background(), conn.Intercept)
+		err := recreateDatabase(context.Background(), conn.Intercept)
 		// Check error
 		assert.ErrorContains(t, err, `ERROR: database "postgres" is used by an active logical replication slot (SQLSTATE 55006)`)
 	})
@@ -257,25 +257,38 @@ func TestRestartDatabase(t *testing.T) {
 		utils.RestId = "test-rest"
 		gock.New(utils.Docker.DaemonHost()).
 			Post("/v" + utils.Docker.ClientVersion() + "/containers/" + utils.RestId + "/kill").
-			Reply(http.StatusServiceUnavailable)
+			Reply(http.StatusOK)
 		// Restarts storage-api
 		utils.StorageId = "test-storage"
 		gock.New(utils.Docker.DaemonHost()).
 			Post("/v" + utils.Docker.ClientVersion() + "/containers/" + utils.StorageId + "/restart").
-			Reply(http.StatusServiceUnavailable)
+			Reply(http.StatusOK)
 		// Restarts gotrue
 		utils.GotrueId = "test-auth"
 		gock.New(utils.Docker.DaemonHost()).
 			Post("/v" + utils.Docker.ClientVersion() + "/containers/" + utils.GotrueId + "/restart").
-			Reply(http.StatusServiceUnavailable)
+			Reply(http.StatusOK)
 		// Restarts realtime
 		utils.RealtimeId = "test-realtime"
 		gock.New(utils.Docker.DaemonHost()).
 			Post("/v" + utils.Docker.ClientVersion() + "/containers/" + utils.RealtimeId + "/restart").
-			Reply(http.StatusServiceUnavailable)
+			Reply(http.StatusOK)
+		// Wait for services ready
+		for _, container := range []string{utils.StorageId, utils.GotrueId} {
+			gock.New(utils.Docker.DaemonHost()).
+				Get("/v" + utils.Docker.ClientVersion() + "/containers/" + container + "/json").
+				Reply(http.StatusOK).
+				JSON(types.ContainerJSON{ContainerJSONBase: &types.ContainerJSONBase{
+					State: &types.ContainerState{
+						Running: true,
+						Health:  &types.Health{Status: "healthy"},
+					},
+				}})
+		}
 		// Run test
-		RestartDatabase(context.Background(), io.Discard)
+		err := RestartDatabase(context.Background(), io.Discard)
 		// Check error
+		assert.NoError(t, err)
 		assert.Empty(t, apitest.ListUnmatchedRequests())
 	})
 
@@ -289,11 +302,10 @@ func TestRestartDatabase(t *testing.T) {
 			Post("/v" + utils.Docker.ClientVersion() + "/containers/" + utils.DbId + "/restart").
 			Reply(http.StatusServiceUnavailable)
 		// Run test
-		var stderr bytes.Buffer
-		RestartDatabase(context.Background(), &stderr)
+		err := RestartDatabase(context.Background(), io.Discard)
 		// Check error
+		assert.True(t, errdefs.IsUnavailable(err))
 		assert.Empty(t, apitest.ListUnmatchedRequests())
-		assert.Contains(t, stderr.String(), "Failed to restart database")
 	})
 
 	t.Run("timeout health check", func(t *testing.T) {
@@ -306,8 +318,9 @@ func TestRestartDatabase(t *testing.T) {
 			Post("/v" + utils.Docker.ClientVersion() + "/containers/" + utils.DbId + "/restart").
 			Reply(http.StatusOK)
 		// Run test
-		RestartDatabase(context.Background(), io.Discard)
+		err := RestartDatabase(context.Background(), io.Discard)
 		// Check error
+		assert.ErrorIs(t, err, ErrDatabase)
 		assert.Empty(t, apitest.ListUnmatchedRequests())
 	})
 }