emjga1 has asked for the wisdom of the Perl Monks concerning the following question:

Folks

I have written a Perl Script to auto generate a Nagios configuration file

While the code works very well. Snipit of it is below. It does not look good and is vary hard to maintanin or to add new bits to.

Can any body point me in the right direction to do things a bit better. e.g possable use of Forms

Thanks

Matt

Snipit of code - 

foreach (@MASTER) {
        chomp;
        @line = split(/\t/,$_);
        $nodename = $line[0];
        $machine_type = $line[2];
        $comment = $line[6];
        $location = $line[8];
        $machine = $line[10];
        $serial = $line[12];
        $fname = $line[16];
        $lname = $line[18];
        $phone = $line[20];
        $racf = $line[22];
        $installed = $line[24];
        $type = $line[26];
#print "NODE $nodename TYPE $machine_type COMMENT $comment LOCATION $l
+ocation MACHINE $machine SERIAL $serial Fist na
me $fname Last name $lname Phone $phone RACF $racf $type\n";
        nslookup();

        if (@found = grep(/\b$nodename\b/,@exceptions)) {
                print "found $nodename in exceptions - ignoring\n";
        } else {

        #if (@found = grep(/\b$type\b/,server)) {
        #        print "Node $nodename is a server\n";
        #} else { 
        #print "Node $nodename is a workstation\n"; }


        print "Doing $nodename\n";
        print autoconf "\n# Host $nodename $machine_type check host al
+ive \n";
        print autoconf "define host{\n";
        print autoconf "use     generic-host\n";
        print autoconf "host_name       $nodename\n";
        print autoconf "alias   $comment\n";
        print autoconf "address $IPnumber\n";
# If a Sun T3 then use normal ping command and not the New ping
        if (@T3 = grep(/\b$machine_type\b/,"sun_t3")) {
        print autoconf "check_command   check-host-alive-ping\n";
        } else {
        print autoconf "check_command   check-host-alive\n";
        }
        print autoconf "max_check_attempts      10\n";
        print autoconf "notification_interval   7\n";
        print autoconf "notification_period     24x7\n";
        print autoconf "notification_options    d,u,r\n";
        print autoconf "}\n";
        print autoconf " \n";
        print autoconf "#Serviceescalation definition $nodename $machi
+ne_type PING\n";
        print autoconf "define serviceescalation{\n";
        print autoconf "host_name       $nodename\n";
        print autoconf "service_description     PING\n";
        print autoconf "first_notification      2\n";
        print autoconf "last_notification       2\n";
        print autoconf "contact_groups  $machine_type-admins-escalatio
+n\n";
        print autoconf "notification_interval   0\n";
        print autoconf "}\n";
        print autoconf "\n";
        print autoconf "#Service Definition $nodename $machine_type PI
+NG\n";
        print autoconf "define service{ \n";
        print autoconf "use     generic-service \n";
        print autoconf "host_name       $nodename\n";
        print autoconf "service_description     PING\n";
        print autoconf "is_volatile     0\n";
        print autoconf "check_period    24x7\n";
        print autoconf "max_check_attempts      3\n";
        print autoconf "normal_check_interval   5\n";
        print autoconf "retry_check_interval    1\n";
        print autoconf "contact_groups  $machine_type-admins\n";
        print autoconf "notification_interval   7\n";
        print autoconf "notification_period     24x7\n";
        print autoconf "notification_options    c,r\n";
# If a Sun T3 then use normal ping command and not the New ping
        if (@T3 = grep(/\b$machine_type\b/,"sun_t3")) {
        print autoconf "check_command   check_ping\n";
        } else {
        print autoconf "check_command   check_newping\n";
        }
        print autoconf "}\n";
        print autoconf "\n";

        print autoconfnscgi "\n# Extended info for client $nodename $m
+achine_type using info.cgi script\n";


                if (@SUN = grep(/\b$machine_type\b/,"sun")) {

                        print autoconfnscgi "define hostextinfo{\n";
                        print autoconfnscgi "host_name  $nodename\n";
                        print autoconfnscgi "notes_url  info.cgi?host=
+$nodename&type=$machine_type&comment=$comment&l
ocation=$location&machine=$machine&serial=$serial&fname=$fname&lname=$
+lname&phone=$phone&racf=$racf\n";
                        print autoconfnscgi "icon_image $machine_type.
+gif\n";

[download]

update (broquaint): dropped <code> tags and added formatting
(ybiC): balanced <readmore> tags, remove trainling (Long) from title

Replies are listed 'Best First'.
Re: Better use of perl print statment for Nagios, (Long)
by liz (Monsignor) on Jul 30, 2003 at 13:58 UTC
    Seems like you could use "here docs" (perldoc perlop): 
    print autoconf <<EOD;

# Host $nodename $machine_type check host alive
define host{
  use     generic-host
  host_name       $nodename
  alias   $comment
  address $IPnumber
(yadayadayada)
EOD

    [download]
    Liz
[reply]
[d/l]
Re: Better use of perl print statment for Nagios, (Long)
by l2kashe (Deacon) on Jul 30, 2003 at 14:51 UTC

    A couple of thoughts

    Since it appears that the data you are interested in is always in the same offsets of your split you could define them once, and then use them throught out the rest of the code. (You could also use Constant, but read up on the docs and do a super search, as interesting problems can arise from using the module). You could use the select function to clean up the prints, or a HERE document. If you end up going with HERE docs, I would suggest doing whatever "extra" logic prior to, so that you don't need to end the print, perform a simple test and reinitiate printing.. Ala

    # snipped for brevity's sake
my $nodename = '0';
my $m_type   = '2';
my $comment  = '6'; # etc... 

for ( @MASTER ) {
   my($chk_cmd, $chk_ping);
   chomp;
   my $line = [ split("\t") ];

# If its a t3 array these lines need to be different, do it 
# now as opposed to later
   if ( $line->[$m_type] eq 'sun_t3') {
      $chk_cmd  = "check_command\tcheck-host-alive-ping\n";
      $chk_ping = "check_command\tcheck_ping\n";
   } else {
      $chk_cmd  = "check_command\tcheck-host-alive\n";
      $chk_ping = "check_command\tcheck_newping\n";
   }

   print autoconf<<EOF;

   # Host $line->[$nodename] $line->[$m_type] check host alive
   define host {
      use     generic-host
      host_name       $line->[$nodename]
      alias   $line->[$comment]
      address $IPnumber
      $chk_cmd
   }
   EOF
}

    [download]

    Also since quite a bit of that content is relativly static, I might end up using an array to store bits and pieces, then joining them into single scalars ala my $out = join("\n", @data); to further clean up the output.

    use perl;

[reply]
[d/l]
Re: Better use of perl print statment for Nagios, (Long)
by BrowserUk (Patriarch) on Jul 30, 2003 at 15:58 UTC

    There are many things that you could do to make this more readable/maintainabe.

    You could avoid having to use the filehandle on all your print statements by using perlfunc:select to make your file.

    You can avoid having to explicitly put "\n" on the end of each line by setting local $\="\n" (see perlvar:$OUTPUT_RECORD_SEPERATOR). This can also be achieved using -l (el) on the command line or shebang line.

    You could avoid having to use the print keyword on every line by using multiline quotes. Eg. 

    print 
"this is line 1 $somevar
this is line 2 $someothervar

this is line 4
this is line 5
";

    [download]

    You could avoid the long block initialising all the individual variables using array slices. 

    use constant { NODE=>0, TYPE=>1, COMMENT=>2 ...};
my @d;
@d = ( split '\t' )[ 0,2,6,8,10,12,16,18,20,22,24,26 ];

select AUTOCONF;
print "
# Host @d[NODE, TYPE, CHECK, HOST, ALIVE]
define host{\n
use   generic-host
host_name    $d[NODE]
alias  $d[COMMENT]
address $d[IP]
max_check_attempts    10
notification_interval   7
notification_period   24x7
notification_options   d,u,r
}
";

    [download]

    Here docs or formats might be even better, but the whole thing is screaming out for a template system like Text::Template.

    Examine what is said, not who speaks.
    "Efficiency is intelligent laziness." -David Dunham
    "When I'm working on a problem, I never think about beauty. I think only how to solve the problem. But when I have finished, if the solution is not beautiful, I know it is wrong." -Richard Buckminster Fuller
[reply]
[d/l]
[select]
Re: Better use of perl print statment for Nagios
by emjga1 (Novice) on Jul 31, 2003 at 09:03 UTC
    Thanks to everyone for there kind and very helpfull input

    I think in the first instance I will look at using

    Here Docs , (Liz)

    Long term goal to look at Template system like Text::Template , (BrowserUk)

    Matt

[reply]

Back to Seekers of Perl Wisdom