Skip to content

Commit

Permalink
Ensure pod infra containers have an exit command
Browse files Browse the repository at this point in the history
Most Libpod containers are made via `pkg/specgen/generate` which
includes code to generate an appropriate exit command which will
handle unmounting the container's storage, cleaning up the
container's network, etc. There is one notable exception: pod
infra containers, which are made entirely within Libpod and do
not touch pkg/specgen. As such, no cleanup process, network never
cleaned up, bad things can happen.

There is good news, though - it's not that difficult to add this,
and it's done in this PR. Generally speaking, we don't allow
passing options directly to the infra container at create time,
but we do (optionally) proxy a pre-approved set of options into
it when we create it. Add ExitCommand to these options, and set
it at time of pod creation using the same code we use to generate
exit commands for normal containers.

Fixes #7103

Signed-off-by: Matthew Heon <mheon@redhat.com>

<MH: Fixed cherry-pick conflicts>

Signed-off-by: Matthew Heon <mheon@redhat.com>
  • Loading branch information
mheon committed Aug 21, 2020
1 parent e61d4c1 commit 881af31
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 6 deletions.
20 changes: 20 additions & 0 deletions libpod/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -2036,3 +2036,23 @@ func WithPodHostNetwork() PodCreateOption {
return nil
}
}

// WithPodInfraExitCommand sets an exit command for the pod's infra container.
// Semantics are identical to WithExitCommand() above - the ID of the container
// will be appended to the end of the provided command (note that this will
// specifically be the ID of the infra container *and not the pod's id*.
func WithPodInfraExitCommand(exitCmd []string) PodCreateOption {
return func(pod *Pod) error {
if pod.valid {
return define.ErrPodFinalized
}

if !pod.config.InfraContainer.HasInfraContainer {
return errors.Wrapf(define.ErrInvalidArg, "cannot configure pod infra container exit command as no infra container is being created")
}

pod.config.InfraContainer.ExitCommand = exitCmd

return nil
}
}
11 changes: 10 additions & 1 deletion libpod/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,15 @@ type podState struct {
InfraContainerID string
}

// InfraContainerConfig is the configuration for the pod's infra container
// InfraContainerConfig is the configuration for the pod's infra container.
// Generally speaking, these are equivalent to container configuration options
// you will find in container_config.go (and even named identically), save for
// HasInfraContainer (which determines if an infra container is even created -
// if it is false, no other options in this struct will be used) and HostNetwork
// (this involves the created OCI spec, and as such is not represented directly
// in container_config.go).
// Generally speaking, aside from those two exceptions, these options will set
// the equivalent field in the container's configuration.
type InfraContainerConfig struct {
ConmonPidFile string `json:"conmonPidFile"`
HasInfraContainer bool `json:"makeInfraContainer"`
Expand All @@ -96,6 +104,7 @@ type InfraContainerConfig struct {
UseImageHosts bool `json:"useImageHosts,omitempty"`
HostAdd []string `json:"hostsAdd,omitempty"`
Networks []string `json:"networks,omitempty"`
ExitCommand []string `json:"exitCommand,omitempty"`
}

// ID retrieves the pod's ID
Expand Down
6 changes: 6 additions & 0 deletions libpod/runtime_pod_infra_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ func (r *Runtime) makeInfraContainer(ctx context.Context, p *Pod, imgName, rawIm
return nil, errors.Wrapf(err, "error removing network namespace from pod %s infra container", p.ID())
}

// For each option in InfraContainerConfig - if set, pass into
// the infra container we're creating with the appropriate
// With... option.
if p.config.InfraContainer.StaticIP != nil {
options = append(options, WithStaticIP(p.config.InfraContainer.StaticIP))
}
Expand All @@ -107,6 +110,9 @@ func (r *Runtime) makeInfraContainer(ctx context.Context, p *Pod, imgName, rawIm
if len(p.config.InfraContainer.HostAdd) > 0 {
options = append(options, WithHosts(p.config.InfraContainer.HostAdd))
}
if len(p.config.InfraContainer.ExitCommand) > 0 {
options = append(options, WithExitCommand(p.config.InfraContainer.ExitCommand))
}
}

g.SetRootReadonly(true)
Expand Down
6 changes: 3 additions & 3 deletions pkg/bindings/test/pods_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ var _ = Describe("Podman pods", func() {
Expect(response.State).To(Equal(define.PodStateExited))
for _, i := range response.Containers {
Expect(define.StringToContainerStatus(i.State)).
To(Equal(define.ContainerStateStopped))
To(Equal(define.ContainerStateExited))
}

// Stop an already stopped pod
Expand Down Expand Up @@ -309,7 +309,7 @@ var _ = Describe("Podman pods", func() {
Expect(response.State).To(Equal(define.PodStateExited))
for _, i := range response.Containers {
Expect(define.StringToContainerStatus(i.State)).
To(Equal(define.ContainerStateStopped))
To(Equal(define.ContainerStateExited))
}
_, err = pods.Stop(bt.conn, newpod2, nil)
Expect(err).To(BeNil())
Expand All @@ -318,7 +318,7 @@ var _ = Describe("Podman pods", func() {
Expect(response.State).To(Equal(define.PodStateExited))
for _, i := range response.Containers {
Expect(define.StringToContainerStatus(i.State)).
To(Equal(define.ContainerStateStopped))
To(Equal(define.ContainerStateExited))
}
_, err = pods.Prune(bt.conn)
Expect(err).To(BeNil())
Expand Down
16 changes: 14 additions & 2 deletions pkg/specgen/generate/pod_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@ func MakePod(p *specgen.PodSpecGenerator, rt *libpod.Runtime) (*libpod.Pod, erro
if err := p.Validate(); err != nil {
return nil, err
}
options, err := createPodOptions(p)
options, err := createPodOptions(p, rt)
if err != nil {
return nil, err
}
return rt.NewPod(context.Background(), options...)
}

func createPodOptions(p *specgen.PodSpecGenerator) ([]libpod.PodCreateOption, error) {
func createPodOptions(p *specgen.PodSpecGenerator, rt *libpod.Runtime) ([]libpod.PodCreateOption, error) {
var (
options []libpod.PodCreateOption
)
Expand All @@ -31,6 +31,18 @@ func createPodOptions(p *specgen.PodSpecGenerator) ([]libpod.PodCreateOption, er
return nil, err
}
options = append(options, nsOptions...)

// Make our exit command
storageConfig := rt.StorageConfig()
runtimeConfig, err := rt.GetConfig()
if err != nil {
return nil, err
}
exitCommand, err := CreateExitCommandArgs(storageConfig, runtimeConfig, logrus.IsLevelEnabled(logrus.DebugLevel), false, false)
if err != nil {
return nil, errors.Wrapf(err, "error creating infra container exit command")
}
options = append(options, libpod.WithPodInfraExitCommand(exitCommand))
}
if len(p.CgroupParent) > 0 {
options = append(options, libpod.WithPodCgroupParent(p.CgroupParent))
Expand Down

0 comments on commit 881af31

Please sign in to comment.