Skip to content

Commit

Permalink
test: make Postgres tests more stable (#3857)
Browse files Browse the repository at this point in the history
* test: make Postgres machines bigger

shared-cpu-1x might be too small.

* test: wait machines longer

`fly pg import` starts a disposable machine. Sometimes that lingers
longer than we expected.

* test: make sure Postgres is up before `fly pg import`
  • Loading branch information
kzys authored Aug 19, 2024
1 parent 0ee4071 commit 885dd16
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 33 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ require (
github.com/buildpacks/pack v0.35.1
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/coder/websocket v1.8.12
Expand Down Expand Up @@ -143,6 +142,7 @@ require (
github.com/beorn7/perks v1.0.1 // indirect
github.com/buildpacks/imgutil v0.0.0-20240605145725-186f89b2d168 // indirect
github.com/buildpacks/lifecycle v0.19.6 // indirect
github.com/cenkalti/backoff/v4 v4.3.0 // indirect
github.com/cespare/xxhash/v2 v2.3.0 // indirect
github.com/chrismellard/docker-credential-acr-env v0.0.0-20230304212654-82a0ddb27589 // indirect
github.com/cloudflare/circl v1.3.7 // indirect
Expand Down
82 changes: 50 additions & 32 deletions test/preflight/fly_postgres_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"testing"
"time"

"github.com/cenkalti/backoff/v4"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
fly "github.com/superfly/fly-go"
Expand Down Expand Up @@ -44,7 +43,10 @@ func TestPostgres_autostart(t *testing.T) {

appName := f.CreateRandomAppName()

f.Fly("pg create --org %s --name %s --region %s --initial-cluster-size 1 --vm-size shared-cpu-1x --volume-size 1", f.OrgSlug(), appName, f.PrimaryRegion())
f.Fly(
"pg create --org %s --name %s --region %s --initial-cluster-size 1 --vm-size %s --volume-size 1",
f.OrgSlug(), appName, f.PrimaryRegion(), postgresMachineSize,
)
machList := f.MachinesList(appName)
require.Equal(t, 1, len(machList), "expected exactly 1 machine after launch")
firstMachine := machList[0]
Expand Down Expand Up @@ -149,6 +151,30 @@ func TestPostgres_haConfigSave(t *testing.T) {
f.Fly("config validate")
}

const postgresMachineSize = "shared-cpu-4x"

// assertMachineCount checks the number of machines for the given app.
func assertMachineCount(tb testing.TB, f *testlib.FlyctlTestEnv, appName string, expected int) {
tb.Helper()

machines := f.MachinesList(appName)
var s string

for _, m := range machines {
s += fmt.Sprintf("machine %s (image: %s)", m.ID, m.FullImageRef())
}
assert.Len(tb, machines, expected, "expected %d machine(s) but got %s", expected, s)
}

// assertPostgresIsUp checks that the given Postgres server is really up.
// Even after "fly pg create", sometimes the server is not ready for accepting connections.
func assertPostgresIsUp(tb testing.TB, f *testlib.FlyctlTestEnv, appName string) {
tb.Helper()

ssh := f.FlyAllowExitFailure(`ssh console -a %s -u postgres -C "psql -p 5433 -h /run/postgresql -c 'SELECT 1'"`, appName)
assert.Equal(tb, 0, ssh.ExitCode(), "failed to connect to postgres at %s: %s", appName, ssh.StdErr())
}

func TestPostgres_ImportSuccess(t *testing.T) {
f := testlib.NewTestEnvFromEnv(t)

Expand All @@ -162,25 +188,16 @@ func TestPostgres_ImportSuccess(t *testing.T) {
secondAppName := f.CreateRandomAppName()

f.Fly(
"pg create --org %s --name %s --region %s --initial-cluster-size 1 --vm-size shared-cpu-1x --volume-size 1 --password x",
f.OrgSlug(), firstAppName, f.PrimaryRegion(),
"pg create --org %s --name %s --region %s --initial-cluster-size 1 --vm-size %s --volume-size 1 --password x",
f.OrgSlug(), firstAppName, f.PrimaryRegion(), postgresMachineSize,
)
f.Fly(
"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(),
"pg create --org %s --name %s --region %s --initial-cluster-size 1 --vm-size %s --volume-size 1",
f.OrgSlug(), secondAppName, f.PrimaryRegion(), postgresMachineSize,
)
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")
assert.EventuallyWithT(t, func(c *assert.CollectT) {
assertPostgresIsUp(t, f, firstAppName)
}, 1*time.Minute, 10*time.Second)

f.Fly(
"ssh console -a %s -u postgres -C \"psql -p 5433 -h /run/postgresql -c 'CREATE TABLE app_name (app_name TEXT)'\"",
Expand All @@ -192,8 +209,8 @@ func TestPostgres_ImportSuccess(t *testing.T) {
)

f.Fly(
"pg import -a %s --region %s --vm-size shared-cpu-1x postgres://postgres:x@%s.internal/postgres",
secondAppName, f.PrimaryRegion(), firstAppName,
"pg import -a %s --region %s --vm-size %s postgres://postgres:x@%s.internal/postgres",
secondAppName, f.PrimaryRegion(), postgresMachineSize, firstAppName,
)

result := f.Fly(
Expand All @@ -204,10 +221,9 @@ func TestPostgres_ImportSuccess(t *testing.T) {
require.Contains(f, output, firstAppName)

// Wait for the importer machine to be destroyed.
require.EventuallyWithT(t, func(c *assert.CollectT) {
ml := f.MachinesList(secondAppName)
require.Equal(c, 1, len(ml))
}, 10*time.Second, 1*time.Second, "import machine not destroyed")
assert.EventuallyWithT(t, func(c *assert.CollectT) {
assertMachineCount(t, f, secondAppName, 1)
}, 1*time.Minute, 10*time.Second, "import machine not destroyed")
}

func TestPostgres_ImportFailure(t *testing.T) {
Expand All @@ -222,20 +238,22 @@ func TestPostgres_ImportFailure(t *testing.T) {
appName := f.CreateRandomAppName()

f.Fly(
"pg create --org %s --name %s --region %s --initial-cluster-size 1 --vm-size shared-cpu-1x --volume-size 1 --password x",
f.OrgSlug(), appName, f.PrimaryRegion(),
"pg create --org %s --name %s --region %s --initial-cluster-size 1 --vm-size %s --volume-size 1 --password x",
f.OrgSlug(), appName, f.PrimaryRegion(), postgresMachineSize,
)
assert.EventuallyWithT(t, func(c *assert.CollectT) {
assertPostgresIsUp(t, f, appName)
}, 1*time.Minute, 10*time.Second)

result := f.FlyAllowExitFailure(
"pg import -a %s --region %s --vm-size shared-cpu-1x postgres://postgres:x@%s.internal/test",
appName, f.PrimaryRegion(), appName,
"pg import -a %s --region %s --vm-size %s postgres://postgres:x@%s.internal/test",
appName, f.PrimaryRegion(), postgresMachineSize, appName,
)
require.NotEqual(f, 0, result.ExitCode())
require.Contains(f, result.StdOut().String(), "database \"test\" does not exist")

// Wait for the importer machine to be destroyed.
require.EventuallyWithT(t, func(c *assert.CollectT) {
ml := f.MachinesList(appName)
assert.Equal(c, 1, len(ml))
}, 10*time.Second, 1*time.Second, "import machine not destroyed")
assert.EventuallyWithT(t, func(c *assert.CollectT) {
assertMachineCount(t, f, appName, 1)
}, 1*time.Minute, 10*time.Second, "import machine not destroyed")
}

0 comments on commit 885dd16

Please sign in to comment.