-
Notifications
You must be signed in to change notification settings - Fork 85
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
ignore Read-only file system error on sysctl set #910
base: main
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: haircommander The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This is probably going to break certain functionality (internal networks, definitely, probably also IPv6 will act strange since we can't configure neighbor advertisements properly). |
though in those cases, they were broken to begin with. This is just making a fatal error non fatal. If netavark truly needed to set the sysctls then something will break down the line I think |
Signed-off-by: Peter Hunt <[email protected]>
dfea64e
to
a6eeb11
Compare
I would at least have a warning printed (although I guess that may defeat the purpose if every single container start-up prints these). Ignoring these problems will turn into a nightmare of random support requests why xyz isn't working in their env and network issues are already a pain to deal with due to often lacking reproducer and/or race conditions. I don't what to deal with more of these. I think we should still make the caller enforce that the sysctls are already set to the right values, i.e. it should be possible to set |
so the code already does that. if the sysctl is set to the right value it skips. the problem is when setting values like I am open to logging something if we skip, or passing an option to skip from podman. TBH I have hit other issues when testing with this patch so I'm not even sure if we should go down this route... |
That is why I suggested the use of the special interface name Anyhow I tend to agree this all is less then ideal and would not be future proof, i.e. if we start setting a new sysctl then the parent containers config would need to be updated with it as well. It would be hard to maintain as each update could result in breaking changes for users going that route. |
fixes #825
Needed for running podman in a kubernetes pod. I am investigating if we can not set Proc to unmasked and get away with enough podman operations.