-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Add support for runtime flags in containers.conf #27310
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
base: main
Are you sure you want to change the base?
Conversation
Added a way to define default runtime flags in config. Fixes: https://github.com/containers/common/issues/715 Default runtime flags should be defined as shown below: [engine.runtimes_flags] runsc = [ "net-raw", ] crun = [ "debug", ] Signed-off-by: Rosvaldas Atstupėnas <[email protected]>
[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 |
f62fd54
to
f7a60eb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps an entry in containers.conf (https://github.com/containers/container-libs/blob/main/common/pkg/config/containers.conf) and the associated man page would be helpful.
They are already documented there https://github.com/containers/container-libs/blob/c4c5e00ad22d4280b27b45fe274a24e72b938fb6/common/pkg/config/containers.conf#L875-L878 |
test/system/030-run.bats
Outdated
containersConf=$PODMAN_TMPDIR/containers.conf | ||
cat >$containersConf <<EOF | ||
[engine] | ||
runtime="crun" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ricardobranco777 Do you have crun installed on your test system? I think this must use $(podman_runtime)
instead to use the proper test runtime so the right error can be triggered
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ricardobranco777 Do you have crun installed on your test system?
Both crun & runc can be installed in the openQA SUT VM.
I think this must use
$(podman_runtime)
instead to use the proper test runtime so the right error can be triggered
Agree. This test has passed on systems without crun, I guess because podman picks runc if crun is not available..
[engine.runtimes_flags] | ||
crun = [ | ||
"invalidflag", | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit testing wise it would be nice to define some other runtimes and ensure flags are indeed only set for each specific runtime.
Though given the "urgency" for this I am fine with the test as is minus the other comment
Signed-off-by: Matt Heon <[email protected]>
f7a60eb
to
43ff7a4
Compare
Tests are green, this is ready |
Replaces #26892
Does this PR introduce a user-facing change?