I guess this line should not reference $cbsd_mq_router_config: victoria_metrics_args=${cbsd_mq_router_config-"--storageDataPath=/var/db/victoria-metrics --retentionPeriod=1 --httpListenAddr=:8428"} https://github.com/freebsd/freebsd-ports/blob/da7851dcc648e66d2036f471c3084eb943063d3a/net-mgmt/victoria-metrics/files/victoria-metrics.in#L17 Also, it would be nice to map "service victoria-metrics reload" to a `kill -HUP`, as it is a supported signal to reload (scrape) configuration. cheers,
(In reply to Lapo Luchini from comment #0) > I guess this line should not reference $cbsd_mq_router_config: I've also noticed it earlier and asked Oleg, he said it was a secret undocumented vmetrics' option. ;-) I agree that it looks a bit weird to see embedded vague references to other, not directly related port(s), but you should probably talk to Oleg about it. > Also, it would be nice to map "service victoria-metrics reload" to a `kill -HUP` Could you try adding "extra_commands=reload" line to the rc script and see if it works as expected?
Ah, sorry, I misread that as "use these defaults if original variable is not null" but that's ${name:-} not ${name-} (which I can't find in "man sh", oh well). > Could you try adding "extra_commands=reload" line to the rc script I tried: it does. :) % tail -n 2 /var/log/victoria_metrics/victoria_metrics.log 2021-05-13T08:24:12.607Z info /wrkdirs/usr/ports/net-mgmt/victoria-metrics/work/VictoriaMetrics-1.59.0/lib/promscrape/scraper.go:123 SIGHUP received; reloading Prometheus configs from "/usr/local/etc/victoria-scrape.yml"
I checked that "extra_commands=reload" also works in (vmutils') /usr/local/etc/rc.d/vmagent
Created attachment 225114 [details] net-mgmt/victoria-metrics: fix args string, ca_root_nss deps - fix args string in rc.d - added ca_root_nss runtime dependencies
Sorry, I forgot to remove cbsd_mq_router_config when Alexey informed me about it. This is a copy-paste error from my previous port. I added a patch with a fix. Also added: - dependency on ca_root_nss certificates - rc.d reload extra command
So far so good. I was looking at their OpenBSD port (https://github.com/VictoriaMetrics/VictoriaMetrics/tree/master/ports/OpenBSD) and noticed that they use shorter /var/db path and user/group names. I'm considering doing the same for our port. Any objections? -USERS= victoria-metrics -GROUPS= victoria-metrics -VICTORIA_DATA?= /var/db/victoria-metrics +USERS= vmetrics +GROUPS= vmetrics +VICTORIA_DATA?= /var/db/vmetrics
(In reply to Alexey Dokuchaev from comment #6) > I'm considering doing the same for our port. On the other hands, this would diverge from the rc-script which bakes full name in many places... Need to think more about it.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=69d8c3f90fd43297b044f3f99d99084216c9dc14 commit 69d8c3f90fd43297b044f3f99d99084216c9dc14 Author: Alexey Dokuchaev <danfe@FreeBSD.org> AuthorDate: 2021-05-20 10:18:36 +0000 Commit: Alexey Dokuchaev <danfe@FreeBSD.org> CommitDate: 2021-05-20 10:26:54 +0000 net-mgmt/victoria-metrics: port had been improved (+) Add missing run-time dependency on `security/ca_root_nss': apparently, every Go application that uses TLS needs root CA certificates installed in the default place, which are used for verifying signatures provided by peers during establishing TLS connections. While here, remove accidental reference to `cbsd_mq_router_config' in the rc script and allow to reload (scrape) configuration by sending SIGHUP to processes. PR: 255822 net-mgmt/victoria-metrics/Makefile | 3 +++ net-mgmt/victoria-metrics/files/victoria-metrics.in | 3 ++- net-mgmt/victoria-metrics/files/vmagent.in | 1 + 3 files changed, 6 insertions(+), 1 deletion(-)
(In reply to Alexey Dokuchaev from comment #6) I always like the idea of minimizing the difference between upstream and port. Linux packages use 'victoriametrics': https://github.com/patsevanton/victoriametrics-rpm/commit/8e38be7cd96e72a388cf58da414fc70ff3ef3ea2 However, I cannot find a reason why in the original patch I started using 'victoria-metrics'
(In reply to olevole from comment #9) > I always like the idea of minimizing the difference between upstream and port. Right. Let's see if upstream makes any moves to unite the spelling in popular GNU/Linux packages and its own OpenBSD port example. I've kept the status quo for now.
PS: dependency on ca_root_nss won't be necessary after either of this lands: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=256003 https://github.com/golang/go/pull/46276
> However, I cannot find a reason why in the original patch I started using 'victoria-metrics' I don't know about your reason, of course, but in my own porting attempt I was using "the executable name" as a base, and that has the hyphen.
(In reply to Lapo Luchini from comment #11) > PS: dependency on ca_root_nss won't be necessary after either of this lands No problem, we can always remove explicit dependency once it's no longer necessary.