modules/dnsmasq_dhcp: respect conf-file/conf-dir #239
modules/dnsmasq_dhcp: respect conf-file/conf-dir #239kovetskiy wants to merge 3 commits intonetdata:masterfrom
Conversation
kovetskiy
changed the title
kovetskiy
changed the title
|
@ilyam8: could you take a look at the PR? |
* dnsmasq_dhcp respects conf-file=<path> configuration lines and reads
specified files
* dnsmasq_dhcp respects conf-dir=<path>[,<suffix-include-or-exclude>...]
configuration lines and recursively reads specified directories and
files in these directories
* fix unexpected behavior: dnsmasq_dhcp reads contents of 'dnsmasq.d' directory
by default (dnsmasq doesn't do that if 'conf-dir' is not specified)
|
I've force-pushed the changes. |
kovetskiy
force-pushed
the
master
branch
2 times, most recently
from
3191564 to
8264de3
Compare
|
upd: pushed even more bad file names |
| } | ||
|
|
||
| for _, file := range includeFiles { | ||
| files := d.findConfigs(file) |
There was a problem hiding this comment.
i think we need to avoid infinite recursion here (it is not infinite, i know)
that is what i get, fatal error doesnt look good
[ DEBUG ] dnsmasq_dhcp[dnsmasq_dhcp] autodetection.go:126 664436 opening/etc/dnsmasq.conf
[ DEBUG ] dnsmasq_dhcp[dnsmasq_dhcp] autodetection.go:126 664437 opening/etc/dnsmasq.conf
[ DEBUG ] dnsmasq_dhcp[dnsmasq_dhcp] autodetection.go:126 664438 opening/etc/dnsmasq.conf
runtime: goroutine stack exceeds 1000000000-byte limit
fatal error: stack overflow
| // findConfigs is tolerant to IO errors and finds as maximum config | ||
| // files as possible, therefore if an error occurrs during scanning process, | ||
| // it will be just logged with warning severity | ||
| func (d *DnsmasqDHCP) findConfigs(confPath string) []string { |
There was a problem hiding this comment.
findConfigs returns configs, right? but it doesnt include entry config (confPath string). Looks a little bit lame to me, not sure. What do you think?
having it this way we need to
configs := d.findConfigs(d.ConfPath)
configs = append([]string{d.ConfPath}, configs...)
^^ looks like a workaround or smth
There was a problem hiding this comment.
Didn't touch that logic though
https://github.com/netdata/go.d.plugin/pull/239/files#diff-daece282ff93570d72a99d7ffa78dfd5L20
There was a problem hiding this comment.
i know, i know.
it was
findConfigurations(confDir string) ([]string, error)
which is ok (but not good ofc), if finds configs in dir.
we can fix it later.
One more thing i noticed, your implementation ignores ConfDir.
There was a problem hiding this comment.
Well, maybe we need to get rid of ConfigDir
All we need is the entry config
CONFIG FILE
At startup, dnsmasq reads /etc/dnsmasq.conf, if it exists. (On FreeBSD, the file is /usr/local/etc/dnsmasq.conf ) (but see the -C and -7 options.) The format of this file consists of one option per line, exactly as the long options detailed in the OPTIONS section but without the
leading "--". Lines starting with # are comments and ignored. For options which may only be specified once, the configuration file overrides the command line. Quoting is allowed in a config file: between " quotes the special meanings of ,:. and # are removed and the following
escapes are allowed: \\ \" \t \e \b \r and \n. The later corresponding to tab, escape, backspace, return and newline.
There was a problem hiding this comment.
On debian it reads /etc/default/dnsmasq
from
/etc/init.d/dnsmasq
NAME=dnsmasq
DESC="DNS forwarder and DHCP server"
# Most configuration options in /etc/default/dnsmasq are deprecated
# but still honoured.
ENABLED=1
if [ -r /etc/default/$NAME ]; then
. /etc/default/$NAME
fi
There was a problem hiding this comment.
ok, it seems /etc/default/dnsmasq doesnt work, i found in README in dnsmasq.d
# All files in this directory will be read by dnsmasq as
# configuration files, except if their names end in
# ".dpkg-dist",".dpkg-old" or ".dpkg-new"
#
# This can be changed by editing /etc/default/dnsmasq
but, as i said it doesnt work, so we can ignore it.
There was a problem hiding this comment.
i finally read man dnsmasq lmao
-C, --conf-file=<file>
Specify a different configuration file. The conf-file option is also allowed in configuration files, to include multiple configuration files. A filename of "-" causes dnsmasq to read configuration from stdin.
-7, --conf-dir=<directory>[,<file-extension>......],
Read all the files in the given directory as configuration files. If extension(s) are given, any files which end in those extensions are skipped. Any files whose names end in ~ or start with . or start and end with # are always skipped. If the extension starts with * then
only files which have that extension are loaded. So --conf-dir=/path/to/dir,*.conf loads all files with the suffix .conf in /path/to/dir. This flag may be given on the command line or in a configuration file. If giving it on the command line, be sure to escape * charac‐
ters.
it looks like we need both config_path and config_dir since both of them can be specified via command line.
3 participants
#230
dnsmasq_dhcp respects conf-file/conf-dir directive
specified files
configuration lines and recursively reads specified directories and
files in these directories
by default (dnsmasq doesn't do that if 'conf-dir' is not specified)
Haven't added the following option:
It will be done in a separated PR.