Merge pull request #12354 from Luap99/exit-command

Do not store the exit command in container config

5 files changed, 58 insertions, 71 deletions

diff --git a/pkg/api/handlers/compat/exec.go b/pkg/api/handlers/compat/exec.go
index ea61a1013..76f720bf2 100644
--- a/pkg/api/handlers/compat/exec.go
+++ b/pkg/api/handlers/compat/exec.go
@@ -12,7 +12,7 @@ import (
 	"github.com/containers/podman/v3/pkg/api/handlers/utils"
 	"github.com/containers/podman/v3/pkg/api/server/idle"
 	api "github.com/containers/podman/v3/pkg/api/types"
-	"github.com/containers/podman/v3/pkg/specgen/generate"
+	"github.com/containers/podman/v3/pkg/specgenutil"
 	"github.com/gorilla/mux"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -65,7 +65,7 @@ func ExecCreateHandler(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	// Automatically log to syslog if the server has log-level=debug set
-	exitCommandArgs, err := generate.CreateExitCommandArgs(storageConfig, runtimeConfig, logrus.IsLevelEnabled(logrus.DebugLevel), true, true)
+	exitCommandArgs, err := specgenutil.CreateExitCommandArgs(storageConfig, runtimeConfig, logrus.IsLevelEnabled(logrus.DebugLevel), true, true)
 	if err != nil {
 		utils.InternalServerError(w, err)
 		return
diff --git a/pkg/checkpoint/checkpoint_restore.go b/pkg/checkpoint/checkpoint_restore.go
index 3a300daaf..85fe6a77e 100644
--- a/pkg/checkpoint/checkpoint_restore.go
+++ b/pkg/checkpoint/checkpoint_restore.go
@@ -239,11 +239,6 @@ func CRImportCheckpoint(ctx context.Context, runtime *libpod.Runtime, restoreOpt
 		}
 	}
 
-	// Check if the ExitCommand points to the correct container ID
-	if containerConfig.ExitCommand[len(containerConfig.ExitCommand)-1] != containerConfig.ID {
-		return nil, errors.Errorf("'ExitCommandID' uses ID %s instead of container ID %s", containerConfig.ExitCommand[len(containerConfig.ExitCommand)-1], containerConfig.ID)
-	}
-
 	containers = append(containers, container)
 	return containers, nil
 }
diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go
index e04c7a38a..631eb3a43 100644
--- a/pkg/domain/infra/abi/containers.go
+++ b/pkg/domain/infra/abi/containers.go
@@ -29,6 +29,7 @@ import (
 	"github.com/containers/podman/v3/pkg/signal"
 	"github.com/containers/podman/v3/pkg/specgen"
 	"github.com/containers/podman/v3/pkg/specgen/generate"
+	"github.com/containers/podman/v3/pkg/specgenutil"
 	"github.com/containers/podman/v3/pkg/util"
 	"github.com/containers/storage"
 	"github.com/pkg/errors"
@@ -657,7 +658,7 @@ func makeExecConfig(options entities.ExecOptions, rt *libpod.Runtime) (*libpod.E
 		return nil, errors.Wrapf(err, "error retrieving Libpod configuration to build exec exit command")
 	}
 	// TODO: Add some ability to toggle syslog
-	exitCommandArgs, err := generate.CreateExitCommandArgs(storageConfig, runtimeConfig, false, false, true)
+	exitCommandArgs, err := specgenutil.CreateExitCommandArgs(storageConfig, runtimeConfig, logrus.IsLevelEnabled(logrus.DebugLevel), false, true)
 	if err != nil {
 		return nil, errors.Wrapf(err, "error constructing exit command for exec session")
 	}
diff --git a/pkg/specgen/generate/container_create.go b/pkg/specgen/generate/container_create.go
index f90fef9e8..df5d2e8ff 100644
--- a/pkg/specgen/generate/container_create.go
+++ b/pkg/specgen/generate/container_create.go
@@ -3,17 +3,14 @@ package generate
 import (
 	"context"
 	"fmt"
-	"os"
 	"path/filepath"
 	"strings"
 
 	cdi "github.com/container-orchestrated-devices/container-device-interface/pkg"
 	"github.com/containers/common/libimage"
-	"github.com/containers/common/pkg/config"
 	"github.com/containers/podman/v3/libpod"
 	"github.com/containers/podman/v3/pkg/specgen"
 	"github.com/containers/podman/v3/pkg/util"
-	"github.com/containers/storage/types"
 	spec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/opencontainers/selinux/go-selinux/label"
 	"github.com/pkg/errors"
@@ -163,15 +160,6 @@ func MakeContainer(ctx context.Context, rt *libpod.Runtime, s *specgen.SpecGener
 	}
 	options = append(options, opts...)
 
-	var exitCommandArgs []string
-
-	exitCommandArgs, err = CreateExitCommandArgs(rt.StorageConfig(), rtc, logrus.IsLevelEnabled(logrus.DebugLevel), s.Remove, false)
-	if err != nil {
-		return nil, nil, nil, err
-	}
-
-	options = append(options, libpod.WithExitCommand(exitCommandArgs))
-
 	if len(s.Aliases) > 0 {
 		options = append(options, libpod.WithNetworkAliases(s.Aliases))
 	}
@@ -500,54 +488,3 @@ func createContainerOptions(ctx context.Context, rt *libpod.Runtime, s *specgen.
 	}
 	return options, nil
 }
-
-func CreateExitCommandArgs(storageConfig types.StoreOptions, config *config.Config, syslog, rm, exec bool) ([]string, error) {
-	// We need a cleanup process for containers in the current model.
-	// But we can't assume that the caller is Podman - it could be another
-	// user of the API.
-	// As such, provide a way to specify a path to Podman, so we can
-	// still invoke a cleanup process.
-
-	podmanPath, err := os.Executable()
-	if err != nil {
-		return nil, err
-	}
-
-	command := []string{podmanPath,
-		"--root", storageConfig.GraphRoot,
-		"--runroot", storageConfig.RunRoot,
-		"--log-level", logrus.GetLevel().String(),
-		"--cgroup-manager", config.Engine.CgroupManager,
-		"--tmpdir", config.Engine.TmpDir,
-		"--cni-config-dir", config.Network.NetworkConfigDir,
-	}
-	if config.Engine.OCIRuntime != "" {
-		command = append(command, []string{"--runtime", config.Engine.OCIRuntime}...)
-	}
-	if storageConfig.GraphDriverName != "" {
-		command = append(command, []string{"--storage-driver", storageConfig.GraphDriverName}...)
-	}
-	for _, opt := range storageConfig.GraphDriverOptions {
-		command = append(command, []string{"--storage-opt", opt}...)
-	}
-	if config.Engine.EventsLogger != "" {
-		command = append(command, []string{"--events-backend", config.Engine.EventsLogger}...)
-	}
-
-	if syslog {
-		command = append(command, "--syslog")
-	}
-	command = append(command, []string{"container", "cleanup"}...)
-
-	if rm {
-		command = append(command, "--rm")
-	}
-
-	// This has to be absolutely last, to ensure that the exec session ID
-	// will be added after it by Libpod.
-	if exec {
-		command = append(command, "--exec")
-	}
-
-	return command, nil
-}
diff --git a/pkg/specgenutil/util.go b/pkg/specgenutil/util.go
index 15676d086..b47082b7f 100644
--- a/pkg/specgenutil/util.go
+++ b/pkg/specgenutil/util.go
@@ -3,10 +3,13 @@ package specgenutil
 import (
 	"io/ioutil"
 	"net"
+	"os"
 	"strconv"
 	"strings"
 
+	"github.com/containers/common/pkg/config"
 	"github.com/containers/podman/v3/libpod/network/types"
+	storageTypes "github.com/containers/storage/types"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
@@ -272,3 +275,54 @@ func parseAndValidatePort(port string) (uint16, error) {
 	}
 	return uint16(num), nil
 }
+
+func CreateExitCommandArgs(storageConfig storageTypes.StoreOptions, config *config.Config, syslog, rm, exec bool) ([]string, error) {
+	// We need a cleanup process for containers in the current model.
+	// But we can't assume that the caller is Podman - it could be another
+	// user of the API.
+	// As such, provide a way to specify a path to Podman, so we can
+	// still invoke a cleanup process.
+
+	podmanPath, err := os.Executable()
+	if err != nil {
+		return nil, err
+	}
+
+	command := []string{podmanPath,
+		"--root", storageConfig.GraphRoot,
+		"--runroot", storageConfig.RunRoot,
+		"--log-level", logrus.GetLevel().String(),
+		"--cgroup-manager", config.Engine.CgroupManager,
+		"--tmpdir", config.Engine.TmpDir,
+		"--cni-config-dir", config.Network.NetworkConfigDir,
+	}
+	if config.Engine.OCIRuntime != "" {
+		command = append(command, []string{"--runtime", config.Engine.OCIRuntime}...)
+	}
+	if storageConfig.GraphDriverName != "" {
+		command = append(command, []string{"--storage-driver", storageConfig.GraphDriverName}...)
+	}
+	for _, opt := range storageConfig.GraphDriverOptions {
+		command = append(command, []string{"--storage-opt", opt}...)
+	}
+	if config.Engine.EventsLogger != "" {
+		command = append(command, []string{"--events-backend", config.Engine.EventsLogger}...)
+	}
+
+	if syslog {
+		command = append(command, "--syslog")
+	}
+	command = append(command, []string{"container", "cleanup"}...)
+
+	if rm {
+		command = append(command, "--rm")
+	}
+
+	// This has to be absolutely last, to ensure that the exec session ID
+	// will be added after it by Libpod.
+	if exec {
+		command = append(command, "--exec")
+	}
+
+	return command, nil
+}