Skip to content
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

Mask dmesg_restric as part of procSysKernel handler #77

Merged
merged 1 commit into from
Nov 3, 2022

Conversation

@felipecrs
Copy link
Contributor Author

@rodnymolina this indeed fix the issue. Can be tested with:

$ docker run --rm --runtime sysbox-runc ubuntu sysctl kernel.dmesg_restrict=0
kernel.dmesg_restrict = 0

Without these changes, it fails with:

$ docker run --rm --runtime sysbox-runc ubuntu sysctl kernel.dmesg_restrict=0
sysctl: permission denied on key "kernel.dmesg_restrict", ignoring

@felipecrs felipecrs marked this pull request as ready for review November 1, 2022 00:07
@felipecrs felipecrs changed the title Mask dmesg_restric Mask dmesg_restric Nov 1, 2022
@rodnymolina
Copy link
Member

rodnymolina commented Nov 2, 2022

@felipecrs, your changes look good but there are a couple of issues that need to be addressed before you merge this PR:

  1. The sysbox branch that you have used for this PR is not based off of our latest master branch, so a few of our integration testcases are failing. Unfortunately, we are having some issues with our CI right now, so I cloned your branch and manually ran the tests, which confirmed what I just mentioned:
$ git log --oneline -10 mask-dmesg_restrict
74f0254 (felipe-fork/mask-dmesg_restrict, felipe-pr-77) Mask dmesg_restric
95a773a Lower log level of error message from err -> warn.
2234afb Lower sysbox-fs log message from error to debug.
c1ed450 Address review comments for previous commits

$ git show 95a773a | egrep Date
Date:   Sat Mar 19 20:43:56 2022 +0000
  1. There are a few minor changes required on top of your current commit. I took care of those and submit a new commit on top of your personal mask-dmesg_restrict branch, please add this one to your PR update.
$ git log --oneline -10
71b225f (HEAD -> mask-dmesg_restrict, felipe-fork/mask-dmesg_restrict) Adjust 'dmesg_restrict' supported range as part of procSysKernel handler
74f0254 (felipe-pr-77) Mask dmesg_restrict

@felipecrs felipecrs force-pushed the mask-dmesg_restrict branch 2 times, most recently from 86d3754 to f8aa00f Compare November 2, 2022 23:32
@felipecrs
Copy link
Contributor Author

felipecrs commented Nov 2, 2022

Hi @rodnymolina, sorry about that. I was applying the patch on top of v0.5.2, and that's why it was outdated. I totally forgot to update it while creating the PR.

Thanks for finishing it as well. I also squashed your commit into mine to keep the history clean.

Co-authored-by: Rodny Molina <rodny.molina@docker.com>
Signed-off-by: Felipe Santos <felipecassiors@gmail.com>
Signed-off-by: Rodny Molina <rodny.molina@docker.com>
@felipecrs felipecrs changed the title Mask dmesg_restric Mask dmesg_restric as part of procSysKernel handler Nov 2, 2022
@rodnymolina
Copy link
Member

Thanks @felipecrs -- I just verified that things are looking good now.

Copy link
Member

@rodnymolina rodnymolina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG

@rodnymolina rodnymolina merged commit 4ef8ac1 into nestybox:master Nov 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support writing to /proc/sys/kernel/dmesg_restrict inside system containers
2 participants