From 557b65e0925ed9f707da60776603a460a77803e9 Mon Sep 17 00:00:00 2001 From: Erik Sjölund Date: Mon, 11 Jul 2022 21:59:32 +0200 Subject: [CI:DOCS] Improve language. Fix spelling and typos. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Correct spelling and typos. * Improve language. Co-authored-by: Ed Santiago Signed-off-by: Erik Sjölund --- libpod/container_internal.go | 10 +++++----- libpod/container_internal_linux.go | 7 ++----- libpod/define/container_inspect.go | 6 +++--- libpod/define/errors.go | 2 +- libpod/define/volume_inspect.go | 2 +- libpod/events/nullout.go | 2 +- libpod/oci_conmon_linux.go | 4 ++-- libpod/runtime_volume_linux.go | 4 ++-- 8 files changed, 17 insertions(+), 20 deletions(-) (limited to 'libpod') diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 6a98466c2..caf7f3849 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -130,7 +130,7 @@ func (c *Container) bundlePath() string { return c.config.StaticDir } -// ControlSocketPath returns the path to the containers control socket for things like tty +// ControlSocketPath returns the path to the container's control socket for things like tty // resizing func (c *Container) ControlSocketPath() string { return filepath.Join(c.bundlePath(), "ctl") @@ -513,8 +513,8 @@ func (c *Container) setupStorage(ctx context.Context) error { return fmt.Errorf("error creating container storage: %w", containerInfoErr) } - // only reconfig IDMappings if layer was mounted from storage - // if its a external overlay do not reset IDmappings + // Only reconfig IDMappings if layer was mounted from storage. + // If it's an external overlay do not reset IDmappings. if !c.config.RootfsOverlay { c.config.IDMappings.UIDMap = containerInfo.UIDMap c.config.IDMappings.GIDMap = containerInfo.GIDMap @@ -794,7 +794,7 @@ func (c *Container) save() error { } // Checks the container is in the right state, then initializes the container in preparation to start the container. -// If recursive is true, each of the containers dependencies will be started. +// If recursive is true, each of the container's dependencies will be started. // Otherwise, this function will return with error if there are dependencies of this container that aren't running. func (c *Container) prepareToStart(ctx context.Context, recursive bool) (retErr error) { // Container must be created or stopped to be started @@ -1255,7 +1255,7 @@ func (c *Container) stop(timeout uint) error { // If the container is running in a PID Namespace, then killing the // primary pid is enough to kill the container. If it is not running in // a pid namespace then the OCI Runtime needs to kill ALL processes in - // the containers cgroup in order to make sure the container is stopped. + // the container's cgroup in order to make sure the container is stopped. all := !c.hasNamespace(spec.PIDNamespace) // We can't use --all if Cgroups aren't present. // Rootless containers with Cgroups v1 and NoCgroups are both cases diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index 390e95258..46e525add 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -184,11 +184,8 @@ func (c *Container) prepare() error { func (c *Container) isWorkDirSymlink(resolvedPath string) bool { // We cannot create workdir since explicit --workdir is // set in config but workdir could also be a symlink. - // If its a symlink lets check if resolved link is present - // on the container or not. - - // If we can resolve symlink and resolved link is present on the container - // then return nil cause its a valid use-case. + // If it's a symlink, check if the resolved target is present in the container. + // If so, that's a valid use case: return nil. maxSymLinks := 0 for { diff --git a/libpod/define/container_inspect.go b/libpod/define/container_inspect.go index ccc4ae00f..e6a34ba61 100644 --- a/libpod/define/container_inspect.go +++ b/libpod/define/container_inspect.go @@ -506,8 +506,8 @@ type InspectContainerHostConfig struct { // CpuRealtimeRuntime is the length of time (in microseconds) allocated // for realtime tasks within every CpuRealtimePeriod. CpuRealtimeRuntime int64 `json:"CpuRealtimeRuntime"` - // CpusetCpus is the is the set of CPUs that the container will execute - // on. Formatted as `0-3` or `0,2`. Default (if unset) is all CPUs. + // CpusetCpus is the set of CPUs that the container will execute on. + // Formatted as `0-3` or `0,2`. Default (if unset) is all CPUs. CpusetCpus string `json:"CpusetCpus"` // CpusetMems is the set of memory nodes the container will use. // Formatted as `0-3` or `0,2`. Default (if unset) is all memory nodes. @@ -544,7 +544,7 @@ type InspectContainerHostConfig struct { OomKillDisable bool `json:"OomKillDisable"` // Init indicates whether the container has an init mounted into it. Init bool `json:"Init,omitempty"` - // PidsLimit is the maximum number of PIDs what may be created within + // PidsLimit is the maximum number of PIDs that may be created within // the container. 0, the default, indicates no limit. PidsLimit int64 `json:"PidsLimit"` // Ulimits is a set of ulimits that will be set within the container. diff --git a/libpod/define/errors.go b/libpod/define/errors.go index b858e1989..fd27e89de 100644 --- a/libpod/define/errors.go +++ b/libpod/define/errors.go @@ -205,7 +205,7 @@ var ( // Useful for potentially long running tasks. ErrCanceled = errors.New("cancelled by user") - // ErrConmonVersionFormat is used when the expected versio-format of conmon + // ErrConmonVersionFormat is used when the expected version format of conmon // has changed. ErrConmonVersionFormat = "conmon version changed format" ) diff --git a/libpod/define/volume_inspect.go b/libpod/define/volume_inspect.go index f731a8735..9279812da 100644 --- a/libpod/define/volume_inspect.go +++ b/libpod/define/volume_inspect.go @@ -45,7 +45,7 @@ type InspectVolumeData struct { // GID is the GID that the volume was created with. GID int `json:"GID,omitempty"` // Anonymous indicates that the volume was created as an anonymous - // volume for a specific container, and will be be removed when any + // volume for a specific container, and will be removed when any // container using it is removed. Anonymous bool `json:"Anonymous,omitempty"` // MountCount is the number of times this volume has been mounted. diff --git a/libpod/events/nullout.go b/libpod/events/nullout.go index 3eca9e8db..587a1b98b 100644 --- a/libpod/events/nullout.go +++ b/libpod/events/nullout.go @@ -19,7 +19,7 @@ func (e EventToNull) Read(ctx context.Context, options ReadOptions) error { } // NewNullEventer returns a new null eventer. You should only do this for -// the purposes on internal libpod testing. +// the purposes of internal libpod testing. func NewNullEventer() Eventer { return EventToNull{} } diff --git a/libpod/oci_conmon_linux.go b/libpod/oci_conmon_linux.go index 121e750f4..abc579519 100644 --- a/libpod/oci_conmon_linux.go +++ b/libpod/oci_conmon_linux.go @@ -1388,7 +1388,7 @@ func (r *ConmonOCIRuntime) sharedConmonArgs(ctr *Container, cuuid, bundlePath, p } func startCommand(cmd *exec.Cmd, ctr *Container) error { - // Make sure to unset the NOTIFY_SOCKET and reset if afterwards if needed. + // Make sure to unset the NOTIFY_SOCKET and reset it afterwards if needed. switch ctr.config.SdNotifyMode { case define.SdNotifyModeContainer, define.SdNotifyModeIgnore: if ctr.notifySocket != "" { @@ -1667,7 +1667,7 @@ func httpAttachNonTerminalCopy(container *net.UnixConn, http *bufio.ReadWriter, // multiplexing by Conmon). headerLen := uint32(numR - 1) // Practically speaking, we could make this buf[0] - 1, - // but we need to validate it anyways... + // but we need to validate it anyway. switch buf[0] { case AttachPipeStdin: headerBuf = makeHTTPAttachHeader(0, headerLen) diff --git a/libpod/runtime_volume_linux.go b/libpod/runtime_volume_linux.go index a751d75d2..d2fe9f2fb 100644 --- a/libpod/runtime_volume_linux.go +++ b/libpod/runtime_volume_linux.go @@ -31,7 +31,7 @@ func (r *Runtime) NewVolume(ctx context.Context, options ...VolumeCreateOption) // newVolume creates a new empty volume with the given options. // The createPluginVolume can be set to true to make it not create the volume in the volume plugin, -// this is required for the UpdateVolumePlugins() function. If you are not sure set this to false. +// this is required for the UpdateVolumePlugins() function. If you are not sure, set this to false. func (r *Runtime) newVolume(noCreatePluginVolume bool, options ...VolumeCreateOption) (_ *Volume, deferredErr error) { volume := newVolume(r) for _, option := range options { @@ -216,7 +216,7 @@ func (r *Runtime) UpdateVolumePlugins(ctx context.Context) *define.VolumeReload for _, vol := range libpodVolumes { if vol.UsesVolumeDriver() { if _, ok := allPluginVolumes[vol.Name()]; !ok { - // The volume is no longer in the plugin, lets remove it from the libpod db. + // The volume is no longer in the plugin. Let's remove it from the libpod db. if err := r.removeVolume(ctx, vol, false, nil, true); err != nil { if errors.Is(err, define.ErrVolumeBeingUsed) { // Volume is still used by at least one container. This is very bad, -- cgit v1.2.3-54-g00ecf From cfb768a36d91a68c0e8fbbdf2b6f2ee175485e88 Mon Sep 17 00:00:00 2001 From: Mikhail Khachayants Date: Tue, 12 Jul 2022 10:37:46 +0300 Subject: fix wrong log message on Trace level [NO NEW TESTS NEEDED] Empty path to runtime binary was printed instead of a real path. Before fix: TRAC[0000] found runtime "" TRAC[0000] found runtime "" After: TRAC[0000] found runtime "/usr/bin/crun" TRAC[0000] found runtime "/usr/bin/runc" Signed-off-by: Mikhail Khachayants --- libpod/oci_conmon_linux.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'libpod') diff --git a/libpod/oci_conmon_linux.go b/libpod/oci_conmon_linux.go index abc579519..cb76de72c 100644 --- a/libpod/oci_conmon_linux.go +++ b/libpod/oci_conmon_linux.go @@ -133,7 +133,7 @@ func newConmonOCIRuntime(name string, paths []string, conmonPath string, runtime continue } foundPath = true - logrus.Tracef("found runtime %q", runtime.path) + logrus.Tracef("found runtime %q", path) runtime.path = path break } -- cgit v1.2.3-54-g00ecf From 183fdea5f595886c799f804d3e7483d3bb0139c8 Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Wed, 6 Jul 2022 07:23:36 -0400 Subject: Use SafeChown rather then chown for volumes on NFS NFS Servers will thrown ENOTSUPP error if you attempt to chown a directory to the same UID and GID as the directory already has. If volumes are stored on NFS directories this throws an ugly error and then works on the next try. Bottom line don't chown directories that already have the correct UID and GID. Fixes: https://github.com/containers/podman/issues/14766 [NO NEW TESTS NEEDED] Difficult to setup an NFS Server in testing. Signed-off-by: Daniel J Walsh --- libpod/runtime_volume_linux.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'libpod') diff --git a/libpod/runtime_volume_linux.go b/libpod/runtime_volume_linux.go index d2fe9f2fb..1f354e41b 100644 --- a/libpod/runtime_volume_linux.go +++ b/libpod/runtime_volume_linux.go @@ -16,6 +16,7 @@ import ( "github.com/containers/podman/v4/libpod/events" volplugin "github.com/containers/podman/v4/libpod/plugin" "github.com/containers/storage/drivers/quota" + "github.com/containers/storage/pkg/idtools" "github.com/containers/storage/pkg/stringid" pluginapi "github.com/docker/go-plugins-helpers/volume" "github.com/sirupsen/logrus" @@ -101,14 +102,14 @@ func (r *Runtime) newVolume(noCreatePluginVolume bool, options ...VolumeCreateOp if err := os.MkdirAll(volPathRoot, 0700); err != nil { return nil, fmt.Errorf("creating volume directory %q: %w", volPathRoot, err) } - if err := os.Chown(volPathRoot, volume.config.UID, volume.config.GID); err != nil { + if err := idtools.SafeChown(volPathRoot, volume.config.UID, volume.config.GID); err != nil { return nil, fmt.Errorf("chowning volume directory %q to %d:%d: %w", volPathRoot, volume.config.UID, volume.config.GID, err) } fullVolPath := filepath.Join(volPathRoot, "_data") if err := os.MkdirAll(fullVolPath, 0755); err != nil { return nil, fmt.Errorf("creating volume directory %q: %w", fullVolPath, err) } - if err := os.Chown(fullVolPath, volume.config.UID, volume.config.GID); err != nil { + if err := idtools.SafeChown(fullVolPath, volume.config.UID, volume.config.GID); err != nil { return nil, fmt.Errorf("chowning volume directory %q to %d:%d: %w", fullVolPath, volume.config.UID, volume.config.GID, err) } if err := LabelVolumePath(fullVolPath); err != nil { -- cgit v1.2.3-54-g00ecf From c3c07ed09eb9e5998f2c67f1054850a5b8266aae Mon Sep 17 00:00:00 2001 From: Urvashi Mohnani Date: Tue, 19 Jul 2022 11:58:27 -0400 Subject: Update init ctr default for play kube Update the init container type default to once instead of always to match k8s behavior. Add a new annotation that can be used to change the init ctr type in the kube yaml. Signed-off-by: Urvashi Mohnani --- docs/source/markdown/podman-play-kube.1.md | 2 +- libpod/define/annotations.go | 5 +++++ pkg/domain/infra/abi/play.go | 6 +++++- test/e2e/play_kube_test.go | 29 +++++++++++++++++++++++++++-- 4 files changed, 38 insertions(+), 4 deletions(-) (limited to 'libpod') diff --git a/docs/source/markdown/podman-play-kube.1.md b/docs/source/markdown/podman-play-kube.1.md index 47e438bd6..66341d875 100644 --- a/docs/source/markdown/podman-play-kube.1.md +++ b/docs/source/markdown/podman-play-kube.1.md @@ -22,7 +22,7 @@ Currently, the supported Kubernetes kinds are: Only two volume types are supported by play kube, the *hostPath* and *persistentVolumeClaim* volume types. For the *hostPath* volume type, only the *default (empty)*, *DirectoryOrCreate*, *Directory*, *FileOrCreate*, *File*, *Socket*, *CharDevice* and *BlockDevice* subtypes are supported. Podman interprets the value of *hostPath* *path* as a file path when it contains at least one forward slash, otherwise Podman treats the value as the name of a named volume. When using a *persistentVolumeClaim*, the value for *claimName* is the name for the Podman named volume. -Note: When playing a kube YAML with init containers, the init container will be created with init type value `always`. +Note: When playing a kube YAML with init containers, the init container will be created with init type value `once`. To change the default type, use the `io.podman.annotations.init.container.type` annotation to set the type to `always`. Note: *hostPath* volume types created by play kube will be given an SELinux shared label (z), bind mounts are not relabeled (use `chcon -t container_file_t -R `). diff --git a/libpod/define/annotations.go b/libpod/define/annotations.go index 8f5279981..580286d6a 100644 --- a/libpod/define/annotations.go +++ b/libpod/define/annotations.go @@ -135,6 +135,11 @@ const ( // creating a checkpoint image to specify the name of host distribution on // which the checkpoint was created. CheckpointAnnotationDistributionName = "io.podman.annotations.checkpoint.distribution.name" + + // InitContainerType is used by play kube when playing a kube yaml to specify the type + // of the init container. + InitContainerType = "io.podman.annotations.init.container.type" + // MaxKubeAnnotation is the max length of annotations allowed by Kubernetes. MaxKubeAnnotation = 63 ) diff --git a/pkg/domain/infra/abi/play.go b/pkg/domain/infra/abi/play.go index c913fdb69..8b47eff53 100644 --- a/pkg/domain/infra/abi/play.go +++ b/pkg/domain/infra/abi/play.go @@ -507,13 +507,17 @@ func (ic *ContainerEngine) playKubePod(ctx context.Context, podName string, podY for k, v := range podSpec.PodSpecGen.Labels { // add podYAML labels labels[k] = v } + initCtrType := annotations[define.InitContainerType] + if initCtrType == "" { + initCtrType = define.OneShotInitContainer + } specgenOpts := kube.CtrSpecGenOptions{ Annotations: annotations, ConfigMaps: configMaps, Container: initCtr, Image: pulledImage, - InitContainerType: define.AlwaysInitContainer, + InitContainerType: initCtrType, Labels: labels, LogDriver: options.LogDriver, LogOptions: options.LogOptions, diff --git a/test/e2e/play_kube_test.go b/test/e2e/play_kube_test.go index 457aaebb2..9e2d4de19 100644 --- a/test/e2e/play_kube_test.go +++ b/test/e2e/play_kube_test.go @@ -1559,8 +1559,10 @@ var _ = Describe("Podman play kube", func() { }) // If you have an init container in the pod yaml, podman should create and run the init container with play kube - It("podman play kube test with init containers", func() { - pod := getPod(withPodInitCtr(getCtr(withImage(ALPINE), withCmd([]string{"echo", "hello"}), withInitCtr(), withName("init-test"))), withCtr(getCtr(withImage(ALPINE), withCmd([]string{"top"})))) + // With annotation set to always + It("podman play kube test with init containers and annotation set", func() { + // With the init container type annotation set to always + pod := getPod(withAnnotation("io.podman.annotations.init.container.type", "always"), withPodInitCtr(getCtr(withImage(ALPINE), withCmd([]string{"echo", "hello"}), withInitCtr(), withName("init-test"))), withCtr(getCtr(withImage(ALPINE), withCmd([]string{"top"})))) err := generateKubeYaml("pod", pod, kubeYaml) Expect(err).To(BeNil()) @@ -1585,6 +1587,29 @@ var _ = Describe("Podman play kube", func() { Expect(inspect.OutputToString()).To(ContainSubstring("running")) }) + // If you have an init container in the pod yaml, podman should create and run the init container with play kube + // Using default init container type (once) + It("podman play kube test with init container type set to default value", func() { + // Using the default init container type (once) + pod := getPod(withPodInitCtr(getCtr(withImage(ALPINE), withCmd([]string{"echo", "hello"}), withInitCtr(), withName("init-test"))), withCtr(getCtr(withImage(ALPINE), withCmd([]string{"top"})))) + err := generateKubeYaml("pod", pod, kubeYaml) + Expect(err).To(BeNil()) + + kube := podmanTest.Podman([]string{"play", "kube", kubeYaml}) + kube.WaitWithDefaultTimeout() + Expect(kube).Should(Exit(0)) + + // Expect the number of containers created to be 2, infra and regular container + numOfCtrs := podmanTest.NumberOfContainers() + Expect(numOfCtrs).To(Equal(2)) + + // Regular container should be in running state + inspect := podmanTest.Podman([]string{"inspect", "--format", "{{.State.Status}}", "testPod-" + defaultCtrName}) + inspect.WaitWithDefaultTimeout() + Expect(inspect).Should(Exit(0)) + Expect(inspect.OutputToString()).To(ContainSubstring("running")) + }) + // If you supply only args for a Container, the default Entrypoint defined in the Docker image is run with the args that you supplied. It("podman play kube test correct command with only set args in yaml file", func() { pod := getPod(withCtr(getCtr(withImage(REGISTRY_IMAGE), withCmd(nil), withArg([]string{"echo", "hello"})))) -- cgit v1.2.3-54-g00ecf From 73ecc5a4bdd620124650dbdb621df667beb8438f Mon Sep 17 00:00:00 2001 From: Aditya R Date: Tue, 12 Jul 2022 18:34:44 +0530 Subject: pkg,libpod: remove pkg/hooks and use hooks from c/common PR https://github.com/containers/common/pull/1071 moved `pkg/hooks` to `c/common` hence remove that from podman and use `pkg/hooks` from `c/common` [NO NEW TESTS NEEDED] [NO TESTS NEEDED] Signed-off-by: Aditya R --- libpod/container_internal.go | 4 +- pkg/hooks/0.1.0/hook.go | 88 -------- pkg/hooks/0.1.0/hook_test.go | 182 ---------------- pkg/hooks/1.0.0/hook.go | 89 -------- pkg/hooks/1.0.0/hook_test.go | 214 ------------------- pkg/hooks/1.0.0/when.go | 96 --------- pkg/hooks/1.0.0/when_test.go | 329 ----------------------------- pkg/hooks/README.md | 22 -- pkg/hooks/docs/.gitignore | 1 - pkg/hooks/docs/oci-hooks.5.md | 182 ---------------- pkg/hooks/exec/exec.go | 69 ------ pkg/hooks/exec/exec_test.go | 222 ------------------- pkg/hooks/exec/runtimeconfigfilter.go | 72 ------- pkg/hooks/exec/runtimeconfigfilter_test.go | 265 ----------------------- pkg/hooks/hooks.go | 145 ------------- pkg/hooks/hooks_test.go | 218 ------------------- pkg/hooks/monitor.go | 66 ------ pkg/hooks/monitor_test.go | 324 ---------------------------- pkg/hooks/read.go | 101 --------- pkg/hooks/read_test.go | 194 ----------------- pkg/hooks/version.go | 6 - 21 files changed, 2 insertions(+), 2887 deletions(-) delete mode 100644 pkg/hooks/0.1.0/hook.go delete mode 100644 pkg/hooks/0.1.0/hook_test.go delete mode 100644 pkg/hooks/1.0.0/hook.go delete mode 100644 pkg/hooks/1.0.0/hook_test.go delete mode 100644 pkg/hooks/1.0.0/when.go delete mode 100644 pkg/hooks/1.0.0/when_test.go delete mode 100644 pkg/hooks/README.md delete mode 100644 pkg/hooks/docs/.gitignore delete mode 100644 pkg/hooks/docs/oci-hooks.5.md delete mode 100644 pkg/hooks/exec/exec.go delete mode 100644 pkg/hooks/exec/exec_test.go delete mode 100644 pkg/hooks/exec/runtimeconfigfilter.go delete mode 100644 pkg/hooks/exec/runtimeconfigfilter_test.go delete mode 100644 pkg/hooks/hooks.go delete mode 100644 pkg/hooks/hooks_test.go delete mode 100644 pkg/hooks/monitor.go delete mode 100644 pkg/hooks/monitor_test.go delete mode 100644 pkg/hooks/read.go delete mode 100644 pkg/hooks/read_test.go delete mode 100644 pkg/hooks/version.go (limited to 'libpod') diff --git a/libpod/container_internal.go b/libpod/container_internal.go index caf7f3849..7e330430c 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -22,12 +22,12 @@ import ( "github.com/containers/common/pkg/cgroups" "github.com/containers/common/pkg/chown" "github.com/containers/common/pkg/config" + "github.com/containers/common/pkg/hooks" + "github.com/containers/common/pkg/hooks/exec" cutil "github.com/containers/common/pkg/util" "github.com/containers/podman/v4/libpod/define" "github.com/containers/podman/v4/libpod/events" "github.com/containers/podman/v4/pkg/ctime" - "github.com/containers/podman/v4/pkg/hooks" - "github.com/containers/podman/v4/pkg/hooks/exec" "github.com/containers/podman/v4/pkg/lookup" "github.com/containers/podman/v4/pkg/rootless" "github.com/containers/podman/v4/pkg/selinux" diff --git a/pkg/hooks/0.1.0/hook.go b/pkg/hooks/0.1.0/hook.go deleted file mode 100644 index c3df5fa1d..000000000 --- a/pkg/hooks/0.1.0/hook.go +++ /dev/null @@ -1,88 +0,0 @@ -// Package hook is the 0.1.0 hook configuration structure. -package hook - -import ( - "encoding/json" - "errors" - "strings" - - current "github.com/containers/podman/v4/pkg/hooks/1.0.0" - rspec "github.com/opencontainers/runtime-spec/specs-go" -) - -// Version is the hook configuration version defined in this package. -const Version = "0.1.0" - -// Hook is the hook configuration structure. -type Hook struct { - Hook *string `json:"hook"` - Arguments []string `json:"arguments,omitempty"` - - // https://github.com/cri-o/cri-o/pull/1235 - Stages []string `json:"stages"` - Stage []string `json:"stage"` - - Cmds []string `json:"cmds,omitempty"` - Cmd []string `json:"cmd,omitempty"` - - Annotations []string `json:"annotations,omitempty"` - Annotation []string `json:"annotation,omitempty"` - - HasBindMounts *bool `json:"hasbindmounts,omitempty"` -} - -func Read(content []byte) (hook *current.Hook, err error) { - var raw Hook - - if err = json.Unmarshal(content, &raw); err != nil { - return nil, err - } - - if raw.Hook == nil { - return nil, errors.New("missing required property: hook") - } - - if raw.Stages == nil { - raw.Stages = raw.Stage - } else if raw.Stage != nil { - return nil, errors.New("cannot set both 'stage' and 'stages'") - } - if raw.Stages == nil { - return nil, errors.New("missing required property: stages") - } - - if raw.Cmds == nil { - raw.Cmds = raw.Cmd - } else if raw.Cmd != nil { - return nil, errors.New("cannot set both 'cmd' and 'cmds'") - } - - if raw.Annotations == nil { - raw.Annotations = raw.Annotation - } else if raw.Annotation != nil { - return nil, errors.New("cannot set both 'annotation' and 'annotations'") - } - - hook = ¤t.Hook{ - Version: current.Version, - Hook: rspec.Hook{ - Path: *raw.Hook, - }, - When: current.When{ - Commands: raw.Cmds, - HasBindMounts: raw.HasBindMounts, - Or: true, - }, - Stages: raw.Stages, - } - if raw.Arguments != nil { - hook.Hook.Args = append([]string{*raw.Hook}, raw.Arguments...) - } - if raw.Annotations != nil { - hook.When.Annotations = map[string]string{ - ".*": strings.Join(raw.Annotations, "|"), - } - } - - return hook, nil -} diff --git a/pkg/hooks/0.1.0/hook_test.go b/pkg/hooks/0.1.0/hook_test.go deleted file mode 100644 index 5beadbaaf..000000000 --- a/pkg/hooks/0.1.0/hook_test.go +++ /dev/null @@ -1,182 +0,0 @@ -package hook - -import ( - "testing" - - current "github.com/containers/podman/v4/pkg/hooks/1.0.0" - rspec "github.com/opencontainers/runtime-spec/specs-go" - "github.com/stretchr/testify/assert" -) - -func TestGood(t *testing.T) { - hook, err := Read([]byte("{\"hook\": \"/a/b/c\", \"stages\": [\"prestart\"], \"cmds\": [\"sh\"]}")) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, ¤t.Hook{ - Version: current.Version, - Hook: rspec.Hook{ - Path: "/a/b/c", - }, - When: current.When{ - Commands: []string{"sh"}, - Or: true, - }, - Stages: []string{"prestart"}, - }, hook) -} - -func TestInvalidJSON(t *testing.T) { - _, err := Read([]byte("{")) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^unexpected end of JSON input$", err.Error()) -} - -func TestArguments(t *testing.T) { - hook, err := Read([]byte("{\"hook\": \"/a/b/c\", \"arguments\": [\"d\", \"e\"], \"stages\": [\"prestart\"], \"cmds\": [\"sh\"]}")) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, ¤t.Hook{ - Version: current.Version, - Hook: rspec.Hook{ - Path: "/a/b/c", - Args: []string{"/a/b/c", "d", "e"}, - }, - When: current.When{ - Commands: []string{"sh"}, - Or: true, - }, - Stages: []string{"prestart"}, - }, hook) -} - -func TestEmptyObject(t *testing.T) { - _, err := Read([]byte("{}")) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^missing required property: hook$", err.Error()) -} - -func TestNoStages(t *testing.T) { - _, err := Read([]byte("{\"hook\": \"/a/b/c\"}")) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^missing required property: stages$", err.Error()) -} - -func TestStage(t *testing.T) { - hook, err := Read([]byte("{\"hook\": \"/a/b/c\", \"stage\": [\"prestart\"]}")) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, ¤t.Hook{ - Version: current.Version, - Hook: rspec.Hook{ - Path: "/a/b/c", - }, - When: current.When{Or: true}, - Stages: []string{"prestart"}, - }, hook) -} - -func TestStagesAndStage(t *testing.T) { - _, err := Read([]byte("{\"hook\": \"/a/b/c\", \"stages\": [\"prestart\"], \"stage\": [\"prestart\"]}")) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^cannot set both 'stage' and 'stages'$", err.Error()) -} - -func TestCmd(t *testing.T) { - hook, err := Read([]byte("{\"hook\": \"/a/b/c\", \"stage\": [\"prestart\"], \"cmd\": [\"sh\"]}")) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, ¤t.Hook{ - Version: current.Version, - Hook: rspec.Hook{ - Path: "/a/b/c", - }, - When: current.When{ - Commands: []string{"sh"}, - Or: true, - }, - Stages: []string{"prestart"}, - }, hook) -} - -func TestCmdsAndCmd(t *testing.T) { - _, err := Read([]byte("{\"hook\": \"/a/b/c\", \"stages\": [\"prestart\"], \"cmds\": [\"sh\"], \"cmd\": [\"true\"]}")) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^cannot set both 'cmd' and 'cmds'$", err.Error()) -} - -func TestAnnotations(t *testing.T) { - hook, err := Read([]byte("{\"hook\": \"/a/b/c\", \"stage\": [\"prestart\"], \"annotations\": [\"a\", \"b\"]}")) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, ¤t.Hook{ - Version: current.Version, - Hook: rspec.Hook{ - Path: "/a/b/c", - }, - When: current.When{ - Annotations: map[string]string{".*": "a|b"}, - Or: true, - }, - Stages: []string{"prestart"}, - }, hook) -} - -func TestAnnotation(t *testing.T) { - hook, err := Read([]byte("{\"hook\": \"/a/b/c\", \"stage\": [\"prestart\"], \"annotation\": [\"a\", \"b\"]}")) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, ¤t.Hook{ - Version: current.Version, - Hook: rspec.Hook{ - Path: "/a/b/c", - }, - When: current.When{ - Annotations: map[string]string{".*": "a|b"}, - Or: true, - }, - Stages: []string{"prestart"}, - }, hook) -} - -func TestAnnotationsAndAnnotation(t *testing.T) { - _, err := Read([]byte("{\"hook\": \"/a/b/c\", \"stages\": [\"prestart\"], \"annotations\": [\"a\"], \"annotation\": [\"b\"]}")) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^cannot set both 'annotation' and 'annotations'$", err.Error()) -} - -func TestHasBindMounts(t *testing.T) { - hook, err := Read([]byte("{\"hook\": \"/a/b/c\", \"stage\": [\"prestart\"], \"hasbindmounts\": true}")) - if err != nil { - t.Fatal(err) - } - hasBindMounts := true - assert.Equal(t, ¤t.Hook{ - Version: current.Version, - Hook: rspec.Hook{ - Path: "/a/b/c", - }, - When: current.When{ - HasBindMounts: &hasBindMounts, - Or: true, - }, - Stages: []string{"prestart"}, - }, hook) -} diff --git a/pkg/hooks/1.0.0/hook.go b/pkg/hooks/1.0.0/hook.go deleted file mode 100644 index 71f940a64..000000000 --- a/pkg/hooks/1.0.0/hook.go +++ /dev/null @@ -1,89 +0,0 @@ -// Package hook is the 1.0.0 hook configuration structure. -package hook - -import ( - "encoding/json" - "errors" - "fmt" - "os" - "regexp" - - rspec "github.com/opencontainers/runtime-spec/specs-go" -) - -// Version is the hook configuration version defined in this package. -const Version = "1.0.0" - -// Hook is the hook configuration structure. -type Hook struct { - Version string `json:"version"` - Hook rspec.Hook `json:"hook"` - When When `json:"when"` - Stages []string `json:"stages"` -} - -// Read reads hook JSON bytes, verifies them, and returns the hook configuration. -func Read(content []byte) (hook *Hook, err error) { - if err = json.Unmarshal(content, &hook); err != nil { - return nil, err - } - return hook, nil -} - -// Validate performs load-time hook validation. -func (hook *Hook) Validate(extensionStages []string) (err error) { - if hook == nil { - return errors.New("nil hook") - } - - if hook.Version != Version { - return fmt.Errorf("unexpected hook version %q (expecting %v)", hook.Version, Version) - } - - if hook.Hook.Path == "" { - return errors.New("missing required property: hook.path") - } - - if _, err := os.Stat(hook.Hook.Path); err != nil { - return err - } - - for key, value := range hook.When.Annotations { - if _, err = regexp.Compile(key); err != nil { - return fmt.Errorf("invalid annotation key %q: %w", key, err) - } - if _, err = regexp.Compile(value); err != nil { - return fmt.Errorf("invalid annotation value %q: %w", value, err) - } - } - - for _, command := range hook.When.Commands { - if _, err = regexp.Compile(command); err != nil { - return fmt.Errorf("invalid command %q: %w", command, err) - } - } - - if hook.Stages == nil { - return errors.New("missing required property: stages") - } - - validStages := map[string]bool{ - "createContainer": true, - "createRuntime": true, - "prestart": true, - "poststart": true, - "poststop": true, - "startContainer": true, - } - for _, stage := range extensionStages { - validStages[stage] = true - } - - for _, stage := range hook.Stages { - if !validStages[stage] { - return fmt.Errorf("unknown stage %q", stage) - } - } - - return nil -} diff --git a/pkg/hooks/1.0.0/hook_test.go b/pkg/hooks/1.0.0/hook_test.go deleted file mode 100644 index bd6d6b654..000000000 --- a/pkg/hooks/1.0.0/hook_test.go +++ /dev/null @@ -1,214 +0,0 @@ -package hook - -import ( - "os" - "path/filepath" - "runtime" - "testing" - - rspec "github.com/opencontainers/runtime-spec/specs-go" - "github.com/stretchr/testify/assert" -) - -// path is the path to an example hook executable. -var path string - -func TestGoodRead(t *testing.T) { - hook, err := Read([]byte("{\"version\": \"1.0.0\", \"hook\": {\"path\": \"/a/b/c\"}, \"when\": {\"always\": true}, \"stages\": [\"prestart\"]}")) - if err != nil { - t.Fatal(err) - } - always := true - assert.Equal(t, &Hook{ - Version: Version, - Hook: rspec.Hook{ - Path: "/a/b/c", - }, - When: When{ - Always: &always, - }, - Stages: []string{"prestart"}, - }, hook) -} - -func TestInvalidJSON(t *testing.T) { - _, err := Read([]byte("{")) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^unexpected end of JSON input$", err.Error()) -} - -func TestGoodValidate(t *testing.T) { - always := true - hook := &Hook{ - Version: Version, - Hook: rspec.Hook{ - Path: path, - }, - When: When{ - Always: &always, - }, - Stages: []string{"prestart"}, - } - err := hook.Validate([]string{}) - if err != nil { - t.Fatal(err) - } -} - -func TestNilValidation(t *testing.T) { - var hook *Hook - err := hook.Validate([]string{}) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^nil hook$", err.Error()) -} - -func TestWrongVersion(t *testing.T) { - hook := Hook{Version: "0.1.0"} - err := hook.Validate([]string{}) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^unexpected hook version \"0.1.0\" \\(expecting 1.0.0\\)$", err.Error()) -} - -func TestNoHookPath(t *testing.T) { - hook := Hook{ - Version: "1.0.0", - Hook: rspec.Hook{}, - } - err := hook.Validate([]string{}) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^missing required property: hook.path$", err.Error()) -} - -func TestUnknownHookPath(t *testing.T) { - hook := Hook{ - Version: "1.0.0", - Hook: rspec.Hook{ - Path: filepath.Join("does", "not", "exist"), - }, - } - err := hook.Validate([]string{}) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^stat does/not/exist: no such file or directory$", err.Error()) - if !os.IsNotExist(err) { - t.Fatal("opaque wrapping for not-exist errors") - } -} - -func TestNoStages(t *testing.T) { - hook := Hook{ - Version: "1.0.0", - Hook: rspec.Hook{ - Path: path, - }, - } - err := hook.Validate([]string{}) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^missing required property: stages$", err.Error()) -} - -func TestInvalidStage(t *testing.T) { - hook := Hook{ - Version: "1.0.0", - Hook: rspec.Hook{ - Path: path, - }, - Stages: []string{"does-not-exist"}, - } - err := hook.Validate([]string{}) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^unknown stage \"does-not-exist\"$", err.Error()) -} - -func TestExtensionStage(t *testing.T) { - hook := Hook{ - Version: "1.0.0", - Hook: rspec.Hook{ - Path: path, - }, - Stages: []string{"prestart", "b"}, - } - err := hook.Validate([]string{"a", "b", "c"}) - if err != nil { - t.Fatal(err) - } -} - -func TestInvalidAnnotationKey(t *testing.T) { - hook := Hook{ - Version: "1.0.0", - Hook: rspec.Hook{ - Path: path, - }, - When: When{ - Annotations: map[string]string{ - "[": "a", - }, - }, - Stages: []string{"prestart"}, - } - err := hook.Validate([]string{}) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^invalid annotation key \"\\[\": error parsing regexp: .*", err.Error()) -} - -func TestInvalidAnnotationValue(t *testing.T) { - hook := Hook{ - Version: "1.0.0", - Hook: rspec.Hook{ - Path: path, - }, - When: When{ - Annotations: map[string]string{ - "a": "[", - }, - }, - Stages: []string{"prestart"}, - } - err := hook.Validate([]string{}) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^invalid annotation value \"\\[\": error parsing regexp: .*", err.Error()) -} - -func TestInvalidCommand(t *testing.T) { - hook := Hook{ - Version: "1.0.0", - Hook: rspec.Hook{ - Path: path, - }, - When: When{ - Commands: []string{"["}, - }, - Stages: []string{"prestart"}, - } - err := hook.Validate([]string{}) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^invalid command \"\\[\": error parsing regexp: .*", err.Error()) -} - -func init() { - if runtime.GOOS != "windows" { - path = "/bin/sh" - } else { - panic("we need a reliable executable path on Windows") - } -} diff --git a/pkg/hooks/1.0.0/when.go b/pkg/hooks/1.0.0/when.go deleted file mode 100644 index a1351890f..000000000 --- a/pkg/hooks/1.0.0/when.go +++ /dev/null @@ -1,96 +0,0 @@ -package hook - -import ( - "errors" - "fmt" - "regexp" - - rspec "github.com/opencontainers/runtime-spec/specs-go" -) - -// When holds hook-injection conditions. -type When struct { - Always *bool `json:"always,omitempty"` - Annotations map[string]string `json:"annotations,omitempty"` - Commands []string `json:"commands,omitempty"` - HasBindMounts *bool `json:"hasBindMounts,omitempty"` - - // Or enables any-of matching. - // - // Deprecated: this property is for is backwards-compatibility with - // 0.1.0 hooks. It will be removed when we drop support for them. - Or bool `json:"-"` -} - -// Match returns true if the given conditions match the configuration. -func (when *When) Match(config *rspec.Spec, annotations map[string]string, hasBindMounts bool) (match bool, err error) { - matches := 0 - - if when.Always != nil { - if *when.Always { - if when.Or { - return true, nil - } - matches++ - } else if !when.Or { - return false, nil - } - } - - if when.HasBindMounts != nil { - if *when.HasBindMounts && hasBindMounts { - if when.Or { - return true, nil - } - matches++ - } else if !when.Or { - return false, nil - } - } - - for keyPattern, valuePattern := range when.Annotations { - match := false - for key, value := range annotations { - match, err = regexp.MatchString(keyPattern, key) - if err != nil { - return false, fmt.Errorf("annotation key: %w", err) - } - if match { - match, err = regexp.MatchString(valuePattern, value) - if err != nil { - return false, fmt.Errorf("annotation value: %w", err) - } - if match { - break - } - } - } - if match { - if when.Or { - return true, nil - } - matches++ - } else if !when.Or { - return false, nil - } - } - - if config.Process != nil && len(when.Commands) > 0 { - if len(config.Process.Args) == 0 { - return false, errors.New("process.args must have at least one entry") - } - command := config.Process.Args[0] - for _, cmdPattern := range when.Commands { - match, err := regexp.MatchString(cmdPattern, command) - if err != nil { - return false, fmt.Errorf("command: %w", err) - } - if match { - return true, nil - } - } - return false, nil - } - - return matches > 0, nil -} diff --git a/pkg/hooks/1.0.0/when_test.go b/pkg/hooks/1.0.0/when_test.go deleted file mode 100644 index 94b0c3830..000000000 --- a/pkg/hooks/1.0.0/when_test.go +++ /dev/null @@ -1,329 +0,0 @@ -package hook - -import ( - "fmt" - "testing" - - rspec "github.com/opencontainers/runtime-spec/specs-go" - "github.com/stretchr/testify/assert" -) - -func TestNoMatch(t *testing.T) { - config := &rspec.Spec{} - for _, o := range []bool{true, false} { - or := o - t.Run(fmt.Sprintf("or %t", or), func(t *testing.T) { - when := When{Or: or} - match, err := when.Match(config, map[string]string{}, false) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, false, match) - }) - } -} - -func TestAlways(t *testing.T) { - config := &rspec.Spec{} - processStruct := &rspec.Process{ - Args: []string{"/bin/sh", "a", "b"}, - } - for _, a := range []bool{true, false} { - always := a - for _, o := range []bool{true, false} { - or := o - for _, p := range []*rspec.Process{processStruct, nil} { - process := p - t.Run(fmt.Sprintf("always %t, or %t, has process %t", always, or, process != nil), func(t *testing.T) { - config.Process = process - when := When{Always: &always, Or: or} - match, err := when.Match(config, map[string]string{}, false) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, always, match) - }) - } - } - } -} - -func TestHasBindMountsAnd(t *testing.T) { - hasBindMounts := true - when := When{HasBindMounts: &hasBindMounts} - config := &rspec.Spec{} - for _, b := range []bool{false, true} { - containerHasBindMounts := b - t.Run(fmt.Sprintf("%t", containerHasBindMounts), func(t *testing.T) { - match, err := when.Match(config, map[string]string{}, containerHasBindMounts) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, containerHasBindMounts, match) - }) - } -} - -func TestHasBindMountsOr(t *testing.T) { - hasBindMounts := true - when := When{HasBindMounts: &hasBindMounts, Or: true} - config := &rspec.Spec{} - for _, b := range []bool{false, true} { - containerHasBindMounts := b - t.Run(fmt.Sprintf("%t", containerHasBindMounts), func(t *testing.T) { - match, err := when.Match(config, map[string]string{}, containerHasBindMounts) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, containerHasBindMounts, match) - }) - } -} - -func TestAnnotations(t *testing.T) { - when := When{ - Annotations: map[string]string{ - "^a$": "^b$", - "^c$": "^d$", - }, - } - config := &rspec.Spec{} - for _, tt := range []struct { - name string - annotations map[string]string - or bool - match bool - }{ - { - name: "matching both, and", - annotations: map[string]string{ - "a": "b", - "c": "d", - "e": "f", - }, - or: false, - match: true, - }, - { - name: "matching one, and", - annotations: map[string]string{ - "a": "b", - }, - or: false, - match: false, - }, - { - name: "matching one, or", - annotations: map[string]string{ - "a": "b", - }, - or: true, - match: true, - }, - { - name: "key-only, or", - annotations: map[string]string{ - "a": "bc", - }, - or: true, - match: false, - }, - { - name: "value-only, or", - annotations: map[string]string{ - "ac": "b", - }, - or: true, - match: false, - }, - } { - test := tt - t.Run(test.name, func(t *testing.T) { - when.Or = test.or - match, err := when.Match(config, test.annotations, false) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, test.match, match) - }) - } -} - -func TestCommands(t *testing.T) { - when := When{ - Commands: []string{ - "^/bin/sh$", - }, - } - config := &rspec.Spec{} - for _, tt := range []struct { - name string - process *rspec.Process - match bool - }{ - { - name: "good", - process: &rspec.Process{ - Args: []string{"/bin/sh", "a", "b"}, - }, - match: true, - }, - { - name: "extra characters", - process: &rspec.Process{ - Args: []string{"/bin/shell", "a", "b"}, - }, - match: false, - }, - { - name: "process unset", - match: false, - }, - } { - test := tt - t.Run(test.name, func(t *testing.T) { - config.Process = test.process - match, err := when.Match(config, map[string]string{}, false) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, test.match, match) - }) - } -} - -func TestCommandsEmptyProcessArgs(t *testing.T) { - when := When{ - Commands: []string{ - "^/bin/sh$", - }, - } - config := &rspec.Spec{ - Process: &rspec.Process{}, - } - _, err := when.Match(config, map[string]string{}, false) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^process\\.args must have at least one entry$", err.Error()) -} - -func TestHasBindMountsAndCommands(t *testing.T) { - hasBindMounts := true - when := When{ - HasBindMounts: &hasBindMounts, - Commands: []string{ - "^/bin/sh$", - }, - } - config := &rspec.Spec{Process: &rspec.Process{}} - for _, tt := range []struct { - name string - command string - hasBindMounts bool - or bool - match bool - }{ - { - name: "both, and", - command: "/bin/sh", - hasBindMounts: true, - or: false, - match: true, - }, - { - name: "both, or", - command: "/bin/sh", - hasBindMounts: true, - or: true, - match: true, - }, - { - name: "bind, and", - command: "/bin/shell", - hasBindMounts: true, - or: false, - match: false, - }, - { - name: "bind, or", - command: "/bin/shell", - hasBindMounts: true, - or: true, - match: true, - }, - { - name: "command, and", - command: "/bin/sh", - hasBindMounts: false, - or: false, - match: false, - }, - { - name: "command, or", - command: "/bin/sh", - hasBindMounts: false, - or: true, - match: true, - }, - { - name: "neither, and", - command: "/bin/shell", - hasBindMounts: false, - or: false, - match: false, - }, - { - name: "neither, or", - command: "/bin/shell", - hasBindMounts: false, - or: true, - match: false, - }, - } { - test := tt - t.Run(test.name, func(t *testing.T) { - config.Process.Args = []string{test.command} - when.Or = test.or - match, err := when.Match(config, map[string]string{}, test.hasBindMounts) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, test.match, match) - }) - } -} - -func TestInvalidRegexp(t *testing.T) { - config := &rspec.Spec{Process: &rspec.Process{Args: []string{"/bin/sh"}}} - for _, tt := range []struct { - name string - when When - expected string - }{ - { - name: "invalid-annotation-key", - when: When{Annotations: map[string]string{"[": "a"}}, - expected: "^annotation key: error parsing regexp: .*", - }, - { - name: "invalid-annotation-value", - when: When{Annotations: map[string]string{"a": "["}}, - expected: "^annotation value: error parsing regexp: .*", - }, - { - name: "invalid-command", - when: When{Commands: []string{"["}}, - expected: "^command: error parsing regexp: .*", - }, - } { - test := tt - t.Run(test.name, func(t *testing.T) { - _, err := test.when.Match(config, map[string]string{"a": "b"}, false) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, test.expected, err.Error()) - }) - } -} diff --git a/pkg/hooks/README.md b/pkg/hooks/README.md deleted file mode 100644 index f6a03a775..000000000 --- a/pkg/hooks/README.md +++ /dev/null @@ -1,22 +0,0 @@ -# OCI Hooks Configuration - -For POSIX platforms, the [OCI runtime configuration][runtime-spec] supports [hooks][spec-hooks] for configuring custom actions related to the life cycle of the container. -The way you enable the hooks above is by editing the OCI runtime configuration before running the OCI runtime (e.g. [`runc`][runc]). -CRI-O and `podman create` create the OCI configuration for you, and this documentation allows developers to configure them to set their intended hooks. - -One problem with hooks is that the runtime actually stalls execution of the container before running the hooks and stalls completion of the container, until all hooks complete. -This can cause some performance issues. -Also a lot of hooks just check if certain configuration is set and then exit early, without doing anything. -For example the [oci-systemd-hook][] only executes if the command is `init` or `systemd`, otherwise it just exits. -This means if we automatically enabled all hooks, every container would have to execute `oci-systemd-hook`, even if they don't run systemd inside of the container. -Performance would also suffer if we executed each hook at each stage ([pre-start][], [post-start][], and [post-stop][]). - -The hooks configuration is documented in [`oci-hooks.5`](docs/oci-hooks.5.md). - -[oci-systemd-hook]: https://github.com/projectatomic/oci-systemd-hook -[post-start]: https://github.com/opencontainers/runtime-spec/blob/v1.0.1/config.md#poststart -[post-stop]: https://github.com/opencontainers/runtime-spec/blob/v1.0.1/config.md#poststop -[pre-start]: https://github.com/opencontainers/runtime-spec/blob/v1.0.1/config.md#prestart -[runc]: https://github.com/opencontainers/runc -[runtime-spec]: https://github.com/opencontainers/runtime-spec/blob/v1.0.1/spec.md -[spec-hooks]: https://github.com/opencontainers/runtime-spec/blob/v1.0.1/config.md#posix-platform-hooks diff --git a/pkg/hooks/docs/.gitignore b/pkg/hooks/docs/.gitignore deleted file mode 100644 index 5d06cd466..000000000 --- a/pkg/hooks/docs/.gitignore +++ /dev/null @@ -1 +0,0 @@ -*.5 diff --git a/pkg/hooks/docs/oci-hooks.5.md b/pkg/hooks/docs/oci-hooks.5.md deleted file mode 100644 index 9a1a35682..000000000 --- a/pkg/hooks/docs/oci-hooks.5.md +++ /dev/null @@ -1,182 +0,0 @@ -% oci-hooks 5 OCI Hooks Configuration -% W. Trevor King -% MAY 2018 - -# NAME - -oci-hooks - OCI hooks configuration directories - -# SYNOPSIS - -`/usr/share/containers/oci/hooks.d/*.json` - -# DESCRIPTION - -Provides a way for users to configure the intended hooks for Open Container Initiative containers so they will only be executed for containers that need their functionality, and then only for the stages where they're needed. - -## Directories - -Hooks are configured with JSON files (ending with a `.json` extension) in a series of hook directories. -The default directory is `/usr/share/containers/oci/hooks.d`, but tools consuming this format may change that default, include additional directories, or provide their callers with ways to adjust the configuration directories. - -If multiple directories are configured, a JSON filename in a preferred directory masks entries with the same filename in directories with lower precedence. For example, if a consuming tool watches for hooks in `/etc/containers/oci/hooks.d` and `/usr/share/containers/oci/hooks.d` (in order of decreasing precedence), then a hook definition in `/etc/containers/oci/hooks.d/01-my-hook.json` will mask any definition in `/usr/share/containers/oci/hooks.d/01-my-hook.json`. - -Tools consuming this format may also opt to monitor the hook directories for changes, in which case they will notice additions, changes, and removals to JSON files without needing to be restarted or otherwise signaled. When the tool monitors multiple hooks directories, the precedence discussed in the previous paragraph still applies. For example, if a consuming tool watches for hooks in `/etc/containers/oci/hooks.d` and `/usr/share/containers/oci/hooks.d` (in order of decreasing precedence), then writing a new hook definition to `/etc/containers/oci/hooks.d/01-my-hook.json` will mask the hook previously loaded from `/usr/share/containers/oci/hooks.d/01-my-hook.json`. Subsequent changes to `/usr/share/containers/oci/hooks.d/01-my-hook.json` will have no effect on the consuming tool as long as `/etc/containers/oci/hooks.d/01-my-hook.json` exists. Removing `/etc/containers/oci/hooks.d/01-my-hook.json` will reload the hook from `/usr/share/containers/oci/hooks.d/01-my-hook.json`. - -Hooks are injected in the order obtained by sorting the JSON file names, after converting them to lower case, based on their Unicode code points. -For example, a matching hook defined in `01-my-hook.json` would be injected before matching hooks defined in `02-another-hook.json` and `01-UPPERCASE.json`. -It is strongly recommended to make the sort order unambiguous depending on an ASCII-only prefix (like the `01`/`02` above). - -Each JSON file should contain an object with one of the following schemas. - -## 1.0.0 Hook Schema - -`version` (required string) - Sets the hook-definition version. For this schema version, the value be `1.0.0`. - -`hook` (required object) - The hook to inject, with the hook-entry schema defined by the 1.0.1 OCI Runtime Specification. - -`when` (required object) - Conditions under which the hook is injected. The following properties can be specified, and at least one must be specified: - - * `always` (optional boolean) - If set `true`, this condition matches. - * `annotations` (optional object) - If all `annotations` key/value pairs match a key/value pair from the configured annotations, this condition matches. - Both keys and values must be POSIX extended regular expressions. - * `commands` (optional array of strings) - If the configured `process.args[0]` matches an entry, this condition matches. - Entries must be POSIX extended regular expressions. - * `hasBindMounts` (optional boolean) - If `hasBindMounts` is true and the caller requested host-to-container bind mounts, this condition matches. - -`stages` (required array of strings) - Stages when the hook must be injected. Entries must be chosen from the 1.0.1 OCI Runtime Specification hook stages or from extension stages supported by the package consumer. - -If *all* of the conditions set in `when` match, then the `hook` must be injected for the stages set in `stages`. - -## 0.1.0 Hook Schema - -`hook` (required string) - Sets `path` in the injected hook. - -`arguments` (optional array of strings) - Additional arguments to pass to the hook. The injected hook's `args` is `hook` with `arguments` appended. - -`stages` (required array of strings) - Stages when the hook must be injected. `stage` is an allowed synonym for this property, but you must not set both `stages` and `stage`. Entries must be chosen from the 1.0.1 OCI Runtime Specification hook stages or from extension stages supported by the package consumer. - -`cmds` (optional array of strings) - The hook must be injected if the configured `process.args[0]` matches an entry. `cmd` is an allowed synonym for this property, but you must not set both `cmds` and `cmd`. Entries must be POSIX extended regular expressions. - -`annotations` (optional array of strings) - The hook must be injected if an `annotations` entry matches a value from the configured annotations. `annotation` is an allowed synonym for this property, but you must not set both `annotations` and `annotation`. Entries must be POSIX extended regular expressions. - -`hasbindmounts` (optional boolean) - The hook must be injected if `hasBindMounts` is true and the caller requested host-to-container bind mounts. - -# EXAMPLE - -## 1.0.0 Hook Schema - -The following configuration injects `oci-systemd-hook` in the pre-start and post-stop stages if `process.args[0]` ends with `/init` or `/systemd`: - -```console -$ cat /etc/containers/oci/hooks.d/oci-systemd-hook.json -{ - "version": "1.0.0", - "hook": { - "path": "/usr/libexec/oci/hooks.d/oci-systemd-hook" - }, - "when": { - "commands": [".*/init$" , ".*/systemd$"] - }, - "stages": ["prestart", "poststop"] -} -``` - -The following example injects `oci-umount --debug` in the pre-start stage if the container is configured to bind-mount host directories into the container. - -```console -$ cat /etc/containers/oci/hooks.d/oci-umount.json -{ - "version": "1.0.0", - "hook": { - "path": "/usr/libexec/oci/hooks.d/oci-umount", - "args": ["oci-umount", "--debug"], - }, - "when": { - "hasBindMounts": true - }, - "stages": ["prestart"] -} -``` - -The following example injects `nvidia-container-runtime-hook prestart` with particular environment variables in the pre-start stage if the container is configured with an `annotations` entry whose key matches `^com\.example\.department$` and whose value matches `.*fluid-dynamics.*`. - -```console -$ cat /etc/containers/oci/hooks.d/nvidia.json -{ - "version": "1.0.0", - "hook": { - "path": "/usr/sbin/nvidia-container-runtime-hook", - "args": ["nvidia-container-runtime-hook", "prestart"], - "env": [ - "NVIDIA_REQUIRE_CUDA=cuda>=9.1", - "NVIDIA_VISIBLE_DEVICES=GPU-fef8089b" - ] - }, - "when": { - "annotations": { - "^com\\.example\\.department$": ".*fluid-dynamics$" - } - }, - "stages": ["prestart"] -} -``` - -## 0.1.0 Hook Schema - -The following configuration injects `oci-systemd-hook` in the pre-start and post-stop stages if `process.args[0]` ends with `/init` or `/systemd`: - -```console -$ cat /etc/containers/oci/hooks.d/oci-systemd-hook.json -{ - "cmds": [".*/init$" , ".*/systemd$"], - "hook": "/usr/libexec/oci/hooks.d/oci-systemd-hook", - "stages": ["prestart", "poststop"] -} -``` - -The following example injects `oci-umount --debug` in the pre-start stage if the container is configured to bind-mount host directories into the container. - -```console -$ cat /etc/containers/oci/hooks.d/oci-umount.json -{ - "hook": "/usr/libexec/oci/hooks.d/oci-umount", - "arguments": ["--debug"], - "hasbindmounts": true, - "stages": ["prestart"] -} -``` - -The following example injects `nvidia-container-runtime-hook prestart` in the pre-start stage if the container is configured with an `annotations` entry whose value matches `.*fluid-dynamics.*`. - -```console -$ cat /etc/containers/oci/hooks.d/osystemd-hook.json -{ - "hook": "/usr/sbin/nvidia-container-runtime-hook", - "arguments": ["prestart"], - "annotations: [".*fluid-dynamics.*"], - "stages": ["prestart"] -} -``` - -# SEE ALSO - -`oci-systemd-hook(1)`, `oci-umount(1)`, `locale(7)` - -* [OCI Runtime Specification, 1.0.1, POSIX-platform hooks](https://github.com/opencontainers/runtime-spec/blob/v1.0.1/config.md#posix-platform-hooks) -* [OCI Runtime Specification, 1.0.1, process](https://github.com/opencontainers/runtime-spec/blob/v1.0.1/config.md#process) -* [POSIX extended regular expressions (EREs)](https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap09.html#tag_09_04) diff --git a/pkg/hooks/exec/exec.go b/pkg/hooks/exec/exec.go deleted file mode 100644 index bc639245f..000000000 --- a/pkg/hooks/exec/exec.go +++ /dev/null @@ -1,69 +0,0 @@ -// Package exec provides utilities for executing Open Container Initiative runtime hooks. -package exec - -import ( - "bytes" - "context" - "fmt" - "io" - osexec "os/exec" - "time" - - rspec "github.com/opencontainers/runtime-spec/specs-go" - "github.com/sirupsen/logrus" -) - -// DefaultPostKillTimeout is the recommended default post-kill timeout. -const DefaultPostKillTimeout = time.Duration(10) * time.Second - -// Run executes the hook and waits for it to complete or for the -// context or hook-specified timeout to expire. -func Run(ctx context.Context, hook *rspec.Hook, state []byte, stdout io.Writer, stderr io.Writer, postKillTimeout time.Duration) (hookErr, err error) { - cmd := osexec.Cmd{ - Path: hook.Path, - Args: hook.Args, - Env: hook.Env, - Stdin: bytes.NewReader(state), - Stdout: stdout, - Stderr: stderr, - } - if cmd.Env == nil { - cmd.Env = []string{} - } - - if hook.Timeout != nil { - var cancel context.CancelFunc - ctx, cancel = context.WithTimeout(ctx, time.Duration(*hook.Timeout)*time.Second) - defer cancel() - } - - err = cmd.Start() - if err != nil { - return err, err - } - exit := make(chan error, 1) - go func() { - err := cmd.Wait() - if err != nil { - err = fmt.Errorf("executing %v: %w", cmd.Args, err) - } - exit <- err - }() - - select { - case err = <-exit: - return err, err - case <-ctx.Done(): - if err := cmd.Process.Kill(); err != nil { - logrus.Errorf("Failed to kill pid %v", cmd.Process) - } - timer := time.NewTimer(postKillTimeout) - defer timer.Stop() - select { - case <-timer.C: - err = fmt.Errorf("failed to reap process within %s of the kill signal", postKillTimeout) - case err = <-exit: - } - return err, ctx.Err() - } -} diff --git a/pkg/hooks/exec/exec_test.go b/pkg/hooks/exec/exec_test.go deleted file mode 100644 index 1e105373d..000000000 --- a/pkg/hooks/exec/exec_test.go +++ /dev/null @@ -1,222 +0,0 @@ -package exec - -import ( - "bytes" - "context" - "fmt" - "os" - "runtime" - "strings" - "testing" - "time" - - rspec "github.com/opencontainers/runtime-spec/specs-go" - "github.com/stretchr/testify/assert" -) - -// path is the path to an example hook executable. -var path string - -// unavoidableEnvironmentKeys may be injected even if the hook -// executable is executed with a requested empty environment. -var unavoidableEnvironmentKeys []string - -func TestRun(t *testing.T) { - ctx := context.Background() - hook := &rspec.Hook{ - Path: path, - Args: []string{"sh", "-c", "cat"}, - } - var stderr, stdout bytes.Buffer - hookErr, err := Run(ctx, hook, []byte("{}"), &stdout, &stderr, DefaultPostKillTimeout) - if err != nil { - t.Fatal(err) - } - if hookErr != nil { - t.Fatal(hookErr) - } - assert.Equal(t, "{}", stdout.String()) - assert.Equal(t, "", stderr.String()) -} - -func TestRunIgnoreOutput(t *testing.T) { - ctx := context.Background() - hook := &rspec.Hook{ - Path: path, - Args: []string{"sh", "-c", "cat"}, - } - hookErr, err := Run(ctx, hook, []byte("{}"), nil, nil, DefaultPostKillTimeout) - if err != nil { - t.Fatal(err) - } - if hookErr != nil { - t.Fatal(hookErr) - } -} - -func TestRunFailedStart(t *testing.T) { - ctx := context.Background() - hook := &rspec.Hook{ - Path: "/does/not/exist", - } - hookErr, err := Run(ctx, hook, []byte("{}"), nil, nil, DefaultPostKillTimeout) - if err == nil { - t.Fatal("unexpected success") - } - if !os.IsNotExist(err) { - t.Fatal(err) - } - assert.Equal(t, err, hookErr) -} - -func parseEnvironment(input string) (env map[string]string, err error) { - env = map[string]string{} - lines := strings.Split(input, "\n") - for i, line := range lines { - if line == "" && i == len(lines)-1 { - continue // no content after the terminal newline - } - keyValue := strings.SplitN(line, "=", 2) - if len(keyValue) < 2 { - return env, fmt.Errorf("no = in environment line: %q", line) - } - env[keyValue[0]] = keyValue[1] - } - for _, key := range unavoidableEnvironmentKeys { - delete(env, key) - } - return env, nil -} - -func TestRunEnvironment(t *testing.T) { - ctx := context.Background() - hook := &rspec.Hook{ - Path: path, - Args: []string{"sh", "-c", "env"}, - } - for _, tt := range []struct { - name string - env []string - expected map[string]string - }{ - { - name: "unset", - expected: map[string]string{}, - }, - { - name: "set empty", - env: []string{}, - expected: map[string]string{}, - }, - { - name: "set", - env: []string{ - "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", - "TERM=xterm", - }, - expected: map[string]string{ - "PATH": "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", - "TERM": "xterm", - }, - }, - } { - test := tt - t.Run(test.name, func(t *testing.T) { - var stderr, stdout bytes.Buffer - hook.Env = test.env - hookErr, err := Run(ctx, hook, []byte("{}"), &stdout, &stderr, DefaultPostKillTimeout) - if err != nil { - t.Fatal(err) - } - if hookErr != nil { - t.Fatal(hookErr) - } - assert.Equal(t, "", stderr.String()) - - env, err := parseEnvironment(stdout.String()) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, test.expected, env) - }) - } -} - -func TestRunCancel(t *testing.T) { - hook := &rspec.Hook{ - Path: path, - Args: []string{"sh", "-c", "echo waiting; sleep 2; echo done"}, - } - one := 1 - for _, tt := range []struct { - name string - contextTimeout time.Duration - hookTimeout *int - expectedHookError string - expectedRunError error - expectedStdout string - }{ - { - name: "no timeouts", - expectedStdout: "waiting\ndone\n", - }, - { - name: "context timeout", - contextTimeout: time.Duration(1) * time.Second, - expectedStdout: "waiting\n", - expectedHookError: "^executing \\[sh -c echo waiting; sleep 2; echo done]: signal: killed$", - expectedRunError: context.DeadlineExceeded, - }, - { - name: "hook timeout", - hookTimeout: &one, - expectedStdout: "waiting\n", - expectedHookError: "^executing \\[sh -c echo waiting; sleep 2; echo done]: signal: killed$", - expectedRunError: context.DeadlineExceeded, - }, - } { - test := tt - t.Run(test.name, func(t *testing.T) { - ctx := context.Background() - var stderr, stdout bytes.Buffer - if test.contextTimeout > 0 { - var cancel context.CancelFunc - ctx, cancel = context.WithTimeout(ctx, test.contextTimeout) - defer cancel() - } - hook.Timeout = test.hookTimeout - hookErr, err := Run(ctx, hook, []byte("{}"), &stdout, &stderr, DefaultPostKillTimeout) - assert.Equal(t, test.expectedRunError, err) - if test.expectedHookError == "" { - if hookErr != nil { - t.Fatal(hookErr) - } - } else { - assert.Regexp(t, test.expectedHookError, hookErr.Error()) - } - assert.Equal(t, "", stderr.String()) - assert.Equal(t, test.expectedStdout, stdout.String()) - }) - } -} - -func TestRunKillTimeout(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), time.Duration(500)*time.Millisecond) - defer cancel() - hook := &rspec.Hook{ - Path: path, - Args: []string{"sh", "-c", "sleep 1"}, - } - hookErr, err := Run(ctx, hook, []byte("{}"), nil, nil, time.Duration(0)) - assert.Equal(t, context.DeadlineExceeded, err) - assert.Regexp(t, "^(failed to reap process within 0s of the kill signal|executing \\[sh -c sleep 1]: signal: killed)$", hookErr) -} - -func init() { - if runtime.GOOS != "windows" { - path = "/bin/sh" - unavoidableEnvironmentKeys = []string{"PWD", "SHLVL", "_"} - } else { - panic("we need a reliable executable path on Windows") - } -} diff --git a/pkg/hooks/exec/runtimeconfigfilter.go b/pkg/hooks/exec/runtimeconfigfilter.go deleted file mode 100644 index 72d4b8979..000000000 --- a/pkg/hooks/exec/runtimeconfigfilter.go +++ /dev/null @@ -1,72 +0,0 @@ -package exec - -import ( - "bytes" - "context" - "encoding/json" - "fmt" - "reflect" - "time" - - "github.com/davecgh/go-spew/spew" - spec "github.com/opencontainers/runtime-spec/specs-go" - "github.com/pmezard/go-difflib/difflib" - "github.com/sirupsen/logrus" -) - -var spewConfig = spew.ConfigState{ - Indent: " ", - DisablePointerAddresses: true, - DisableCapacities: true, - SortKeys: true, -} - -// RuntimeConfigFilter calls a series of hooks. But instead of -// passing container state on their standard input, -// RuntimeConfigFilter passes the proposed runtime configuration (and -// reads back a possibly-altered form from their standard output). -func RuntimeConfigFilter(ctx context.Context, hooks []spec.Hook, config *spec.Spec, postKillTimeout time.Duration) (hookErr, err error) { - data, err := json.Marshal(config) - if err != nil { - return nil, err - } - for i, hook := range hooks { - hook := hook - var stdout bytes.Buffer - hookErr, err = Run(ctx, &hook, data, &stdout, nil, postKillTimeout) - if err != nil { - return hookErr, err - } - - data = stdout.Bytes() - var newConfig spec.Spec - err = json.Unmarshal(data, &newConfig) - if err != nil { - logrus.Debugf("invalid JSON from config-filter hook %d:\n%s", i, string(data)) - return nil, fmt.Errorf("unmarshal output from config-filter hook %d: %w", i, err) - } - - if !reflect.DeepEqual(config, &newConfig) { - oldConfig := spewConfig.Sdump(config) - newConfig := spewConfig.Sdump(&newConfig) - diff, err := difflib.GetUnifiedDiffString(difflib.UnifiedDiff{ - A: difflib.SplitLines(oldConfig), - B: difflib.SplitLines(newConfig), - FromFile: "Old", - FromDate: "", - ToFile: "New", - ToDate: "", - Context: 1, - }) - if err == nil { - logrus.Debugf("precreate hook %d made configuration changes:\n%s", i, diff) - } else { - logrus.Warnf("Precreate hook %d made configuration changes, but we could not compute a diff: %v", i, err) - } - } - - *config = newConfig - } - - return nil, nil -} diff --git a/pkg/hooks/exec/runtimeconfigfilter_test.go b/pkg/hooks/exec/runtimeconfigfilter_test.go deleted file mode 100644 index a4e9b1fdb..000000000 --- a/pkg/hooks/exec/runtimeconfigfilter_test.go +++ /dev/null @@ -1,265 +0,0 @@ -package exec - -import ( - "context" - "encoding/json" - "errors" - "os" - "testing" - "time" - - spec "github.com/opencontainers/runtime-spec/specs-go" - "github.com/stretchr/testify/assert" -) - -func TestRuntimeConfigFilter(t *testing.T) { - unexpectedEndOfJSONInput := json.Unmarshal([]byte("{\n"), nil) //nolint:govet // this should force the error - fileMode := os.FileMode(0600) - rootUint32 := uint32(0) - binUser := int(1) - for _, tt := range []struct { - name string - contextTimeout time.Duration - hooks []spec.Hook - input *spec.Spec - expected *spec.Spec - expectedHookError string - expectedRunError error - expectedRunErrorString string - }{ - { - name: "no-op", - hooks: []spec.Hook{ - { - Path: path, - Args: []string{"sh", "-c", "cat"}, - }, - }, - input: &spec.Spec{ - Version: "1.0.0", - Root: &spec.Root{ - Path: "rootfs", - }, - }, - expected: &spec.Spec{ - Version: "1.0.0", - Root: &spec.Root{ - Path: "rootfs", - }, - }, - }, - { - name: "device injection", - hooks: []spec.Hook{ - { - Path: path, - Args: []string{"sh", "-c", `sed 's|\("gid":0}\)|\1,{"path": "/dev/sda","type":"b","major":8,"minor":0,"fileMode":384,"uid":0,"gid":0}|'`}, - }, - }, - input: &spec.Spec{ - Version: "1.0.0", - Root: &spec.Root{ - Path: "rootfs", - }, - Linux: &spec.Linux{ - Devices: []spec.LinuxDevice{ - { - Path: "/dev/fuse", - Type: "c", - Major: 10, - Minor: 229, - FileMode: &fileMode, - UID: &rootUint32, - GID: &rootUint32, - }, - }, - }, - }, - expected: &spec.Spec{ - Version: "1.0.0", - Root: &spec.Root{ - Path: "rootfs", - }, - Linux: &spec.Linux{ - Devices: []spec.LinuxDevice{ - { - Path: "/dev/fuse", - Type: "c", - Major: 10, - Minor: 229, - FileMode: &fileMode, - UID: &rootUint32, - GID: &rootUint32, - }, - { - Path: "/dev/sda", - Type: "b", - Major: 8, - Minor: 0, - FileMode: &fileMode, - UID: &rootUint32, - GID: &rootUint32, - }, - }, - }, - }, - }, - { - name: "chaining", - hooks: []spec.Hook{ - { - Path: path, - Args: []string{"sh", "-c", `sed 's|\("gid":0}\)|\1,{"path": "/dev/sda","type":"b","major":8,"minor":0,"fileMode":384,"uid":0,"gid":0}|'`}, - }, - { - Path: path, - Args: []string{"sh", "-c", `sed 's|/dev/sda|/dev/sdb|'`}, - }, - }, - input: &spec.Spec{ - Version: "1.0.0", - Root: &spec.Root{ - Path: "rootfs", - }, - Linux: &spec.Linux{ - Devices: []spec.LinuxDevice{ - { - Path: "/dev/fuse", - Type: "c", - Major: 10, - Minor: 229, - FileMode: &fileMode, - UID: &rootUint32, - GID: &rootUint32, - }, - }, - }, - }, - expected: &spec.Spec{ - Version: "1.0.0", - Root: &spec.Root{ - Path: "rootfs", - }, - Linux: &spec.Linux{ - Devices: []spec.LinuxDevice{ - { - Path: "/dev/fuse", - Type: "c", - Major: 10, - Minor: 229, - FileMode: &fileMode, - UID: &rootUint32, - GID: &rootUint32, - }, - { - Path: "/dev/sdb", - Type: "b", - Major: 8, - Minor: 0, - FileMode: &fileMode, - UID: &rootUint32, - GID: &rootUint32, - }, - }, - }, - }, - }, - { - name: "context timeout", - contextTimeout: time.Duration(1) * time.Second, - hooks: []spec.Hook{ - { - Path: path, - Args: []string{"sh", "-c", "sleep 2"}, - }, - }, - input: &spec.Spec{ - Version: "1.0.0", - Root: &spec.Root{ - Path: "rootfs", - }, - }, - expected: &spec.Spec{ - Version: "1.0.0", - Root: &spec.Root{ - Path: "rootfs", - }, - }, - expectedHookError: "^executing \\[sh -c sleep 2]: signal: killed$", - expectedRunError: context.DeadlineExceeded, - }, - { - name: "hook timeout", - hooks: []spec.Hook{ - { - Path: path, - Args: []string{"sh", "-c", "sleep 2"}, - Timeout: &binUser, - }, - }, - input: &spec.Spec{ - Version: "1.0.0", - Root: &spec.Root{ - Path: "rootfs", - }, - }, - expected: &spec.Spec{ - Version: "1.0.0", - Root: &spec.Root{ - Path: "rootfs", - }, - }, - expectedHookError: "^executing \\[sh -c sleep 2]: signal: killed$", - expectedRunError: context.DeadlineExceeded, - }, - { - name: "invalid JSON", - hooks: []spec.Hook{ - { - Path: path, - Args: []string{"sh", "-c", "echo '{'"}, - }, - }, - input: &spec.Spec{ - Version: "1.0.0", - Root: &spec.Root{ - Path: "rootfs", - }, - }, - expected: &spec.Spec{ - Version: "1.0.0", - Root: &spec.Root{ - Path: "rootfs", - }, - }, - expectedRunError: unexpectedEndOfJSONInput, - expectedRunErrorString: unexpectedEndOfJSONInput.Error(), - }, - } { - test := tt - t.Run(test.name, func(t *testing.T) { - ctx := context.Background() - if test.contextTimeout > 0 { - var cancel context.CancelFunc - ctx, cancel = context.WithTimeout(ctx, test.contextTimeout) - defer cancel() - } - hookErr, err := RuntimeConfigFilter(ctx, test.hooks, test.input, DefaultPostKillTimeout) - if test.expectedRunError != nil { - if test.expectedRunErrorString != "" { - assert.Contains(t, err.Error(), test.expectedRunErrorString) - } else { - assert.True(t, errors.Is(err, test.expectedRunError)) - } - } - if test.expectedHookError == "" { - if hookErr != nil { - t.Fatal(hookErr) - } - } else { - assert.Regexp(t, test.expectedHookError, hookErr.Error()) - } - assert.Equal(t, test.expected, test.input) - }) - } -} diff --git a/pkg/hooks/hooks.go b/pkg/hooks/hooks.go deleted file mode 100644 index 14f98b1de..000000000 --- a/pkg/hooks/hooks.go +++ /dev/null @@ -1,145 +0,0 @@ -// Package hooks implements hook configuration and handling for CRI-O and libpod. -package hooks - -import ( - "context" - "fmt" - "os" - "sort" - "strings" - "sync" - - current "github.com/containers/podman/v4/pkg/hooks/1.0.0" - rspec "github.com/opencontainers/runtime-spec/specs-go" - "github.com/sirupsen/logrus" -) - -// Version is the current hook configuration version. -const Version = current.Version - -const ( - // DefaultDir is the default directory containing system hook configuration files. - DefaultDir = "/usr/share/containers/oci/hooks.d" - - // OverrideDir is the directory for hook configuration files overriding the default entries. - OverrideDir = "/etc/containers/oci/hooks.d" -) - -// Manager provides an opaque interface for managing CRI-O hooks. -type Manager struct { - hooks map[string]*current.Hook - directories []string - extensionStages []string - lock sync.Mutex -} - -type namedHook struct { - name string - hook *current.Hook -} - -// New creates a new hook manager. Directories are ordered by -// increasing preference (hook configurations in later directories -// override configurations with the same filename from earlier -// directories). -// -// extensionStages allows callers to add additional stages beyond -// those specified in the OCI Runtime Specification and to control -// OCI-defined stages instead of delegating to the OCI runtime. See -// Hooks() for more information. -func New(ctx context.Context, directories []string, extensionStages []string) (manager *Manager, err error) { - manager = &Manager{ - hooks: map[string]*current.Hook{}, - directories: directories, - extensionStages: extensionStages, - } - - for _, dir := range directories { - err = ReadDir(dir, manager.extensionStages, manager.hooks) - if err != nil && !os.IsNotExist(err) { - return nil, err - } - } - - return manager, nil -} - -// filenames returns sorted hook entries. -func (m *Manager) namedHooks() (hooks []*namedHook) { - m.lock.Lock() - defer m.lock.Unlock() - - hooks = make([]*namedHook, len(m.hooks)) - i := 0 - for name, hook := range m.hooks { - hooks[i] = &namedHook{ - name: name, - hook: hook, - } - i++ - } - - return hooks -} - -// Hooks injects OCI runtime hooks for a given container configuration. -// -// If extensionStages was set when initializing the Manager, -// matching hooks requesting those stages will be returned in -// extensionStageHooks. This takes precedence over their inclusion in -// the OCI configuration. For example: -// -// manager, err := New(ctx, []string{DefaultDir}, []string{"poststop"}) -// extensionStageHooks, err := manager.Hooks(config, annotations, hasBindMounts) -// -// will have any matching post-stop hooks in extensionStageHooks and -// will not insert them into config.Hooks.Poststop. -func (m *Manager) Hooks(config *rspec.Spec, annotations map[string]string, hasBindMounts bool) (extensionStageHooks map[string][]rspec.Hook, err error) { - hooks := m.namedHooks() - sort.Slice(hooks, func(i, j int) bool { return strings.ToLower(hooks[i].name) < strings.ToLower(hooks[j].name) }) - localStages := map[string]bool{} // stages destined for extensionStageHooks - for _, stage := range m.extensionStages { - localStages[stage] = true - } - for _, namedHook := range hooks { - match, err := namedHook.hook.When.Match(config, annotations, hasBindMounts) - if err != nil { - return extensionStageHooks, fmt.Errorf("matching hook %q: %w", namedHook.name, err) - } - if match { - logrus.Debugf("hook %s matched; adding to stages %v", namedHook.name, namedHook.hook.Stages) - if config.Hooks == nil { - config.Hooks = &rspec.Hooks{} - } - for _, stage := range namedHook.hook.Stages { - if _, ok := localStages[stage]; ok { - if extensionStageHooks == nil { - extensionStageHooks = map[string][]rspec.Hook{} - } - extensionStageHooks[stage] = append(extensionStageHooks[stage], namedHook.hook.Hook) - } else { - switch stage { - case "createContainer": - config.Hooks.CreateContainer = append(config.Hooks.CreateContainer, namedHook.hook.Hook) - case "createRuntime": - config.Hooks.CreateRuntime = append(config.Hooks.CreateRuntime, namedHook.hook.Hook) - case "prestart": - config.Hooks.Prestart = append(config.Hooks.Prestart, namedHook.hook.Hook) - case "poststart": - config.Hooks.Poststart = append(config.Hooks.Poststart, namedHook.hook.Hook) - case "poststop": - config.Hooks.Poststop = append(config.Hooks.Poststop, namedHook.hook.Hook) - case "startContainer": - config.Hooks.StartContainer = append(config.Hooks.StartContainer, namedHook.hook.Hook) - default: - return extensionStageHooks, fmt.Errorf("hook %q: unknown stage %q", namedHook.name, stage) - } - } - } - } else { - logrus.Debugf("hook %s did not match", namedHook.name) - } - } - - return extensionStageHooks, nil -} diff --git a/pkg/hooks/hooks_test.go b/pkg/hooks/hooks_test.go deleted file mode 100644 index d5d0c2a32..000000000 --- a/pkg/hooks/hooks_test.go +++ /dev/null @@ -1,218 +0,0 @@ -package hooks - -import ( - "context" - "fmt" - "io/ioutil" - "path/filepath" - "runtime" - "testing" - - current "github.com/containers/podman/v4/pkg/hooks/1.0.0" - rspec "github.com/opencontainers/runtime-spec/specs-go" - "github.com/stretchr/testify/assert" -) - -// path is the path to an example hook executable. -var path string - -func TestGoodNew(t *testing.T) { - ctx := context.Background() - - dir := t.TempDir() - - for i, name := range []string{ - "01-my-hook.json", - "01-UPPERCASE.json", - "02-another-hook.json", - } { - jsonPath := filepath.Join(dir, name) - var extraStages string - if i == 0 { - extraStages = ", \"poststart\", \"poststop\"" - } - err := ioutil.WriteFile(jsonPath, []byte(fmt.Sprintf("{\"version\": \"1.0.0\", \"hook\": {\"path\": \"%s\", \"timeout\": %d}, \"when\": {\"always\": true}, \"stages\": [\"prestart\"%s]}", path, i+1, extraStages)), 0644) - if err != nil { - t.Fatal(err) - } - } - - manager, err := New(ctx, []string{dir}, []string{}) - if err != nil { - t.Fatal(err) - } - - config := &rspec.Spec{} - extensionStageHooks, err := manager.Hooks(config, map[string]string{}, false) - if err != nil { - t.Fatal(err) - } - - one := 1 - two := 2 - three := 3 - assert.Equal(t, &rspec.Hooks{ - Prestart: []rspec.Hook{ - { - Path: path, - Timeout: &one, - }, - { - Path: path, - Timeout: &two, - }, - { - Path: path, - Timeout: &three, - }, - }, - Poststart: []rspec.Hook{ - { - Path: path, - Timeout: &one, - }, - }, - Poststop: []rspec.Hook{ - { - Path: path, - Timeout: &one, - }, - }, - }, config.Hooks) - - var nilExtensionStageHooks map[string][]rspec.Hook - assert.Equal(t, nilExtensionStageHooks, extensionStageHooks) -} - -func TestBadNew(t *testing.T) { - ctx := context.Background() - - dir := t.TempDir() - - jsonPath := filepath.Join(dir, "a.json") - err := ioutil.WriteFile(jsonPath, []byte("{\"version\": \"-1\"}"), 0644) - if err != nil { - t.Fatal(err) - } - - _, err = New(ctx, []string{dir}, []string{}) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^parsing hook \"[^\"]*a.json\": unrecognized hook version: \"-1\"$", err.Error()) -} - -func TestBrokenMatch(t *testing.T) { - manager := Manager{ - hooks: map[string]*current.Hook{ - "a.json": { - Version: current.Version, - Hook: rspec.Hook{ - Path: "/a/b/c", - }, - When: current.When{ - Commands: []string{"["}, - }, - Stages: []string{"prestart"}, - }, - }, - } - config := &rspec.Spec{ - Process: &rspec.Process{ - Args: []string{"/bin/sh"}, - }, - } - extensionStageHooks, err := manager.Hooks(config, map[string]string{}, false) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^matching hook \"a\\.json\": command: error parsing regexp: .*", err.Error()) - - var nilExtensionStageHooks map[string][]rspec.Hook - assert.Equal(t, nilExtensionStageHooks, extensionStageHooks) -} - -func TestInvalidStage(t *testing.T) { - always := true - manager := Manager{ - hooks: map[string]*current.Hook{ - "a.json": { - Version: current.Version, - Hook: rspec.Hook{ - Path: "/a/b/c", - }, - When: current.When{ - Always: &always, - }, - Stages: []string{"does-not-exist"}, - }, - }, - } - extensionStageHooks, err := manager.Hooks(&rspec.Spec{}, map[string]string{}, false) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^hook \"a\\.json\": unknown stage \"does-not-exist\"$", err.Error()) - - var nilExtensionStageHooks map[string][]rspec.Hook - assert.Equal(t, nilExtensionStageHooks, extensionStageHooks) -} - -func TestExtensionStage(t *testing.T) { - always := true - manager := Manager{ - hooks: map[string]*current.Hook{ - "a.json": { - Version: current.Version, - Hook: rspec.Hook{ - Path: "/a/b/c", - }, - When: current.When{ - Always: &always, - }, - Stages: []string{"prestart", "poststop", "a", "b"}, - }, - }, - extensionStages: []string{"poststop", "a", "b", "c"}, - } - - config := &rspec.Spec{} - extensionStageHooks, err := manager.Hooks(config, map[string]string{}, false) - if err != nil { - t.Fatal(err) - } - - assert.Equal(t, &rspec.Hooks{ - Prestart: []rspec.Hook{ - { - Path: "/a/b/c", - }, - }, - }, config.Hooks) - - assert.Equal(t, map[string][]rspec.Hook{ - "poststop": { - { - Path: "/a/b/c", - }, - }, - "a": { - { - Path: "/a/b/c", - }, - }, - "b": { - { - Path: "/a/b/c", - }, - }, - }, extensionStageHooks) -} - -func init() { - if runtime.GOOS != "windows" { - path = "/bin/sh" - } else { - panic("we need a reliable executable path on Windows") - } -} diff --git a/pkg/hooks/monitor.go b/pkg/hooks/monitor.go deleted file mode 100644 index d2d7140a5..000000000 --- a/pkg/hooks/monitor.go +++ /dev/null @@ -1,66 +0,0 @@ -package hooks - -import ( - "context" - - current "github.com/containers/podman/v4/pkg/hooks/1.0.0" - "github.com/fsnotify/fsnotify" - "github.com/sirupsen/logrus" -) - -// Monitor dynamically monitors hook directories for additions, -// updates, and removals. -// -// This function writes two empty structs to the sync channel: the -// first is written after the watchers are established and the second -// when this function exits. The expected usage is: -// -// ctx, cancel := context.WithCancel(context.Background()) -// sync := make(chan error, 2) -// go m.Monitor(ctx, sync) -// err := <-sync // block until writers are established -// if err != nil { -// return err // failed to establish watchers -// } -// // do stuff -// cancel() -// err = <-sync // block until monitor finishes -func (m *Manager) Monitor(ctx context.Context, sync chan<- error) { - watcher, err := fsnotify.NewWatcher() - if err != nil { - sync <- err - return - } - defer watcher.Close() - - for _, dir := range m.directories { - err = watcher.Add(dir) - if err != nil { - logrus.Errorf("Failed to watch %q for hooks", dir) - sync <- err - return - } - logrus.Debugf("monitoring %q for hooks", dir) - } - - sync <- nil - - for { - select { - case event := <-watcher.Events: - m.hooks = make(map[string]*current.Hook) - for _, dir := range m.directories { - err = ReadDir(dir, m.extensionStages, m.hooks) - if err != nil { - logrus.Errorf("Failed loading hooks for %s: %v", event.Name, err) - } - } - case <-ctx.Done(): - err = ctx.Err() - logrus.Debugf("hook monitoring canceled: %v", err) - sync <- err - close(sync) - return - } - } -} diff --git a/pkg/hooks/monitor_test.go b/pkg/hooks/monitor_test.go deleted file mode 100644 index 1067d2920..000000000 --- a/pkg/hooks/monitor_test.go +++ /dev/null @@ -1,324 +0,0 @@ -package hooks - -import ( - "context" - "fmt" - "io/ioutil" - "os" - "path/filepath" - "testing" - "time" - - rspec "github.com/opencontainers/runtime-spec/specs-go" - "github.com/stretchr/testify/assert" -) - -func TestMonitorOneDirGood(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) - dir := t.TempDir() - - manager, err := New(ctx, []string{dir}, []string{}) - if err != nil { - t.Fatal(err) - } - - sync := make(chan error, 2) - go manager.Monitor(ctx, sync) - err = <-sync - if err != nil { - t.Fatal(err) - } - - jsonPath := filepath.Join(dir, "a.json") - - t.Run("good-addition", func(t *testing.T) { - err = ioutil.WriteFile(jsonPath, []byte(fmt.Sprintf("{\"version\": \"1.0.0\", \"hook\": {\"path\": \"%s\"}, \"when\": {\"always\": true}, \"stages\": [\"prestart\", \"poststart\", \"poststop\"]}", path)), 0644) - if err != nil { - t.Fatal(err) - } - - time.Sleep(100 * time.Millisecond) // wait for monitor to notice - - config := &rspec.Spec{} - _, err = manager.Hooks(config, map[string]string{}, false) - if err != nil { - t.Fatal(err) - } - - assert.Equal(t, &rspec.Hooks{ - Prestart: []rspec.Hook{ - { - Path: path, - }, - }, - Poststart: []rspec.Hook{ - { - Path: path, - }, - }, - Poststop: []rspec.Hook{ - { - Path: path, - }, - }, - }, config.Hooks) - }) - - t.Run("good-removal", func(t *testing.T) { - err = os.Remove(jsonPath) - if err != nil { - t.Fatal(err) - } - - time.Sleep(100 * time.Millisecond) // wait for monitor to notice - - config := &rspec.Spec{} - expected := config.Hooks - _, err = manager.Hooks(config, map[string]string{}, false) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, expected, config.Hooks) - }) - - t.Run("bad-addition", func(t *testing.T) { - err = ioutil.WriteFile(jsonPath, []byte("{\"version\": \"-1\"}"), 0644) - if err != nil { - t.Fatal(err) - } - - time.Sleep(100 * time.Millisecond) // wait for monitor to notice - - config := &rspec.Spec{} - expected := config.Hooks - _, err = manager.Hooks(config, map[string]string{}, false) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, expected, config.Hooks) - - err = os.Remove(jsonPath) - if err != nil { - t.Fatal(err) - } - }) - - cancel() - err = <-sync - assert.Equal(t, context.Canceled, err) -} - -func TestMonitorTwoDirGood(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) - primaryDir := t.TempDir() - fallbackDir := t.TempDir() - - manager, err := New(ctx, []string{fallbackDir, primaryDir}, []string{}) - if err != nil { - t.Fatal(err) - } - - sync := make(chan error, 2) - go manager.Monitor(ctx, sync) - err = <-sync - if err != nil { - t.Fatal(err) - } - - fallbackPath := filepath.Join(fallbackDir, "a.json") - fallbackJSON := []byte(fmt.Sprintf("{\"version\": \"1.0.0\", \"hook\": {\"path\": \"%s\"}, \"when\": {\"always\": true}, \"stages\": [\"prestart\"]}", path)) - fallbackInjected := &rspec.Hooks{ - Prestart: []rspec.Hook{ - { - Path: path, - }, - }, - } - - t.Run("good-fallback-addition", func(t *testing.T) { - err = ioutil.WriteFile(fallbackPath, fallbackJSON, 0644) - if err != nil { - t.Fatal(err) - } - - time.Sleep(100 * time.Millisecond) // wait for monitor to notice - - config := &rspec.Spec{} - _, err = manager.Hooks(config, map[string]string{}, false) - if err != nil { - t.Fatal(err) - } - - assert.Equal(t, fallbackInjected, config.Hooks) - }) - - primaryPath := filepath.Join(primaryDir, "a.json") - primaryJSON := []byte(fmt.Sprintf("{\"version\": \"1.0.0\", \"hook\": {\"path\": \"%s\", \"timeout\": 1}, \"when\": {\"always\": true}, \"stages\": [\"prestart\"]}", path)) - one := 1 - primaryInjected := &rspec.Hooks{ - Prestart: []rspec.Hook{ - { - Path: path, - Timeout: &one, - }, - }, - } - - t.Run("good-primary-override", func(t *testing.T) { - err = ioutil.WriteFile(primaryPath, primaryJSON, 0644) - if err != nil { - t.Fatal(err) - } - - time.Sleep(100 * time.Millisecond) // wait for monitor to notice - - config := &rspec.Spec{} - _, err = manager.Hooks(config, map[string]string{}, false) - if err != nil { - t.Fatal(err) - } - - assert.Equal(t, primaryInjected, config.Hooks) - }) - - t.Run("good-fallback-removal", func(t *testing.T) { - err = os.Remove(fallbackPath) - if err != nil { - t.Fatal(err) - } - - time.Sleep(100 * time.Millisecond) // wait for monitor to notice - - config := &rspec.Spec{} - _, err = manager.Hooks(config, map[string]string{}, false) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, primaryInjected, config.Hooks) // masked by primary - }) - - t.Run("good-fallback-restore", func(t *testing.T) { - err = ioutil.WriteFile(fallbackPath, fallbackJSON, 0644) - if err != nil { - t.Fatal(err) - } - - time.Sleep(100 * time.Millisecond) // wait for monitor to notice - - config := &rspec.Spec{} - _, err = manager.Hooks(config, map[string]string{}, false) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, primaryInjected, config.Hooks) // masked by primary - }) - - primaryPath2 := filepath.Join(primaryDir, "0a.json") // 0a because it will be before a.json alphabetically - - t.Run("bad-primary-new-addition", func(t *testing.T) { - err = ioutil.WriteFile(primaryPath2, []byte("{\"version\": \"-1\"}"), 0644) - if err != nil { - t.Fatal(err) - } - - time.Sleep(100 * time.Millisecond) // wait for monitor to notice - - config := &rspec.Spec{} - fmt.Println("expected: ", config.Hooks) - expected := primaryInjected // 0a.json is bad, a.json is still good - _, err = manager.Hooks(config, map[string]string{}, false) - fmt.Println("actual: ", config.Hooks) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, expected, config.Hooks) - }) - - t.Run("bad-primary-same-addition", func(t *testing.T) { - err = ioutil.WriteFile(primaryPath, []byte("{\"version\": \"-1\"}"), 0644) - if err != nil { - t.Fatal(err) - } - - time.Sleep(100 * time.Millisecond) // wait for monitor to notice - - config := &rspec.Spec{} - expected := fallbackInjected - _, err = manager.Hooks(config, map[string]string{}, false) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, expected, config.Hooks) - }) - - t.Run("good-primary-removal", func(t *testing.T) { - err = os.Remove(primaryPath) - if err != nil { - t.Fatal(err) - } - - time.Sleep(100 * time.Millisecond) // wait for monitor to notice - - config := &rspec.Spec{} - _, err = manager.Hooks(config, map[string]string{}, false) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, fallbackInjected, config.Hooks) - }) - - t.Run("good-non-json-addition", func(t *testing.T) { - err = ioutil.WriteFile(filepath.Join(fallbackDir, "README"), []byte("Hello"), 0644) - if err != nil { - t.Fatal(err) - } - - time.Sleep(100 * time.Millisecond) // wait for monitor to notice - - config := &rspec.Spec{} - _, err = manager.Hooks(config, map[string]string{}, false) - if err != nil { - t.Fatal(err) - } - - assert.Equal(t, fallbackInjected, config.Hooks) - }) - - t.Run("good-fallback-removal", func(t *testing.T) { - err = os.Remove(fallbackPath) - if err != nil { - t.Fatal(err) - } - - time.Sleep(100 * time.Millisecond) // wait for monitor to notice - - config := &rspec.Spec{} - expected := config.Hooks - _, err = manager.Hooks(config, map[string]string{}, false) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, expected, config.Hooks) - }) - - cancel() - err = <-sync - assert.Equal(t, context.Canceled, err) -} - -func TestMonitorBadWatcher(t *testing.T) { - ctx := context.Background() - - manager, err := New(ctx, []string{}, []string{}) - if err != nil { - t.Fatal(err) - } - manager.directories = []string{"/does/not/exist"} - - sync := make(chan error, 2) - go manager.Monitor(ctx, sync) - err = <-sync - if !os.IsNotExist(err) { - t.Fatal("opaque wrapping for not-exist errors") - } -} diff --git a/pkg/hooks/read.go b/pkg/hooks/read.go deleted file mode 100644 index 379ed67ef..000000000 --- a/pkg/hooks/read.go +++ /dev/null @@ -1,101 +0,0 @@ -// Package hooks implements CRI-O's hook handling. -package hooks - -import ( - "encoding/json" - "errors" - "fmt" - "io/ioutil" - "os" - "path/filepath" - "strings" - - old "github.com/containers/podman/v4/pkg/hooks/0.1.0" - current "github.com/containers/podman/v4/pkg/hooks/1.0.0" - "github.com/sirupsen/logrus" -) - -type reader func(content []byte) (*current.Hook, error) - -var ( - // ErrNoJSONSuffix represents hook-add attempts where the filename - // does not end in '.json'. - ErrNoJSONSuffix = errors.New("hook filename does not end in '.json'") - - // Readers registers per-version hook readers. - Readers = map[string]reader{} -) - -// Read reads a hook JSON file, verifies it, and returns the hook configuration. -func Read(path string, extensionStages []string) (*current.Hook, error) { - if !strings.HasSuffix(path, ".json") { - return nil, ErrNoJSONSuffix - } - content, err := ioutil.ReadFile(path) - if err != nil { - return nil, err - } - hook, err := read(content) - if err != nil { - return nil, fmt.Errorf("parsing hook %q: %w", path, err) - } - err = hook.Validate(extensionStages) - return hook, err -} - -func read(content []byte) (hook *current.Hook, err error) { - var ver version - if err := json.Unmarshal(content, &ver); err != nil { - return nil, fmt.Errorf("version check: %w", err) - } - reader, ok := Readers[ver.Version] - if !ok { - return nil, fmt.Errorf("unrecognized hook version: %q", ver.Version) - } - - hook, err = reader(content) - if err != nil { - return hook, fmt.Errorf("%v: %w", ver.Version, err) - } - return hook, err -} - -// ReadDir reads hook JSON files from a directory into the given map, -// clobbering any previous entries with the same filenames. -func ReadDir(path string, extensionStages []string, hooks map[string]*current.Hook) error { - logrus.Debugf("reading hooks from %s", path) - files, err := ioutil.ReadDir(path) - if err != nil { - return err - } - res := err - for _, file := range files { - filePath := filepath.Join(path, file.Name()) - hook, err := Read(filePath, extensionStages) - if err != nil { - if err == ErrNoJSONSuffix { - continue - } - if os.IsNotExist(err) { - if err2, ok := err.(*os.PathError); ok && err2.Path == filePath { - continue - } - } - if res == nil { - res = err - } else { - res = fmt.Errorf("%v: %w", err, res) - } - continue - } - hooks[file.Name()] = hook - logrus.Debugf("added hook %s", filePath) - } - return res -} - -func init() { - Readers[current.Version] = current.Read - Readers[old.Version] = old.Read - Readers[""] = old.Read -} diff --git a/pkg/hooks/read_test.go b/pkg/hooks/read_test.go deleted file mode 100644 index 381d66bbe..000000000 --- a/pkg/hooks/read_test.go +++ /dev/null @@ -1,194 +0,0 @@ -package hooks - -import ( - "fmt" - "io/ioutil" - "os" - "path/filepath" - "testing" - - current "github.com/containers/podman/v4/pkg/hooks/1.0.0" - rspec "github.com/opencontainers/runtime-spec/specs-go" - "github.com/stretchr/testify/assert" -) - -func TestNoJSONSuffix(t *testing.T) { - _, err := Read("abc", []string{}) - assert.Equal(t, err, ErrNoJSONSuffix) -} - -func TestUnknownPath(t *testing.T) { - _, err := Read(filepath.Join("does", "not", "exist.json"), []string{}) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^open does/not/exist.json: no such file or directory$", err.Error()) - if !os.IsNotExist(err) { - t.Fatal("opaque wrapping for not-exist errors") - } -} - -func TestGoodFile(t *testing.T) { - dir := t.TempDir() - - jsonPath := filepath.Join(dir, "hook.json") - err := ioutil.WriteFile(jsonPath, []byte(fmt.Sprintf("{\"version\": \"1.0.0\", \"hook\": {\"path\": \"%s\"}, \"when\": {\"always\": true}, \"stages\": [\"prestart\"]}", path)), 0644) - if err != nil { - t.Fatal(err) - } - - hook, err := Read(jsonPath, []string{}) - if err != nil { - t.Fatal(err) - } - always := true - assert.Equal(t, ¤t.Hook{ - Version: current.Version, - Hook: rspec.Hook{ - Path: path, - }, - When: current.When{ - Always: &always, - }, - Stages: []string{"prestart"}, - }, hook) -} - -func TestBadFile(t *testing.T) { - dir := t.TempDir() - - path := filepath.Join(dir, "hook.json") - err := ioutil.WriteFile(path, []byte("{\"version\": \"1.0.0\", \"hook\": \"not-a-string\"}"), 0644) - if err != nil { - t.Fatal(err) - } - - _, err = Read(path, []string{}) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^parsing hook \"[^\"]*hook.json\": 1.0.0: json: cannot unmarshal string into Go struct field Hook.hook of type specs.Hook$", err.Error()) -} - -func TestGoodBytes(t *testing.T) { - hook, err := read([]byte("{\"version\": \"1.0.0\", \"hook\": {\"path\": \"/a/b/c\"}, \"when\": {\"always\": true}, \"stages\": [\"prestart\"]}")) - if err != nil { - t.Fatal(err) - } - always := true - assert.Equal(t, ¤t.Hook{ - Version: current.Version, - Hook: rspec.Hook{ - Path: "/a/b/c", - }, - When: current.When{ - Always: &always, - }, - Stages: []string{"prestart"}, - }, hook) -} - -func TestInvalidJSON(t *testing.T) { - _, err := read([]byte("{")) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^version check: unexpected end of JSON input$", err.Error()) -} - -func TestInvalidVersion(t *testing.T) { - _, err := read([]byte("{\"version\": \"-1\"}")) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^unrecognized hook version: \"-1\"$", err.Error()) -} - -func TestInvalidCurrentJSON(t *testing.T) { - _, err := read([]byte("{\"version\": \"1.0.0\", \"hook\": \"not-a-string\"}")) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^1.0.0: json: cannot unmarshal string into Go struct field Hook.hook of type specs.Hook$", err.Error()) -} - -func TestGoodDir(t *testing.T) { - dir := t.TempDir() - - err := ioutil.WriteFile(filepath.Join(dir, "README"), []byte("not a hook"), 0644) - if err != nil { - t.Fatal(err) - } - - jsonPath := filepath.Join(dir, "a.json") - err = ioutil.WriteFile(jsonPath, []byte(fmt.Sprintf("{\"version\": \"1.0.0\", \"hook\": {\"path\": \"%s\"}, \"when\": {\"always\": true}, \"stages\": [\"prestart\"]}", path)), 0644) - if err != nil { - t.Fatal(err) - } - - hooks := map[string]*current.Hook{} - err = ReadDir(dir, []string{}, hooks) - if err != nil { - t.Fatal(err) - } - - always := true - assert.Equal(t, map[string]*current.Hook{ - "a.json": { - Version: current.Version, - Hook: rspec.Hook{ - Path: path, - }, - When: current.When{ - Always: &always, - }, - Stages: []string{"prestart"}, - }, - }, hooks) -} - -func TestUnknownDir(t *testing.T) { - hooks := map[string]*current.Hook{} - err := ReadDir(filepath.Join("does", "not", "exist"), []string{}, hooks) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^open does/not/exist: no such file or directory$", err.Error()) - if !os.IsNotExist(err) { - t.Fatal("opaque wrapping for not-exist errors") - } -} - -func TestBadDir(t *testing.T) { - dir := t.TempDir() - - jsonPath := filepath.Join(dir, "a.json") - err := ioutil.WriteFile(jsonPath, []byte("{\"version\": \"-1\"}"), 0644) - if err != nil { - t.Fatal(err) - } - - hooks := map[string]*current.Hook{} - err = ReadDir(dir, []string{}, hooks) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^parsing hook \"[^\"]*a.json\": unrecognized hook version: \"-1\"$", err.Error()) -} - -func TestHookExecutableDoesNotExit(t *testing.T) { - dir := t.TempDir() - - jsonPath := filepath.Join(dir, "hook.json") - err := ioutil.WriteFile(jsonPath, []byte("{\"version\": \"1.0.0\", \"hook\": {\"path\": \"/does/not/exist\"}, \"when\": {\"always\": true}, \"stages\": [\"prestart\"]}"), 0644) - if err != nil { - t.Fatal(err) - } - - hooks := map[string]*current.Hook{} - err = ReadDir(dir, []string{}, hooks) - if err == nil { - t.Fatal("unexpected success") - } - assert.Regexp(t, "^stat /does/not/exist: no such file or directory$", err.Error()) -} diff --git a/pkg/hooks/version.go b/pkg/hooks/version.go deleted file mode 100644 index 637d8e2f4..000000000 --- a/pkg/hooks/version.go +++ /dev/null @@ -1,6 +0,0 @@ -package hooks - -// version a structure for checking the version of a hook configuration. -type version struct { - Version string `json:"version"` -} -- cgit v1.2.3-54-g00ecf From f6d18ed41cb4ce0fd59090d2114f728d557638db Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Wed, 20 Jul 2022 12:36:02 +0200 Subject: fix goroutine leaks in events and logs backend When running a single podman logs this is not really important since we will exit when we finish reading the logs. However for the system service this is very important. Leaking goroutines will cause an increased memory and CPU ussage over time. Both the the event and log backend have goroutine leaks with both the file and journald drivers. The journald backend has the problem that journal.Wait(IndefiniteWait) will block until we get a new journald event. So when a client closes the connection the goroutine would still wait until there is a new journal entry. To fix this we just wait for a maximum of 5 seconds, after that we can check if the client connection was closed and exit correctly in this case. For the file backend we can fix this by waiting for either the log line or context cancel at the same time. Currently it would block waiting for new log lines and only check afterwards if the client closed the connection and thus hang forever if there are no new log lines. [NO NEW TESTS NEEDED] I am open to ideas how we can test memory leaks in CI. To test manually run a container like this: `podman run --log-driver $driver --name test -d alpine sh -c 'i=1; while [ "$i" -ne 1000 ]; do echo "line $i"; i=$((i + 1)); done; sleep inf'` where `$driver` can be either `journald` or `k8s-file`. Then start the podman system service and use: `curl -m 1 --output - --unix-socket $XDG_RUNTIME_DIR/podman/podman.sock -v 'http://d/containers/test/logs?follow=1&since=0&stderr=1&stdout=1' &>/dev/null` to get the logs from the API and then it closes the connection after 1 second. Now run the curl command several times and check the memory usage of the service. Fixes #14879 Signed-off-by: Paul Holzinger --- libpod/container_log.go | 14 ++++++++++---- libpod/container_log_linux.go | 9 +++++++-- libpod/events/journal_linux.go | 13 +++++++++++-- libpod/events/logfile.go | 26 ++++++++++---------------- 4 files changed, 38 insertions(+), 24 deletions(-) (limited to 'libpod') diff --git a/libpod/container_log.go b/libpod/container_log.go index a9e0fe065..c49b54eb1 100644 --- a/libpod/container_log.go +++ b/libpod/container_log.go @@ -10,6 +10,7 @@ import ( "github.com/containers/podman/v4/libpod/define" "github.com/containers/podman/v4/libpod/events" "github.com/containers/podman/v4/libpod/logs" + "github.com/nxadm/tail" "github.com/nxadm/tail/watch" "github.com/sirupsen/logrus" ) @@ -74,14 +75,19 @@ func (c *Container) readFromLogFile(ctx context.Context, options *logs.LogOption go func() { defer options.WaitGroup.Done() - - for line := range t.Lines { + var line *tail.Line + var ok bool + for { select { case <-ctx.Done(): // the consumer has cancelled + t.Kill(errors.New("hangup by client")) return - default: - // fallthrough + case line, ok = <-t.Lines: + if !ok { + // channel was closed + return + } } nll, err := logs.NewLogLine(line.Text) if err != nil { diff --git a/libpod/container_log_linux.go b/libpod/container_log_linux.go index 0686caed2..7e95f2449 100644 --- a/libpod/container_log_linux.go +++ b/libpod/container_log_linux.go @@ -178,8 +178,13 @@ func (c *Container) readFromJournal(ctx context.Context, options *logs.LogOption if !options.Follow || !containerCouldBeLogging { return } - // Sleep until something's happening on the journal. - journal.Wait(sdjournal.IndefiniteWait) + + // journal.Wait() is blocking, this would cause the goroutine to hang forever + // if no more journal entries are generated and thus if the client + // has closed the connection in the meantime to leak memory. + // Waiting only 5 seconds makes sure we can check if the client closed in the + // meantime at least every 5 seconds. + journal.Wait(5 * time.Second) continue } lastReadCursor = cursor diff --git a/libpod/events/journal_linux.go b/libpod/events/journal_linux.go index 0a0a768d0..16ef6504f 100644 --- a/libpod/events/journal_linux.go +++ b/libpod/events/journal_linux.go @@ -141,9 +141,18 @@ func (e EventJournalD) Read(ctx context.Context, options ReadOptions) error { if !options.Stream || (len(options.Until) > 0 && time.Now().After(untilTime)) { break } - t := sdjournal.IndefiniteWait + + // j.Wait() is blocking, this would cause the goroutine to hang forever + // if no more journal entries are generated and thus if the client + // has closed the connection in the meantime to leak memory. + // Waiting only 5 seconds makes sure we can check if the client closed in the + // meantime at least every 5 seconds. + t := 5 * time.Second if len(options.Until) > 0 { - t = time.Until(untilTime) + until := time.Until(untilTime) + if until < t { + t = until + } } _ = j.Wait(t) continue diff --git a/libpod/events/logfile.go b/libpod/events/logfile.go index 4dafd8600..c7dbf4850 100644 --- a/libpod/events/logfile.go +++ b/libpod/events/logfile.go @@ -108,23 +108,19 @@ func (e EventLogFile) Read(ctx context.Context, options ReadOptions) error { } }() } - funcDone := make(chan bool) - copy := true - go func() { - select { - case <-funcDone: - // Do nothing - case <-ctx.Done(): - copy = false - t.Kill(errors.New("hangup by client")) - } - }() - for line := range t.Lines { + var line *tail.Line + var ok bool + for { select { case <-ctx.Done(): // the consumer has cancelled + t.Kill(errors.New("hangup by client")) return nil - default: + case line, ok = <-t.Lines: + if !ok { + // channel was closed + return nil + } // fallthrough } @@ -138,12 +134,10 @@ func (e EventLogFile) Read(ctx context.Context, options ReadOptions) error { default: return fmt.Errorf("event type %s is not valid in %s", event.Type.String(), e.options.LogFilePath) } - if copy && applyFilters(event, filterMap) { + if applyFilters(event, filterMap) { options.EventChannel <- event } } - funcDone <- true - return nil } // String returns a string representation of the logger -- cgit v1.2.3-54-g00ecf From 3edaa174e8bc22f57818a69b284b7535dd99a657 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Thu, 21 Jul 2022 09:44:20 +0200 Subject: libpod: create /etc/passwd if missing create the /etc/passwd and /etc/group files if they are missing in the image. Closes: https://github.com/containers/podman/issues/14966 Signed-off-by: Giuseppe Scrivano --- libpod/container_internal_linux.go | 41 ++------------------------------------ test/e2e/run_passwd_test.go | 7 ++++++- 2 files changed, 8 insertions(+), 40 deletions(-) (limited to 'libpod') diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index 46e525add..a131ab367 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -2886,23 +2886,6 @@ func (c *Container) generatePasswdAndGroup() (string, string, error) { } } - // Next, check if the container even has a /etc/passwd or /etc/group. - // If it doesn't we don't want to create them ourselves. - if needPasswd { - exists, err := c.checkFileExistsInRootfs("/etc/passwd") - if err != nil { - return "", "", err - } - needPasswd = exists - } - if needGroup { - exists, err := c.checkFileExistsInRootfs("/etc/group") - if err != nil { - return "", "", err - } - needGroup = exists - } - // If we don't need a /etc/passwd or /etc/group at this point we can // just return. if !needPasswd && !needGroup { @@ -2947,7 +2930,7 @@ func (c *Container) generatePasswdAndGroup() (string, string, error) { return "", "", fmt.Errorf("error looking up location of container %s /etc/passwd: %w", c.ID(), err) } - f, err := os.OpenFile(containerPasswd, os.O_APPEND|os.O_WRONLY, 0600) + f, err := os.OpenFile(containerPasswd, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0600) if err != nil { return "", "", fmt.Errorf("container %s: %w", c.ID(), err) } @@ -2993,7 +2976,7 @@ func (c *Container) generatePasswdAndGroup() (string, string, error) { return "", "", fmt.Errorf("error looking up location of container %s /etc/group: %w", c.ID(), err) } - f, err := os.OpenFile(containerGroup, os.O_APPEND|os.O_WRONLY, 0600) + f, err := os.OpenFile(containerGroup, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0600) if err != nil { return "", "", fmt.Errorf("container %s: %w", c.ID(), err) } @@ -3112,26 +3095,6 @@ func (c *Container) cleanupOverlayMounts() error { return overlay.CleanupContent(c.config.StaticDir) } -// Check if a file exists at the given path in the container's root filesystem. -// Container must already be mounted for this to be used. -func (c *Container) checkFileExistsInRootfs(file string) (bool, error) { - checkPath, err := securejoin.SecureJoin(c.state.Mountpoint, file) - if err != nil { - return false, fmt.Errorf("cannot create path to container %s file %q: %w", c.ID(), file, err) - } - stat, err := os.Stat(checkPath) - if err != nil { - if os.IsNotExist(err) { - return false, nil - } - return false, fmt.Errorf("container %s: %w", c.ID(), err) - } - if stat.IsDir() { - return false, nil - } - return true, nil -} - // Creates and mounts an empty dir to mount secrets into, if it does not already exist func (c *Container) createSecretMountDir() error { src := filepath.Join(c.state.RunDir, "/run/secrets") diff --git a/test/e2e/run_passwd_test.go b/test/e2e/run_passwd_test.go index 411e12218..20a6ee3b1 100644 --- a/test/e2e/run_passwd_test.go +++ b/test/e2e/run_passwd_test.go @@ -66,10 +66,15 @@ RUN rm -f /etc/passwd /etc/shadow /etc/group USER 1000`, ALPINE) imgName := "testimg" podmanTest.BuildImage(dockerfile, imgName, "false") - session := podmanTest.Podman([]string{"run", "--rm", imgName, "ls", "/etc/"}) + session := podmanTest.Podman([]string{"run", "--passwd=false", "--rm", imgName, "ls", "/etc/"}) session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) Expect(session.OutputToString()).To(Not(ContainSubstring("passwd"))) + + // test that the /etc/passwd file is created + session = podmanTest.Podman([]string{"run", "--rm", "--user", "0:0", imgName, "ls", "/etc/passwd"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) }) It("podman run with no user specified does not change --group specified", func() { -- cgit v1.2.3-54-g00ecf From 772e883f83a46bc113c1ed26aea29bf5dcf5675a Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Fri, 22 Jul 2022 13:49:49 +0200 Subject: container wait: improve error message Improve the error message when looking up the exit code of a container. The state of the container may help us track down #14859 which flakes rarely and is impossible to reproduce on my machine. [NO NEW TESTS NEEDED] Signed-off-by: Valentin Rothberg --- libpod/container_api.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'libpod') diff --git a/libpod/container_api.go b/libpod/container_api.go index 742eb6d3e..2ff4bfe08 100644 --- a/libpod/container_api.go +++ b/libpod/container_api.go @@ -555,7 +555,7 @@ func (c *Container) WaitForExit(ctx context.Context, pollInterval time.Duration) // The container never ran. return true, 0, nil } - return true, -1, err + return true, -1, fmt.Errorf("%w (container in state %s)", err, c.state.State) } return true, exitCode, nil -- cgit v1.2.3-54-g00ecf From 976f818f1ea0779da37a3e100056df69b306df1f Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Tue, 19 Jul 2022 17:41:12 +0200 Subject: libpod: do not lock all containers on pod rm do not attempt to lock all containers on pod rm since it can cause deadlocks when other podman cleanup processes are attempting to lock the same containers in a different order. [NO NEW TESTS NEEDED] Closes: https://github.com/containers/podman/issues/14929 Signed-off-by: Giuseppe Scrivano --- libpod/runtime_pod_linux.go | 98 ++++++++++----------------------------------- 1 file changed, 22 insertions(+), 76 deletions(-) (limited to 'libpod') diff --git a/libpod/runtime_pod_linux.go b/libpod/runtime_pod_linux.go index 75ff24e41..b22a0dbb6 100644 --- a/libpod/runtime_pod_linux.go +++ b/libpod/runtime_pod_linux.go @@ -7,7 +7,6 @@ import ( "context" "errors" "fmt" - "os" "path" "path/filepath" "strings" @@ -18,7 +17,6 @@ import ( "github.com/containers/podman/v4/libpod/events" "github.com/containers/podman/v4/pkg/rootless" "github.com/containers/podman/v4/pkg/specgen" - runcconfig "github.com/opencontainers/runc/libcontainer/configs" "github.com/sirupsen/logrus" ) @@ -214,83 +212,37 @@ func (r *Runtime) removePod(ctx context.Context, p *Pod, removeCtrs, force bool, return fmt.Errorf("pod %s contains containers and cannot be removed: %w", p.ID(), define.ErrCtrExists) } - // Go through and lock all containers so we can operate on them all at - // once. - // First loop also checks that we are ready to go ahead and remove. - containersLocked := true + ctrNamedVolumes := make(map[string]*ContainerNamedVolume) + + var removalErr error for _, ctr := range ctrs { - ctrLock := ctr.lock - ctrLock.Lock() - defer func() { - if containersLocked { + err := func() error { + ctrLock := ctr.lock + ctrLock.Lock() + defer func() { ctrLock.Unlock() - } - }() + }() - // If we're force-removing, no need to check status. - if force { - continue - } - - // Sync all containers - if err := ctr.syncContainer(); err != nil { - return err - } - - // Ensure state appropriate for removal - if err := ctr.checkReadyForRemoval(); err != nil { - return fmt.Errorf("pod %s has containers that are not ready to be removed: %w", p.ID(), err) - } - } - - // We're going to be removing containers. - // If we are Cgroupfs cgroup driver, to avoid races, we need to hit - // the pod and conmon Cgroups with a PID limit to prevent them from - // spawning any further processes (particularly cleanup processes) which - // would prevent removing the Cgroups. - if p.runtime.config.Engine.CgroupManager == config.CgroupfsCgroupsManager { - // Get the conmon Cgroup - conmonCgroupPath := filepath.Join(p.state.CgroupPath, "conmon") - conmonCgroup, err := cgroups.Load(conmonCgroupPath) - if err != nil && err != cgroups.ErrCgroupDeleted && err != cgroups.ErrCgroupV1Rootless { - logrus.Errorf("Retrieving pod %s conmon cgroup %s: %v", p.ID(), conmonCgroupPath, err) - } - - // New resource limits - resLimits := new(runcconfig.Resources) - resLimits.PidsLimit = 1 // Inhibit forks with very low pids limit - - // Don't try if we failed to retrieve the cgroup - if err == nil { - if err := conmonCgroup.Update(resLimits); err != nil && !os.IsNotExist(err) { - logrus.Warnf("Error updating pod %s conmon cgroup PID limit: %v", p.ID(), err) + if err := ctr.syncContainer(); err != nil { + return err } - } - } - - var removalErr error - ctrNamedVolumes := make(map[string]*ContainerNamedVolume) + for _, vol := range ctr.config.NamedVolumes { + ctrNamedVolumes[vol.Name] = vol + } - // Second loop - all containers are good, so we should be clear to - // remove. - for _, ctr := range ctrs { - // Remove the container. - // Do NOT remove named volumes. Instead, we're going to build a - // list of them to be removed at the end, once the containers - // have been removed by RemovePodContainers. - for _, vol := range ctr.config.NamedVolumes { - ctrNamedVolumes[vol.Name] = vol - } + return r.removeContainer(ctx, ctr, force, false, true, timeout) + }() - if err := r.removeContainer(ctx, ctr, force, false, true, timeout); err != nil { - if removalErr == nil { - removalErr = err - } else { - logrus.Errorf("Removing container %s from pod %s: %v", ctr.ID(), p.ID(), err) - } + if removalErr == nil { + removalErr = err + } else { + logrus.Errorf("Removing container %s from pod %s: %v", ctr.ID(), p.ID(), err) } } + if removalErr != nil { + return removalErr + } // Clear infra container ID before we remove the infra container. // There is a potential issue if we don't do that, and removal is @@ -326,12 +278,6 @@ func (r *Runtime) removePod(ctx context.Context, p *Pod, removeCtrs, force bool, } } - // let's unlock the containers so if there is any cleanup process, it can terminate its execution - for _, ctr := range ctrs { - ctr.lock.Unlock() - } - containersLocked = false - // Remove pod cgroup, if present if p.state.CgroupPath != "" { logrus.Debugf("Removing pod cgroup %s", p.state.CgroupPath) -- cgit v1.2.3-54-g00ecf