Run systemd service with full permissions required for cgroup v2 device controller. #4272

r10r · 2020-10-15T20:41:45Z

Running cri-o on a cgroup v2 unified system systemd.unified_cgroup_hierarchy=1 cgroup_no_v1=all I noticed that
the runtime (lxc / crio-lxc) fails to create the BPF filter for the cgroup device controller (with EPERM).
After allowing the crio.service to run with full privileges the runtime can create the BPF filter.

openshift-ci-robot · 2020-10-15T20:41:47Z

@r10r: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot · 2020-10-15T20:41:54Z

Hi @r10r. Thanks for your PR.

I'm waiting for a cri-o member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

This is required for container runtimes using the the cgroup2 device controller which is implemented as BPF filter. Signed-off-by: Ruben Jenster <r.jenster@drachenfels.de>

codecov · 2020-10-15T21:01:50Z

Codecov Report

Merging #4272 (3125f89) into main (af6e36a) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4272   +/-   ##
=======================================
  Coverage   38.57%   38.57%           
=======================================
  Files         111      111           
  Lines        8897     8897           
=======================================
  Hits         3432     3432           
  Misses       5081     5081           
  Partials      384      384

openshift-ci-robot · 2020-10-15T22:46:52Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mrunalp, r10r

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~~ [mrunalp]

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

mrunalp · 2020-10-15T22:47:00Z

/ok-to-test

r10r · 2020-10-16T13:04:33Z

/retest

r10r · 2020-10-16T19:45:53Z

@mrunalp Why are the build jobs failing? Because I force-pushed to the branch referenced by the pull request ?

haircommander · 2020-10-19T13:48:24Z

hm, we're getting this error:


TASK [enable and start CRI-O] **************************************************
task path: /go/src/github.com/cri-o/cri-o/contrib/test/integration/e2e-base.yml:25
fatal: [localhost]: FAILED! => {"changed": false, "msg": "Error loading unit file 'crio': org.freedesktop.DBus.Error.InvalidArgs \"Invalid argument\""}
	to retry, use: --limit @/go/src/github.com/cri-o/cri-o/contrib/test/integration/main.retry

on rhel 7

the others may be flakes.

/retest

(I expect the RHEL boxes to still fail, we'll need to get fancy with our installation script, I can get you a patch when we see it passes on fedora)

r10r · 2020-10-20T13:58:17Z

@haircommander Sorry for buggin - but I don't quite understand the failure - Is the systemd version on RHEL boxes to old to support the '+' permission flag ? I search in the systemd sources, but I can't really determine when the '+' flag was introduced. It's easier to look if a specific version supports the flag.

Can you tell me the systemd version used on the RHEL box ?

haircommander · 2020-10-20T14:18:36Z

I think some of those failures were infra failures. I'm going to re-run to see what the state of the world actually is

/retest

haircommander · 2020-10-20T23:46:22Z

/test e2e_fedora

haircommander · 2020-10-20T23:47:40Z

/test e2e_crun

r10r · 2020-11-04T07:46:36Z

/retest

r10r · 2020-11-05T09:45:03Z

@haircommander this seems to be a long journey - the builds are still failing. Did you make some changes to the infrastructure ? Does the builds work without the change ?

haircommander · 2020-11-05T14:09:47Z

I am sorry I have not had a chance to look at this. I will try later today

haircommander · 2020-11-13T21:43:04Z

hey @r10r sorry again, this keeps falling off of my radar. what happens when you apply this patch:

diff --git a/contrib/test/integration/build/cri-o.yml b/contrib/test/integration/build/cri-o.yml
index 2aa23e465..b447d41cd 100644
--- a/contrib/test/integration/build/cri-o.yml
+++ b/contrib/test/integration/build/cri-o.yml
@@ -26,6 +26,13 @@
     regexp: '^Restart='
     line: 'Restart=no'
 
+- name: Remove prefix of ExecStart for older systemd
+  lineinfile:
+    path: "{{ ansible_env.GOPATH }}/src/github.com/cri-o/cri-o/contrib/systemd/crio.service"
+    regexp: '^ExecStart=+/usr/local/bin/crio'
+    line: 'ExecStart=/usr/local/bin/crio'
+  when: (ansible_distribution == 'RedHat' and ansible_distribution_major_version|int < 8)
+
 - name: install cri-o
   make:
     target: install

Signed-off-by: Ruben Jenster <r.jenster@drachenfels.de>

openshift-merge-robot · 2020-11-18T22:05:35Z

@r10r: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/openshift-jenkins/critest_rhel	`3125f89`	link	`/test critest_rhel`
ci/openshift-jenkins/e2e_features_rhel	`3125f89`	link	`/test e2e_features_rhel`
ci/openshift-jenkins/e2e_rhel	`3125f89`	link	`/test e2e_rhel`
ci/openshift-jenkins/e2e_crun_cgroupv2	`3125f89`	link	`/test e2e_cgroupv2`
ci/openshift-jenkins/e2e_fedora	`3125f89`	link	`/test e2e_fedora`
ci/openshift-jenkins/e2e_crun	`3125f89`	link	`/test e2e_crun`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

github-actions · 2022-06-30T00:02:56Z

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

r10r · 2022-11-17T13:01:14Z

/retest

openshift-ci · 2023-05-24T03:58:39Z

@r10r: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/ci-critest	`3125f89`	link	true	`/test ci-critest`
ci/prow/ci-rhel-integration	`3125f89`	link	true	`/test ci-rhel-integration`
ci/prow/ci-rhel-e2e	`3125f89`	link	true	`/test ci-rhel-e2e`
ci/prow/ci-crun-e2e	`3125f89`	link	true	`/test ci-crun-e2e`
ci/prow/ci-cgroupv2-e2e	`3125f89`	link	true	`/test ci-cgroupv2-e2e`
ci/prow/ci-e2e-conmonrs	`3125f89`	link	true	`/test ci-e2e-conmonrs`
ci/prow/ci-e2e	`3125f89`	link	true	`/test ci-e2e`
ci/prow/ci-cgroupv2-e2e-crun	`3125f89`	link	true	`/test ci-cgroupv2-e2e-crun`
ci/prow/e2e-gcp-ovn	`3125f89`	link	true	`/test e2e-gcp-ovn`
ci/prow/ci-e2e-evented-pleg	`3125f89`	link	true	`/test ci-e2e-evented-pleg`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

sohankunkerkar · 2023-05-25T17:11:46Z

closing this in favor of #6970

r10r requested review from mrunalp and runcom as code owners October 15, 2020 20:41

openshift-ci-robot requested review from haircommander and rhatdan October 15, 2020 20:41

Run systemd service with full privileges.

8a19a63

This is required for container runtimes using the the cgroup2 device controller which is implemented as BPF filter. Signed-off-by: Ruben Jenster <r.jenster@drachenfels.de>

r10r force-pushed the master branch from c9f21af to 8a19a63 Compare October 15, 2020 20:44

openshift-ci-robot added dco-signoff: yes Indicates the PR's author has DCO signed all their commits. and removed dco-signoff: no Indicates the PR's author has not DCO signed all their commits. labels Oct 15, 2020

mrunalp approved these changes Oct 15, 2020

View reviewed changes

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

openshift-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Oct 15, 2020

openshift-ci-robot added dco-signoff: no Indicates the PR's author has not DCO signed all their commits. and removed dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Nov 18, 2020

Remove full privileges prefix '+' of ExecStart for older systemd.

3125f89

Signed-off-by: Ruben Jenster <r.jenster@drachenfels.de>

r10r force-pushed the master branch from c1f6460 to 3125f89 Compare November 18, 2020 18:37

openshift-ci-robot added dco-signoff: yes Indicates the PR's author has DCO signed all their commits. and removed dco-signoff: no Indicates the PR's author has not DCO signed all their commits. labels Nov 18, 2020

github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 30, 2022

haircommander removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 24, 2023

sohankunkerkar self-assigned this May 24, 2023

sohankunkerkar closed this May 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Run systemd service with full permissions required for cgroup v2 device controller. #4272

Run systemd service with full permissions required for cgroup v2 device controller. #4272

r10r commented Oct 15, 2020

openshift-ci-robot commented Oct 15, 2020

openshift-ci-robot commented Oct 15, 2020

codecov bot commented Oct 15, 2020 •

edited

openshift-ci-robot commented Oct 15, 2020

mrunalp commented Oct 15, 2020

r10r commented Oct 16, 2020

r10r commented Oct 16, 2020

haircommander commented Oct 19, 2020

r10r commented Oct 20, 2020

haircommander commented Oct 20, 2020

haircommander commented Oct 20, 2020

haircommander commented Oct 20, 2020

r10r commented Nov 4, 2020

r10r commented Nov 5, 2020

haircommander commented Nov 5, 2020

haircommander commented Nov 13, 2020

openshift-merge-robot commented Nov 18, 2020

github-actions bot commented Jun 30, 2022

r10r commented Nov 17, 2022

openshift-ci bot commented May 24, 2023

sohankunkerkar commented May 25, 2023

Run systemd service with full permissions required for cgroup v2 device controller. #4272

Run systemd service with full permissions required for cgroup v2 device controller. #4272

Conversation

r10r commented Oct 15, 2020

openshift-ci-robot commented Oct 15, 2020

openshift-ci-robot commented Oct 15, 2020

codecov bot commented Oct 15, 2020 • edited

Codecov Report

openshift-ci-robot commented Oct 15, 2020

mrunalp commented Oct 15, 2020

r10r commented Oct 16, 2020

r10r commented Oct 16, 2020

haircommander commented Oct 19, 2020

r10r commented Oct 20, 2020

haircommander commented Oct 20, 2020

haircommander commented Oct 20, 2020

haircommander commented Oct 20, 2020

r10r commented Nov 4, 2020

r10r commented Nov 5, 2020

haircommander commented Nov 5, 2020

haircommander commented Nov 13, 2020

openshift-merge-robot commented Nov 18, 2020

github-actions bot commented Jun 30, 2022

r10r commented Nov 17, 2022

openshift-ci bot commented May 24, 2023

sohankunkerkar commented May 25, 2023

codecov bot commented Oct 15, 2020 •

edited