1
\$\begingroup\$

I have the below code which i'm working to copy(using rsync) the contents From remote host labserver01 and dumps those into the directory /var/log/infoSec/ on the base system from where scripts runs and this works correctly and sends e-mail to the recipients, However i'm also figuring out to include a way to send e-mail even if it fails.

I'm Just wondering if there is better way do this, i'm sure there will be more elegant ways.

Appreciate any idea and review in advance.

#!/usr/bin/python3
from email.mime.multipart import MIMEMultipart
from email.mime.text import MIMEText
import smtplib
import subprocess
import argparse
import sys
import os

#Dir Structure
dest_dir = "/infralogs/external_dns_logs"
rsync_user = "root"

email_sender = "dnslogger@udalt.com"
email_receiver = "gusain@udalt.com"
msg = ""

parser = argparse.ArgumentParser()
parser.add_argument("-n","--hosts",dest="hosts",help="enter remote host/hosts name, comma seperated",metavar="HOSTS")
parser.add_argument("-s","--src",dest="source",help="source file/directory",metavar="SOURCE")
parser.add_argument("-e","--exclude",dest="exclude",help="Exclude files/Directories, comma seperated list",metavar="EXCLUDE")

if len(sys.argv) < 7:
    print(len(sys.argv))
    parser.print_help()
    parser.exit()

args = parser.parse_args()

def sync(host,dest_dir):
    exclude = ""
    if not os.path.exists(dest_dir):
        os.mkdir(dest_dir)
    if ',' in args.exclude:
        for excl in args.exclude.split(','):
            exclude = exclude + " --exclude " + excl
        cmd = "rsync -e 'ssh -o StrictHostKeyChecking=no' -auPz %s %s@%s:%s %s/"%(exclude,rsync_user,host,args.source,dest_dir)
    else:
        cmd = "rsync -e 'ssh -o StrictHostKeyChecking=no' -auPz --exclude %s %s@%s:%s %s/"%(args.exclude,rsync_user,host,args.source,dest_dir)
    cmd_content = cmd
    p = subprocess.Popen(cmd,shell=True)
    p.wait()
    print("DONE")
    return cmd_content + " Rsync process completed."  # returns the msg to the caller

msglist = []             # a list to store the cmd_contents for the mail body

if ',' in args.hosts:
    for host in args.hosts.split(','):
        dest = dest_dir + "/" + host
        msglist.append(sync(host,dest))
else:
    dest = dest_dir + "/" + args.hosts
    msglist.append(sync(args.hosts,dest))

msg = "\n".join(msglist)                 # combine all cmd_contents, one per line


try:
    Mail = smtplib.SMTP('mailserver.global.udalt.com', 25, 'localhost.udalt.com')
    mail_obj = MIMEMultipart('alternative')
    mail_obj["From"] = email_sender
    mail_obj["To"] = email_receiver
    mail_obj["Cc"] = "gusain@udalt.com"
    mail_obj["Subject"] = "Rsync process completed Successfully."
    mail_obj.attach(MIMEText(msg, 'plain'))
    Mail.sendmail(from_addr=[email_sender], to_addrs=[email_receiver],msg=mail_obj.as_string())
    print("Mail Sent to %s" % (email_sender))
except Exception as error:
    print("Mail Failed - {}".format(error))

Command Execution method:

$ /usr/bin/dns_rsync.py -n labserver01 -s /var/log/infoSec/ -e "null"

\$\endgroup\$
1
\$\begingroup\$

Review

  1. Constants should be UPPER_SNAKE_CASE
  2. More (and better) functions!

    • Lot of this code is in the global namespace, which is bad (not maintainable)
    • Why is sync doing argument parsing?

      You should split up your code into individual instructions

      Same goes for the logic of hosts splitting, these really should be part of another function (maybe parse_arguments)

  3. Go with new style formatting or fstrings python3.6+

  4. Simplify the split(',') using join()
  5. Don't do manual path appending but use os.path.join()
  6. Top it off with a if __name__ == '__main__'

Code

#!/usr/bin/python3
from email.mime.multipart import MIMEMultipart
from email.mime.text import MIMEText
import smtplib
import subprocess
import argparse
import os

DEST_DIR = "/infralogs/external_dns_logs"
RSYNC_USER = "root"

EMAIL_SENDER = "dnslogger@udalt.com"
EMAIL_RECIEVER = "gusain@udalt.com"
EMAIL_CC = "gusain@udalt.com"

def parse_arguments():
    parser = argparse.ArgumentParser()
    parser.add_argument("-n","--hosts", dest="hosts", help="enter remote host/hosts name, comma seperated", metavar="HOSTS")
    parser.add_argument("-s","--src", dest="source", help="source file/directory", metavar="SOURCE")
    parser.add_argument("-e","--exclude", dest="exclude", help="Exclude files/Directories, comma seperated list", metavar="EXCLUDE")
    args = parser.parse_args()

    if args.exclude is None or args.source is None or args.hosts is None:
        parser.print_help()
        parser.exit()

    args.exclude = " --exclude " + " --exclude".join(args.exclude.split(','))
    args.hosts = args.hosts.split(',')
    return args.hosts, args.source, args.exclude

def sync(hosts, src, exclude):
    for host in hosts:
        host = host.strip()
        dest = os.path.join(DEST_DIR, host)
        cmd = f"rsync -e 'ssh -o StrictHostKeyChecking=no' -auPz {exclude} {RSYNC_USER}@{host}:{src} {dest}/"
        p = subprocess.Popen(cmd, shell=True)
        p.wait()
        yield f"{cmd} Rsync process completed."

def mail(message):
    try:
        Mail = smtplib.SMTP('mailserver.global.udalt.com', 25, 'localhost.udalt.com')
        mail_obj = MIMEMultipart('alternative')
        mail_obj["From"] = EMAIL_SENDER
        mail_obj["To"] = EMAIL_RECIEVER
        mail_obj["Cc"] = EMAIL_CC
        mail_obj["Subject"] = "Rsync process completed Successfully."
        mail_obj.attach(MIMEText(msg, 'plain'))
        Mail.sendmail(from_addr=[EMAIL_SENDER], to_addrs=[EMAIL_RECIEVER],msg=mail_obj.as_string())
        print(f"Mail Sent to {EMAIL_SENDER}")
        print(message)
    except Exception as error:
        print(f"Mail Failed - {error}")

if __name__ == '__main__':
    if not os.path.exists(DEST_DIR):
        os.mkdir(DEST_DIR)

    for msg in sync(*parse_arguments()):
        mail(msg)
\$\endgroup\$
  • \$\begingroup\$ @ Ludisposed, thanks for the review , this looks awesome and i'll borrow the suggestions and code styles as mentions, i'm reading on the reviewed code. \$\endgroup\$ – newTOcode Sep 20 '18 at 12:43

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.