pgstef / check_pgbackrest

pgBackRest backup check plugin for Nagios
PostgreSQL License
35 stars 14 forks source link

pgbackrest version check fails #36

Closed netphantm closed 2 days ago

netphantm commented 6 days ago

hi getting this error in icinga2:

Remote command execution failed: Argument "2.52.1" isn't numeric in numeric lt (<) at check_pgbackrest line 591.

seems like they added a minor version and that check is not working with 2.52.1 cos it's not numeric..

momo4200 commented 6 days ago

Hi there,

simply add cut in sub pgbackrest_version :

before : my $pgbackrest_version = $version_cmd | sed -e s/pgBackRest\\ // | sed -e s/dev//;

after : my $pgbackrest_version = $version_cmd | sed -e s/pgBackRest\\ // | sed -e s/dev// | cut -d . -f1,2;

It will work after that.

pgstef commented 6 days ago

Hi there,

simply add cut in sub pgbackrest_version :

before : my $pgbackrest_version = $version_cmd | sed -e s/pgBackRest\\ // | sed -e s/dev//;

after : my $pgbackrest_version = $version_cmd | sed -e s/pgBackRest\\ // | sed -e s/dev// | cut -d . -f1,2;

It will work after that.

That solution would work for short term fix indeed. Although, before adding it, I'd like to think if it would be meaningful to support the 3 digits for the future or if it isn't necessary.

dwsteele commented 6 days ago

I'd like to think if it would be meaningful to support the 3 digits for the future or if it isn't necessary.

We are planning to go to three-part semver versions as early as October. I'll be submitting a PR for that soon-ish.

mbanck-ntap commented 4 days ago

I don't think enhancement is the right classification for this, isn't it breaking check-pgbackrest for everybody on the current pgBackRest release? The enhancement would be handling this gracefully in the future, but a hot-fix is needed ASAP I think.

momo4200 commented 4 days ago

Hi there, why don't you put release instead of third digit ?

Pgbackrest 2.52 release X

If it is the same branch it is easier to solve check_pgbackrest then.

Thanks

pgstef commented 4 days ago

Hi there, why don't you put release instead of third digit ?

Pgbackrest 2.52 release X

If it is the same branch it is easier to solve check_pgbackrest then.

Thanks

That's a pgBackRest decision, not check_pgbackrest. Three-part semver versions is pretty common.

pgstef commented 4 days ago

I don't think enhancement is the right classification for this, isn't it breaking check-pgbackrest for everybody on the current pgBackRest release? The enhancement would be handling this gracefully in the future, but a hot-fix is needed ASAP I think.

It's a behavior change of another software, not from this one. So from my point of view, this is an enhancement to work with more recent releases.

I've got a working solution already (which allows to add another service check too), although I would have been tempted to wait for the pgBackRest PR to exactly figure out how this would be handled in the future. I first need to fix the regression testing suite anyway and then I'll push a new release.

dwsteele commented 4 days ago

We are planning for three part (semver) release versions from October onwards, starting (probably) with 3.0.0. We have already discussed this with packagers and the patch version didn't cause any problems for pgbackrest so we went forward with it.

Sorry for the trouble.

pgstef commented 4 days ago

Sorry for the trouble.

I'll update the regex to work with 2.53dev, 2.52.1, 3.0.0 and 3.0.1dev numbering. As soon as I can get the regression test suite cleaned-up and fixed, I'll push a new release.

pgstef commented 3 days ago

FWIW, I've pushed an update to support this version change earlier today and just fixed the CI which seems happy about it.

I would have liked to wait for pgBackRest 2.53 with the lock refactoring but I understand it would be better to have a new release ASAP. I'll double check tomorrow in the morning and create a new release if everything's fine.

pgstef commented 2 days ago

Release 2.4 is available. Let's hope the packages would be ready and available soon. Closing this issue since this release should fix the problem.

Kind Regards