-
Notifications
You must be signed in to change notification settings - Fork 89
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
Fix spaces in output of sysctl -n <key> #42
Conversation
I think I see what you're doing here, but I also think that it fixes some cases and breaks others. See #40 for some details. Basically, we would need to mangle both the value we are trying to set/compare and the value the system reports if we want to avoid breakage. This is what my module was doing in the past, but it also had its own downsides : The code is more complex and more fragile, and AFAICT some sysctl values could contain multiple spaces on purpose (such as kernel.core_pattern), though in practice it might not be relevant... For me it makes no doubt that the proper solution is to make sure the values being set are exactly the same as the ones sysctl reports, this means using tabs as-is. This is up to the users of the module. The real problem is that people have been used to setting mangled values, such as grouping kernel.sem with spaces instead of tabs... Would a clear mention of this in the README be enough, perhaps? |
\s includes [ \t\r\n\f]. So it should not break others. yes you are right that both value return by the command and the one provided should be parsed or provide the same that sysctl reports. I started to look at parsing the value provided to match the one parsed with sed and i haven't finished. |
i have added parsing of the value provided. Let me know what you think. |
@thias que penses-tu du dernier changement? |
@thias what do you think? |
@thias please let us know what you think of this change. We could use this in our environment. Thanks! |
👍 |
@juliengk I was about to hack something up to solve this very issue. One thing I've noticed from testing in a shell is that there is a difference between single vs double quotes for those sed statements - http://stackoverflow.com/questions/6697753/difference-between-single-and-double-quotes-in-bash When I use |
I've decided to just pass in my Hiera variables with the tabs because I think it's the cleanest solution. |
I see this after an upgrade...this looks to fix it for me
|
I can confirm that @benohara code change made {my} runs idempotent. |
No description provided.