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

Reload also when current system values differ #56153

Open
wants to merge 1 commit into
base: devel
from

Conversation

Projects
None yet
3 participants
@kustodian
Copy link
Contributor

commented May 7, 2019

SUMMARY

Previously if sysctl_set=no (which is the default) this module only checked for changes in the sysctl.conf file to decide whether it should reload it or not. This means that if the values in the conf file are the same as they are set with the module, but the current values on the system are different, that this module wouldn't apply the changes on the system and thus the value set with the module wouldn't be applied on the OS. This isn't obvious and it doesn't make sense that the module works like that by default, especially because there is a separate option reload. Now sysctl will also check if the current value differs on the system and if it does, it will reload the file again.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

sysctl

Reload also when current system values differ
Previously if `sysctl_set=no` (which is the default) this module only
checked for changes in the sysctl.conf file to decide whether it should
reload it or not. This means that if the values in the conf file are the
same as they are set with the module, but the current values on the
system are different, that this module wouldn't apply the changes on the
system and thus the value set with the module wouldn't be applied on the
OS. This isn't obvious and it doesn't make sense that the module works
like that by default, especially because there is a separate option
`reload`. Now sysctl will also check if the current value differs on the
system and if it does, it will reload the file again.
@kustodian

This comment has been minimized.

Copy link
Contributor Author

commented May 7, 2019

This is the follow up to #55695. I'm thinking of updating the tests to test these new changes (at least in check mode), but I would rather do it once #55695 is merged, so that I don't have to deal with conflicts in the test playbook.

@ansibot

This comment has been minimized.

Copy link
Contributor

commented May 7, 2019

@kustodian

This comment has been minimized.

Copy link
Contributor Author

commented May 7, 2019

Also tests for sysctl module in Docker are rather bad because the /proc filesystem is mounted read-only, so we cannot actually test if sysctl is setting any values correctly or not. It could only work in privileged mode, but privileged mode is not available on Shippable. I thought about a way to workaround this and the only way I found was to build/write a mock version of sysctl which would work inside containers, but that seems just too much work.

@ansibot ansibot added the stale_ci label May 16, 2019

@jillr jillr requested a review from bcoca May 16, 2019

@jillr jillr removed the needs_triage label May 16, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.