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

feat: Introduce xymon-client plugin #4260

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Conversation

kumy
Copy link
Contributor

@kumy kumy commented Sep 27, 2024

Closes #4259
Relates opnsense/tools#434

image

@kumy
Copy link
Contributor Author

kumy commented Sep 27, 2024

shown on dashboard Services widget

Can someone explain how to add the service to the widget? 🙏

image

@Monviech
Copy link
Member

Monviech commented Sep 27, 2024

@kumy You create an inc file for your plugin like this, if I remember correctly. It returns the services.

function caddy_services()

@kumy kumy force-pushed the issue-4259 branch 5 times, most recently from b33cde7 to 9d9929b Compare September 28, 2024 06:37
@kumy
Copy link
Contributor Author

kumy commented Sep 28, 2024

@Monviech Thanks! Added 🕺

@kumy
Copy link
Contributor Author

kumy commented Sep 28, 2024

Another question, the service is not automatically started after boot. Anything special to do? 🙏

EDIT: problably something related to bootup

'bootup' => ['miniupnpd_configure_do'],

EDIT2: According to https://docs.opnsense.org/development/backend/overview.html#bootup, method 7 seems the new way. Trying…

EDIT3: got it working.
EDIT4: will rename xymon to xymonclient

@kumy kumy force-pushed the issue-4259 branch 2 times, most recently from 59695e0 to a319db5 Compare September 28, 2024 10:42
@kumy kumy mentioned this pull request Oct 12, 2024
3 tasks
'stop' => ['xymonclient stop'],
],
'name' => 'xymonclient',
'pidfile' => "/usr/local/www/xymon/client/logs/clientlaunch.$fqdn.pid"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'pidfile' => "/usr/local/www/xymon/client/logs/clientlaunch.$fqdn.pid"
'pidfile' => "/var/run/xymonclient.pid"

Two reasons, when making the pid dynamic, restart behavior might get flaky and the usual directory is /var/run and not exposed via the webserver.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AdSchellevis
This is not my choice. It's defined in file /usr/local/etc/rc.d/xymon-client from the package itself.

Excerpt:

#!/bin/sh

# PROVIDE: xymon_client
# REQUIRE: DAEMON
# KEYWORD: shutdown

. /etc/rc.subr

name=xymon_client
rcvar=xymon_client_enable

load_rc_config "$name"
: ${xymon_client_enable:=NO}
: ${xymon_client_user:=xymon}

pidfile="/usr/local/www/xymon/client/logs/clientlaunch.`hostname`.pid"
command=/usr/local/www/xymon/client/bin/xymonlaunch
command_args="--config=/usr/local/www/xymon/client/etc/clientlaunch.cfg --log=/usr/local/www/xymon/client/logs/clientlaunch.log --pidfile=${pidfile}"
start_precmd=xymon_precmd

[…]

The variable pidfile is not overridable as it uses = and not something like : ${pidfile:=...}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can change the port rc.d script to better handle an external pid. Eventually we need to do something about the FreeBSD port itself though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, done see latest commit and PR opnsense/ports#206

# to load values from our own template

INCLUDE="include /usr/local/etc/xymon/xymonclient.cfg"
FILE=/usr/local/www/xymon/client/etc/xymonclient.cfg
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when possible, I would prevent saving settings in the www directory, the post install script can be avoided by shipping the xymonclient.cfg for this plugin and using the template directory to flush it to disk (probably in stead of /usr/local/etc/xymon/xymonclient.cfg further below)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AdSchellevis like https://github.com/opnsense/plugins/pull/4260/files#r1835329433

I don't think this would be possible. The file /usr/local/www/xymon/client/etc/xymonclient.cfg is statically coded from file /usr/local/www/xymon/client/etc/clientlaunch.cfg

# cat /usr/local/www/xymon/client/etc/clientlaunch.cfg
#
# The clientlaunch.cfg file is loaded by "xymonlaunch".
# It controls which of the Xymon client-side modules to run, 
# (both the main client "xymonclient.sh" and any client-side
# extensions); how often, and with which parameters, options 
# and environment variables.
#
# Note: On the Xymon *server* itself, this file is normally 
#       NOT used. Instead, both the client- and server-tasks
#       are controlled by the tasks.cfg file.
#

# msgcache is used for passive clients, that cannot connect
# directly to the Xymon server. This is not the default
# setup, so this task is normally disabled.
[msgcache]
	DISABLED
	ENVFILE $XYMONCLIENTHOME/etc/xymonclient.cfg
	CMD $XYMONCLIENTHOME/bin/msgcache --no-daemon --pidfile=$XYMONCLIENTLOGS/msgcache.pid
	LOGFILE $XYMONCLIENTLOGS/msgcache.log

# The main client task
[client]
	ENVFILE $XYMONCLIENTHOME/etc/xymonclient.cfg
	CMD $XYMONCLIENTHOME/bin/xymonclient.sh 
	LOGFILE $XYMONCLIENTLOGS/xymonclient.log
	INTERVAL 5m

Which is called by the /usr/local/etc/rc.d/xymon-client from the non-overridable variable command_args="--config=/usr/local/www/xymon/client/etc/clientlaunch.cfg --log=/usr/local/www/xymon/client/logs/clientlaunch.log --pidfil e=${pidfile}"

Were you suggesting to completely overwrite the /usr/local/etc/rc.d/xymon-client file from the package to pass our own arguments?

Copy link
Contributor Author

@kumy kumy Nov 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did some tests to override the /usr/local/etc/rc.d/xymon-client and setting pidfile="/var/run/xymonclient.pid" (as suggested here). This is preventing the service to start as /var/run/ is owned by root:wheel and perms are 0755 and the service is started as user xymon which don't have permission there.

EDIT: I circumvented this by patching the "forked" rc file and added the pid file creation and give it proper permission.

Next issue is giving execution bit to the /usr/local/etc/rc.d/xymon-client now managed by the template system :(

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Next issue is giving execution bit to the /usr/local/etc/rc.d/xymon-client now managed by the template system :(

Looks like I can't set it to executable

https://github.com/opnsense/core/blob/84437b3812d959a7aca7f1af8d6c528d8683607d/src/opnsense/service/modules/template.py#L287-L289

So I don't know how I can accomplish what you requested with this comment, neither how to change the pidfile path

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File removed here in favor of ports update see PR opnsense/ports#206

@AdSchellevis
Copy link
Member

@kumy did a very quick review and left some remarks/tips.

@kumy
Copy link
Contributor Author

kumy commented Nov 9, 2024

@AdSchellevis thanks for the review.

Except the 2 comments left open above, everything else has been adjusted.

@AdSchellevis
Copy link
Member

@kumy ok, maybe the better question is then if there's anything in /usr/local/www/xymon/ that should be accessible (without any constraints) via http[s]. If that's not the case, we can try to search for alternatives, either by pushing a new WWWDIR during build in https://github.com/opnsense/ports/blob/3c574e41e4e45df72cbb64c060a5637dd1d64f29/net-mgmt/xymon-client/files/xymon-client.in#L16 or making a copy of the rc file which ships to something like /usr/local/etc/rc.d/opn-xymon-client with the proper attributes set.

@kumy
Copy link
Contributor Author

kumy commented Nov 9, 2024

there's anything in /usr/local/www/xymon/ that should be accessible (without any constraints) via http[s]

The client does not publish any webservice. It collects status/metrics and send them to the Xymon Server(s).

making a copy of the rc file which ships to something like /usr/local/etc/rc.d/opn-xymon-client with the proper attributes set.

So a dedicated/forked rc init script.

@AdSchellevis Couldn't we do like Debian and just add one line to the /etc/xymon/xymonclient.cfg (eg Debian do: include /etc/default/xymon-client at the top of file.)
This is technically what I did/need with the PostInstall script... I just add include /usr/local/etc/xymon/xymonclient.cfg

Do you have any pointer/example to create a patch for https://github.com/opnsense/ports/tree/3c574e41e4e45df72cbb64c060a5637dd1d64f29/net-mgmt/xymon-client. We would need to patch a file coming from the xymon-4.3.30.tar.gz.

Example from Debian:

$ cat /etc/xymon/xymonclient.cfg
# Environment settings for the Xymon client.

include /etc/default/xymon-client
include /var/run/xymon/bbdisp-runtime.cfg
CONFIGCLASS="$SERVEROSTYPE"     # Default configuration class for logfiles

PATH="/bin:/usr/bin:/sbin:/usr/sbin:/usr/local/bin:/usr/local/sbin:/usr/lib/xymon/client/bin"  # PATH setting for the client scripts.
SHELL="/bin/sh"				# Shell to use when forking programs

# You normally don't need to modify anything below here
XYMONDPORT="1984"                   # Portnumber where xymond listens
XYMONCLIENTHOME="/usr/lib/xymon/client"
XYMONHOME="$XYMONCLIENTHOME"       # Directory for the Xymon client files
XYMON="$XYMONHOME/bin/xymon"          # The Xymon client "xymon" utility
XYMONTMP="/var/lib/xymon/tmp"     # Where we may store temporary files.
XYMONCLIENTLOGS="/var/log/xymon"  # Where we store the client logfiles

include /var/run/xymon/xymonclient-include.cfg

# Options to logfetch, the xymon binary which examines log files for recent changes.
LOGFETCHOPTS=""
[…]
$ cat /etc/default/xymon-client
# Configure the Xymon client settings.

# You MUST set the list of Xymon servers that this
# client reports to.
# It is good to use IP-addresses here instead of DNS
# names - DNS might not work if there's a problem.
#
# E.g. (a single Xymon server)
#   XYMONSERVERS="192.168.1.1"
# or (multiple servers)
#   XYMONSERVERS="10.0.0.1 192.168.1.1"

XYMONSERVERS="myxymonserver.example"

# The defaults usually suffice for the rest of this file, 
# but you can tweak the hostname that the client reports 
# data with, and the OS name used (typically needed only on 
# RHEL or RHAS servers).

# CLIENTHOSTNAME=""
# CLIENTOS="rhel3"

CLIENTHOSTNAME="myhostname.example"

# Optionally mount a tmpfs on /var/lib/xymon/tmp. On client systems this has
# the advantage that client reports will continue to work even if the /var
# filesystem is out of space. This setting is ignored on server systems where
# this directory contains important information that must be preserved over
# reboots (xymond.chk).

#TMPFSSIZE="5000000"

@AdSchellevis
Copy link
Member

Patching in ports should work similar to the change already in the Makefile (https://github.com/opnsense/ports/blob/3c574e41e4e45df72cbb64c060a5637dd1d64f29/net-mgmt/xymon-client/files/patch-Makefile). If I'm not mistaken (@fichtner usually handles these), you can drop a file there and build / install from the ports directory in the usuall way.

@kumy
Copy link
Contributor Author

kumy commented Nov 11, 2024

@AdSchellevis / @fichtner This PR and opnsense/ports#206 should be ready for another review round 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Plugin for xymon-client
4 participants