Merge pull request #372 from ssm/feature/more-robust-async

Feature/more robust async

lib/Munin/Common/Utils.pm

@@ -0,0 +1,59 @@
+package Munin::Common::Utils;
+
+use strict;
+use warnings;
+
+use Exporter ();
+our @ISA = qw/Exporter/;
+our @EXPORT_OK = qw( is_valid_hostname );
+
+use Params::Validate qw( :all );
+use List::Util qw( all any );
+
+### Set operations #############################################################
+
+sub is_valid_hostname {
+    my ($hostname) = validate_pos(@_, { type => SCALAR } );
+
+    # anything?
+    return unless $hostname;
+
+    # total length
+    return if length($hostname) > 255;
+
+    # each part
+    my @parts = (split(/[.]/, $hostname));
+    return if any { length > 63 } @parts;
+    return if any { ! /^[a-z0-9\-]+$/ } @parts;
+
+    return $hostname;
+
+}
+
+
+1;
+
+__END__
+
+
+=head1 NAME
+
+Munin::Common::Utils - Various utility functions
+
+
+=head1 SYNOPSIS
+
+  use Munin::Common::Utils qw( ... );
+
+
+=head1 SUBROUTINES
+
+=over
+
+=item B<is_valid_hostname($hostname)>
+
+Returns $hostname if it is syntactically valid, or an undef if not.
+
+=back
+
+=cut

lib/Munin/Node/SpoolReader.pm

@@ -13,25 +13,34 @@ use Munin::Common::Defaults;
 use Munin::Common::SyncDictFile;
 use Munin::Common::Logger;
 
+use Params::Validate qw(:all);
+
 use Munin::Node::Config;
 my $config = Munin::Node::Config->instance;
 
 
-sub new
-{
-    my ($class, %args) = @_;
-
-    $args{spooldir} or croak "no spooldir provided";
+sub new {
+    my $class = shift;
+    my $validated = validate (
+        @_, {
+            spooldir => {
+                type    => SCALAR,
+                default => $Munin::Common::Defaults::MUNIN_SPOOLDIR
+            },
+        }
+    );
+    my $self = bless {}, $class;
 
-    opendir $args{spooldirhandle}, $args{spooldir}
-        or croak "Could not open spooldir '$args{spooldir}': $!";
+    $self->{spooldir} = $validated->{'spooldir'};
 
-    $args{metadata} = _init_metadata($args{spooldir});
+    my $spooldirhandle;
+    opendir $spooldirhandle, $self->{spooldir}
+      or croak "Could not open spooldir '$self->{spooldir}': $!";
+    $self->{spooldirhandle} = $spooldirhandle;
 
-    # TODO: paranoia check?  except dir doesn't (currently) have to be
-    # root-owned.
+    $self->{metadata} = _init_metadata($self->{spooldir});
 
-    return bless \%args, $class;
+    return $self;
 }
 
 

lib/Munin/Node/SpoolWriter.pm

@@ -12,51 +12,61 @@ use Fcntl qw(:DEFAULT :flock);
 use Munin::Common::Defaults;
 use Munin::Common::SyncDictFile;
 use Munin::Common::Logger;
+use Munin::Common::Utils qw( is_valid_hostname );
 
+use Params::Validate qw( :all );
+use List::Util qw( first );
 
 use constant DEFAULT_TIME => 86_400;      # put 1 day of results into a spool file
 use constant MAXIMUM_AGE  => 7;           # remove spool files more than a week old
+use constant DEFAULT_HOSTNAME => 'munin.example.com';
 
 sub _snap_to_epoch_boundary { my $self = shift; return $_[0] - ($_[0] % $self->{interval_size}) }
 
+sub new {
+    my $class = shift;
+    my $validated = validate(
+        @_, {
+            spooldir => {
+                type => SCALAR,
+                default => $Munin::Common::Defaults::MUNIN_SPOOLDIR,
+            },
+            interval_size => { type => SCALAR, optional => 1 },
+            interval_keep => { type => SCALAR, optional => 1 },
+            hostname      => { type => SCALAR, optional => 1 },
+        }
+    );
+    my $self = bless {}, $class;
+
+    $self->{spooldir} = $validated->{spooldir};
+
+    my $spooldirhandle;
+    opendir $spooldirhandle, $self->{spooldir}
+      or croak "Could not open spooldir '$self->{spooldir}': $!";
+    $self->{spooldirhandle} = $spooldirhandle;
+
+    $self->{metadata} = _init_metadata($self->{spooldir});
+
+    $self->{interval_size} = first { defined($_) and $_ > 0 } (
+        $validated->{interval_size},
+        $self->{metadata}->{interval_size},
+        DEFAULT_TIME
+    );
+
+    $self->{interval_keep} = first { defined($_) and $_ > 0 } (
+        $validated->{interval_keep},
+        $self->{metadata}->{interval_keep},
+        MAXIMUM_AGE,
+    );
+
+    $self->{hostname} = first { defined($_) and is_valid_hostname($_) } (
+        $validated->{hostname},
+        $self->{metadata}->{hostname},
+        DEFAULT_HOSTNAME,
+    );
+
+    return $self;
 
-sub new
-{
-    my ($class, %args) = @_;
-
-    $args{spooldir} or croak "no spooldir provided";
-
-    opendir $args{spooldirhandle}, $args{spooldir}
-        or croak "Could not open spooldir '$args{spooldir}': $!";
-
-    $args{metadata} = _init_metadata($args{spooldir});
-
-    if(!$args{interval_size} && (my $interval_size = get_metadata(\%args, "interval_size"))) {
-	    $args{interval_size} = $interval_size;
-    }
-
-    if(!$args{interval_keep} && (my $interval_keep = get_metadata(\%args, "interval_keep"))) {
-	    $args{interval_keep} = $interval_keep;
-    }
-
-    if(!$args{hostname} && (my $hostname = get_metadata(\%args, "hostname"))) {
-	    $args{hostname} = $hostname;
-    }
-
-    $args{interval_size} = DEFAULT_TIME unless ($args{interval_size});
-    $args{interval_keep} = MAXIMUM_AGE unless ($args{interval_keep});
-    $args{hostname} = "unknown" unless ($args{hostname});
-
-    set_metadata(\%args, "interval_size", $args{interval_size}) if $args{interval_size} != get_metadata(\%args, "interval_size");
-    set_metadata(\%args, "interval_keep", $args{interval_keep}) if $args{interval_keep} != get_metadata(\%args, "interval_keep");
-    set_metadata(\%args, "hostname", $args{hostname}) if $args{hostname} ne get_metadata(\%args, "hostname");
-
-    # TODO: paranoia check?  except dir doesn't (currently) have to be
-    # root-owned.
-
-    # TODO: set umask
-
-    return bless \%args, $class;
 }
 
 

script/munin-async

@@ -28,6 +28,8 @@ use Pod::Usage;
 use Munin::Node::SpoolReader;
 use Munin::Node::SpoolWriter;
 use Munin::Common::Defaults;
+use Munin::Common::Logger;
+use Munin::Common::Utils qw( is_valid_hostname );
 
 # Disable buffering
 $| = 1;
@@ -43,6 +45,7 @@ my $cleanupandexit;
 
 my $verbose;
 my $debug;
+my $screen;
 my $help;
 
 GetOptions(
@@ -57,6 +60,7 @@ GetOptions(
         "help|h" => \$help,
         "verbose|v" => \$verbose,
         "debug" => \$debug,
+        "screen" => \$screen,
 ) or pod2usage(1);
 if ($help) {
         pod2usage(1);
@@ -68,6 +72,20 @@ if ($cleanupandexit) {
 	exit;
 }
 
+if ($overridehost) {
+    if (! is_valid_hostname($overridehost)) {
+        CRITICAL(sprintf("invalid hostname: %s\n", $overridehost));
+        exit(65);
+    }
+}
+
+if ( $verbose || $debug || $screen ) {
+    my %log;
+    $log{output} = 'screen' if $screen;
+    $log{level}  = 'info'   if $verbose;
+    $log{level}  = 'debug'  if $debug;
+    Munin::Common::Logger::configure(%log);
+}
 
 # Use STDIN/STDOUT, in order to be : 
 # 1. secure over internet (SSH), munin-node needs only 
@@ -82,7 +100,7 @@ $hostname = $spoolreader->get_metadata("hostname") || hostname();
 $hostname = $overridehost if $overridehost;
 chomp($hostname);
 
-die "spooldir [$SPOOLDIR] not found" unless -d $SPOOLDIR;
+INFO("Starting");
 
 print "# munin node at $hostname\n";
 
@@ -154,5 +172,8 @@ munin-async [options]
                                   Note that without this flag, the "fetch" 
 				  command is disabled.
 
+     --screen                   Log to screen instead of syslog
+     --debug                    Log debug messages
      -v --verbose               Be verbose
      -h --help                  View this message	
+

script/munin-asyncd

@@ -30,6 +30,7 @@ use List::Util qw(min max);
 
 use Munin::Node::SpoolWriter;
 use Munin::Common::Defaults;
+use Munin::Common::Logger;
 
 my $host = "localhost:4949";
 my $metahostname;
@@ -42,6 +43,7 @@ my $nocleanup;
 my $do_fork;
 my $verbose;
 my $debug;
+my $screen;
 my $help;
 my $update_rate = 300;
 my @nodes;
@@ -62,21 +64,35 @@ GetOptions(
 	"verbose|v" => \$verbose,
 	"nocleanup|n" => \$nocleanup,
 	"debug" => \$debug,
+        "screen" => \$screen,
 ) or pod2usage(1);
 if ($help) {
 	pod2usage(1);
 }
 
+if ( $verbose || $debug || $screen ) {
+    my %log;
+    $log{output} = 'screen' if $screen;
+    $log{level}  = 'info'   if $verbose;
+    $log{level}  = 'debug'  if $debug;
+    Munin::Common::Logger::configure(%log);
+}
+
 # $minrate defaults to $update_rate.
 $minrate = $update_rate if ! defined $minrate;
 
 # Debug implies Verbose
 $verbose = 1 if $debug;
 
+unless (-d $SPOOLDIR) {
+	mkpath($SPOOLDIR, { verbose => $verbose, } ) 
+		or LOGCROAK("Cannot create '$SPOOLDIR': $!");
+}
+
 my $sock = new IO::Socket::INET(
 	PeerAddr        => "$host",
 	Proto   => 'tcp'
-) || die "Error creating socket: $!";
+) || LOGCROAK("Error creating socket: $!");
 my $nodeheader = <$sock>;
 print STDERR "[sock][>] nodes\n" if $debug;
 print $sock "nodes\n";
@@ -113,14 +129,14 @@ my $process_name = "main";
 
 my $plugins = {};
 {
-	print STDERR "[$$][$process_name] Reading config from $host\n" if $verbose;
+	INFO("[$$][$process_name] Reading config from $host");
 	my $sock = new IO::Socket::INET( 
 		PeerAddr	=> "$host", 
 		Proto	=> 'tcp'
 	) || die "Error creating socket: $!"; 
 
 	local $0 = "munin-asyncd [$metahostname] [list]";
-	print STDERR "[sock][>] cap multigraph\n" if $debug;
+	DEBUG("[sock][>] cap multigraph");
 	print $sock "cap multigraph\n";
 	<$sock>; # Read the first header comment line
 	<$sock>; # Read the multigraph response line
@@ -236,10 +252,10 @@ MAIN: while($keepgoing) {
 	my $sleep_sec = $when_next - time;
 
 	if ($sleep_sec > 0) {
-		print STDERR "[$$][$process_name] Sleeping $sleep_sec sec\n" if $verbose;
+		INFO("[$$][$process_name] Sleeping $sleep_sec sec");
 		sleep $sleep_sec;
 	} else {
-		print STDERR "[$$][$process_name] Already late : should sleep $sleep_sec sec\n" if $verbose;
+		INFO("[$$][$process_name] Already late : should sleep $sleep_sec sec\n");
 	}
 }
 
@@ -284,13 +300,13 @@ sub fetch_data
 			push @$output_rows, "update_rate $update_rate";
 		}
 
-		print STDERR "[$$][$process_name][>][$plugin] asking for data\n" if $verbose;
-		print STDERR "[$$][$process_name][>][$plugin][sock] fetch $plugin\n" if $debug;
+		INFO("[$$][$process_name][>][$plugin] asking for data\n");
+                DEBUG("[sock][>][$plugin] fetch $plugin\n");
 		print $sock "fetch $plugin\n";
 
 		while(my $line = <$sock>) {
 			chomp($line);
-			print STDERR "[$$][$process_name][<][$plugin][sock] $line\n" if $debug;
+			DEBUG("[sock][<][$plugin] $line");
 
 			if ($line =~ m/^\./) {
 				# Starting with . => end
@@ -327,5 +343,7 @@ munin-asyncd [options]
      -n --nocleanup             Disable automated spool dir cleanup
 
         --fork                  Do fork
+        --screen                Log to screen instead of syslog
+        --debug                 Log debug messages
      -v --verbose               Be verbose
      -h --help                  View this message