Remove detection for scope properties, which have always been broken #1978
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The detection for scope properties (whether scope units support DefaultDependencies= or Delegate=) has always been broken, since systemd refuses to create scopes unless at least one PID is attached to it (and this has been so since scope units were introduced in systemd v205.) This can be seen in journal logs whenever a container is started with libpod: Feb 11 15:08:07 myhost systemd[1]: libcontainer-12345-systemd-test-default-dependencies.scope: Scope has no PIDs. Refusing. Feb 11 15:08:07 myhost systemd[1]: libcontainer-12345-systemd-test-default-dependencies.scope: Scope has no PIDs. Refusing. Since this logic never worked, just assume both attributes are supported (which is what the code does when detection fails for this reason, since it's looking for an "unknown attribute" or "read-only attribute" to mark them as false) and skip the detection altogether. Signed-off-by: Filipe Brandenburger <filbranden@google.com>
|
Maybe its possible for us to inject a dummy command/pid to the scope to detect this properly, however i'm not sure if its worth it, given that most stable distribution now ships systemd with capabilities that we are checking here i think. @mrunalp @crosbymichael any thoughts ? |
|
LGTM |
Yes, my thoughts exactly. Since the introduction of the feature, this detection never really worked (was always assumed "true") and nobody ever complained, so probably OK to keep this now... I'm actually also looking at possibly removing the slice detection too (and just keeping Delegate=no there), since in latest systemd that's not possible and in older systemd it didn't do much. But that's for another PR. 😄 Thanks for the quick review! |
|
LGTM |
1 similar comment
|
LGTM |
thaJeztah
added a commit
to thaJeztah/containerd
that referenced
this pull request
Mar 7, 2019
This includes an improved fix for CVE-2019-5736 to reduce the increased memory-consumption introduced by the original patch, RHEL 7.6 getting into a loop due to a kernel bug in those kernels, and improve compatibility with older kernels. changes included: - opencontainers/runc#1973 Vendor opencontainers/runtime-spec 29686dbc - opencontainers/runc#1978 Remove detection for scope properties, which have always been broken - opencontainers/runc#1963 Vendor in go-criu and use it for CRIU's RPC definition - opencontainers/runc#1995 exec: expose --preserve-fds - opencontainers/runc#2000 fix preserve-fds flag may cause runc hang - opencontainers/runc#1968 Create bind mount mountpoints during restore - opencontainers/runc#1984 nsenter: cloned_binary: "memfd" cleanups Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
added a commit
to thaJeztah/containerd
that referenced
this pull request
Mar 7, 2019
This includes an improved fix for CVE-2019-5736 to reduce the increased memory-consumption introduced by the original patch, RHEL 7.6 getting into a loop due to a kernel bug in those kernels, and improve compatibility with older kernels. changes included: - opencontainers/runc#1973 Vendor opencontainers/runtime-spec 29686dbc - opencontainers/runc#1978 Remove detection for scope properties, which have always been broken - opencontainers/runc#1963 Vendor in go-criu and use it for CRIU's RPC definition - opencontainers/runc#1995 exec: expose --preserve-fds - opencontainers/runc#2000 fix preserve-fds flag may cause runc hang - opencontainers/runc#1968 Create bind mount mountpoints during restore - opencontainers/runc#1984 nsenter: cloned_binary: "memfd" cleanups Signed-off-by: Sebastiaan van Stijn <github@gone.nl> (cherry picked from commit b8d40b3) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
added a commit
to thaJeztah/containerd
that referenced
this pull request
Mar 7, 2019
This includes an improved fix for CVE-2019-5736 to reduce the increased memory-consumption introduced by the original patch, RHEL 7.6 getting into a loop due to a kernel bug in those kernels, and improve compatibility with older kernels. changes included: - opencontainers/runc#1973 Vendor opencontainers/runtime-spec 29686dbc - opencontainers/runc#1978 Remove detection for scope properties, which have always been broken - opencontainers/runc#1963 Vendor in go-criu and use it for CRIU's RPC definition - opencontainers/runc#1995 exec: expose --preserve-fds - opencontainers/runc#2000 fix preserve-fds flag may cause runc hang - opencontainers/runc#1968 Create bind mount mountpoints during restore - opencontainers/runc#1984 nsenter: cloned_binary: "memfd" cleanups Signed-off-by: Sebastiaan van Stijn <github@gone.nl> (cherry picked from commit b8d40b3) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
rphillips
added a commit
to rphillips/origin
that referenced
this pull request
Apr 10, 2019
…rties, which have always been broken ref: opencontainers/runc#1978 fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1698543
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The detection for scope properties (whether scope units support
DefaultDependencies=orDelegate=) has always been broken, since systemd refuses to create scopes unless at least one PID is attached to it (and this has been so since scope units were introduced in systemd v205, see systemd commit v204-230-g6c12b52e1964.)This can be seen in journal logs whenever a container is started with libpod:
Since this logic never worked, just assume both attributes are supported (which is what the code does when detection fails for this reason, since it's looking for an "unknown attribute" or "read-only attribute" to mark them as false) and skip the detection altogether.