mikaku / Monitorix

Monitorix is a free, open source, lightweight system monitoring tool.
https://www.monitorix.org
GNU General Public License v2.0
1.12k stars 167 forks source link

BUG monitorix.conf overwrite #414

Closed Saentist closed 2 years ago

Saentist commented 2 years ago

When used make install-systemd-all (posibly in other manual install types) monitorix.conf is overwriten, without any question. With dpkg -i monitorix.deb it ask do we want to overwrite config file.

Sugestion: adding simple logic to Makefile IF monitorix.conf exist in /etc/monitorix THEN copy to monitorix.conf.default ELSE just copy it ;)

No config lost.

 make install-systemd-all
Installing script and modules...
install -p -d "/usr/bin"
install -p -m755 monitorix "/usr/bin/monitorix"
install -p -d "/var/lib/monitorix"
install -p -d "/var/lib/monitorix"
install -p -d "/var/lib/monitorix/www"
install -p -d "/var/lib/monitorix/www/cgi"
install -p -dm755 "/var/lib/monitorix/www/imgs"
install -p -m755 monitorix.cgi "/var/lib/monitorix/www/cgi/monitorix.cgi"
install -p -m644 logo_bot.png "/var/lib/monitorix/www/logo_bot.png"
install -p -m644 logo_top.png "/var/lib/monitorix/www/logo_top.png"
install -p -m644 monitorixico.png "/var/lib/monitorix/www/monitorixico.png"
install -p -d "/var/lib/monitorix/www/css"
install -p -m644 css/black.css "/var/lib/monitorix/www/css/black.css"
install -p -m644 css/white.css "/var/lib/monitorix/www/css/white.css"
install -p -d "/etc/monitorix"
install -p -m644 monitorix.conf "/etc/monitorix/monitorix.conf"
...
mikaku commented 2 years ago

I'll see what can I do. Meanwhile read this FAQ to configure Monitorix properly.

mikaku commented 2 years ago

@Saentist, please, check this patch and let me know if it works.

Saentist commented 2 years ago

@mikaku not work

/etc/monitorix# ll
drwxr-xr-x   3 root root  4096 мар 23 13:13 ./
drwxr-xr-x 164 root root 12288 мар 23 02:59 ../
drwxr-xr-x   2 root root  4096 мар 16 16:14 conf.d/
-rw-r--r--   1 root root 34812 мар 16 22:22 monitorix.conf
-rw-r--r--   1 root root 28996 мар 17 00:02 monitorix.conf~

work in opposite

ifneq ("$(wildcard $(PATH_TO_FILE))","")
FILE_EXISTS = 1
else
FILE_EXISTS = 0
endif

https://fedingo.com/how-to-check-if-file-exists-in-shell-script/

Saentist commented 2 years ago

its not bad to add somewhere on systemd section

sudo systemctl daemon-reload
sudo systemctl enable monitorix.service
sudo systemctl reload-or-restart monitorix
mikaku commented 2 years ago

work in opposite

Please elaborate.

Saentist commented 2 years ago

script backup (somewhere) current config and place default config from repo in config folder

again we go to this old situation https://github.com/mikaku/Monitorix/issues/295

mikaku commented 2 years ago

script backup (somewhere) current config and place default config from repo in config folder

Sorry, I don't know if I understand you correctly.

You mean that the new monitorix.conf is renamed as monitorix.conf~ and your current configuration file is not touched?

Saentist commented 2 years ago

no monitorix.conf in /etc/monitorix is rewritten from repo where is backuped one i don't know monitorix.conf~ is backup from nano editor, ignore this file

file='/etc/monitorix/monitorix.conf'
if [ -f $file ]; then
    #code to be run if file exists
else
    #code to be run if file does not exist
fi

If i read correctly in current Makefile $(INSTALL_DATA) $(BACKUP) $(PN).conf "$(DESTDIR)$(CONFDIR)/$(PN)/$(PN).conf'' is install -p -m644 -b monitorix.conf "$(DESTDIR)/etc/monitorix/monitorix/conf"

there is no clear what is variable =''$(DESTDIR)'', guess is file system root ''/''

mikaku commented 2 years ago

monitorix.conf~ is backup from nano editor, ignore this file

No, the monitorix.conf~ is created by the command install when executing Makefile. It is the backup of the previous configuration file before installing the new one.

there is no clear what is variable =''$(DESTDIR)'', guess is file system root ''/''

$(DESTDIR) has no value, and since all pathname are absolute this puts this variable as a user decision if you have installed Monitorix on a different place.

If you set DESTDIR=/home/peter/ the file Makefile will install Monitorix inside such directory. Of course, you'll have to setup the configuration file accordingly.

Saentist commented 2 years ago

No, the monitorix.conf~ is created by the command install when executing Makefile. It is the backup of the previous configuration file before installing the new one.

So work in opposite if file monitorix.conf exist renamed as monitorix.conf~ and monitorix.conf copyed I cant see logic in this, user to go manually and rename files, to see his working config. And most worst is if open it with standard nano editor monitorix.conf backup is gone

if /etc/monitorix/monitorix.conf file exist, then script must copy repo file as monitorix.conf.default to /etc/monitorix else just copy to newer empty folder /etc/monitorix

This is most simple logic, config is there and work, and there is also a default new config with eventually newer features.

mikaku commented 2 years ago

And most worst is if open it with standard nano editor monitorix.conf backup is gone

I can change the suffix of the backed up file with --suffix=.bak, the rest of behavior is how install works.

mikaku commented 2 years ago

This is most simple logic, config is there and work, and there is also a default new config with eventually newer features.

I agree, it means then we cannot use this functionality of install. I'll see what can I do.

Saentist commented 2 years ago

But why you want to backup working config, and replace it with default not working one?

mikaku commented 2 years ago

But why you want to backup working config, and replace it with default not working one?

Because you shouldn't use the monitorix.conf to configure your Monitorix. Instead, you should create your own configuration file in conf.d/.

This way your Monitorix is always up-to-date.

IzzySoft commented 2 years ago

Apart from that, nice thing with RPM (and DEB) packaging:

%config(noreplace) %{_sysconfdir}/monitorix/monitorix.conf

If the user has modified the file, the installer (rpm/dpkg) will ask the user what to do then: install the new, keep the old, view the diff, edit.

But much better if you as a user put your own changes into separate files in conf.d/. You only need to put the sections you modified there, not the complete config. And you can have multiple files for easier maintenance. One example is already there with the .deb

%config(noreplace) %{_sysconfdir}/monitorix/conf.d/00-debian.conf

Whatever you place there in separate files has no risk to be overwritten. This was introduced to Monitorix some years ago for exactly this purpose.

mikaku commented 2 years ago

As @Saentist observed, the parameter -b (backup) in the command install when using Makefile to install Monitorix, works in the opposite direction than the common package managers (rpm / dpkg) do. That is, those common package managers will keep your configuration intact and will place the new configuration file apart (e.g. will be suffixed as .rpmnew in the case of rpm).

Since almost no one is using the Makefile instead the its own package manager to install Monitorix, I'm not sure if it worth the work to force Makefile to behave like the common package managers.

Thoughts?

Saentist commented 2 years ago

Since almost no one is using the Makefile instead the its own package manager to install Monitorix, I'm not sure if it worth the work to force Makefile to behave like the common package managers.

Except people with use git version with all new stuff. Why not try to implement some of examples above?

mikaku commented 2 years ago

Why not try to implement some of examples above?

The syntax of the examples above is for a .spec file only, and they are already implemented.

IzzySoft commented 2 years ago

instead the its own package manager

Good point: we package managers use that to build the packages :rofl: But as in that case the install always happens to an empty dir (for a clean package), we can ignore that :wink: And apologies if I caused confusion with the .spec examples.

That apart, as Jordi wrote: never modify the original config, always use conf.d for your own stuff – and all is well. I fully agree it's not worth reworking the Makefile logic. If you stick to the rules, there's no need for that – and if you don't: well… I'd say then it's your own responsibility. No warranties on overclocked CPUs :wink:

Saentist commented 2 years ago

Why not try to implement some of examples above?

The syntax of the examples above is for a .spec file only, and they are already implemented.

What .spec? Example is for makefile

ifneq ("$(wildcard $(PATH_TO_FILE))","")
FILE_EXISTS = 1
else
FILE_EXISTS = 0
endif

there is lot of resources https://makefiletutorial.com

IzzySoft commented 2 years ago

What is a spec file?

Saentist commented 2 years ago

What is a spec file?

Please read first post. Problem is not in packages Problem reported is when is used make install-systemd-all Makefile is afected by this issue.

If problem was in packages, i'll ask why in your repo there is no "Nightly builds".

mikaku commented 2 years ago

there is lot of resources https://makefiletutorial.com/

I'll take a look, thanks for the URL.

mikaku commented 2 years ago

This commit will save as .bak your current config file.

I don't know enough the Makefile syntax to provide a better solution, sorry. If you want to improve it then provide yourself a modified version of Makefile.