Fix missing OCI Runtime #7126

mheon · 2020-07-28T20:31:47Z

Problem: when we create a container with --runtime=/PATH/TO/RUNTIME and then call other Podman commands without that flag, Podman may fail because it can't find the OCI runtime.

Solution: make containers remember the full path of the OCI runtime they were created with (if and only if they were made with an OCI runtime specified by full path) and re-initialize the OCI runtime on subsequently retrieving the container from the database (if and only if the runtime does not already exist).

mheon · 2020-07-28T20:34:25Z

This needs tests, but they can't be E2E tests (all of those specify a full --runtime with every invocation of Podman). I'll look into adding a system test.

TomSweeneyRedHat · 2020-07-28T23:07:32Z

libpod/boltdb_state_internal.go

+			// If the path starts with a / and exists, make a new
+			// OCI runtime for it using the full path.
+			if strings.HasPrefix(runtimeName, "/") {
+				if stat, err := os.Stat(runtimeName); err == nil && !stat.IsDir() {


do you need to worry about a symlink here?

TomSweeneyRedHat · 2020-07-28T23:08:04Z

@mheon all kinds of test unhappiness

rhatdan · 2020-08-01T12:21:22Z

LGTM

mheon · 2020-08-07T13:22:00Z

Rebased

openshift-ci-robot · 2020-08-07T13:22:02Z

[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:

~~OWNERS~~ [mheon]

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

mheon · 2020-08-10T14:18:25Z

Rebased, should go green

vrothberg

/lgtm hold

vrothberg · 2020-08-11T06:47:27Z

Restarted what looked like flakes

vrothberg · 2020-08-11T06:47:37Z

/lgtm hold

rhatdan · 2020-08-11T09:58:44Z

Looks like your tests are failing?

edsantiago · 2020-08-11T11:54:25Z

Failures are in podman-remote only, f31 and f32, both:

[+0097s] not ok 27 podman run : full path to --runtime is preserved
         ...
         # # podman-remote --url unix:/tmp/podman.G3zrQE inspect --format {{.OCIRuntime}} f0386ab7ed5f1aa0e68dfee9b56744d75d200c7bc6bdfa9fc04ad8c8a68fcc48
         # crun
         # #/vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
         # #|     FAIL: FIXME
         # #| expected: '/usr/bin/crun'
         # #|   actual: 'crun'
         # #\^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

mheon · 2020-08-12T18:02:32Z

Remote seems to completely ignore the --runtime option... I'm going to disable the test as remote.

vrothberg · 2020-09-07T13:52:49Z

@mheon can you rebase?

mheon · 2020-09-09T20:01:20Z

Added another commit that should (theoretically) properly handle --runtime for the podman-remote case.

rhatdan · 2020-09-11T19:24:40Z

@mheon need a rebase. Would like to get this in, to fix https://bugzilla.redhat.com/show_bug.cgi?id=1860176

mheon · 2020-09-11T20:06:17Z

Test failures are legitimate. I know what has to be done, but it'll have to wait until Monday, too busy trying to get 2.1.0-RC1 out the door today.

rhatdan · 2020-09-15T20:34:56Z

LGTM

github-actions · 2020-10-16T00:19:56Z

A friendly reminder that this PR had no activity for 30 days.

rhatdan · 2020-10-16T11:39:03Z

@mheon Can we get this Rebased and moved forward?

mheon · 2020-10-19T15:14:17Z

Rebased, let's see what broke

rhatdan · 2020-10-20T11:31:39Z

@mheon needs another rebase.

Say I start a container with the flag `--runtime /usr/local/sbin/crun`. I then stop the container, and restart it without the flag. We previously stored the runtime in use by a container only by basename when given a path, so the container only knows that it's using the `crun` OCI runtime - and on being restarted without the flag, it will use the system crun, not my special crun build. Using the full path as the name in these cases ensures we will still use the correct runtime, even on subsequent runs of Podman. Signed-off-by: Matthew Heon <[email protected]>

When an OCI runtime is given by full path, we need to ensure we use the same runtime on subsequent use. Unfortunately, users are often not considerate enough to use the same `--runtime` flag every time they invoke runtime - and if the runtime was not in containers.conf, that means we don't have it stored inn the libpod Runtime. Fortunately, since we have the full path, we can initialize the OCI runtime for use at the point where we pull the container from the database. Signed-off-by: Matthew Heon <[email protected]>

My patches to fix `--runtime /usr/bin/crun` being allowed to use a different version of the crun runtime revealed a problem: we were actually relying on that exact behavior in our E2E tests. We specified the runtime path as `/usr/bin/runc` for the Ubuntu tests, but that didn't exist, so Podman was actively looking for a different, usable runc binary and using that, instead of the path we explicitly hardcoded. Fixing the bug broke this, and thus broke the tests. Instead of hard-coding OCI runtime paths, swap to just using the runtime name, `runc` or `crun`, and letting Podman figure out where the runtime lives - it's quite good at that. This should un-break the tests and make them more durable. Signed-off-by: Matthew Heon <[email protected]>

Signed-off-by: Matthew Heon <[email protected]>

mheon · 2020-10-20T15:11:06Z

@containers/podman-maintainers This should be ready to merge.

jwhonce · 2020-10-20T15:19:01Z

/lgtm

openshift-ci-robot requested review from giuseppe and vrothberg July 28, 2020 20:31

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 28, 2020

TomSweeneyRedHat reviewed Jul 28, 2020

View reviewed changes

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 30, 2020

mheon force-pushed the fix_missing_ociruntime branch 2 times, most recently from b1465a5 to fe9461d Compare July 30, 2020 14:52

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 30, 2020

mheon mentioned this pull request Jul 31, 2020

Lower missing runtime from Errorf to Infof #7077

Closed

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 7, 2020

mheon force-pushed the fix_missing_ociruntime branch from fe9461d to 5e5e6f5 Compare August 7, 2020 13:21

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 7, 2020

mheon force-pushed the fix_missing_ociruntime branch 2 times, most recently from adf2e7d to a615b88 Compare August 10, 2020 14:18

mheon force-pushed the fix_missing_ociruntime branch from a615b88 to e565396 Compare August 10, 2020 15:30

vrothberg reviewed Aug 11, 2020

View reviewed changes

mheon force-pushed the fix_missing_ociruntime branch from e565396 to e30f9e1 Compare August 12, 2020 18:03

mheon force-pushed the fix_missing_ociruntime branch from e30f9e1 to 1f98838 Compare September 7, 2020 14:54

mheon force-pushed the fix_missing_ociruntime branch from c65d45b to c90ab7f Compare September 9, 2020 20:59

mheon force-pushed the fix_missing_ociruntime branch from c90ab7f to c8e13d8 Compare September 14, 2020 17:28

github-actions bot added the stale-pr label Oct 16, 2020

mheon force-pushed the fix_missing_ociruntime branch from c8e13d8 to a3373e5 Compare October 19, 2020 15:14

mheon force-pushed the fix_missing_ociruntime branch 3 times, most recently from 72af2f9 to eaea608 Compare October 19, 2020 17:16

mheon added 3 commits October 20, 2020 09:20

mheon force-pushed the fix_missing_ociruntime branch from 97305e6 to 07221d9 Compare October 20, 2020 13:21

Add a system test to verify --runtime is preserved

1b49333

Signed-off-by: Matthew Heon <[email protected]>

mheon force-pushed the fix_missing_ociruntime branch from 07221d9 to 1b49333 Compare October 20, 2020 13:24

openshift-ci-robot assigned jwhonce Oct 20, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 20, 2020

openshift-merge-robot merged commit 3668211 into containers:master Oct 20, 2020

edsantiago mentioned this pull request Oct 20, 2020

APIv2: /info is getting slower #8076

Closed

didier-durand mentioned this pull request Nov 28, 2020

podman run --runtime= --rm fails with with kata 2.0.1 (works fine with 1.12) kata-containers/kata-containers#1152

Closed

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 24, 2023

github-actions bot locked as resolved and limited conversation to collaborators Sep 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix missing OCI Runtime #7126

Fix missing OCI Runtime #7126

mheon commented Jul 28, 2020

mheon commented Jul 28, 2020

TomSweeneyRedHat Jul 28, 2020

TomSweeneyRedHat commented Jul 28, 2020

rhatdan commented Aug 1, 2020

mheon commented Aug 7, 2020

openshift-ci-robot commented Aug 7, 2020

mheon commented Aug 10, 2020

vrothberg left a comment

vrothberg commented Aug 11, 2020

vrothberg commented Aug 11, 2020

rhatdan commented Aug 11, 2020

edsantiago commented Aug 11, 2020

mheon commented Aug 12, 2020

vrothberg commented Sep 7, 2020

mheon commented Sep 9, 2020

rhatdan commented Sep 11, 2020

mheon commented Sep 11, 2020

rhatdan commented Sep 15, 2020

github-actions bot commented Oct 16, 2020

rhatdan commented Oct 16, 2020

mheon commented Oct 19, 2020

rhatdan commented Oct 20, 2020

mheon commented Oct 20, 2020

jwhonce commented Oct 20, 2020

Fix missing OCI Runtime #7126

Fix missing OCI Runtime #7126

Conversation

mheon commented Jul 28, 2020

mheon commented Jul 28, 2020

TomSweeneyRedHat Jul 28, 2020

Choose a reason for hiding this comment

TomSweeneyRedHat commented Jul 28, 2020

rhatdan commented Aug 1, 2020

mheon commented Aug 7, 2020

openshift-ci-robot commented Aug 7, 2020

mheon commented Aug 10, 2020

vrothberg left a comment

Choose a reason for hiding this comment

vrothberg commented Aug 11, 2020

vrothberg commented Aug 11, 2020

rhatdan commented Aug 11, 2020

edsantiago commented Aug 11, 2020

mheon commented Aug 12, 2020

vrothberg commented Sep 7, 2020

mheon commented Sep 9, 2020

rhatdan commented Sep 11, 2020

mheon commented Sep 11, 2020

rhatdan commented Sep 15, 2020

github-actions bot commented Oct 16, 2020

rhatdan commented Oct 16, 2020

mheon commented Oct 19, 2020

rhatdan commented Oct 20, 2020

mheon commented Oct 20, 2020

jwhonce commented Oct 20, 2020