Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: allow for postgres image to be specified #2158

Merged
merged 1 commit into from
Jul 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions backend/controller/pubsub/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,13 @@ func TestConsumptionDelay(t *testing.T) {
SELECT created_at, ROW_NUMBER() OVER (ORDER BY created_at) AS row_num
FROM (
select * from topic_events order by created_at
)
) AS sub_event_times
),
async_call_times AS (
SELECT created_at, ROW_NUMBER() OVER (ORDER BY created_at) AS row_num
FROM (
select * from async_calls ac order by created_at
)
) AS sub_async_calls
)
SELECT COUNT(*)
FROM event_times
Expand Down
2 changes: 1 addition & 1 deletion cmd/ftl/cmd_dev.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func (d *devCmd) Run(ctx context.Context, projConfig projectconfig.Config) error
}

if d.InitDB {
dsn, err := d.ServeCmd.setupDB(ctx)
dsn, err := d.ServeCmd.setupDB(ctx, d.ServeCmd.DatabaseImage)
if err != nil {
return fmt.Errorf("failed to setup database: %w", err)
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/ftl/cmd_schema_import.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ func query(ctx context.Context, prompt string) error {
func (s *schemaImportCmd) setup(ctx context.Context) error {
logger := log.FromContext(ctx)

exists, err := container.DoesExist(ctx, ollamaContainerName)
exists, err := container.DoesExist(ctx, ollamaContainerName, optional.None[string]())
if err != nil {
return err
}
Expand Down
9 changes: 5 additions & 4 deletions cmd/ftl/cmd_serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ type serveCmd struct {
Stop bool `help:"Stop the running FTL instance. Can be used with --background to restart the server" default:"false"`
StartupTimeout time.Duration `help:"Timeout for the server to start up." default:"1m"`
ObservabilityConfig observability.Config `embed:"" prefix:"o11y-"`
DatabaseImage string `help:"The container image to start for the database" default:"postgres:15.4" env:"FTL_DATABASE_IMAGE" hidden:""`
controller.CommonConfig
}

Expand Down Expand Up @@ -87,7 +88,7 @@ func (s *serveCmd) run(ctx context.Context, projConfig projectconfig.Config, ini
logger.Infof("Starting FTL with %d controller(s)", s.Controllers)

// Bring up the DB and DAL.
dsn, err := s.setupDB(ctx)
dsn, err := s.setupDB(ctx, s.DatabaseImage)
if err != nil {
return err
}
Expand Down Expand Up @@ -299,13 +300,13 @@ func isServeRunning(logger *log.Logger) (bool, error) {
return true, nil
}

func (s *serveCmd) setupDB(ctx context.Context) (string, error) {
func (s *serveCmd) setupDB(ctx context.Context, image string) (string, error) {
logger := log.FromContext(ctx)

recreate := s.Recreate
port := s.DBPort

exists, err := container.DoesExist(ctx, ftlContainerName)
exists, err := container.DoesExist(ctx, ftlContainerName, optional.Some(image))
if err != nil {
return "", err
}
Expand All @@ -320,7 +321,7 @@ func (s *serveCmd) setupDB(ctx context.Context) (string, error) {
return "", fmt.Errorf("failed to close listener: %w", err)
}

err = container.RunDB(ctx, ftlContainerName, s.DBPort)
err = container.RunDB(ctx, ftlContainerName, s.DBPort, image)
if err != nil {
return "", err
}
Expand Down
2 changes: 1 addition & 1 deletion docker-compose.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
services:
db:
image: postgres
image: postgres:15.4
command: postgres
user: postgres
# For local debugging
Expand Down
29 changes: 20 additions & 9 deletions internal/container/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@ var dockerClient = once.Once(func(ctx context.Context) (*client.Client, error) {
return client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation())
})

func DoesExist(ctx context.Context, name string) (bool, error) {
func DoesExist(ctx context.Context, name string, image optional.Option[string]) (bool, error) {
cli, err := dockerClient.Get(ctx)
logger := log.FromContext(ctx)
if err != nil {
return false, err
}
Expand All @@ -37,8 +38,20 @@ func DoesExist(ctx context.Context, name string) (bool, error) {
if err != nil {
return false, fmt.Errorf("failed to list containers: %w", err)
}

return len(containers) > 0, nil
if len(containers) == 0 {
return false, nil
}
imageName, ok := image.Get()
if !ok {
return true, nil
}
for _, c := range containers {
if c.Image != imageName {
logger.Infof("possible database version mismatch, expecting to use container image %s for container with name %s, bit it was already running with image %s", image, name, c.Image)
break
}
}
return true, nil
}

// Pull pulls the given image.
Expand Down Expand Up @@ -105,28 +118,26 @@ func Run(ctx context.Context, image, name string, hostPort, containerPort int, v
}

// RunDB runs a new detached postgres container with the given name and exposed port.
func RunDB(ctx context.Context, name string, port int) error {
func RunDB(ctx context.Context, name string, port int, image string) error {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We lean on Option[T] heavily in the codebase where semantically relevant, rather than implicit zero values, so let's replace this with optional.Option[string] to make it clear that it can be omitted.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that RunDB does actually need this, it is DoesExist where it is optional

cli, err := dockerClient.Get(ctx)
if err != nil {
return err
}

const containerName = "postgres"

exists, err := DoesExist(ctx, containerName)
exists, err := DoesExist(ctx, name, optional.Some(image))
if err != nil {
return err
}

if !exists {
err = Pull(ctx, "postgres:latest")
err = Pull(ctx, image)
if err != nil {
return err
}
}

config := container.Config{
Image: "postgres:latest",
Image: image,
Env: []string{"POSTGRES_PASSWORD=secret"},
User: "postgres",
Cmd: []string{"postgres"},
Expand Down
Loading