Add support for MidnightBSD to a number of plugins #1342

laffer1 · 2020-07-07T21:25:22Z

No description provided.

coveralls · 2020-07-07T21:31:01Z

Pull Request Test Coverage Report for Build 3007

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 52.42%

Totals
Change from base Build 3000:	0.0%
Covered Lines:	1083
Relevant Lines:	2066

💛 - Coveralls

h01ger · 2020-07-08T07:50:01Z

On Tue, Jul 07, 2020 at 02:25:41PM -0700, Lucas Holt wrote: -- Commit Summary -- * Add support for MidnightBSD to a number of plugins -- File Changes -- M contrib/plugin-gallery/print-gallery.awk (1) M doc/installation/install.rst (17) A plugins/node.d.midnightbsd/coretemp (100) A plugins/node.d.midnightbsd/cpu (103) A plugins/node.d.midnightbsd/dev_cpu_ (138) A plugins/node.d.midnightbsd/df (57) A plugins/node.d.midnightbsd/df_inode (69) A plugins/node.d.midnightbsd/if_ (109) A plugins/node.d.midnightbsd/if_errcoll_ (80)

[10 more plugins] is midnightbsd really so much different from FreeBSD (from which it was forked), NetBSD and OpenBSD that it needs special plugins?

…

-- cheers, Holger

------------------------------------------------------------------------------- holger@(debian|reproducible-builds|layer-acht).org PGP fingerprint: B8BF 5413 7B09 D35C F026 FE9D 091A B856 069A AA1C

laffer1 · 2020-07-08T14:28:19Z

It seemed to be the pattern with the others. I can modify the node.d.freebsd scripts instead to do an OS check and skip the legacy freebsd 4 and lower cases instead.

I will have to hack up build.pl though since it's doing this and midnightbsd is a supported platform in perl.

File::Find::find( { wanted => &_find_plugins_wanted },
'plugins/node.d', "plugins/node.d.$^O" );

h01ger · 2020-07-08T18:39:08Z

On Wed, Jul 08, 2020 at 07:28:34AM -0700, Lucas Holt wrote: It seemed to be the pattern with the others. I can modify the node.d.freebsd scripts instead to do an OS check and skip the legacy freebsd 4 and lower cases instead.

I think that's preferable, thanks.

…

-- cheers, Holger

------------------------------------------------------------------------------- holger@(debian|reproducible-builds|layer-acht).org PGP fingerprint: B8BF 5413 7B09 D35C F026 FE9D 091A B856 069A AA1C

sumpfralle

Thank you for your contribution!

After going through half of the plugins (and commenting them) I noticed, that these look very similar to the set of files below plugins/node.d.freebsd.

Maybe you could describe, where are the relevant differences? Maybe these can be incorporated without creating a code copy (which would increase the maintenance burden)?

sumpfralle · 2020-07-08T20:47:37Z

plugins/node.d.midnightbsd/coretemp

+	if [ -n "$cpus" ] ; then
+		echo "yes"
+	else
+		echo "no"


It is always helpful for users, if no is suffixed with a reason (e.g. no (empty output of 'sysctl dev.cpu')).

sumpfralle · 2020-07-08T20:50:06Z

plugins/node.d.midnightbsd/coretemp

+	reqcpus
+	for cpu in $cpus ; do
+		echo -n "CPU$cpu.value "
+		sysctl -n dev.cpu.$cpu.temperature | tr -d C


Please quote the variable. shellcheck will appreciate this ...

sumpfralle · 2020-07-08T20:50:53Z

plugins/node.d.midnightbsd/coretemp

+	;;
+config)
+	config
+        ;;


Please reduce indentation.

sumpfralle · 2020-07-08T20:51:39Z

plugins/node.d.midnightbsd/cpu

+			echo yes
+			exit 0
+		else
+			echo no


Please add a reason for no.

sumpfralle · 2020-07-08T20:52:26Z

plugins/node.d.midnightbsd/cpu

+			exit 0
+		fi
+	else
+		echo no


Please add something like (executable 'sysctl' not found).

sumpfralle · 2020-07-08T21:04:52Z

plugins/node.d.midnightbsd/if_errcoll_

+		netstat -i -b -n | sed -n -e '/^faith/d' -e '/^lo[0-9]/d' -e '/^pf(log|sync)/d' -e '/<Link#[0-9]*>/s/\** .*//p'
+		exit 0
+	else
+		exit 1


Maybe add a helpful error message?

sumpfralle · 2020-07-08T21:05:04Z

plugins/node.d.midnightbsd/if_errcoll_

+	echo 'graph_category network'
+	echo "graph_info This graph shows the amount of errors and collisions on the $INTERFACE network interface."
+	echo 'ierrors.label Input Errors'
+        echo 'ierrors.type COUNTER'


Please reduce indentation.

sumpfralle · 2020-07-08T21:05:16Z

plugins/node.d.midnightbsd/if_errcoll_

+	exit 0
+fi;
+
+/usr/bin/netstat -i -b -n -I $INTERFACE | awk '


Please quote $INTERFACE.

sumpfralle · 2020-07-08T21:05:41Z

plugins/node.d.midnightbsd/if_packets_

+		echo yes
+		exit 0
+	else
+		echo "no (/usr/bin/netstat not found)"


Please mention /sbin/ifconfig.

sumpfralle · 2020-07-08T21:06:03Z

plugins/node.d.midnightbsd/if_packets_

+}'
+
+else
+	/usr/bin/netstat -i -b -n -I $INTERFACE | awk '


Please quote $INTERFACE.

laffer1 · 2020-07-09T05:43:14Z

I pushed a newer commit that removes the entire node.d.midnightbsd directory and uses the existing freebsd scripts with modifications to check uname output as needed.

sumpfralle

Thank your for minimizing your set of changes! This looks fine to me now.

Please take a look at my minor nitpicks below and please squash your commits into one.

Thank you!

sumpfralle · 2020-07-09T20:13:36Z

plugins/node.d/smart_

+                    # Ignore Memory Disks, CD-ROM drives and Floppy
+                    continue
+                try:
+                    verboselog('Trying '+drive+'...')


Please add spaces around operators (+). PEP8 (and flake8) would appreciate this.

sumpfralle · 2020-07-09T20:17:42Z

plugins/node.d.freebsd/cpu

 	OSV=`$SYSCTL_BIN -n kern.osrelease | cut -f1 -d.`
-	if [ "$OSV" = "4" ]; then
+	if [ $(uname -s) = "MidnightBSD" ]; then
+	  STATUNITS=`$SYSCTL_BIN -n kern.clockrate | cut -f13 -d' '`


I think, you mixed up tabs and spaces here.

Additionally it would be great, if you could quote $(uname -s) in order to avoid and potential whitespace issues (and to please shellcheck).

After taking another look at it: maybe you can just add the condition to the branch for "$OSV -ge "5" below?

I changed the order so that the $(uname -s) = "MidnightBSD" || "$OSV -ge "5" check is before "$OSV" = "4" instead. Otherwise, when MidnightBSD hits version 4, things will break.

sumpfralle · 2020-07-09T21:17:10Z

plugins/node.d/smart_

@@ -589,6 +589,23 @@ def find_smart_drives():
                    continue
        except Exception as exc:
            verboselog('Failed to list FreeBSD disks: {}'.format(exc))
+    elif os.uname()[0] == "MidnightBSD":


After taking another look at this: I think, you can avoid code duplication here by simply changing the above condition from elif os.uname()[0] == "NetBSD" to elif os.uname()[0] in {"NetBSD", "MidnightBSD"}:.

And maybe customize the final error message as verboselog('Failed to list {} disks: {}'.format(os.uname()[0], exc))

sumpfralle · 2020-07-09T21:18:54Z

plugins/node.d/hddtemp_smartctl

@@ -187,6 +187,10 @@ if ($^O eq 'linux') {
  opendir(DEV, '/dev');
  @drives = grep /^(ada?|da)[0-9]+$/, readdir DEV;
  closedir(DEV);
+} elsif ($^O eq 'midnightbsd') {


This is the same as above - maybe just use } elsif (($^O eq 'freebsd') || ($^O eq 'midnightbsd')) {.

Every line counts :)

laffer1 · 2020-07-13T17:33:44Z

I've made the changes requested.

daftaupe · 2022-03-10T14:51:40Z

Hello, is that still expected to be merged someday ?
It would be a good basis to add DragonFlyBSD support :)

sumpfralle reviewed Jul 8, 2020

View reviewed changes

sumpfralle reviewed Jul 9, 2020

View reviewed changes

Add support for MidnightBSD to a number of plugins

b15fb12

laffer1 force-pushed the midnightbsd branch from 24d3404 to b15fb12 Compare July 13, 2020 17:29

steveschnepp added the [affects] branch master label Oct 29, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for MidnightBSD to a number of plugins #1342

Add support for MidnightBSD to a number of plugins #1342

laffer1 commented Jul 7, 2020

coveralls commented Jul 7, 2020 •

edited

h01ger commented Jul 8, 2020 via email

laffer1 commented Jul 8, 2020 •

edited

h01ger commented Jul 8, 2020 via email

sumpfralle left a comment

sumpfralle Jul 8, 2020

sumpfralle Jul 8, 2020

sumpfralle Jul 8, 2020

sumpfralle Jul 8, 2020

sumpfralle Jul 8, 2020

sumpfralle Jul 8, 2020

sumpfralle Jul 8, 2020

sumpfralle Jul 8, 2020

sumpfralle Jul 8, 2020

sumpfralle Jul 8, 2020

laffer1 commented Jul 9, 2020

sumpfralle left a comment

sumpfralle Jul 9, 2020

sumpfralle Jul 9, 2020

sumpfralle Jul 9, 2020

laffer1 Jul 13, 2020

sumpfralle Jul 9, 2020

sumpfralle Jul 9, 2020 •

edited

laffer1 commented Jul 13, 2020

daftaupe commented Mar 10, 2022

Add support for MidnightBSD to a number of plugins #1342

Are you sure you want to change the base?

Add support for MidnightBSD to a number of plugins #1342

Conversation

laffer1 commented Jul 7, 2020

coveralls commented Jul 7, 2020 • edited

Pull Request Test Coverage Report for Build 3007

💛 - Coveralls

h01ger commented Jul 8, 2020 via email

laffer1 commented Jul 8, 2020 • edited

h01ger commented Jul 8, 2020 via email

sumpfralle left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

laffer1 commented Jul 9, 2020

sumpfralle left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sumpfralle Jul 9, 2020 • edited

Choose a reason for hiding this comment

laffer1 commented Jul 13, 2020

daftaupe commented Mar 10, 2022

coveralls commented Jul 7, 2020 •

edited

laffer1 commented Jul 8, 2020 •

edited

sumpfralle Jul 9, 2020 •

edited