Skip to content
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.

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

Begin adding support for multiple OCI runtimes #3378

Merged
merged 6 commits into from
Jun 21, 2019

Conversation

mheon
Copy link
Member

@mheon mheon commented Jun 19, 2019

Containers already remember the runtime they were created with in their configs. Now, let's allow them to request that runtime on subsequent runs of Podman. This greatly improves support for running multiple OCI runtimes at once - containers started on crun or kata will continue to use those runtimes even if the default runtime swaps to runc, for example.

Allow Podman containers to request to use a specific OCI runtime
if multiple runtimes are configured. This is the first step to
properly supporting containers in a multi-runtime environment.

The biggest changes are that all OCI runtimes are now initialized
when Podman creates its runtime, and containers now use the
runtime requested in their configuration (instead of always the
default runtime).

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
We may want to ship configurations including more than one
runtime configuration - for example, crun and runc and kata, all
configured. However, we don't want to make these extra runtimes
hard requirements, so let's not fatally error when we can't find
their executables.

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mheon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L labels Jun 19, 2019
Signed-off-by: Matthew Heon <matthew.heon@pm.me>
@mheon
Copy link
Member Author

mheon commented Jun 19, 2019

Step 2 from here is add support for probing runtime features on those runtimes, so I can be able to detect whether a runtime supports something (for example, opencontainers/runc#2062 - which we need to prevent systemd from closing down our scopes immediately on shutdown request - but probably isn't as relevant to, say, kata).

@mheon
Copy link
Member Author

mheon commented Jun 20, 2019

The problem in CI is the --runtime flag, which appears to be setting the default OCI runtime to a path, instead of a name. I'll poke around tomorrow and get it resolved.

ctr.ociRuntime = s.runtime.defaultOCIRuntime
} else {
ociRuntime, ok := s.runtime.ociRuntimes[ctr.config.OCIRuntime]
if !ok {
Copy link
Member

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

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

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

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

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.

This is done by the --runtime flag, and as such, by all our CI.

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
Try and locate the right runtime by using the basename of the
path.

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
@mheon
Copy link
Member Author

mheon commented Jun 20, 2019

[+0750s] Error: requested OCI runtime /usr/bin/runc is not available: invalid argument

Do we not have runc in the test images? What's going on here?

@mheon
Copy link
Member Author

mheon commented Jun 20, 2019

Oh wait, I got it. Working on it now.

Use name of the default runtime, instead of the OCIRuntime config
option, which may include a full path.

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
@mheon
Copy link
Member Author

mheon commented Jun 20, 2019

Prow appears to be blowing up. Infra problems?

@mheon
Copy link
Member Author

mheon commented Jun 20, 2019

/retest

@mheon
Copy link
Member Author

mheon commented Jun 20, 2019

Tests going green, ready for review

@rhatdan
Copy link
Member

rhatdan commented Jun 21, 2019

LGTM
@giuseppe PTAL

@giuseppe
Copy link
Member

tested locally

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 21, 2019
@openshift-merge-robot openshift-merge-robot merged commit 7d8aba9 into containers:master Jun 21, 2019
This was referenced Jun 21, 2019
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 26, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants