Remotely collect server data using Net::OpenSSH

Question

I am newbie in Perl programming and currently trying to use Net::OpenSSH module in my code. My code as below which the task is to run multiple commands in remote server:

foreach $s (@servers) {
my $ssh = Net::OpenSSH->new("$username\@$s", timeout=>30);
$ssh->error and die "Unable to connect: " . $ssh->error;
print "Connected to $s\n";
my $fh = $ssh->capture("df -k /home/ | tail -1") or die "Unable to run command\n";
my @df_arr = split(/\s+/, $fh);
print "$s: Disk space /home/ = $df_arr[3] \n";
my $fh1 = $ssh->capture("svmon -G -O unit=GB | grep memory") or die "Unable to run command\n";
my @sv_arr = split(/\s+/, $fh1);
print "$s: Free memory = $sv_arr[3] \n\n";
close $fh;
undef $ssh;
}

This code is not so nice since I plan to simplify and reduce a line as many as possible.

Are there any techniques or methods that I can use to simplify this code?

• 200_success: Why not use standard tools, such as SNMP, instead of writing your own hacks?
• chicks: SNMP is a hard to manage, but there are a slow of alternatives for this that have more history and ease of use like nagios, OMD, or munin. I'm assuming this question is just a learning exercise and not the beginnings of a new monitoring system.
• Mathieu Guindon: This exact same question/code was posted here on September 6, 2017 12:28 UTC. Is it your code?
• MrAZ: @Mat'sMug Hi, Im not posting my code to that website...it seem someone post it there.

Answer 1

Looks good to me. Kudos for the "... or die" idiom. More informative output would be "unable to run df on $s" or "unable to run svmon on $s".

You don't have to do tail or grep on the far end, as a line of perl could do that. But it is a perfectly clean and sensible approach, which I advocate keeping.

If you wind up with many such monitoring commands, you may find it convenient to move the post-processing into perl commands that can be refactored out as utility functions.

• simbabque: I can only add that proper indentation and more verbose variable names would improve legibility.

Answer 2

You should add indentation inside the loop:

foreach $s (@servers) {
 my $ssh = Net::OpenSSH->new("$username\@$s", timeout=>30);

---

Consider this line of code:

foreach $s (@servers) {

The variable name $s is not too descriptive. It would be better as $server.

Also, it should be localized to the loop using the my keyword.

Since foreach is identical to for, you can just shorten it to for .

for my $server (@servers) {

---

It is great that you check success of the commands. As the other answer mentioned, it would be better from the user perspective to generate unique messages for the df and svmon commands.

You can also add the $ssh->error output to those messages as you did when you checked new().

---

Again, the variable name $fh is not too descriptive, and that name is often associated with open file handles.

It might be better as $output.

You can also reuse $output instead of creating another variable ($fh1).

Since capture does not return a file handle, there is no need to call close on it. This line can be deleted:

close $fh;