Merge branch '152-pre-release-issues' into 'master'

fix: pre-release Docker container issues (#152):
- resolve docker volume mappings and remove indirect dump container dirs
- use dynamic names for auxiliary containers to be able to run multiple database lab instances on the same machine
- automatic pull absent Docker images
- add container logs output on error

See merge request postgres-ai/database-lab!141

Author: agneum

pkg/retrieval/engine/postgres/initialize/logical/dump.go

@@ -36,8 +36,7 @@ const (
 	DumpJobType = "logical-dump"
 
 	// Defines dump options.
-	dumpContainerName = "retrieval_logical_dump"
-	dumpContainerDir  = "/tmp"
+	dumpContainerPrefix = "dblab_ld_"
 
 	// Defines dump source types.
 	sourceTypeLocal  = "local"
@@ -190,13 +189,17 @@ func (d *DumpJob) setupDumper() error {
 	return errors.Errorf("unknown source type given: %v", d.Source.Type)
 }
 
+func (d *DumpJob) dumpContainerName() string {
+	return dumpContainerPrefix + d.globalCfg.InstanceID
+}
+
 // Name returns a name of the job.
 func (d *DumpJob) Name() string {
 	return d.name
 }
 
 // Run starts the job.
-func (d *DumpJob) Run(ctx context.Context) error {
+func (d *DumpJob) Run(ctx context.Context) (err error) {
 	log.Msg(fmt.Sprintf("Run job: %s. Options: %v", d.Name(), d.DumpOptions))
 
 	isEmpty, err := tools.IsEmptyDirectory(d.globalCfg.DataDir)
@@ -212,6 +215,10 @@ func (d *DumpJob) Run(ctx context.Context) error {
 		log.Msg("The data directory is not empty. Existing data may be overwritten.")
 	}
 
+	if err := tools.PullImage(ctx, d.dockerClient, d.DockerImage); err != nil {
+		return errors.Wrap(err, "failed to scan pulling image response")
+	}
+
 	cont, err := d.dockerClient.ContainerCreate(ctx,
 		&container.Config{
 			Env:         d.getEnvironmentVariables(),
@@ -223,21 +230,27 @@ func (d *DumpJob) Run(ctx context.Context) error {
 			NetworkMode: d.getContainerNetworkMode(),
 		},
 		&network.NetworkingConfig{},
-		dumpContainerName,
+		d.dumpContainerName(),
 	)
 	if err != nil {
 		log.Err(err)
 
-		return errors.Wrap(err, "failed to create container")
+		return errors.Wrapf(err, "failed to create container %q", d.dumpContainerName())
 	}
 
 	defer tools.RemoveContainer(ctx, d.dockerClient, cont.ID, tools.StopTimeout)
 
+	defer func() {
+		if err != nil {
+			tools.PrintContainerLogs(ctx, d.dockerClient, d.dumpContainerName())
+		}
+	}()
+
 	if err := d.dockerClient.ContainerStart(ctx, cont.ID, types.ContainerStartOptions{}); err != nil {
-		return errors.Wrap(err, "failed to start container")
+		return errors.Wrapf(err, "failed to start container %q", d.dumpContainerName())
 	}
 
-	log.Msg(fmt.Sprintf("Running container: %s. ID: %v", dumpContainerName, cont.ID))
+	log.Msg(fmt.Sprintf("Running container: %s. ID: %v", d.dumpContainerName(), cont.ID))
 
 	if err := tools.CheckContainerReadiness(ctx, d.dockerClient, cont.ID); err != nil {
 		return errors.Wrap(err, "failed to readiness check")
@@ -258,7 +271,7 @@ func (d *DumpJob) Run(ctx context.Context) error {
 	})
 
 	if err != nil {
-		return errors.Wrap(err, "failed to create an exec command")
+		return errors.Wrap(err, "failed to create dump command")
 	}
 
 	if len(d.Partial.Tables) > 0 {
@@ -343,11 +356,12 @@ func (d *DumpJob) getDumpContainerPath() string {
 
 func (d *DumpJob) getEnvironmentVariables() []string {
 	envs := []string{
-		"PGDATA=" + pgDataContainerDir,
+		"PGDATA=" + d.globalCfg.DataDir,
 		"POSTGRES_HOST_AUTH_METHOD=trust",
 	}
 
 	if d.DumpOptions.Source.Type == sourceTypeLocal && d.DumpOptions.Source.Connection.Port == defaults.Port {
+		log.Msg(fmt.Sprintf("The default PostgreSQL port is busy, trying to use an alternative one: %d", reservePort))
 		envs = append(envs, "PGPORT="+strconv.Itoa(reservePort))
 	}
 
@@ -360,15 +374,15 @@ func (d *DumpJob) getMountVolumes() []mount.Mount {
 		{
 			Type:   mount.TypeBind,
 			Source: d.globalCfg.DataDir,
-			Target: pgDataContainerDir,
+			Target: d.globalCfg.DataDir,
 		},
 	}
 
 	if d.DumpOptions.DumpFile != "" {
 		mounts = append(mounts, mount.Mount{
 			Type:   mount.TypeBind,
 			Source: filepath.Dir(d.DumpOptions.DumpFile),
-			Target: dumpContainerDir,
+			Target: filepath.Dir(d.DumpOptions.DumpFile),
 		})
 	}
 

pkg/retrieval/engine/postgres/initialize/logical/restore.go

@@ -33,10 +33,8 @@ const (
 	RestoreJobType = "logical-restore"
 
 	// const defines restore options.
-	restoreContainerName = "retrieval_logical_restore"
-	pgDataContainerDir   = "/var/lib/postgresql/pgdata"
-	dumpContainerFile    = "/tmp/db.dump"
-	defaultParallelJobs  = 1
+	restoreContainerPrefix = "dblab_lr_"
+	defaultParallelJobs    = 1
 )
 
 // RestoreJob defines a logical restore job.
@@ -80,10 +78,6 @@ func NewJob(cfg config.JobConfig, docker *client.Client, globalCfg *dblabCfg.Glo
 
 	restoreJob.setDefaults()
 
-	if err := restoreJob.dbMarker.CreateConfig(); err != nil {
-		return nil, errors.Wrap(err, "failed to create a DBMarker config of the database")
-	}
-
 	return restoreJob, nil
 }
 
@@ -94,32 +88,41 @@ func (r *RestoreJob) setDefaults() {
 	}
 }
 
+func (r *RestoreJob) restoreContainerName() string {
+	return restoreContainerPrefix + r.globalCfg.InstanceID
+}
+
 // Name returns a name of the job.
 func (r *RestoreJob) Name() string {
 	return r.name
 }
 
 // Run starts the job.
-func (r *RestoreJob) Run(ctx context.Context) error {
+func (r *RestoreJob) Run(ctx context.Context) (err error) {
 	log.Msg(fmt.Sprintf("Run job: %s. Options: %v", r.Name(), r.RestoreOptions))
 
 	isEmpty, err := tools.IsEmptyDirectory(r.globalCfg.DataDir)
 	if err != nil {
-		return errors.Wrap(err, "failed to explore the data directory")
+		return errors.Wrapf(err, "failed to explore the data directory %q", r.globalCfg.DataDir)
 	}
 
 	if !isEmpty {
 		if !r.ForceInit {
-			return errors.New("the data directory is not empty. Use 'forceInit' or empty the data directory")
+			return errors.Errorf("the data directory %q is not empty. Use 'forceInit' or empty the data directory",
+				r.globalCfg.DataDir)
 		}
 
-		log.Msg("The data directory is not empty. Existing data may be overwritten.")
+		log.Msg(fmt.Sprintf("The data directory %q is not empty. Existing data may be overwritten.", r.globalCfg.DataDir))
+	}
+
+	if err := tools.PullImage(ctx, r.dockerClient, r.RestoreOptions.DockerImage); err != nil {
+		return errors.Wrap(err, "failed to scan image pulling response")
 	}
 
 	cont, err := r.dockerClient.ContainerCreate(ctx,
 		&container.Config{
 			Env: []string{
-				"PGDATA=" + pgDataContainerDir,
+				"PGDATA=" + r.globalCfg.DataDir,
 			},
 			Image:       r.RestoreOptions.DockerImage,
 			Healthcheck: health.GetConfig(),
@@ -129,29 +132,35 @@ func (r *RestoreJob) Run(ctx context.Context) error {
 				{
 					Type:   mount.TypeBind,
 					Source: r.RestoreOptions.DumpFile,
-					Target: dumpContainerFile,
+					Target: r.RestoreOptions.DumpFile,
 				},
 				{
 					Type:   mount.TypeBind,
 					Source: r.globalCfg.DataDir,
-					Target: pgDataContainerDir,
+					Target: r.globalCfg.DataDir,
 				},
 			},
 		},
 		&network.NetworkingConfig{},
-		restoreContainerName,
+		r.restoreContainerName(),
 	)
 	if err != nil {
-		return errors.Wrap(err, "failed to create container")
+		return errors.Wrapf(err, "failed to create container %q", r.restoreContainerName())
 	}
 
 	defer tools.RemoveContainer(ctx, r.dockerClient, cont.ID, tools.StopTimeout)
 
+	defer func() {
+		if err != nil {
+			tools.PrintContainerLogs(ctx, r.dockerClient, r.restoreContainerName())
+		}
+	}()
+
 	if err := r.dockerClient.ContainerStart(ctx, cont.ID, types.ContainerStartOptions{}); err != nil {
-		return errors.Wrap(err, "failed to start a container")
+		return errors.Wrapf(err, "failed to start container %q", r.restoreContainerName())
 	}
 
-	log.Msg(fmt.Sprintf("Running container: %s. ID: %v", restoreContainerName, cont.ID))
+	log.Msg(fmt.Sprintf("Running container: %s. ID: %v", r.restoreContainerName(), cont.ID))
 
 	if err := r.markDatabase(ctx, cont.ID); err != nil {
 		return errors.Wrap(err, "failed to mark the database")
@@ -172,19 +181,19 @@ func (r *RestoreJob) Run(ctx context.Context) error {
 	})
 
 	if err != nil {
-		return errors.Wrap(err, "failed to create an exec command")
+		return errors.Wrap(err, "failed to create restore command")
 	}
 
 	if len(r.Partial.Tables) > 0 {
 		log.Msg("Partial restore will be run. Tables for restoring: ", strings.Join(r.Partial.Tables, ", "))
 	}
 
 	if err := r.dockerClient.ContainerExecStart(ctx, execCommand.ID, types.ExecStartCheck{Tty: true}); err != nil {
-		return errors.Wrap(err, "failed to run the exec command")
+		return errors.Wrap(err, "failed to run restore command")
 	}
 
 	if err := tools.InspectCommandResponse(ctx, r.dockerClient, cont.ID, execCommand.ID); err != nil {
-		return errors.Wrap(err, "failed to exec the restore command")
+		return errors.Wrap(err, "failed to exec restore command")
 	}
 
 	if err := recalculateStats(ctx, r.dockerClient, cont.ID, buildAnalyzeCommand(Connection{
@@ -209,6 +218,10 @@ func (r *RestoreJob) markDatabase(ctx context.Context, contID string) error {
 		r.dbMark.DataStateAt = dataStateAt
 	}
 
+	if err := r.dbMarker.CreateConfig(); err != nil {
+		return errors.Wrap(err, "failed to create a DBMarker config of the database")
+	}
+
 	if err := r.dbMarker.SaveConfig(r.dbMark); err != nil {
 		return errors.Wrap(err, "failed to mark the database")
 	}
@@ -217,20 +230,22 @@ func (r *RestoreJob) markDatabase(ctx context.Context, contID string) error {
 }
 
 func (r *RestoreJob) retrieveDataStateAt(ctx context.Context, contID string) (string, error) {
-	restoreMetaCmd := []string{"sh", "-c", "pg_restore -l " + dumpContainerFile + " | head -n 10"}
+	restoreMetaCmd := []string{"sh", "-c", "pg_restore -l " + r.RestoreOptions.DumpFile + " | head -n 10"}
+
+	log.Dbg("Running a restore metadata command: ", restoreMetaCmd)
 
 	execCommand, err := r.dockerClient.ContainerExecCreate(ctx, contID, types.ExecConfig{
 		AttachStdout: true,
 		AttachStderr: true,
 		Cmd:          restoreMetaCmd,
 	})
 	if err != nil {
-		return "", errors.Wrap(err, "failed to create a restore meta command")
+		return "", errors.Wrap(err, "failed to create a restore metadata command")
 	}
 
 	execAttach, err := r.dockerClient.ContainerExecAttach(ctx, execCommand.ID, types.ExecStartCheck{})
 	if err != nil {
-		return "", errors.Wrap(err, "failed to exec a restore meta command")
+		return "", errors.Wrap(err, "failed to exec a restore metadata command")
 	}
 
 	defer execAttach.Close()
@@ -258,7 +273,7 @@ func (r *RestoreJob) buildLogicalRestoreCommand() []string {
 		restoreCmd = append(restoreCmd, "-t", table)
 	}
 
-	restoreCmd = append(restoreCmd, dumpContainerFile)
+	restoreCmd = append(restoreCmd, r.RestoreOptions.DumpFile)
 
 	return restoreCmd
 }

pkg/retrieval/engine/postgres/initialize/logical/restore_test.go

@@ -22,6 +22,7 @@ func TestRestoreCommandBuilding(t *testing.T) {
 				DBName:       "testDB",
 				ParallelJobs: 1,
 				ForceInit:    false,
+				DumpFile:     "/tmp/db.dump",
 			},
 			Command: []string{"pg_restore", "-U", "postgres", "-C", "-d", "testDB", "-j", "1", "/tmp/db.dump"},
 		},
@@ -30,13 +31,14 @@ func TestRestoreCommandBuilding(t *testing.T) {
 				ParallelJobs: 4,
 				ForceInit:    true,
 			},
-			Command: []string{"pg_restore", "-U", "postgres", "-C", "-d", "postgres", "--clean", "--if-exists", "-j", "4", "/tmp/db.dump"},
+			Command: []string{"pg_restore", "-U", "postgres", "-C", "-d", "postgres", "--clean", "--if-exists", "-j", "4", ""},
 		},
 		{
 			CopyOptions: RestoreOptions{
 				DBName:       "testDB",
 				ParallelJobs: 1,
 				Partial:      Partial{Tables: []string{"test", "users"}},
+				DumpFile:     "/tmp/db.dump",
 			},
 			Command: []string{"pg_restore", "-U", "postgres", "-C", "-d", "testDB", "-j", "1", "-t", "test", "-t", "users", "/tmp/db.dump"},
 		},