Script works, but I think it could be better and faster

Hi All,

I'm new to the forum and to bash scripting. I did some stuff with VB.net, Batch, and VBScripting in the past, but because I shifted over to Linux, I am learning to script in Bash at this moment. So bear with me if I seem to script like a newbie, that's just because I am :wink:

OK, I wrote this script. It does the job, but it's rather slow. I get the server names, files, and password from separate text files. I place the servers and files into a list and loop them, but I configured only 2 local VM's at the moment and it runs rather slow already. I think I know why... It is logging into the server every time and then gets the file(s) I need. How do I overcome this? So, in my opinion, it just needs to login once and then gets the files off and after that move on to the other server.
Let me know what your thoughts are

Here it is:

#!/usr/bin/env bash

############################################ Global Declarations #################################################

home_dir="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null && pwd )"
dest_dir="/home/cornelis/config_files_properties"
ssh_user="root"
ssh_pass=$home_dir"/password_file" # Not needed on the script server
logfile="/tmp/config_files_properties.log"


############################################### Do the job #####################################################

## first thing to check if we are root, if not close
#
#if [ $(whoami) != 'root' ]; then
#        echo "Must be root to run $0"
#        exit 1;
#fi
# redirect all output to log file

exec &> >(while read -r line; do printf '%s %s\n' "$(date)" "$line"; done >> $logfile)

## check if destination folder exists and if not make it

if [[ ! -d "$dest_dir" ]]; then
    mkdir -p $dest_dir
fi

# Deleting existing files in the destination folder (if empty send error to /dev/null)

# extra error handling just in case, before removing anything

cd $dest_dir
if [[ "$?" = "0" ]]; then


# empty folder

shopt -s dotglob && rm -r $dest_dir/* > /dev/null 2>&1

# get the files off the servers and place them in the destination folder in its own folder

index=0
while read line; do
  server_array[index]="$line"
    for server in ${server_array[*]}
    do
    # create the folders named per server
        mkdir -p $dest_dir/$server
          #find the correct files
          index2=0
         while read line2; do
           file_array[index2]="$line2"
           for file in ${file_array[*]}
            do
            #copy the files to the folders
           sshpass -f $ssh_pass scp -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -rpqv $ssh_user@$server:$file $dest_dir/$server
           done
          done < $home_dir/files
    done

done < $home_dir/test_server_names
# change the directory and run SED on the files and folders
cd $dest_dir || exit 1

find ./ -type f -print0 | xargs -0 sed -Ei 's/([^&]password=)([^\t "]*)/\1xxxxxx/gI; s/(password.*>).*(<)/\1xxxxxx\2/gI; s/(password" value=").[^"]*/\1xxxxxx/gI; s/=admin:.[^\t ]*/=admin:xxxxxx/g; /"Password">/ {n;N;s/([\t ]*).*(\n.*<\/)/\1xxxxxx\2/gI}'

#zip the folders into one zip file

zip -rq9 properties.zip *


else
	echo "Cannot change directory to delete old files! Aborting! " 1>&2 >> $logfile
	exit 1
fi



# open the folder with contents

xdg-open $dest_dir

I don't see an immediate reason why your script should run slow. Try using the -x ( xtrace option to isolate where / which line it has its difficulties; mayhap the remote login?
Please post all files' contents so people get a chance to follow what you are doing.

Some remarks:

  • you use arrays, but don't increment / modify the respective indices?
  • you're using FOUR nested loops, to what avail?
  • how do you expect to use ssh_pass variable / file on various servers?

Can you explain how to implement that xtrace? Is this an option on SCP? I can't find it in the manpage

And here is my reason that I think it can be better. It logs in each time it needs a file, instead of log in once and then gets ALL the files off and after that, it should log off and log on to the next server. in my noob opinion, this would save time. Correct me if I am wrong. I just don't know how to do that.
For two servers now it takes about 10 seconds, but in production, it will be a lot more and some are fast responding and some are not, so I want to have the script as fast as possible.

there is not much to it.
I have three plain text files
1 with two servers in it, each server on its own line like:
192.168.122.7
192.168.122.74

1 with the path to the files needed like this:
/path/to/file
/path/to/file2

also each on its own line.

the last file is the password file and has only one password in it. I don't need the password in production, because the script server already has root access to all the other servers and doesn't need to log in.

Some remarks:
[QOUTE]- you use arrays, but don't increment / modify the respective indices?
[/quote]

do you have a link to an explanation what you mean, so I check that and learn?

one for error handling (show a message and exit the script when the folder that needs to be empty, doesn't exist.)
one to loop through the servers
one to create the folders per server name
one to loop through the file paths

As I explained above I don't need that in production but have that in place for my test VM's, they both have the same password, so I don't need to run through that file.

I included my log file to this message

Thanks for the quick response btw!

With kind regards,

Cornelis

btw I forgot to mention that one VM has the files and the other doesn't, just for testing purposes. SO you will see that in the log.

Your shell - which you failed to mention, BTW - should provide the -x (xtrace) option to print to stderr the commands / lines it is about to execute.
One option to accelerate the script would be to optimise the order in which the files are accessed / evaluated, and to drop the for loops. On top, there's no need to read the files' file again and again, if the files don't change with the respective servers. Read once and build the scp into a variable.Try something along

readarray FNARR < filesfile
while read SVNM
  do CHKDIR/MKDIR serverdir
     scp -options $SVNM/${FNARR[@]} serverdir
  done < serverfile

copying all target files from a single server in one go... This is an untested frame / pseudo code proposal, so YMMV.

Thanks for the reply, I am working with GNU bash, version 4.4.23(1)-release (x86_64-redhat-linux-gnu)
on Fedora 29 at the moment. I use PyCharm as an IDE.
I will give your proposal a try.

Hi,

I try to get my head around it and have narrowed it down to a small test script

############################################## Declarations #######################################################

home_dir="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null && pwd )"
dest_dir="/home/cornelis/config_files_properties"
ssh_user="root"
ssh_pass=$home_dir"/password_file" # Not needed on the script server
logfile="/tmp/config_files_properties.log"


################################################ Do the job #######################################################
#
index=0
readarray FNARR < $home_dir/files
while read SVNM
  do if [[ ! -d "$dest_dir$SVNM" ]]; then
    mkdir -p $dest_dir/$SVNM
fi
     sshpass -f $ssh_pass scp -o GSSAPIAuthentication=no -o GSSAPIKeyExchange=no -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -rpqv $ssh_user@$SVNM:${FNARR[@]} $FNARR

     echo $ssh_user@$SVNM${FNARR[@]} $FNARR
echo $SVNM
  done < $home_dir/test_server_names

xdg-open $dest_dir

What I get now is "No such file or directory" and when I echo the path and server they are correct, plus the file exists on the server.
What am I not getting here? I must be doing something wrong somewhere.

Thanks in advance.

Cornelis

------ Post updated at 03:05 PM ------

Got it working like this now. Already reduced to 1 for loop:

index=0
readarray FNARR < $home_dir/files
while read SVNM
  do if [[ ! -d "$dest_dir/$SVNM" ]]; then
    mkdir -p $dest_dir/$SVNM
     fi
    for file in ${FNARR[*]}
                    do
   sshpass -f $ssh_pass scp -o GSSAPIAuthentication=no -o GSSAPIKeyExchange=no -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -rpqv $ssh_user@$SVNM:$file $dest_dir/$SVNM
    done

   done < $home_dir/test_server_names

xdg-open $dest_dir
1 Like

Thanks for trying - that's exactly the way to take. Reduce complexity, here: to one single file, and proceed step by step.

scp - as can cp - can copy multiple files into a target directory in one go. Your bash version offers "Parameter Expansion / Pattern substitution", so try like

scp  ${FNARR[@]/#/$ssh_user@$SVNM:} $SVNM

That doesn't seem to work properly. It places the files on one line again and I can't get it fixed....
here is what it logs:

+ for file in ${FNARR
[*]}
+ sshpass -f /home/cornelis/PycharmProjects/BashProjects/conf_files_properties/password_file scp -o GSSAPIAuthentication=no -o GSSAPIKeyExchange=no -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -rpq root@192.168.122.74:/etc/httpd22/conf.d/proxy_ajp.conf root@192.168.122.74:/etc/httpd22/conf.d/mod_cluster.conf root@192.168.122.74:/etc/httpd22/conf.d/mod_jk_workers.prorperties root@192.168.122.74:/etc/httpd22/conf.d/mod_jk.conf root@192.168.122.74:/etc/famed/defacto/components.properties root@192.168.122.74:/etc/jbossas/domain/domain.xml root@192.168.122.74:/etc/jbossas/domain/host.xml root@192.168.122.74:/etc/jbossas/domain/host-master.xml root@192.168.122.74:/etc/jbossas/domain/host-slave.xml /home/cornelis/config_files_properties/192.168.122.74
scp: /etc/httpd22/conf.d/proxy_ajp.conf: No such file or directory

I can see why it can't find it, but the for loop doesn't fix that anymore.

I have it like this at the moment:

index=0
readarray FNARR < $home_dir/files
while read SVNM
  do if [[ ! -d "$dest_dir/$SVNM" ]]; then
    mkdir -p $dest_dir/$SVNM
     fi
    for file in ${FNARR
[*]}
        do

   sshpass -f $ssh_pass scp -o GSSAPIAuthentication=no -o GSSAPIKeyExchange=no -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -rpq ${FNARR[@]/#/$ssh_user@$SVNM:} $dest_dir/$SVNM



   #sshpass -f $ssh_pass scp -o GSSAPIAuthentication=no -o GSSAPIKeyExchange=no -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -rpqv $ssh_user@$SVNM:$file $dest_dir/$SVNM
        done

   done < $home_dir/test_server_names

xdg-open $dest_dir

Does /etc/httpd22/conf.d/proxy_ajp.conf exist on 192.168.122.74 ?

And, what did we (better: you) find earlier? Prove validity of approach with a single or few files ...

On 192.168.122.74 they do exist, but it seems like it just gets to that entry only and doesn't read the list anymore.