Merge pull request #8 from poliva83/master

Version 0.4.6 - 5/13/2016

CHANGELOG.md

@@ -1,4 +1,8 @@
 # Changelog
+## 0.4.6 (5/3/2016)
+- Added begin/rescue around convergence_strategy deletion in destroy_machine method to avoid failures
+  when the client/node objects are not present
+- Fixed rubocop errors
 
 ## 0.4.5 (4/28/2016)
 - Added OneFlow resources (see readme): one_flow_template, one_flow_service

lib/chef/provider/one_template.rb

@@ -139,12 +139,10 @@ def create_one_template(template_str)
       end
 
       def create_template
-        if new_resource.template_file && new_resource.template.size > 0
-          fail("Attributes 'template_file' and 'template' are mutually " \
-               'exclusive.')
-        elsif new_resource.template_file
+        fail("Attributes 'template_file' and 'template' are mutually 'exclusive.'") if new_resource.template_file && !new_resource.template.empty?
+        if new_resource.template_file
           ::File.read(new_resource.template_file)
-        elsif new_resource.template.size > 0
+        elsif !new_resource.template.empty?
           driver.one.create_template(new_resource.template)
         else
           fail("Missing attribute 'template_file' or 'template' in " \

lib/chef/provider/one_user.rb

@@ -70,17 +70,14 @@ def exists?(filter)
       end
 
       action :update do
-        if exists?(:id => @new_resource.user_id, :name => @new_resource.name)
-          fail "':template' or ':template_file' attribute missing" if !@new_resource.template && !@new_resource.template_file
-          # hash = @current_user.to_hash
-          tpl = new_driver.one.create_template(@new_resource.template) if @new_resource.template
-          tpl = ::File.read(@new_resource.template_file) if @new_resource.template_file
+        fail "user '#{new_resource.name}' does not exists" unless exists?(:id => @new_resource.user_id, :name => @new_resource.name)
+        fail "':template' or ':template_file' attribute missing" unless @new_resource.template || @new_resource.template_file
 
-          rc = @current_user.update(tpl, true)
-          fail "failed to update user '#{@new_resource.name}': #{rc.message}" if OpenNebula.is_error?(rc)
-        else
-          fail "user '#{new_resource.name}' does not exists"
-        end
+        tpl = new_driver.one.create_template(@new_resource.template) if @new_resource.template
+        tpl = ::File.read(@new_resource.template_file) if @new_resource.template_file
+
+        rc = @current_user.update(tpl, true)
+        fail "failed to update user '#{@new_resource.name}': #{rc.message}" if OpenNebula.is_error?(rc)
       end
 
       protected

lib/chef/provider/one_vnet.rb

@@ -86,11 +86,8 @@ def exists?(filter)
           else
             same = ar_pool[0]['AR']['SIZE'].to_i == @new_resource.size
           end
-          if same
-            action_handler.report_progress "vnet '#{@new_resource.name}' already exists - (up to date)"
-          else
-            fail "vnet '#{@new_resource.name}' exists with different configuration"
-          end
+          fail "vnet '#{@new_resource.name}' exists with different configuration" unless same
+          action_handler.report_progress "vnet '#{@new_resource.name}' already exists - (up to date)"
         else
           fail "parent network '#{@new_resource.network}' does not exist" unless exists?(:id => @new_resource.network)
           action_handler.perform_action "reserved vnet '#{@new_resource.name}'" do

lib/chef/provisioning/opennebula_driver/credentials.rb

@@ -37,7 +37,7 @@ def initialize(options = {})
         end
 
         def default
-          fail 'No credentials loaded!  Do you have a ~/.one/one_auth file?' if @credentials.size == 0
+          fail 'No credentials loaded!  Do you have a ~/.one/one_auth file?' if @credentials.empty?
           @credentials[ENV['ONE_DEFAULT_PROFILE'] || 'default'] || @credentials.first[1]
         end
 

lib/chef/provisioning/opennebula_driver/driver.rb

@@ -53,6 +53,7 @@ def with_flow_url(url)
 
 class Chef
   module Provisioning
+    # OpenNebulaDriver module.
     module OpenNebulaDriver
       def self.match_driver_url(url, allow_nil_profile = false)
         scan = url.match(%r/opennebula:(https?:\/\/[^:\/]+ (?::[0-9]{2,5})? (?:\/[^:\s]+) ) :?([^:\s]+)?/x)
@@ -78,19 +79,15 @@ def self.get_onelib(args)
         fail "OpenNebula credentials cannot be #{credentials.class}, it must be a String" unless credentials.is_a?(String)
         fail "OpenNebula endpoint cannot be #{endpoint.class}, it must be a String" unless endpoint.is_a?(String)
         fail "OpenNebula options cannot be #{options.class}, it must be a Hash" unless options.is_a?(Hash)
-        server_version, _ = Open3.capture2e("ruby #{File.dirname(__FILE__)}/../driver_init/server_version.rb #{endpoint} #{credentials} #{options.to_json.inspect}")
+        server_version, = Open3.capture2e("ruby #{File.dirname(__FILE__)}/../driver_init/server_version.rb #{endpoint} #{credentials} #{options.to_json.inspect}")
         server_version.strip!
         fail server_version unless server_version =~ /^\d+\.\d+(?:\.\d+)?$/
         begin
           gem 'opennebula', "~> #{server_version}"
           require 'opennebula'
         rescue Gem::LoadError => e
           e_inspect = e.inspect
-          if e_inspect.include?('already activated')
-            Chef::Log.warn(e_inspect)
-          else
-            raise e
-          end
+          raise e unless e_inspect.include?('already activated')
         end
         require 'chef/provisioning/opennebula_driver/one_lib'
         gem_version = Gem.loaded_specs['opennebula'].version.to_s
@@ -305,15 +302,17 @@ def destroy_machine(action_handler, machine_spec, machine_options)
               end
               fail "Failed to destroy '#{instance.name}'.  Current state: #{instance.state_str}" if instance.state_str != 'DONE'
             end
+          elsif machine_spec.reference
+            Chef::Log.info("vm #{machine_spec.name} (#{machine_spec.reference['instance_id']}) does not exist - (up to date)")
           else
-            if machine_spec.reference
-              Chef::Log.info("vm #{machine_spec.name} (#{machine_spec.reference['instance_id']}) does not exist - (up to date)")
-            else
-              Chef::Log.info("vm #{machine_spec.name} does not exist - (up to date)")
-            end
+            Chef::Log.info("vm #{machine_spec.name} does not exist - (up to date)")
+          end
+          begin
+            strategy = convergence_strategy_for(machine_spec, machine_options)
+            strategy.cleanup_convergence(action_handler, machine_spec)
+          rescue Net::HTTPServerException => e
+            raise unless e.response.code == '404'
           end
-          strategy = convergence_strategy_for(machine_spec, machine_options)
-          strategy.cleanup_convergence(action_handler, machine_spec)
         end
 
         # Stop the given machine.
@@ -580,12 +579,12 @@ def populate_img_object(image_spec, new_image)
         def instance_for(machine_spec)
           instance = nil
           if machine_spec.reference
-            current_endpoint, _ = Chef::Provisioning::OpenNebulaDriver.match_driver_url(machine_spec.driver_url, true)
+            current_endpoint, = Chef::Provisioning::OpenNebulaDriver.match_driver_url(machine_spec.driver_url, true)
             fail "Cannot move '#{machine_spec.name}' from #{current_endpoint} to #{driver_url}: machine moving is not supported.  Destroy and recreate." if current_endpoint != driver_url
             instance = @one.get_resource(:virtualmachine, :id => machine_spec.reference['instance_id'].to_i)
             machine_spec.driver_url = @driver_url_with_profile
           elsif machine_spec.location
-            current_endpoint, _ = Chef::Provisioning::OpenNebulaDriver.match_driver_url(machine_spec.driver_url, true)
+            current_endpoint, = Chef::Provisioning::OpenNebulaDriver.match_driver_url(machine_spec.driver_url, true)
             fail "Cannot move '#{machine_spec.name}' from #{current_endpoint} to #{driver_url}: machine moving is not supported.  Destroy and recreate." if current_endpoint != driver_url
             instance = @one.get_resource(:virtualmachine, :id => machine_spec.location['server_id'].to_i)
             machine_spec.driver_url = @driver_url_with_profile
@@ -631,7 +630,7 @@ def transport_for(machine_spec, machine_options, _instance)
           }.merge(machine_options[:ssh_options] || {})
           ssh_options[:proxy] = Net::SSH::Proxy::Command.new(ssh_options[:proxy]) if ssh_options.key?(:proxy)
 
-          connection_timeout = machine_options[:connection_timeout] || 300
+          connection_timeout = machine_options[:connection_timeout] || 300 # default is 5 min
           username = get_ssh_user(machine_spec, machine_options)
 
           options = {}
@@ -644,16 +643,23 @@ def transport_for(machine_spec, machine_options, _instance)
 
           transport = Chef::Provisioning::Transport::SSH.new(machine_spec.reference['ip'], username, ssh_options, options, config)
 
-          # wait up to 5 min to establish SSH connection
-          connect_sleep = 3
+          rc = retryable_operation("Waiting for SSH connection", connection_timeout.to_i) { transport.available? }
+          fail "Failed to establish SSH connection to '#{machine_spec.name}'" if rc.nil?
+          transport
+        end
+
+        # Retry an operation until the timeout expires.  Will always try at least once.
+        def retryable_operation(msg = "operation", timeout = 15, delay = 3)
+          return nil unless block_given?
           start = Time.now
           loop do
-            break if transport.available?
-            fail "Failed to establish SSH connection to '#{machine_spec.name}'" if Time.now > start + connection_timeout.to_i
-            Chef::Log.info("Waiting for SSH server ...")
-            sleep connect_sleep
+            return true if yield
+            Chef::Log.info(msg)
+            sleep delay
+            break if (Time.now - start) > timeout
           end
-          transport
+          Chef::Log.error("Timed out waiting for operation to complete: '#{msg}'")
+          nil
         end
 
         def convergence_strategy_for(machine_spec, machine_options)

lib/chef/provisioning/opennebula_driver/flow_lib.rb

@@ -238,7 +238,7 @@ def get_ids(type, name)
         # Gets a single ID of a service or template, fails if there's not exactly 1, or returns nil if there 0 and nil_if_none
         def get_unique_id(type, name, nil_if_none = false)
           matches = get_ids(type, name)
-          if matches.length == 0
+          if matches.empty?
             return nil if nil_if_none
             fail "There are no OneFlow #{type}s with the name '#{name}'"
           elsif matches.length > 1
@@ -250,7 +250,7 @@ def get_unique_id(type, name, nil_if_none = false)
 
         # Check if a service or template exists
         def exists?(type, name)
-          get_ids(type, name).length == 0 ? false : true
+          !get_ids(type, name).empty?
         end
 
         # Gets permission of service or template

lib/chef/provisioning/opennebula_driver/one_lib.rb

@@ -152,7 +152,7 @@ def get_resource(resource_type, filter = {})
             next if filter[:uname] && res.to_hash[hash_key]['UNAME'] != filter[:uname]
             resources << res
           end
-          return nil if resources.size == 0
+          return nil if resources.empty?
           return resources[0] if resources.size == 1
           resources
         end
@@ -171,15 +171,38 @@ def wait_for_vm(id, end_state = nil)
           end_state ||= 'RUNNING'
           vm = get_resource(:virtualmachine, :id => id)
           fail "Did not find VM with ID: #{id}" unless vm
-          while vm.lcm_state_str != end_state.upcase
+
+          # Wait up to 10 min for the VM to be ready
+          rc = retryable_operation("wait for VM #{id} to be ready", 600, 2) do
             vm.info
-            Chef::Log.debug("Waiting for VM '#{id}' to be in '#{end_state.upcase}' state: '#{vm.lcm_state_str}'")
-            fail "'#{vm.name}'' failed.  Current state: #{vm.state_str}" if vm.state_str == 'FAILED' || vm.lcm_state_str == 'FAILURE'
-            sleep(2)
+            if vm.lcm_state_str != 'LCM_INIT'
+              short_lcm = OpenNebula::VirtualMachine::SHORT_LCM_STATES[vm.lcm_state_str]
+              fail "'#{vm.name}'' failed.  Current state: #{vm.lcm_state_str}" if short_lcm == 'fail'
+            end
+            fail "'#{vm.name}'' failed.  Current state: #{vm.state_str}" if vm.state_str == 'FAILED'
+            Chef::Log.info("current state: '#{vm.lcm_state_str}'  short: '#{short_lcm}'")
+            OpenNebula::Error.new("Waiting") unless vm.lcm_state_str.casecmp(end_state) == 0
           end
+          fail "wait_for_vm timed out: '#{id}'" if rc.nil?
           vm
         end
 
+        # Retry an OpenNebula operation until the timeout expires.  Will always try at least once.
+        def retryable_operation(msg = "operation", timeout = 15, delay = 2)
+          return nil unless block_given?
+          start = Time.now
+          rc = nil
+          loop do
+            rc = yield
+            return true unless OpenNebula.is_error?(rc)
+            Chef::Log.info(msg)
+            sleep delay
+            break if (Time.now - start) > timeout
+          end
+          Chef::Log.info("Timed out waiting for OpenNebula operation.  Got error #{rc.message} from OpenNebula.")
+          nil
+        end
+
         def rename_vm(res, name)
           rc = res.rename(name)
           raise OpenNebulaException, rc.message if OpenNebula.is_error?(rc)
@@ -245,7 +268,8 @@ def wait_for_img(name, img_id)
           image = nil
           state = 'INIT'
           pool = get_pool(:image)
-          while state == 'INIT' || state == 'LOCKED'
+
+          retryable_operation("wait for IMAGE #{img_id} to be ready", 600, 2) do
             pool.info!(-2, img_id, img_id)
             pool.each do |img|
               next unless img.id == img_id
@@ -255,7 +279,7 @@ def wait_for_img(name, img_id)
               state = cur_state
               break
             end
-            sleep(2)
+            OpenNebula::Error.new("Waiting") if state == 'INIT' || state == 'LOCKED'
           end
           fail "Failed to create #{name} image. State = '#{state}'" if state != 'READY'
           Chef::Log.info("Image #{name} is in READY state")
@@ -364,7 +388,7 @@ def template_from_file(options)
           t_hash = nil
           doc = OpenNebula::CustomObject.new(OpenNebula::CustomObject.build_xml, @client)
           unless OpenNebula.is_error?(doc)
-            rc = doc.allocate("#{File.read(options[:template_file])}")
+            rc = doc.allocate(File.read(options[:template_file]).to_s)
             fail "Failed to allocate OpenNebula document: #{rc.message}" if OpenNebula.is_error?(rc)
             doc.info!
             t_hash = doc.to_hash['DOCUMENT']['TEMPLATE']
@@ -450,16 +474,8 @@ def create_template(t, level = 0)
             else
               comma = (index < count) && level > 0
               level.times { tpl << "  " }
-              if v.is_a?(Integer)
-                tpl << "#{k} = \"#{v}\""
-              elsif v.is_a?(String)
-                # Fix for: template does not support embedded quotation marks
-                # Escaping of " only happens if " is not already escaped, preceded by \\
-                tpl << "#{k} = \"#{v.gsub(/(?<!\\)\"/, '\"')}\""
-              else # any other type
-                # convert to string and print it
-                tpl << "#{k} = \"#{v}\""
-              end
+              txt = v.is_a?(String) ? "#{k} = \"#{v.gsub(/(?<!\\)\"/, '\"')}\"" : "#{k} = \"#{v}\""
+              tpl << txt
               tpl << (comma ? ",\n" : "\n")
               index += 1
             end

lib/chef/provisioning/opennebula_driver/version.rb

@@ -24,7 +24,7 @@ module Provisioning
     # Extending module.
     #
     module OpenNebulaDriver
-      VERSION = '0.4.5'
+      VERSION = '0.4.6'.freeze
     end
   end
 end