-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
build: pass down the cgroup manager to buildah #3944
build: pass down the cgroup manager to buildah #3944
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: giuseppe 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 |
2ca3565
to
42f2acf
Compare
} | ||
if conf.CgroupManager == "systemd" { | ||
runtimeFlags = append(runtimeFlags, "--systemd") | ||
} |
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.
I'm not seeing this --systemd
flag in the buildah bud
or the podman build
man pages. Should it be added?
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.
this option is passed down to the OCI runtime AFAICS, so Buildah doesn't need to know about it
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.
OK, let me reask. Would a user ever have to type --systemd
? If so, then we need to document it in a man page somewhere. If it's all internal, then no need.
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.
We have not had to deal with it yet, but I think adding this to buildah would be useful.
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.
an user could force --runtime-flag systemd
for buildah but I think that is an internal detail and the current documentation covers it
/test images |
LGTM |
Build failures in podman-remote |
Changes LGTM otherwise though |
42f2acf
to
f4affaf
Compare
I am not sure what is the proper fix for it, I've pushed a new version that at least builds now |
f4affaf
to
afe70e7
Compare
Pass down the cgroup manager to use to buildah. Closes: containers#3938 Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
afe70e7
to
06f94be
Compare
/lgtm |
Pass down the cgroup manager to use to buildah.
Closes: #3938
Signed-off-by: Giuseppe Scrivano gscrivan@redhat.com