quattor / configuration-modules-core

Node Configuration Manager Components for Everyone
www.quattor.org
Other
6 stars 54 forks source link

ncm-sysctl throws away errors from sysctl command #1122

Open ned21 opened 7 years ago

ned21 commented 7 years ago

ncm-sysctl runs with -e so any sysctl settings not supported by the kernel are silently ignored. Is this the behaviour we want?

$ sudo ncm-ncd --configure sysctl --verbose

[INFO] NCM-NCD version 14.10.0 started by root at: Tue Jun  6 13:27:24 2017
[VERB] accessing CCM cache manager..
[VERB] getting locked CCM configuration..
[VERB] checking for ncm-ncd locks...
[INFO] executing configure on components....
[INFO] running component: sysctl
---------------------------------------------------------
[VERB] Opening file /etc/sysctl.d/50-quattor.conf
updated /etc/sysctl.d/50-quattor.conf
changed mode(00444) of /etc/sysctl.d/50-quattor.conf
[VERB] File /etc/sysctl.d/50-quattor.conf was modified
[VERB] Changes made to /etc/sysctl.d/50-quattor.conf, running sysctl on it
[VERB] Getting output of command: /sbin/sysctl -e -p /etc/sysctl.d/50-quattor.conf
[INFO] configure on component sysctl executed, 0 errors, 0 warnings

=========================================================

[OK]   0 errors, 0 warnings executing configure

$ sudo sysctl -p /etc/sysctl.d/50-quattor.conf | grep unknown
error: "net.ipv4.tcp_vegas_cong_avoid" is an unknown key
error: "vm.max_writeback_pages" is an unknown key
error: "vm.pagecache" is an unknown key
$
stdweird commented 7 years ago

i'd say no, should report an error, esp since the schema allows anything @ned21 is the exitcode non-zero in the example above?

ned21 commented 7 years ago

My instinct was "no" too but then after some internal discussion, I am not so sure. ERRORs should be programming errors or other unforeseeable/impossible situations. If the sysadmin made a mistake in specifying the sysctl name then in the Quattor model that should be caught prior to deployment. I suspect the problem with specifying a schema for all possible sysctl settings is that it would be huge, and also vary with each kernel version.

stdweird commented 7 years ago

so lacking a stricter schema, there should be an error. but you are right that these are 'configuration' errors; so we should reflect that in the error message.

making a schema is not easy. it will indeed be rather large and will be kernel versioned. i'm not sure how bad it would be (i also don't know how many things people want to change on average).

we could parse sysctl -a output before deploying new config, so we can know upfront what won't work, and report them properly.

but an error (or at the very least a warning) should be reported: whatever you tried to do with the configuration, it didn't work and someone should be made aware of it. not being aware is (imho) worse.

ned21 commented 7 years ago

We would prefer a WARNING over an ERROR although it could be a config option to make it an ERROR.