Skip to content

Begin adding support for multiple OCI runtimes #3378

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
openshift-merge-robot merged 6 commits into from
Jun 21, 2019
Merged

Begin adding support for multiple OCI runtimes #3378

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
92bae8d
Begin adding support for multiple OCI runtimes
mheon Jun 19, 2019
fa0e48f
Make a missing OCI runtime nonfatal
mheon Jun 19, 2019
3d78085
Print container's OCI runtime in `inspect`
mheon Jun 19, 2019
7377870
Properly handle OCI runtime being set to a path
mheon Jun 20, 2019
df43bfe
Handle containers whose OCIRuntime fields are paths
mheon Jun 20, 2019
2ee2404
Properly initialize container OCI runtime
mheon Jun 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions libpod/boltdb_state_internal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package libpod

import (
"bytes"
"path/filepath"
"runtime"
"strings"

Expand Down Expand Up @@ -304,6 +305,23 @@ func (s *BoltState) getContainerFromDB(id []byte, ctr *Container, ctrsBkt *bolt.
}
ctr.lock = lock

if ctr.config.OCIRuntime == "" {
ctr.ociRuntime = s.runtime.defaultOCIRuntime
} else {
// Handle legacy containers which might use a literal path for
// their OCI runtime name.
runtimeName := ctr.config.OCIRuntime
if strings.HasPrefix(runtimeName, "/") {
runtimeName = filepath.Base(runtimeName)
}

ociRuntime, ok := s.runtime.ociRuntimes[runtimeName]
if !ok {
Copy link
Member

@giuseppe giuseppe Jun 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we support specifying the runtime by an absolute path, we needed it to avoid a breaking change. When the runtime starts with / we use directly the specified path without any lookup. Should we take care of this case here and skip the error when ctr.config.OCIRuntime[0] == '/' ?

Copy link
Member Author

@mheon mheon Jun 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be fixed

Copy link
Member Author

@mheon mheon Jun 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, didn't quite catch the case where older containers might be stored with paths starting with a /... For that, I think we can try grabbing the basename and looking up the runtime by that.

Copy link
Member Author

@mheon mheon Jun 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, really fixed now

Copy link
Member

@giuseppe giuseppe Jun 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that is a good fix, thanks.

return errors.Wrapf(ErrInternal, "container %s was created with OCI runtime %s, but that runtime is not available in the current configuration", ctr.ID(), ctr.config.OCIRuntime)
}
ctr.ociRuntime = ociRuntime
}

ctr.runtime = s.runtime
ctr.valid = valid

Expand Down
9 changes: 5 additions & 4 deletions libpod/container.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,10 @@ type Container struct {
// Functions called on a batched container will not lock or sync
batched bool

valid bool
lock lock.Locker
runtime *Runtime
valid bool
lock lock.Locker
runtime *Runtime
ociRuntime *OCIRuntime

rootlessSlirpSyncR *os.File
rootlessSlirpSyncW *os.File
Expand Down Expand Up @@ -789,7 +790,7 @@ func (c *Container) LogDriver() string {

// RuntimeName returns the name of the runtime
func (c *Container) RuntimeName() string {
return c.runtime.ociRuntime.name
return c.config.OCIRuntime
}

// Runtime spec accessors
Expand Down
8 changes: 4 additions & 4 deletions libpod/container_api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ func (c *Container) Kill(signal uint) error {
}

defer c.newContainerEvent(events.Kill)
if err := c.runtime.ociRuntime.killContainer(c, signal); err != nil {
if err := c.ociRuntime.killContainer(c, signal); err != nil {
return err
}

Expand Down Expand Up @@ -280,7 +280,7 @@ func (c *Container) Exec(tty, privileged bool, env, cmd []string, user, workDir

logrus.Debugf("Creating new exec session in container %s with session id %s", c.ID(), sessionID)

execCmd, err := c.runtime.ociRuntime.execContainer(c, cmd, capList, env, tty, workDir, hostUser, sessionID, streams, preserveFDs)
execCmd, err := c.ociRuntime.execContainer(c, cmd, capList, env, tty, workDir, hostUser, sessionID, streams, preserveFDs)
if err != nil {
return errors.Wrapf(err, "error exec %s", c.ID())
}
Expand Down Expand Up @@ -658,7 +658,7 @@ func (c *Container) Sync() error {
(c.state.State != ContainerStateConfigured) &&
(c.state.State != ContainerStateExited) {
oldState := c.state.State
if err := c.runtime.ociRuntime.updateContainerStatus(c, true); err != nil {
if err := c.ociRuntime.updateContainerStatus(c, true); err != nil {
return err
}
// Only save back to DB if state changed
Expand Down Expand Up @@ -715,7 +715,7 @@ func (c *Container) Refresh(ctx context.Context) error {
if len(c.state.ExecSessions) > 0 {
logrus.Infof("Killing %d exec sessions in container %s. They will not be restored after refresh.",
len(c.state.ExecSessions), c.ID())
if err := c.runtime.ociRuntime.execStopContainer(c, c.config.StopTimeout); err != nil {
if err := c.ociRuntime.execStopContainer(c, c.config.StopTimeout); err != nil {
return err
}
}
Expand Down
4 changes: 2 additions & 2 deletions libpod/container_commit.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,11 @@ func (c *Container) Commit(ctx context.Context, destImage string, options Contai
}

if c.state.State == ContainerStateRunning && options.Pause {
if err := c.runtime.ociRuntime.pauseContainer(c); err != nil {
if err := c.ociRuntime.pauseContainer(c); err != nil {
return nil, errors.Wrapf(err, "error pausing container %q", c.ID())
}
defer func() {
if err := c.runtime.ociRuntime.unpauseContainer(c); err != nil {
if err := c.ociRuntime.unpauseContainer(c); err != nil {
logrus.Errorf("error unpausing container %q: %v", c.ID(), err)
}
}()
Expand Down
2 changes: 2 additions & 0 deletions libpod/container_inspect.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ type InspectContainerData struct {
HostsPath string `json:"HostsPath"`
StaticDir string `json:"StaticDir"`
OCIConfigPath string `json:"OCIConfigPath,omitempty"`
OCIRuntime string `json:"OCIRuntime,omitempty"`
LogPath string `json:"LogPath"`
ConmonPidFile string `json:"ConmonPidFile"`
Name string `json:"Name"`
Expand Down Expand Up @@ -274,6 +275,7 @@ func (c *Container) getContainerInspectData(size bool, driverData *driver.Data)
HostsPath: hostsPath,
StaticDir: config.StaticDir,
LogPath: config.LogPath,
OCIRuntime: config.OCIRuntime,
ConmonPidFile: config.ConmonPidFile,
Name: config.Name,
RestartCount: int32(runtimeInfo.RestartCount),
Expand Down
24 changes: 12 additions & 12 deletions libpod/container_internal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ func (c *Container) CheckpointPath() string {

// AttachSocketPath retrieves the path of the container's attach socket
func (c *Container) AttachSocketPath() string {
return filepath.Join(c.runtime.ociRuntime.socketsDir, c.ID(), "attach")
return filepath.Join(c.ociRuntime.socketsDir, c.ID(), "attach")
}

// Get PID file path for a container's exec session
Expand All @@ -138,7 +138,7 @@ func (c *Container) execPidPath(sessionID string) string {

// exitFilePath gets the path to the container's exit file
func (c *Container) exitFilePath() string {
return filepath.Join(c.runtime.ociRuntime.exitsDir, c.ID())
return filepath.Join(c.ociRuntime.exitsDir, c.ID())
}

// Wait for the container's exit file to appear.
Expand All @@ -164,7 +164,7 @@ func (c *Container) waitForExitFileAndSync() error {
return err
}

if err := c.runtime.ociRuntime.updateContainerStatus(c, false); err != nil {
if err := c.ociRuntime.updateContainerStatus(c, false); err != nil {
return err
}

Expand Down Expand Up @@ -299,7 +299,7 @@ func (c *Container) syncContainer() error {
(c.state.State != ContainerStateExited) {
oldState := c.state.State
// TODO: optionally replace this with a stat for the exit file
if err := c.runtime.ociRuntime.updateContainerStatus(c, false); err != nil {
if err := c.ociRuntime.updateContainerStatus(c, false); err != nil {
return err
}
// Only save back to DB if state changed
Expand Down Expand Up @@ -547,8 +547,8 @@ func (c *Container) removeConmonFiles() error {
// Instead of outright deleting the exit file, rename it (if it exists).
// We want to retain it so we can get the exit code of containers which
// are removed (at least until we have a workable events system)
exitFile := filepath.Join(c.runtime.ociRuntime.exitsDir, c.ID())
oldExitFile := filepath.Join(c.runtime.ociRuntime.exitsDir, fmt.Sprintf("%s-old", c.ID()))
exitFile := filepath.Join(c.ociRuntime.exitsDir, c.ID())
oldExitFile := filepath.Join(c.ociRuntime.exitsDir, fmt.Sprintf("%s-old", c.ID()))
if _, err := os.Stat(exitFile); err != nil {
if !os.IsNotExist(err) {
return errors.Wrapf(err, "error running stat on container %s exit file", c.ID())
Expand Down Expand Up @@ -866,7 +866,7 @@ func (c *Container) init(ctx context.Context, retainRetries bool) error {
}

// With the spec complete, do an OCI create
if err := c.runtime.ociRuntime.createContainer(c, c.config.CgroupParent, nil); err != nil {
if err := c.ociRuntime.createContainer(c, c.config.CgroupParent, nil); err != nil {
return err
}

Expand Down Expand Up @@ -1013,7 +1013,7 @@ func (c *Container) start() error {
logrus.Debugf("Starting container %s with command %v", c.ID(), c.config.Spec.Process.Args)
}

if err := c.runtime.ociRuntime.startContainer(c); err != nil {
if err := c.ociRuntime.startContainer(c); err != nil {
return err
}
logrus.Debugf("Started container %s", c.ID())
Expand All @@ -1038,7 +1038,7 @@ func (c *Container) start() error {
func (c *Container) stop(timeout uint) error {
logrus.Debugf("Stopping ctr %s (timeout %d)", c.ID(), timeout)

if err := c.runtime.ociRuntime.stopContainer(c, timeout); err != nil {
if err := c.ociRuntime.stopContainer(c, timeout); err != nil {
return err
}

Expand All @@ -1053,7 +1053,7 @@ func (c *Container) stop(timeout uint) error {

// Internal, non-locking function to pause a container
func (c *Container) pause() error {
if err := c.runtime.ociRuntime.pauseContainer(c); err != nil {
if err := c.ociRuntime.pauseContainer(c); err != nil {
return err
}

Expand All @@ -1066,7 +1066,7 @@ func (c *Container) pause() error {

// Internal, non-locking function to unpause a container
func (c *Container) unpause() error {
if err := c.runtime.ociRuntime.unpauseContainer(c); err != nil {
if err := c.ociRuntime.unpauseContainer(c); err != nil {
return err
}

Expand Down Expand Up @@ -1245,7 +1245,7 @@ func (c *Container) delete(ctx context.Context) (err error) {
span.SetTag("struct", "container")
defer span.Finish()

if err := c.runtime.ociRuntime.deleteContainer(c); err != nil {
if err := c.ociRuntime.deleteContainer(c); err != nil {
return errors.Wrapf(err, "error removing container %s from runtime", c.ID())
}

Expand Down
6 changes: 3 additions & 3 deletions libpod/container_internal_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,7 @@ func (c *Container) checkpointRestoreSupported() (err error) {
if !criu.CheckForCriu() {
return errors.Errorf("Checkpoint/Restore requires at least CRIU %d", criu.MinCriuVersion)
}
if !c.runtime.ociRuntime.featureCheckCheckpointing() {
if !c.ociRuntime.featureCheckCheckpointing() {
return errors.Errorf("Configured runtime does not support checkpoint/restore")
}
return nil
Expand Down Expand Up @@ -575,7 +575,7 @@ func (c *Container) checkpoint(ctx context.Context, options ContainerCheckpointO
return err
}

if err := c.runtime.ociRuntime.checkpointContainer(c, options); err != nil {
if err := c.ociRuntime.checkpointContainer(c, options); err != nil {
return err
}

Expand Down Expand Up @@ -769,7 +769,7 @@ func (c *Container) restore(ctx context.Context, options ContainerCheckpointOpti
if err := c.saveSpec(g.Spec()); err != nil {
return err
}
if err := c.runtime.ociRuntime.createContainer(c, c.config.CgroupParent, &options); err != nil {
if err := c.ociRuntime.createContainer(c, c.config.CgroupParent, &options); err != nil {
return err
}

Expand Down
10 changes: 5 additions & 5 deletions libpod/info.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,12 @@ func (r *Runtime) hostInfo() (map[string]interface{}, error) {
hostDistributionInfo := r.GetHostDistributionInfo()
info["Conmon"] = map[string]interface{}{
"path": r.conmonPath,
"package": r.ociRuntime.conmonPackage(),
"package": r.defaultOCIRuntime.conmonPackage(),
"version": conmonVersion,
}
info["OCIRuntime"] = map[string]interface{}{
"path": r.ociRuntime.path,
"package": r.ociRuntime.pathPackage(),
"path": r.defaultOCIRuntime.path,
"package": r.defaultOCIRuntime.pathPackage(),
"version": ociruntimeVersion,
}
info["Distribution"] = map[string]interface{}{
Expand Down Expand Up @@ -190,12 +190,12 @@ func (r *Runtime) GetConmonVersion() (string, error) {

// GetOCIRuntimePath returns the path to the OCI Runtime Path the runtime is using
func (r *Runtime) GetOCIRuntimePath() string {
return r.ociRuntimePath.Paths[0]
return r.defaultOCIRuntime.path
}

// GetOCIRuntimeVersion returns a string representation of the oci runtimes version
func (r *Runtime) GetOCIRuntimeVersion() (string, error) {
output, err := utils.ExecCmd(r.ociRuntimePath.Paths[0], "--version")
output, err := utils.ExecCmd(r.GetOCIRuntimePath(), "--version")
if err != nil {
return "", err
}
Expand Down
2 changes: 1 addition & 1 deletion libpod/networking_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ func (r *Runtime) setupRootlessNetNS(ctr *Container) (err error) {
defer syncW.Close()

havePortMapping := len(ctr.Config().PortMappings) > 0
apiSocket := filepath.Join(r.ociRuntime.tmpDir, fmt.Sprintf("%s.net", ctr.config.ID))
apiSocket := filepath.Join(ctr.ociRuntime.tmpDir, fmt.Sprintf("%s.net", ctr.config.ID))

cmdArgs := []string{}
if havePortMapping {
Expand Down
52 changes: 40 additions & 12 deletions libpod/oci.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,25 +75,53 @@ type ociError struct {
Msg string `json:"msg,omitempty"`
}

// Make a new OCI runtime with provided options
func newOCIRuntime(oruntime OCIRuntimePath, conmonPath string, conmonEnv []string, cgroupManager string, tmpDir string, logSizeMax int64, noPivotRoot bool, reservePorts bool, supportsJSON bool) (*OCIRuntime, error) {
// Make a new OCI runtime with provided options.
// The first path that points to a valid executable will be used.
func newOCIRuntime(name string, paths []string, conmonPath string, runtimeCfg *RuntimeConfig, supportsJSON bool) (*OCIRuntime, error) {
if name == "" {
return nil, errors.Wrapf(ErrInvalidArg, "the OCI runtime must be provided a non-empty name")
}

runtime := new(OCIRuntime)
runtime.name = oruntime.Name
runtime.path = oruntime.Paths[0]
runtime.name = name
runtime.conmonPath = conmonPath
runtime.conmonEnv = conmonEnv
runtime.cgroupManager = cgroupManager
runtime.tmpDir = tmpDir
runtime.logSizeMax = logSizeMax
runtime.noPivot = noPivotRoot
runtime.reservePorts = reservePorts

runtime.conmonEnv = runtimeCfg.ConmonEnvVars
runtime.cgroupManager = runtimeCfg.CgroupManager
runtime.tmpDir = runtimeCfg.TmpDir
runtime.logSizeMax = runtimeCfg.MaxLogSize
runtime.noPivot = runtimeCfg.NoPivotRoot
runtime.reservePorts = runtimeCfg.EnablePortReservation

// TODO: probe OCI runtime for feature and enable automatically if
// available.
runtime.supportsJSON = supportsJSON

foundPath := false
for _, path := range paths {
stat, err := os.Stat(path)
if err != nil {
if os.IsNotExist(err) {
continue
}
return nil, errors.Wrapf(err, "cannot stat %s", path)
}
if !stat.Mode().IsRegular() {
continue
}
foundPath = true
runtime.path = path
break
}
if !foundPath {
return nil, errors.Wrapf(ErrInvalidArg, "no valid executable found for OCI runtime %s", name)
}

runtime.exitsDir = filepath.Join(runtime.tmpDir, "exits")
runtime.socketsDir = filepath.Join(runtime.tmpDir, "socket")

if cgroupManager != CgroupfsCgroupsManager && cgroupManager != SystemdCgroupsManager {
return nil, errors.Wrapf(ErrInvalidArg, "invalid cgroup manager specified: %s", cgroupManager)
if runtime.cgroupManager != CgroupfsCgroupsManager && runtime.cgroupManager != SystemdCgroupsManager {
return nil, errors.Wrapf(ErrInvalidArg, "invalid cgroup manager specified: %s", runtime.cgroupManager)
}

// Create the exit files and attach sockets directories
Expand Down
2 changes: 1 addition & 1 deletion libpod/pod_api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ func (p *Pod) Kill(signal uint) (map[string]error, error) {
continue
}

if err := ctr.runtime.ociRuntime.killContainer(ctr, signal); err != nil {
if err := ctr.ociRuntime.killContainer(ctr, signal); err != nil {
ctr.lock.Unlock()
ctrErrors[ctr.ID()] = err
continue
Expand Down
Loading