Ruby backup/restore for provisioning

Question

This is my second Ruby coding attempt. Also - this is my first OOP usage in Ruby.

I'm using if __FILE__ == 0ドル form here - same as it is in Python because it's really comfortably as for me. Is there is any objections/restrictions for such usage?

Any other tips/links about this code appreciated as well.

Script intended to create Consul raft-database backup before run destroy.rb from my previous review, or restore database after provisioning.

#!/usr/bin/env ruby
require 'net/ssh'
$me = File.basename(__FILE__)
class CheckIn
 USER = 'knife';
 RSA_KEY = 'ssh/id_rsa';
 def initialize(host)
 @@host = host;
 if File.exist?(RSA_KEY) then
 File.chmod(0600, RSA_KEY);
 else
 abort "[#{$me}] ERROR: no RSA file #{RSA_KEY} found. Exit.\n";
 end
 end
end
class BackupRestore < CheckIn
 def backup()
 _consul_raft_db = '/opt/consul/data/raft/raft.db'
 _root_backup_dir = '/root/vaul_devops_kdbx/backups'
 _raft_backup_file = "#{Time.now.strftime("%H_%M_%S_%d_%m_%y")}_bkp_raft.db"
 Net::SSH.start(@@host, USER, :keys => RSA_KEY) do |ssh|
 backuped = ssh.exec!("
 if test -e #{_consul_raft_db}; then
 echo \"\"
 echo \"OK: Consul DB found - #{_consul_raft_db}.\"
 echo \"\"
 else
 echo \"ERROR: no #{_consul_raft_db} file found. Exit.\"
 exit 1
 fi
 if sudo [ -d #{_root_backup_dir} ]; then
 echo \"OK: #{_root_backup_dir} found.\"
 echo \"\"
 else
 echo \"ERROR: no #{_root_backup_dir} directory found. Exit.\"
 exit 1
 fi
 if sudo cp #{_consul_raft_db} #{_root_backup_dir}/#{_raft_backup_file}; then
 echo \"OK: backup created:\"
 echo \"\"
 sudo ls -l #{_root_backup_dir}/#{_raft_backup_file}
 echo \"\"
 echo \"All backups present on the #{@@host} in the #{_root_backup_dir}:\"
 echo \"\"
 sudo ls -l #{_root_backup_dir}
 echo \"\"
 else
 echo \"\"
 echo \"ERROR: can not complete backup from #{_consul_raft_db} to #{_root_backup_dir}/#{_raft_backup_file}. Exit.\"
 exit 1
 fi
 ")
 puts backuped
 end
 end
 def restore()
 puts "\nRestore will be here\n\n"
 end
end
if __FILE__ == 0ドル
 puts "\n[#{$me}] Consul backup/restore started.\n"
 if ARGV.length == 2 and (ARGV[1] == 'backup' or ARGV[1] == 'restore')
 _host = ARGV[0]
 _mode = ARGV[1]
 puts "\n[#{$me}] Working on the #{_host} in #{_mode} mode.";
 run = BackupRestore.new(_host)
 run.send(_mode)
 else
 abort "\n[#{$me}] ERROR: HOST must be specified as first argument and MODE (backup or restore) - as second one. Exit..\n"
 end
 puts "[#{$me}] Consul backup/restore finished.\n\n"
end

And it's execution - backup:

$ ./scripts/backup.rb setevoy.vault.local backup
[backup.rb] Consul backup/restore started.
[backup.rb] Working on the setevoy.vault.local in backup mode.
OK: Consul DB found - /opt/consul/data/raft/raft.db.
OK: /root/vaul_devops_kdbx/backups found.
OK: backup created:
-rw------- 1 root root 2097152 Feb 17 16:11 /root/vaul_devops_kdbx/backups/16_11_57_17_02_16_bkp_raft.db
All backups present on the setevoy.vault.local in the /root/vaul_devops_kdbx/backups:
total 20248
-rw------- 1 root root 1048576 Feb 17 14:21 14_21_50_17_02_16_bkp_raft.db
[...[
-rw------- 1 root root 2097152 Feb 17 16:11 16_11_57_17_02_16_bkp_raft.db
[backup.rb] Consul backup/restore finished.

and "restore":

$ ./scripts/backup.rb setevoy.vault.local restore
[backup.rb] Consul backup/restore started.
[backup.rb] Working on the setevoy.vault.local in restore mode.
Restore will be here
[backup.rb] Consul backup/restore finished.

• Jörg W Mittag: Why the rush in accepting? I feel honored that you think nobody can do a better job than me, but rest assured: you are wrong ;-)
• setevoy: @JörgWMittag Just was fascinated with your answer :-) If the first point with ; - just stupid habit and few others - came here from my previous Python experience (not to big, although) - every other one really sobered me up. Will see, what others thinking :-)

Answer 1

Unfortunately, I don't have time for a proper review, so I'll stick to the more obvious points:

• Indentation in Ruby is 2 spaces, not 4.
• Not only is your use of semicolons inconsistent (sometimes you use them, sometimes you don't), every single one of those you do have is superfluous. Leave them out. You can use either newline or semicolon as "statement" separator, using both is not necessary.
• The same applies to your usage of the then keyword: sometimes you use it, sometimes you don't, and all uses are superfluous. Leave them out. You can use either the then keyword or a "statement" separator (newline or semicolon) to separate the condition from the then body, using both is not necessary.
• There is a large number of superfluous newlines. You'd generally have one before a new class, module or method definition, but not within – if your method is so long that it needs to be broken down in sections, consider refactoring it.
• You can save yourself a lot of trouble escaping all the double quotes in the giant string by using either some other delimiter (e.g. backuped = ssh.exec!(%Q[ ... ]) or a heredoc.
• Leave out empty parameter lists, e.g. def backup instead of def backup().
• By convention, variable names that start with an underscore _ are used for variables that are ignored. So, you would really only use them for multiple assignment or unused block parameters. In fact, it is more than just a convention: Ruby will warn you about unused local variables, but these warnings are suppressed for variables starting with an underscore, because they are intended as "ignored" variables.
• You should prefer public_send over send. The latter circumvents access protection, the former honors it.
• Prefer &&/|| over and/or. The former actually have the precedence you would expect from them, the latter don't.
• For the string inside the interpolation within your raft_backup_file string, you could use single quotes instead of double quotes. Personally, I always use single quotes unless I actually need to do interpolation or use escape sequences (or there is a single quote in the string). This makes it clear just when looking at the beginning of the string whether or not there is going to be interpolation.

Note that since you use a class variable to store part of your state (@@host), it is impossible to create more than one instance of your class, so it doesn't make sense to have a class at all.

#!/usr/bin/env ruby
require 'net/ssh'
$me = File.basename(__FILE__)
class CheckIn
 USER = 'knife'
 RSA_KEY = 'ssh/id_rsa'
 def initialize(host)
 @@host = host
 if File.exist?(RSA_KEY)
 File.chmod(0600, RSA_KEY)
 else
 abort "[#$me] ERROR: no RSA file #{RSA_KEY} found. Exit.\n"
 end
 end
end
class BackupRestore < CheckIn
 def backup
 consul_raft_db = '/opt/consul/data/raft/raft.db'
 root_backup_dir = '/root/vaul_devops_kdbx/backups'
 raft_backup_file = "#{Time.now.strftime('%H_%M_%S_%d_%m_%y')}_bkp_raft.db"
 Net::SSH.start(@@host, USER, keys: RSA_KEY) do |ssh|
 backuped = ssh.exec!(%Q[
 if test -e #{consul_raft_db}; then
 echo ""
 echo "OK: Consul DB found - #{consul_raft_db}."
 echo ""
 else
 echo "ERROR: no #{consul_raft_db} file found. Exit."
 exit 1
 fi
 if sudo [ -d #{root_backup_dir} ]; then
 echo "OK: #{root_backup_dir} found."
 echo ""
 else
 echo "ERROR: no #{root_backup_dir} directory found. Exit."
 exit 1
 fi
 if sudo cp #{consul_raft_db} #{root_backup_dir}/#{raft_backup_file}; then
 echo "OK: backup created:"
 echo ""
 sudo ls -l #{root_backup_dir}/#{raft_backup_file}
 echo ""
 echo "All backups present on the #@@host in the #{root_backup_dir}:"
 echo ""
 sudo ls -l #{root_backup_dir}
 echo ""
 else
 echo ""
 echo "ERROR: can not complete backup from #{consul_raft_db} to #{root_backup_dir}/#{raft_backup_file}. Exit."
 exit 1
 fi
 ])
 puts backuped
 end
 end
 def restore
 puts "\nRestore will be here\n\n"
 end
end
if __FILE__ == 0ドル
 puts "\n[#$me] Consul backup/restore started.\n"
 if ARGV.length == 2 && (ARGV[1] == 'backup' || ARGV[1] == 'restore')
 host = ARGV[0]
 mode = ARGV[1]
 puts "\n[#$me] Working on the #{host} in #{mode} mode."
 run = BackupRestore.new(host)
 run.public_send(mode)
 else
 abort "\n[#$me] ERROR: HOST must be specified as first argument and MODE (backup or restore) - as second one. Exit..\n"
 end
 puts "[#$me] Consul backup/restore finished.\n\n"
end

• Phrogz: Spot on, except: there's no improper quoting for that string. The syntax highlighting is not interpolation-aware, making it look broken.
• Jörg W Mittag: Hah. You're right. Apparently, not only is SO's syntax highlighter crappy, my brain's is, too. I removed that section.
• setevoy: @JörgWMittag Just came to work and started to edit my script - thanks for your answer :-) One more Q, please: > You'd generally have one before a new class, module or method definition, but not within // why it looks so bad for you - add newlines before/after variables definitions, before/after loops, if/else, methods? As for me - it makes the code more readable... Correct me, please, if I'm wrong.
• Phrogz: @setevoy It's personal preference, but consider this: the more blank lines you have, the less code you can see on screen at once, and thus the less context you have in your vision. If a method is getting longer than a certain limit (which is not a hard number, but consider half a screen) you should probably break it up into smaller methods with logical and helpful names, and clear, targeted goals.
• Jörg W Mittag: I said that it's a general rule, and as you can see in my answer, I did sprinkle some whitespace around for better readability. But other than that, what Phrogz said.