Ruby script that deletes servers from Red Hat Satellite

Question

This script uses the Red Hat Satellite v5.4 API to delete servers which are read in from a file. The servers are listed in the file by hostname but the Satellite API deletes servers by ID number. The script runs the getId call using the hostname and stores the ID ( val[0]["id"] ) in the system_ids array (assuming it is not empty). It then deletes the servers in bulk.

The script assumes the server name is in the host.sub-domain.domain format. One improvement that I can make is to check each hostname and kick it out if the format is not correct. This would make the need to test for an empty system_ids array prior to attempting to delete the servers.

I could also add some error handling, but that is low on my list of priorities.

It works as designed. I'm just hoping to get some tips on streamlining or improving the code.

#!/usr/bin/ruby

require "xmlrpc/client"
require "io/console"

print "Username: "
user = STDIN.gets.chomp
print "Password: "
pass = STDIN.noecho(&:gets).chomp
puts

# These are the parameters we use to create our XMLRPC client
params = {
  host:  "tcecapm1r6cm01.ecap.cciio",
  path:  "/rpc/api",
  use_ssl:  "false",
  user:  user,
  pass:  pass
}

# Get the file we are reading from the command line and
# initialize the array that is used for storing system IDs
server_list = ARGV[0]
system_ids = Array.new


# Create our client using the parameters above
client = XMLRPC::Client.new(params[:host],params[:path])

# Create our Satellite API session
session = client.call("auth.login",params[:user],params[:pass])

# Open the file containing the servers to be deleted and build
# an array of their system IDs for further processing
File.open(server_list, "r") do |file|
  file.each_line do |line|
    val = client.call("system.getId", session, line)
    if val.nil?
      next
    else
      system_ids.push(val[0]["id"]) unless val.empty?
    end
  end
end

client.call("system.deleteSystems", session, system_ids)

Answer 1

The params hash seems like a verbose way to write four variables. You never use params as a whole. Furthermore, params[:use_ssl] is completely unused.

Using XMLRPC::Client.new_from_uri(…) lets you specify the host, path, and protocol in just one variable.

Instead of opening ARGV[0] yourself, just use ARGF — it's even better, in that it can also read its input from STDIN. Instead of using line as the block parameter name, use something descriptive like system_name.

system_ids = Array.new followed by many system_ids.push(…) calls would be better written using map, in my opinion.

def prompt_auth():
  …
end

user, pass = prompt_auth()
uri = 'http://tcecapm1r6cm01.ecap.cciio/rpc/api'

client = XMLRPC::Client.new_from_uri(uri)
session = client.call('auth.login', user, pass)

system_ids = ARGF.map do |system_name|
  system = client.call('system.getId', session, system_name)
  system[0]['id'] unless system.nil? || system.empty?
end.compact
client.call('system.deleteSystems', session, system_ids)