Merge pull request #12281 from vrothberg/fix-12007

fix remote checkpoint/restore

8 files changed, 221 insertions, 80 deletions

diff --git a/pkg/api/handlers/libpod/containers.go b/pkg/api/handlers/libpod/containers.go
index 3aeebc334..b0cec2b1f 100644
--- a/pkg/api/handlers/libpod/containers.go
+++ b/pkg/api/handlers/libpod/containers.go
@@ -1,9 +1,11 @@
 package libpod
 
 import (
+	"fmt"
 	"io/ioutil"
 	"net/http"
 	"os"
+	"strings"
 
 	"github.com/containers/podman/v3/libpod"
 	"github.com/containers/podman/v3/libpod/define"
@@ -206,7 +208,9 @@ func ShowMountedContainers(w http.ResponseWriter, r *http.Request) {
 }
 
 func Checkpoint(w http.ResponseWriter, r *http.Request) {
-	var targetFile string
+	runtime := r.Context().Value(api.RuntimeKey).(*libpod.Runtime)
+	containerEngine := abi.ContainerEngine{Libpod: runtime}
+
 	decoder := r.Context().Value(api.DecoderKey).(*schema.Decoder)
 	query := struct {
 		Keep           bool `schema:"keep"`
@@ -224,66 +228,68 @@ func Checkpoint(w http.ResponseWriter, r *http.Request) {
 			errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String()))
 		return
 	}
+
 	name := utils.GetName(r)
-	runtime := r.Context().Value(api.RuntimeKey).(*libpod.Runtime)
-	ctr, err := runtime.LookupContainer(name)
-	if err != nil {
+	if _, err := runtime.LookupContainer(name); err != nil {
 		utils.ContainerNotFound(w, name, err)
 		return
 	}
+	names := []string{name}
+
+	options := entities.CheckpointOptions{
+		Keep:           query.Keep,
+		LeaveRunning:   query.LeaveRunning,
+		TCPEstablished: query.TCPEstablished,
+		IgnoreRootFS:   query.IgnoreRootFS,
+		PrintStats:     query.PrintStats,
+	}
+
 	if query.Export {
-		tmpFile, err := ioutil.TempFile("", "checkpoint")
+		f, err := ioutil.TempFile("", "checkpoint")
 		if err != nil {
 			utils.InternalServerError(w, err)
 			return
 		}
-		defer os.Remove(tmpFile.Name())
-		if err := tmpFile.Close(); err != nil {
+		defer os.Remove(f.Name())
+		if err := f.Close(); err != nil {
 			utils.InternalServerError(w, err)
 			return
 		}
-		targetFile = tmpFile.Name()
-	}
-	options := libpod.ContainerCheckpointOptions{
-		Keep:           query.Keep,
-		KeepRunning:    query.LeaveRunning,
-		TCPEstablished: query.TCPEstablished,
-		IgnoreRootfs:   query.IgnoreRootFS,
-		PrintStats:     query.PrintStats,
-	}
-	if query.Export {
-		options.TargetFile = targetFile
+		options.Export = f.Name()
 	}
-	criuStatistics, runtimeCheckpointDuration, err := ctr.Checkpoint(r.Context(), options)
+
+	reports, err := containerEngine.ContainerCheckpoint(r.Context(), names, options)
 	if err != nil {
 		utils.InternalServerError(w, err)
 		return
 	}
-	if query.Export {
-		f, err := os.Open(targetFile)
-		if err != nil {
-			utils.InternalServerError(w, err)
+
+	if !query.Export {
+		if len(reports) != 1 {
+			utils.InternalServerError(w, fmt.Errorf("expected 1 restore report but got %d", len(reports)))
 			return
 		}
-		defer f.Close()
-		utils.WriteResponse(w, http.StatusOK, f)
+		if reports[0].Err != nil {
+			utils.InternalServerError(w, reports[0].Err)
+			return
+		}
+		utils.WriteResponse(w, http.StatusOK, reports[0])
+		return
+	}
+
+	f, err := os.Open(options.Export)
+	if err != nil {
+		utils.InternalServerError(w, err)
 		return
 	}
-	utils.WriteResponse(
-		w,
-		http.StatusOK,
-		entities.CheckpointReport{
-			Id:              ctr.ID(),
-			RuntimeDuration: runtimeCheckpointDuration,
-			CRIUStatistics:  criuStatistics,
-		},
-	)
+	defer f.Close()
+	utils.WriteResponse(w, http.StatusOK, f)
 }
 
 func Restore(w http.ResponseWriter, r *http.Request) {
-	var (
-		targetFile string
-	)
+	runtime := r.Context().Value(api.RuntimeKey).(*libpod.Runtime)
+	containerEngine := abi.ContainerEngine{Libpod: runtime}
+
 	decoder := r.Context().Value(api.DecoderKey).(*schema.Decoder)
 	query := struct {
 		Keep            bool   `schema:"keep"`
@@ -295,6 +301,7 @@ func Restore(w http.ResponseWriter, r *http.Request) {
 		IgnoreStaticIP  bool   `schema:"ignoreStaticIP"`
 		IgnoreStaticMAC bool   `schema:"ignoreStaticMAC"`
 		PrintStats      bool   `schema:"printStats"`
+		PublishPorts    string `schema:"publishPorts"`
 	}{
 		// override any golang type defaults
 	}
@@ -303,53 +310,55 @@ func Restore(w http.ResponseWriter, r *http.Request) {
 			errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String()))
 		return
 	}
-	name := utils.GetName(r)
-	runtime := r.Context().Value(api.RuntimeKey).(*libpod.Runtime)
-	ctr, err := runtime.LookupContainer(name)
-	if err != nil {
-		utils.ContainerNotFound(w, name, err)
-		return
+
+	options := entities.RestoreOptions{
+		Name:            query.Name,
+		Keep:            query.Keep,
+		TCPEstablished:  query.TCPEstablished,
+		IgnoreRootFS:    query.IgnoreRootFS,
+		IgnoreVolumes:   query.IgnoreVolumes,
+		IgnoreStaticIP:  query.IgnoreStaticIP,
+		IgnoreStaticMAC: query.IgnoreStaticMAC,
+		PrintStats:      query.PrintStats,
+		PublishPorts:    strings.Fields(query.PublishPorts),
 	}
+
+	var names []string
 	if query.Import {
 		t, err := ioutil.TempFile("", "restore")
 		if err != nil {
 			utils.InternalServerError(w, err)
 			return
 		}
-		defer t.Close()
+		defer os.Remove(t.Name())
 		if err := compat.SaveFromBody(t, r); err != nil {
 			utils.InternalServerError(w, err)
 			return
 		}
-		targetFile = t.Name()
+		options.Import = t.Name()
+	} else {
+		name := utils.GetName(r)
+		if _, err := runtime.LookupContainer(name); err != nil {
+			utils.ContainerNotFound(w, name, err)
+			return
+		}
+		names = []string{name}
 	}
 
-	options := libpod.ContainerCheckpointOptions{
-		Keep:            query.Keep,
-		TCPEstablished:  query.TCPEstablished,
-		IgnoreRootfs:    query.IgnoreRootFS,
-		IgnoreStaticIP:  query.IgnoreStaticIP,
-		IgnoreStaticMAC: query.IgnoreStaticMAC,
-		PrintStats:      query.PrintStats,
-	}
-	if query.Import {
-		options.TargetFile = targetFile
-		options.Name = query.Name
-	}
-	criuStatistics, runtimeRestoreDuration, err := ctr.Restore(r.Context(), options)
+	reports, err := containerEngine.ContainerRestore(r.Context(), names, options)
 	if err != nil {
 		utils.InternalServerError(w, err)
 		return
 	}
-	utils.WriteResponse(
-		w,
-		http.StatusOK,
-		entities.RestoreReport{
-			Id:              ctr.ID(),
-			RuntimeDuration: runtimeRestoreDuration,
-			CRIUStatistics:  criuStatistics,
-		},
-	)
+	if len(reports) != 1 {
+		utils.InternalServerError(w, fmt.Errorf("expected 1 restore report but got %d", len(reports)))
+		return
+	}
+	if reports[0].Err != nil {
+		utils.InternalServerError(w, reports[0].Err)
+		return
+	}
+	utils.WriteResponse(w, http.StatusOK, reports[0])
 }
 
 func InitContainer(w http.ResponseWriter, r *http.Request) {
diff --git a/pkg/bindings/containers/checkpoint.go b/pkg/bindings/containers/checkpoint.go
index 2ad2c6931..7b4ec093d 100644
--- a/pkg/bindings/containers/checkpoint.go
+++ b/pkg/bindings/containers/checkpoint.go
@@ -2,7 +2,9 @@ package containers
 
 import (
 	"context"
+	"io"
 	"net/http"
+	"os"
 
 	"github.com/containers/podman/v3/pkg/bindings"
 	"github.com/containers/podman/v3/pkg/domain/entities"
@@ -23,13 +25,34 @@ func Checkpoint(ctx context.Context, nameOrID string, options *CheckpointOptions
 	if err != nil {
 		return nil, err
 	}
+
+	// "export" is a bool for the server so override it in the parameters
+	// if set.
+	export := false
+	if options.Export != nil && *options.Export != "" {
+		export = true
+		params.Set("export", "true")
+	}
 	response, err := conn.DoRequest(ctx, nil, http.MethodPost, "/containers/%s/checkpoint", params, nil, nameOrID)
 	if err != nil {
 		return nil, err
 	}
 	defer response.Body.Close()
 
-	return &report, response.Process(&report)
+	if !export {
+		return &report, response.Process(&report)
+	}
+
+	f, err := os.OpenFile(*options.Export, os.O_RDWR|os.O_CREATE, 0600)
+	if err != nil {
+		return nil, err
+	}
+	defer f.Close()
+	if _, err := io.Copy(f, response.Body); err != nil {
+		return nil, err
+	}
+
+	return &entities.CheckpointReport{}, nil
 }
 
 // Restore restores a checkpointed container to running. The container is identified by the nameOrID option. All
@@ -47,12 +70,26 @@ func Restore(ctx context.Context, nameOrID string, options *RestoreOptions) (*en
 	if err != nil {
 		return nil, err
 	}
-	// The import key is a reserved golang term
-	params.Del("ImportArchive")
-	if i := options.GetImportAchive(); options.Changed("ImportArchive") {
-		params.Set("import", i)
+
+	for _, p := range options.PublishPorts {
+		params.Add("publishPorts", p)
+	}
+
+	params.Del("ImportArchive") // The import key is a reserved golang term
+
+	// Open the to-be-imported archive if needed.
+	var r io.Reader
+	if i := options.GetImportAchive(); i != "" {
+		params.Set("import", "true")
+		r, err = os.Open(i)
+		if err != nil {
+			return nil, err
+		}
+		// Hard-code the name since it will be ignored in any case.
+		nameOrID = "import"
 	}
-	response, err := conn.DoRequest(ctx, nil, http.MethodPost, "/containers/%s/restore", params, nil, nameOrID)
+
+	response, err := conn.DoRequest(ctx, r, http.MethodPost, "/containers/%s/restore", params, nil, nameOrID)
 	if err != nil {
 		return nil, err
 	}
diff --git a/pkg/bindings/containers/types.go b/pkg/bindings/containers/types.go
index 3a7d5a4c7..4bbb4a62b 100644
--- a/pkg/bindings/containers/types.go
+++ b/pkg/bindings/containers/types.go
@@ -50,12 +50,14 @@ type CheckpointOptions struct {
 	Keep           *bool
 	LeaveRunning   *bool
 	TCPEstablished *bool
+	PrintStats     *bool
 }
 
 //go:generate go run ../generator/generator.go RestoreOptions
 // RestoreOptions are optional options for restoring containers
 type RestoreOptions struct {
 	IgnoreRootfs    *bool
+	IgnoreVolumes   *bool
 	IgnoreStaticIP  *bool
 	IgnoreStaticMAC *bool
 	ImportAchive    *string
@@ -63,6 +65,8 @@ type RestoreOptions struct {
 	Name            *string
 	TCPEstablished  *bool
 	Pod             *string
+	PrintStats      *bool
+	PublishPorts    []string
 }
 
 //go:generate go run ../generator/generator.go CreateOptions
@@ -86,7 +90,8 @@ type ExecInspectOptions struct{}
 //go:generate go run ../generator/generator.go ExecStartOptions
 // ExecStartOptions are optional options for starting
 // exec sessions
-type ExecStartOptions struct{}
+type ExecStartOptions struct {
+}
 
 //go:generate go run ../generator/generator.go HealthCheckOptions
 // HealthCheckOptions are optional options for checking
diff --git a/pkg/bindings/containers/types_checkpoint_options.go b/pkg/bindings/containers/types_checkpoint_options.go
index 7b28c4045..b606922e0 100644
--- a/pkg/bindings/containers/types_checkpoint_options.go
+++ b/pkg/bindings/containers/types_checkpoint_options.go
@@ -91,3 +91,18 @@ func (o *CheckpointOptions) GetTCPEstablished() bool {
 	}
 	return *o.TCPEstablished
 }
+
+// WithPrintStats set field PrintStats to given value
+func (o *CheckpointOptions) WithPrintStats(value bool) *CheckpointOptions {
+	o.PrintStats = &value
+	return o
+}
+
+// GetPrintStats returns value of field PrintStats
+func (o *CheckpointOptions) GetPrintStats() bool {
+	if o.PrintStats == nil {
+		var z bool
+		return z
+	}
+	return *o.PrintStats
+}
diff --git a/pkg/bindings/containers/types_restore_options.go b/pkg/bindings/containers/types_restore_options.go
index 6eea108f4..8817b834b 100644
--- a/pkg/bindings/containers/types_restore_options.go
+++ b/pkg/bindings/containers/types_restore_options.go
@@ -32,6 +32,21 @@ func (o *RestoreOptions) GetIgnoreRootfs() bool {
 	return *o.IgnoreRootfs
 }
 
+// WithIgnoreVolumes set field IgnoreVolumes to given value
+func (o *RestoreOptions) WithIgnoreVolumes(value bool) *RestoreOptions {
+	o.IgnoreVolumes = &value
+	return o
+}
+
+// GetIgnoreVolumes returns value of field IgnoreVolumes
+func (o *RestoreOptions) GetIgnoreVolumes() bool {
+	if o.IgnoreVolumes == nil {
+		var z bool
+		return z
+	}
+	return *o.IgnoreVolumes
+}
+
 // WithIgnoreStaticIP set field IgnoreStaticIP to given value
 func (o *RestoreOptions) WithIgnoreStaticIP(value bool) *RestoreOptions {
 	o.IgnoreStaticIP = &value
@@ -136,3 +151,33 @@ func (o *RestoreOptions) GetPod() string {
 	}
 	return *o.Pod
 }
+
+// WithPrintStats set field PrintStats to given value
+func (o *RestoreOptions) WithPrintStats(value bool) *RestoreOptions {
+	o.PrintStats = &value
+	return o
+}
+
+// GetPrintStats returns value of field PrintStats
+func (o *RestoreOptions) GetPrintStats() bool {
+	if o.PrintStats == nil {
+		var z bool
+		return z
+	}
+	return *o.PrintStats
+}
+
+// WithPublishPorts set field PublishPorts to given value
+func (o *RestoreOptions) WithPublishPorts(value []string) *RestoreOptions {
+	o.PublishPorts = value
+	return o
+}
+
+// GetPublishPorts returns value of field PublishPorts
+func (o *RestoreOptions) GetPublishPorts() []string {
+	if o.PublishPorts == nil {
+		var z []string
+		return z
+	}
+	return o.PublishPorts
+}
diff --git a/pkg/checkpoint/checkpoint_restore.go b/pkg/checkpoint/checkpoint_restore.go
index da82c9745..3a300daaf 100644
--- a/pkg/checkpoint/checkpoint_restore.go
+++ b/pkg/checkpoint/checkpoint_restore.go
@@ -16,6 +16,7 @@ import (
 	"github.com/containers/podman/v3/pkg/domain/entities"
 	"github.com/containers/podman/v3/pkg/errorhandling"
 	"github.com/containers/podman/v3/pkg/specgen/generate"
+	"github.com/containers/podman/v3/pkg/specgenutil"
 	"github.com/containers/storage/pkg/archive"
 	spec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/pkg/errors"
@@ -195,7 +196,12 @@ func CRImportCheckpoint(ctx context.Context, runtime *libpod.Runtime, restoreOpt
 	}
 
 	if len(restoreOptions.PublishPorts) > 0 {
-		ports, err := generate.ParsePortMapping(restoreOptions.PublishPorts, nil)
+		pubPorts, err := specgenutil.CreatePortBindings(restoreOptions.PublishPorts)
+		if err != nil {
+			return nil, err
+		}
+
+		ports, err := generate.ParsePortMapping(pubPorts, nil)
 		if err != nil {
 			return nil, err
 		}
diff --git a/pkg/domain/entities/containers.go b/pkg/domain/entities/containers.go
index 8b7cd62d9..1a4019bb1 100644
--- a/pkg/domain/entities/containers.go
+++ b/pkg/domain/entities/containers.go
@@ -212,7 +212,7 @@ type RestoreOptions struct {
 	Name            string
 	TCPEstablished  bool
 	ImportPrevious  string
-	PublishPorts    []nettypes.PortMapping
+	PublishPorts    []string
 	Pod             string
 	PrintStats      bool
 }
diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go
index 5b5a1912c..152e3c302 100644
--- a/pkg/domain/infra/tunnel/containers.go
+++ b/pkg/domain/infra/tunnel/containers.go
@@ -302,6 +302,14 @@ func (ic *ContainerEngine) ContainerExport(ctx context.Context, nameOrID string,
 }
 
 func (ic *ContainerEngine) ContainerCheckpoint(ctx context.Context, namesOrIds []string, opts entities.CheckpointOptions) ([]*entities.CheckpointReport, error) {
+	options := new(containers.CheckpointOptions)
+	options.WithIgnoreRootfs(opts.IgnoreRootFS)
+	options.WithKeep(opts.Keep)
+	options.WithExport(opts.Export)
+	options.WithTCPEstablished(opts.TCPEstablished)
+	options.WithPrintStats(opts.PrintStats)
+	options.WithLeaveRunning(opts.LeaveRunning)
+
 	var (
 		err  error
 		ctrs = []entities.ListContainer{}
@@ -325,19 +333,36 @@ func (ic *ContainerEngine) ContainerCheckpoint(ctx context.Context, namesOrIds [
 		}
 	}
 	reports := make([]*entities.CheckpointReport, 0, len(ctrs))
-	options := new(containers.CheckpointOptions).WithExport(opts.Export).WithIgnoreRootfs(opts.IgnoreRootFS).WithKeep(opts.Keep)
-	options.WithLeaveRunning(opts.LeaveRunning).WithTCPEstablished(opts.TCPEstablished)
 	for _, c := range ctrs {
 		report, err := containers.Checkpoint(ic.ClientCtx, c.ID, options)
 		if err != nil {
 			reports = append(reports, &entities.CheckpointReport{Id: c.ID, Err: err})
+		} else {
+			reports = append(reports, report)
 		}
-		reports = append(reports, report)
 	}
 	return reports, nil
 }
 
 func (ic *ContainerEngine) ContainerRestore(ctx context.Context, namesOrIds []string, opts entities.RestoreOptions) ([]*entities.RestoreReport, error) {
+	options := new(containers.RestoreOptions)
+	options.WithIgnoreRootfs(opts.IgnoreRootFS)
+	options.WithIgnoreVolumes(opts.IgnoreVolumes)
+	options.WithIgnoreStaticIP(opts.IgnoreStaticIP)
+	options.WithIgnoreStaticMAC(opts.IgnoreStaticMAC)
+	options.WithKeep(opts.Keep)
+	options.WithName(opts.Name)
+	options.WithTCPEstablished(opts.TCPEstablished)
+	options.WithPod(opts.Pod)
+	options.WithPrintStats(opts.PrintStats)
+	options.WithPublishPorts(opts.PublishPorts)
+
+	if opts.Import != "" {
+		options.WithImportAchive(opts.Import)
+		report, err := containers.Restore(ic.ClientCtx, "", options)
+		return []*entities.RestoreReport{report}, err
+	}
+
 	var (
 		err  error
 		ctrs = []entities.ListContainer{}
@@ -360,7 +385,6 @@ func (ic *ContainerEngine) ContainerRestore(ctx context.Context, namesOrIds []st
 		}
 	}
 	reports := make([]*entities.RestoreReport, 0, len(ctrs))
-	options := new(containers.RestoreOptions)
 	for _, c := range ctrs {
 		report, err := containers.Restore(ic.ClientCtx, c.ID, options)
 		if err != nil {