Roll-forward clean up: separate actions and checks

Author: andreyvit

lib/jecloud/application.rb

@@ -107,9 +107,10 @@ def roll_forward!
       next_attempt = nil
       while cont
         update_config do
-          session = Session.new(@config.failures)
-          cont = roll_forward_step! session
-          next_attempt = session.next_attempt
+          Session.new(@config.failures).tap do |session|
+            roll_forward_step! session
+            next_attempt, cont = session.next_attempt, session.any_actions_executed?
+          end
         end
       end
       if next_attempt
@@ -159,70 +160,62 @@ def roll_forward_step! session
           raise ExpectedDelay, "No IP address assigned yet"
         end
 
-        jecloud_installed = false
-        session.action "#{server.uuid}-ssh" do
-          session.action "#{server.uuid}-sudo-test" do
-            x = server_session.sudo!("echo ok").strip
-            if x == 'ok'
-              $log.debug "sudo test ok"
-            else
-              raise UnexpectedExternalProblem, "sudo does not work on #{server.public_ip}"
-            end
+        session.check "#{server.uuid}-sudo-test" do
+          x = server_session.sudo!("echo ok").strip
+          if x == 'ok'
+            $log.debug "sudo test ok"
+          else
+            raise UnexpectedExternalProblem, "sudo does not work on #{server.public_ip}"
           end
+        end
 
+        jecloud_installed = session.check("#{server.uuid}-is-jecloud-up-to-date") do
           jecloud_version = server_session.exec!("jecloud print-version || echo 'NONE'").strip
-          $log.debug "JeCloud version on the server: #{jecloud_version}"
-          if jecloud_version =~ /^\d+\.\d+(?:\.\d+(?:\.\d+)?)?$/
-            if jecloud_version.pad_numbers >= JeCloud::VERSION.pad_numbers
-              $log.debug "JeCloud installed on the server is good enough."
-              jecloud_installed = true
-            end
+          (jecloud_version =~ /^\d+\.\d+(?:\.\d+(?:\.\d+)?)?$/ && jecloud_version.pad_numbers >= JeCloud::VERSION.pad_numbers).tap do |is_good_enough|
+            $log.debug "JeCloud version on the server: #{jecloud_version} (#{is_good_enough ? 'Good to go!' : 'Need to (re)install JeCloud.'})"
           end
+        end
+
+        session.action "#{server.uuid}-install-jecloud", :unless => jecloud_installed do
+          yum_packages = %w/gcc gcc-c++ openssl openssl-devel ruby-devel rubygems git/
+
+          $log.debug "Installing yum packages: #{yum_packages.join(' ')}"
+          server_session.sudo_print!("yum install -y #{yum_packages.join(' ')}")
+          $log.info "Installed yum packages: #{yum_packages.join(' ')}"
+
+          $log.debug "Rebuilding JeCloud locally"
+          puts `rake build`
+          raise UnexpectedExternalProblem, "JeCloud build failed" unless $?.success?
 
-          session.action "#{server.uuid}-install-jecloud", :unless => jecloud_installed do
-            yum_packages = %w/gcc gcc-c++ openssl openssl-devel ruby-devel rubygems git/
-
-            $log.debug "Installing yum packages: #{yum_packages.join(' ')}"
-            server_session.sudo_print!("yum install -y #{yum_packages.join(' ')}")
-            $log.info "Installed yum packages: #{yum_packages.join(' ')}"
-
-            $log.debug "Rebuilding JeCloud locally"
-            puts `rake build`
-            raise UnexpectedExternalProblem, "JeCloud build failed" unless $?.success?
-
-            remote_path = "/tmp/#{File.basename(GEM_FILE)}"
-            $log.debug "Uploading JeCloud gem into #{server.public_ip}:#{remote_path}"
-            server_session.sftp.file.open(remote_path, 'w') do |of|
-              of.write(File.read(GEM_FILE))
-            end
-            server_session.sftp.loop
-
-            $log.debug "Uninstalling old JeCloud version if any"
-            server_session.sudo_print!("gem uninstall --executables jecloud")
-
-            $log.debug "Installing JeCloud gem"
-            server_session.sudo_print!("gem install --no-rdoc --no-ri #{remote_path}")
-
-            jecloud_version = server_session.exec!("jecloud print-version || echo 'NONE'").strip
-            if jecloud_version == JeCloud::VERSION
-              $log.info "Installed JeCloud on #{server.public_ip}"
-            else
-              puts jecloud_version
-              raise UnexpectedExternalProblem, "Installation of JeCloud failed on #{server.public_ip}"
-            end
+          remote_path = "/tmp/#{File.basename(GEM_FILE)}"
+          $log.debug "Uploading JeCloud gem into #{server.public_ip}:#{remote_path}"
+          server_session.sftp.file.open(remote_path, 'w') do |of|
+            of.write(File.read(GEM_FILE))
           end
+          server_session.sftp.loop
 
-          # deployment requested?
-          if server.deployment?
-            # pretend that it succeeded
-            server.deployment = nil
-            return true
+          $log.debug "Uninstalling old JeCloud version if any"
+          server_session.sudo_print!("gem uninstall --executables jecloud")
+
+          $log.debug "Installing JeCloud gem"
+          server_session.sudo_print!("gem install --no-rdoc --no-ri #{remote_path}")
+
+          jecloud_version = server_session.exec!("jecloud print-version || echo 'NONE'").strip
+          if jecloud_version == JeCloud::VERSION
+            $log.info "Installed JeCloud on #{server.public_ip}"
+          else
+            puts jecloud_version
+            raise UnexpectedExternalProblem, "Installation of JeCloud failed on #{server.public_ip}"
           end
         end
+
+        session.action "#{server.uuid}-start-deployment", :if => server.deployment? do
+          # pretend that it succeeded
+          server.deployment = nil
+        end
       end
       server_session.close!
     end
-    return false
   end
 
 private

lib/jecloud/session.rb

@@ -8,15 +8,39 @@ class Session
   def initialize failures
     @failures = failures
     @next_attempt = nil
+    @any_actions_executed = false
   end
 
-  def action name, options={}
+  def any_actions_executed?
+    @any_actions_executed
+  end
+
+  def action name, options={}, &block
+    raise "action must have :if or :unless condition specified" unless options.include?(:if) || options.include?(:unless)
+
+    options = { :if => true, :unless => false }.merge(options)
+    unless options.if && !options.unless
+      $log.debug "Not needed: #{name}"
+      return
+    end
+    run_action_or_check name, &block
+    @any_actions_executed = true  # got here, so action succeeded
+  end
+
+  # check results should be cached eventually, but for now run them every time
+  def check name, options={}, &block
     options = { :if => true, :unless => false }.merge(options)
     unless options.if && !options.unless
       $log.debug "Not needed: #{name}"
       return
     end
 
+    run_action_or_check name, &block
+  end
+
+private
+
+  def run_action_or_check name
     failure = @failures.delete(name) || Hashie::Mash.new
     now = Time.now.to_i
     next_attempt = failure.last && failure.last + failure.delay
@@ -28,7 +52,7 @@ def action name, options={}
     else
       $log.debug "Starting: #{name}"
       begin
-        yield
+        result = yield
         $log.info "Succeeded: #{name}"
       rescue Exception => e
         message = "#{e.class.name}: #{e.message}"
@@ -47,9 +71,8 @@ def action name, options={}
         throw :failed
       end
     end
+    return result
   end
 
-private
-
 end
 end