kpm: fix Tomcat PID detection in system command

This also cleans up the code a little bit.

Signed-off-by: Pierre-Alexandre Meyer <pierre@mouraf.org>

kpm/lib/kpm/account.rb

@@ -73,7 +73,7 @@ class Account
     def initialize(config_file = nil, killbill_api_credentials = nil, killbill_credentials = nil, killbill_url = nil,
                    database_name = nil, database_credentials = nil, database_host = nil, database_port = nil, data_delimiter = nil, logger = nil)
       @killbill_api_key = KILLBILL_API_KEY
-      @killbill_api_secrets = KILLBILL_API_SECRET
+      @killbill_api_secret = KILLBILL_API_SECRET
       @killbill_url = KILLBILL_URL
       @killbill_user = KILLBILL_USER
       @killbill_password = KILLBILL_PASSWORD
@@ -127,7 +127,7 @@ def fetch_export_data(account_id)
         username: @killbill_user,
         password: @killbill_password,
         api_key: @killbill_api_key,
-        api_secret: @killbill_api_secrets
+        api_secret: @killbill_api_secret
       }
 
       begin
@@ -414,7 +414,7 @@ def set_killbill_options(killbill_api_credentials, killbill_credentials, killbil
       unless killbill_api_credentials.nil?
 
         @killbill_api_key = killbill_api_credentials[0]
-        @killbill_api_secrets = killbill_api_credentials[1]
+        @killbill_api_secret = killbill_api_credentials[1]
 
       end
 

kpm/lib/kpm/diagnostic_file.rb

@@ -10,17 +10,17 @@
 module KPM
   class DiagnosticFile
     # Temporary directory
-    TMP_DIR_PREFIX  = 'killbill-diagnostics-'
-    TMP_DIR         = Dir.mktmpdir(TMP_DIR_PREFIX)
-    TMP_LOGS_DIR    = TMP_DIR + File::Separator + 'logs'
+    TMP_DIR_PREFIX = 'killbill-diagnostics-'
+    TMP_DIR = Dir.mktmpdir(TMP_DIR_PREFIX)
+    TMP_LOGS_DIR = TMP_DIR + File::Separator + 'logs'
 
-    TENANT_FILE     = 'tenant_config.data'
-    SYSTEM_FILE     = 'system_configuration.data'
-    ACCOUNT_FILE    = 'account.data'
+    TENANT_FILE = 'tenant_config.data'
+    SYSTEM_FILE = 'system_configuration.data'
+    ACCOUNT_FILE = 'account.data'
 
-    TODAY_DATE      = Date.today.strftime('%m-%d-%y')
-    ZIP_FILE        = 'killbill-diagnostics-' + TODAY_DATE + '.zip'
-    ZIP_LOG_FILE    = 'logs.zip'
+    TODAY_DATE = Date.today.strftime('%m-%d-%y')
+    ZIP_FILE = 'killbill-diagnostics-' + TODAY_DATE + '.zip'
+    ZIP_LOG_FILE = 'logs.zip'
 
     def initialize(config_file = nil, killbill_api_credentials = nil, killbill_credentials = nil, killbill_url = nil,
                    database_name = nil, database_credentials = nil, database_host = nil, database_port = nil, kaui_web_path = nil,
@@ -44,18 +44,18 @@ def initialize(config_file = nil, killbill_api_credentials = nil, killbill_crede
     def export_data(account_id = nil, log_dir = nil)
       self.config = @config_file
 
-      tenant_export_file  = retrieve_tenant_config
-      system_export_file  = retrieve_system_config
+      tenant_export_file = retrieve_tenant_config
+      system_export_file = retrieve_system_config
       account_export_file = retrieve_account_data(account_id) unless account_id.nil?
-      log_files           = retrieve_log_files(log_dir)
+      log_files = retrieve_log_files(log_dir)
 
       raise Interrupt, 'Account id or configuration file not found' unless File.exist?(system_export_file) && File.exist?(tenant_export_file)
 
       zip_file_name = TMP_DIR + File::Separator + ZIP_FILE
 
       Zip::File.open(zip_file_name, Zip::File::CREATE) do |zip_file|
-        zip_file.add(TENANT_FILE,  tenant_export_file)
-        zip_file.add(SYSTEM_FILE,  system_export_file)
+        zip_file.add(TENANT_FILE, tenant_export_file)
+        zip_file.add(SYSTEM_FILE, system_export_file)
         zip_file.add(ACCOUNT_FILE, account_export_file) unless account_id.nil?
         zip_file.add(ZIP_LOG_FILE, log_files) unless log_files.nil?
       end
@@ -79,7 +79,9 @@ def retrieve_tenant_config
       @killbill_url ||= 'http://' + retrieve_config('killbill', 'host').to_s + ':' + retrieve_config('killbill', 'port').to_s unless @config_file.nil?
 
       tenant_config = KPM::TenantConfig.new(@killbill_api_credentials,
-                                            @killbill_credentials, @killbill_url, @logger)
+                                            @killbill_credentials,
+                                            @killbill_url,
+                                            @logger)
       export_file = tenant_config.export
 
       final = TMP_DIR + File::Separator + TENANT_FILE
@@ -91,7 +93,7 @@ def retrieve_tenant_config
 
     def retrieve_system_config
       @logger.info 'Retrieving system configuration'
-      system = KPM::System.new
+      system = KPM::System.new(@logger)
       export_data = system.information(@bundles_dir, true, @config_file, @kaui_web_path, @killbill_web_path)
 
       system_catalina_base(export_data)
@@ -118,13 +120,12 @@ def retrieve_account_data(account_id)
     end
 
     def retrieve_log_files(log_dir)
-      @logger.info 'Collecting log files'
-
       if @catalina_base.nil? && log_dir.nil?
-        @logger.warn 'Unable to find Tomcat process, make sure to run kpm using the same user as the Tomcat process.'
+        @logger.warn "\e[91;1mUnable to find Tomcat process, logs won't be collected: make sure to run kpm using the same user as the Tomcat process or pass the option --log-dir\e[0m"
         return nil
       end
 
+      @logger.info 'Collecting log files'
       log_base = log_dir || (@catalina_base + File::Separator + 'logs')
       log_items = Dir.glob(log_base + File::Separator + '*')
 

kpm/lib/kpm/system.rb

@@ -10,7 +10,8 @@ class System
     DEFAULT_KAUI_SEARCH_BASE_DIR = '**' + File::SEPARATOR + 'kaui'
     DEFAULT_KILLBILL_SEARCH_BASE_DIR = '**' + File::SEPARATOR + 'ROOT'
 
-    def initialize
+    def initialize(logger)
+      @logger = logger
       @formatter = KPM::Formatter.new
     end
 
@@ -271,14 +272,15 @@ def killbill_version(killbill_web_path = nil)
 
     def java_command
       command = 'java -XshowSettings:properties -version 2>&1'
-      apache_tomcat_pid = apache_tomcat_pid
+      apache_tomcat_pid = find_apache_tomcat_pid
+      @logger.debug("Found Tomcat PID: #{apache_tomcat_pid}")
 
       command = "jcmd #{apache_tomcat_pid} VM.system_properties" unless apache_tomcat_pid.nil?
 
       command
     end
 
-    def apache_tomcat_pid
+    def find_apache_tomcat_pid
       apache_tomcat_pid = nil
       `jcmd -l 2>&1`.split("\n").each do |line|
         if /org.apache.catalina/.match(line)

kpm/lib/kpm/tasks.rb

@@ -424,7 +424,7 @@ def inspect
                       desc: 'Path for the killbill web app'
         desc 'system', 'Gather information about the system'
         def system
-          system = KPM::System.new
+          system = KPM::System.new(logger)
           information = system.information(options[:bundles_dir], options[:as_json], options[:config_file], options[:kaui_web_path],
                                            options[:killbill_web_path])
 

kpm/lib/kpm/tenant_config.rb

@@ -9,7 +9,6 @@ class TenantConfig
     # Killbill server
     KILLBILL_HOST = ENV['KILLBILL_HOST'] || '127.0.0.1'
     KILLBILL_URL = "http://#{KILLBILL_HOST}:8080"
-    KILLBILL_API_VERSION = '1.0'
 
     # USER/PWD
     KILLBILL_USER = ENV['KILLBILL_USER'] || 'admin'
@@ -30,7 +29,7 @@ class TenantConfig
 
     def initialize(killbill_api_credentials = nil, killbill_credentials = nil, killbill_url = nil, logger = nil)
       @killbill_api_key = KILLBILL_API_KEY
-      @killbill_api_secrets = KILLBILL_API_SECRET
+      @killbill_api_secret = KILLBILL_API_SECRET
       @killbill_url = KILLBILL_URL
       @killbill_user = KILLBILL_USER
       @killbill_password = KILLBILL_PASSWORD
@@ -42,12 +41,10 @@ def initialize(killbill_api_credentials = nil, killbill_credentials = nil, killb
     def export(key_prefix = nil)
       export_data = fetch_export_data(key_prefix)
 
-      raise Interrupt, 'key_prefix not found' if export_data.empty?
+      raise ArgumentError, "Data for key_prefix=#{key_prefix} not found" if export_data.empty?
 
       export_file = store_into_file(export_data)
 
-      raise Interrupt, 'key_prefix not found' unless File.exist?(export_file)
-
       @logger.info "\e[32mData exported under #{export_file}\e[0m"
 
       export_file
@@ -64,9 +61,9 @@ def fetch_export_data(key_prefix)
 
         if !config_data.empty?
           config_data.each { |data| tenant_config << data }
-          @logger.info "Data for key prefix \e[1m#{prefix}\e[0m was \e[1mfound and is ready to be exported\e[0m."
+          @logger.debug "Data for key prefix \e[1m#{prefix}\e[0m was \e[1mfound and is ready to be exported\e[0m."
         else
-          @logger.info "Data for key prefix \e[1m#{prefix}\e[0m was \e[31mnot found\e[0m."
+          @logger.debug "Data for key prefix \e[1m#{prefix}\e[0m was \e[31mnot found\e[0m."
         end
       end
 
@@ -75,16 +72,19 @@ def fetch_export_data(key_prefix)
 
     def call_client(key_prefix)
       KillBillClient.url = @killbill_url
+      KillBillClient.logger = @logger
       options = {
         username: @killbill_user,
         password: @killbill_password,
         api_key: @killbill_api_key,
-        api_secret: @killbill_api_secrets
+        api_secret: @killbill_api_secret
       }
 
-      tenant_config_data = KillBillClient::Model::Tenant.search_tenant_config(key_prefix, options)
-
-      tenant_config_data
+      begin
+        return KillBillClient::Model::Tenant.search_tenant_config(key_prefix, options)
+      rescue KillBillClient::API::Unauthorized
+        raise ArgumentError, "Unable to export tenant details, wrong credentials? username=#{@killbill_user}, password=#{mask(@killbill_password)}, api_key=#{@killbill_api_key}, api_secret=#{mask(@killbill_api_secret)}"
+      end
     end
 
     def store_into_file(export_data)
@@ -97,20 +97,20 @@ def store_into_file(export_data)
 
     def set_killbill_options(killbill_api_credentials, killbill_credentials, killbill_url)
       unless killbill_api_credentials.nil?
-
         @killbill_api_key = killbill_api_credentials[0]
-        @killbill_api_secrets = killbill_api_credentials[1]
-
+        @killbill_api_secret = killbill_api_credentials[1]
       end
 
       unless killbill_credentials.nil?
-
         @killbill_user = killbill_credentials[0]
         @killbill_password = killbill_credentials[1]
-
       end
 
       @killbill_url = killbill_url unless killbill_url.nil?
     end
+
+    def mask(string, all_but = 3, char = '*')
+      string.gsub(/.(?=.{#{all_but}})/, char)
+    end
   end
 end

kpm/spec/kpm/remote/tenant_config_spec.rb

@@ -10,15 +10,15 @@
 
   let(:user) { 'KPM Tenant Spec' }
   let(:tenant_config_class) do
-    described_class.new([killbill_api_key, killbill_api_secrets],
+    described_class.new([killbill_api_key, killbill_api_secret],
                         [killbill_user, killbill_password], url, logger)
   end
   let(:options) do
     {
       username: killbill_user,
       password: killbill_password,
       api_key: killbill_api_key,
-      api_secret: killbill_api_secrets
+      api_secret: killbill_api_secret
     }
   end
 
@@ -31,7 +31,7 @@
       it 'when initialized with options' do
         tenant_config_class.should be_an_instance_of(KPM::TenantConfig)
         expect(tenant_config_class.instance_variable_get(:@killbill_api_key)).to eq(killbill_api_key)
-        expect(tenant_config_class.instance_variable_get(:@killbill_api_secrets)).to eq(killbill_api_secrets)
+        expect(tenant_config_class.instance_variable_get(:@killbill_api_secret)).to eq(killbill_api_secret)
         expect(tenant_config_class.instance_variable_get(:@killbill_user)).to eq(killbill_user)
         expect(tenant_config_class.instance_variable_get(:@killbill_password)).to eq(killbill_password)
         expect(tenant_config_class.instance_variable_get(:@killbill_url)).to eq(url)

kpm/spec/kpm/unit_mysql/account_spec.rb

@@ -7,7 +7,7 @@
     include_context 'connection_setup'
 
     let(:account_class) do
-      described_class.new(nil, [killbill_api_key, killbill_api_secrets],
+      described_class.new(nil, [killbill_api_key, killbill_api_secret],
                           [killbill_user, killbill_password], url,
                           db_name, [db_username, db_password], db_host, db_port, nil, logger)
     end
@@ -39,7 +39,7 @@
       it 'when initialized with options' do
         account_class.should be_an_instance_of(KPM::Account)
         expect(account_class.instance_variable_get(:@killbill_api_key)).to eq(killbill_api_key)
-        expect(account_class.instance_variable_get(:@killbill_api_secrets)).to eq(killbill_api_secrets)
+        expect(account_class.instance_variable_get(:@killbill_api_secret)).to eq(killbill_api_secret)
         expect(account_class.instance_variable_get(:@killbill_user)).to eq(killbill_user)
         expect(account_class.instance_variable_get(:@killbill_password)).to eq(killbill_password)
         expect(account_class.instance_variable_get(:@killbill_url)).to eq(url)
@@ -354,7 +354,7 @@ def creating_account_with_client
       username: killbill_user,
       password: killbill_password,
       api_key: killbill_api_key,
-      api_secret: killbill_api_secrets
+      api_secret: killbill_api_secret
     }
 
     account = KillBillClient::Model::Account.new

kpm/spec/spec_helper.rb

@@ -26,7 +26,7 @@
   let(:dummy_data_file) { Dir.mktmpdir('dummy') + File::SEPARATOR + 'kbdump' }
   let(:url) { "http://#{yml_file['killbill']['host']}:#{yml_file['killbill']['port']}" }
   let(:killbill_api_key) { yml_file['killbill']['api_key'] }
-  let(:killbill_api_secrets) { yml_file['killbill']['api_secret'] }
+  let(:killbill_api_secret) { yml_file['killbill']['api_secret'] }
   let(:killbill_user) { yml_file['killbill']['user'] }
   let(:killbill_password) { yml_file['killbill']['password'] }
   let(:db_name) { yml_file['database']['name'] }