Improve error trace debuging in ssh key generation;

* additional check: is dir for key exist?;
* save and show exit code, stdout and stderr info;
* add tests;
* refactoring.

Change-Id: Ic53c561fc72abe1f0292ae200aa1d4ced5954083
Closes-Bug: #1268861

astute.gemspec

@@ -14,6 +14,7 @@ Gem::Specification.new do |s|
   s.add_dependency 'mcollective-client', '>= 2.3.1'
   s.add_dependency 'symboltable', '1.0.2'
   s.add_dependency 'rest-client', '~> 1.6.7'
+  s.add_dependency 'popen4', '~> 0.1.2'
 
   s.add_development_dependency 'rake', '10.0.4'
   s.add_development_dependency 'rspec', '2.13.0'

lib/astute/deployment_engine.rb

@@ -13,6 +13,7 @@
 #    under the License.
 
 require 'fileutils'
+require 'popen4'
 
 KEY_DIR = "/var/lib/astute"
 
@@ -118,13 +119,20 @@ module Astute
       Astute.config.PUPPET_SSH_KEYS.each do |key_name|
         dir_path = File.join(KEY_DIR, deployment_id.to_s, key_name)
         key_path = File.join(dir_path, key_name)
+
         FileUtils.mkdir_p dir_path
-        return if File.exist?(key_path) && !overwrite
+        raise DeploymentEngineError, "Could not create directory #{dir_path}" unless File.directory?(dir_path)
+
+        next if File.exist?(key_path) && !overwrite
 
         # Generate 2 keys(<name> and <name>.pub) and save it to <KEY_DIR>/<name>/
         File.delete key_path if File.exist? key_path
-        result = system("ssh-keygen -b 2048 -t rsa -N '' -f #{key_path}")
-        raise "Could not generate ssh key!" unless result
+
+        cmd = "ssh-keygen -b 2048 -t rsa -N '' -f #{key_path} 2>&1"
+        status, stdout, _ = run_system_command cmd
+
+        error_msg = "Could not generate ssh key! Command: #{cmd}, output: #{stdout}, exit code: #{status}"
+        raise DeploymentEngineError, error_msg if status != 0
       end
     end
 
@@ -148,6 +156,12 @@ module Astute
       end
     end
 
+    def run_system_command(cmd)
+      pid, _, stdout, stderr = Open4::popen4 cmd
+      _, status = Process::waitpid2 pid
+      return status.exitstatus, stdout, stderr
+    end
+
     def nodes_status(nodes, status, data_to_merge)
       {
         'nodes' => nodes.map do |n|

lib/astute/exceptions.rb

@@ -23,5 +23,7 @@ module Astute
   class RedhatCheckingError < AstuteError; end
   # Failed to reboot nodes
   class FailedToRebootNodesError < AstuteError; end
+  # Deployment engine error
+  class DeploymentEngineError < AstuteError; end
 
 end

spec/unit/deployment_engine_spec.rb

@@ -188,7 +188,7 @@ describe Astute::DeploymentEngine do
       end
 
       it 'should save files in correct place: KEY_DIR/<name of key>/' do
-        Engine.any_instance.stubs(:system).returns(true)
+        Engine.any_instance.stubs(:run_system_command).returns([0, "", ""])
 
         Dir.mktmpdir do |temp_dir|
           Astute::DeploymentEngine.const_set 'KEY_DIR', temp_dir
@@ -198,31 +198,44 @@ describe Astute::DeploymentEngine do
         end
       end
 
+      it 'should raise error if directory for key was not created' do
+        FileUtils.stubs(:mkdir_p).returns(false)
+        File.stubs(:directory?).returns(false)
+
+        expect { deployer.deploy(nodes) }.to raise_error(Astute::DeploymentEngineError,
+                                                         /Could not create directory/)
+      end
+
       it 'should raise error if ssh key generation fail' do
         FileUtils.stubs(:mkdir_p).returns(true)
-        Engine.any_instance.stubs(:system).returns(false)
+        File.stubs(:directory?).returns(true)
+        Engine.any_instance.stubs(:run_system_command).returns([1, "", ""])
 
-        expect { deployer.deploy(nodes) }.to raise_error('Could not generate ssh key!')
+        expect { deployer.deploy(nodes) }.to raise_error(Astute::DeploymentEngineError,
+                                                         /Could not generate ssh key! Command:/)
       end
 
       it 'should raise error if ssh key generation command not find' do
         FileUtils.stubs(:mkdir_p).returns(true)
-        Engine.any_instance.stubs(:system).returns(nil)
+        File.stubs(:directory?).returns(true)
+        Engine.any_instance.stubs(:run_system_command).returns([127, "Command not found", ""])
 
-        expect { deployer.deploy(nodes) }.to raise_error('Could not generate ssh key!')
+        expect { deployer.deploy(nodes) }.to raise_error(Astute::DeploymentEngineError,
+                                                         /Command not found/)
       end
 
       it 'should run ssh key generation with correct command' do
         FileUtils.stubs(:mkdir_p).returns(true)
+        File.stubs(:directory?).returns(true)
+
         key_path = File.join(Engine::KEY_DIR, nodes.first['deployment_id'].to_s, 'nova', 'nova')
-        Engine.any_instance.expects(:system).with("ssh-keygen -b 2048 -t rsa -N '' -f #{key_path}").returns(true)
+        cmd = "ssh-keygen -b 2048 -t rsa -N '' -f #{key_path} 2>&1"
+        Engine.any_instance.expects(:run_system_command).with(cmd).returns([0, "", ""])
 
         deployer.deploy(nodes)
       end
 
       it 'should not overwrite files' do
-        Engine.any_instance.stubs(:system).returns(true)
-
         Dir.mktmpdir do |temp_dir|
           Astute::DeploymentEngine.const_set 'KEY_DIR', temp_dir
           key_path = File.join(temp_dir,'nova', 'nova')
@@ -235,6 +248,22 @@ describe Astute::DeploymentEngine do
         end
       end
 
+      it 'should check next key if find existing' do
+        Astute.config.PUPPET_SSH_KEYS = ['nova', 'test']
+        nova_key_path = File.join(Engine::KEY_DIR, nodes.first['deployment_id'].to_s, 'nova', 'nova')
+        test_key_path = File.join(Engine::KEY_DIR, nodes.first['deployment_id'].to_s, 'test', 'test')
+
+        FileUtils.stubs(:mkdir_p).returns(true).twice
+        File.stubs(:directory?).returns(true).twice
+
+        File.stubs(:exist?).with(nova_key_path).returns(true)
+        File.stubs(:exist?).with(test_key_path).returns(false)
+
+        Engine.any_instance.expects(:run_system_command).returns([0, "", ""])
+
+        deployer.deploy(nodes)
+      end
+
     end # end context
 
     context 'upload ssh keys' do