[close #109] Allow retrying failed release (#119)

* [close #109] Allow retrying failed release

When a deploy succeeds but the release fails the git remote retains the contents of the `git push`. This means if you immediately try to push again that you'll get a `Everything up-to-date` from the remote and Heroku will not try to re-build your project.

We don't want that behavior, we want to retry on a failed release incase it's intermittent. This PR adds an empty commit when a failed release is detected.

* Test `allow_failure` retry count behavior

When `allow_failure` is set to true then it over-writes the value set by `retries`, this test asserts that to be true.

CHANGELOG.md

@@ -1,5 +1,8 @@
 ## HEAD
 
+- Initializing an `App` can now take a `retries` key to overload the global hatchet env var (https://github.com/heroku/hatchet/pull/119)
+- Calling `App#commit!` now adds an empty commit if there is no changes on disk (https://github.com/heroku/hatchet/pull/119)
+- Bugfix: Failed release phase in deploys can now be re-run (https://github.com/heroku/hatchet/pull/119)
 - Bugfix: Allow `hatchet lock` to be run against new projects (https://github.com/heroku/hatchet/pull/118)
 - Bugfix: Allow `hatchet.lock` file to lock a repo to a different branch than what is specified as default for GitHub (https://github.com/heroku/hatchet/pull/118)
 

README.md

@@ -531,6 +531,8 @@ WARNING: Enabling `run_multi` setting on an app will charge your Heroku account
 WARNING: Do not use `run_multi` if you're not using the `deploy` block syntax or manually call `teardown!` inside the text context [more info about how behavior does not work with the `after` block syntax in rspec](https://github.com/heroku/hatchet/issues/110).
 WARNING: To work, `run_multi` requires your application to have a `web` process associated with it.
 
+- `retries` (Integer): When passed in, this value will be used insead of the global `HATCHET_RETRIES` set via environment variable. When `allow_failures: true` is set as well as a retries value, then the application will not retry pushing to Heroku.
+
 ### App methods:
 
 - `app.set_config()`: Updates the configuration on your app taking in a hash

lib/hatchet/app.rb

@@ -9,7 +9,7 @@ class App
     HATCHET_BUILDPACK_BRANCH = -> { ENV['HATCHET_BUILDPACK_BRANCH'] || ENV['HEROKU_TEST_RUN_BRANCH'] || Hatchet.git_branch }
     BUILDPACK_URL = "https://github.com/heroku/heroku-buildpack-ruby.git"
 
-    attr_reader :name, :stack, :directory, :repo_name, :app_config, :buildpacks, :reaper
+    attr_reader :name, :stack, :directory, :repo_name, :app_config, :buildpacks, :reaper, :max_retries_count
 
     class FailedDeploy < StandardError; end
 
@@ -55,6 +55,7 @@ def initialize(repo_name,
                    buildpack_url: nil,
                    before_deploy: nil,
                    run_multi: ENV["HATCHET_RUN_MULTI"],
+                   retries: RETRIES,
                    config: {}
                   )
       @repo_name     = repo_name
@@ -68,6 +69,7 @@ def initialize(repo_name,
       @buildpacks    = Array(@buildpacks)
       @buildpacks.map! {|b| b == :default ? self.class.default_buildpack : b}
       @run_multi = run_multi
+      @max_retries_count = retries
 
       if run_multi && !ENV["HATCHET_EXPENSIVE_MODE"]
         raise "You're attempting to enable `run_multi: true` mode, but have not enabled `HATCHET_EXPENSIVE_MODE=1` env var to verify you understand the risks"
@@ -129,7 +131,7 @@ def set_lab(lab)
     end
 
     def add_database(plan_name = 'heroku-postgresql:dev', match_val = "HEROKU_POSTGRESQL_[A-Z]+_URL")
-      Hatchet::RETRIES.times.retry do
+      max_retries_count.times.retry do
         # heroku.post_addon(name, plan_name)
         api_rate_limit.call.addon.create(name, plan: plan_name )
         _, value = get_config.detect {|k, v| k.match(/#{match_val}/) }
@@ -316,7 +318,7 @@ def before_deploy(&block)
     end
 
     def commit!
-      local_cmd_exec!('git add .; git commit -m next')
+      local_cmd_exec!('git add .; git commit --allow-empty -m next')
     end
 
     def push_without_retry!
@@ -386,12 +388,12 @@ def deploy(&block)
     end
 
     def push
-      max_retries = @allow_failure ? 1 : RETRIES
-      max_retries.times.retry do |attempt|
+      retry_count = allow_failure? ? 1 : max_retries_count
+      retry_count.times.retry do |attempt|
         begin
           @output = self.push_without_retry!
         rescue StandardError => error
-          puts retry_error_message(error, attempt, max_retries)
+          puts retry_error_message(error, attempt) unless retry_count == 1
           raise error
         end
       end
@@ -400,10 +402,10 @@ def push
     alias :push_with_retry  :push
     alias :push_with_retry! :push_with_retry
 
-    def retry_error_message(error, attempt, max_retries)
+    def retry_error_message(error, attempt)
       attempt += 1
-      return "" if attempt == max_retries
-      msg = "\nRetrying failed Attempt ##{attempt}/#{max_retries} to push for '#{name}' due to error: \n"<<
+      return "" if attempt == max_retries_count
+      msg = "\nRetrying failed Attempt ##{attempt}/#{max_retries_count} to push for '#{name}' due to error: \n"<<
             "#{error.class} #{error.message}\n  #{error.backtrace.join("\n  ")}"
       return msg
     end
@@ -422,7 +424,7 @@ def heroku
 
     def run_ci(timeout: 300, &block)
       in_directory do
-        Hatchet::RETRIES.times.retry do
+        max_retries_count.times.retry do
           result       = create_pipeline
           @pipeline_id = result["id"]
         end
@@ -433,7 +435,7 @@ def run_ci(timeout: 300, &block)
         # that's why we create an app explictly (or maybe it already exists), and then associate it with with the pipeline
         # the app will be auto cleaned up later
         self.setup!
-        Hatchet::RETRIES.times.retry do
+        max_retries_count.times.retry do
           couple_pipeline(@name, @pipeline_id)
         end
 
@@ -446,7 +448,7 @@ def run_ci(timeout: 300, &block)
           api_rate_limit: api_rate_limit
         )
 
-        Hatchet::RETRIES.times.retry do
+        max_retries_count.times.retry do
           test_run.create_test_run
         end
         test_run.wait!(&block)

lib/hatchet/git_app.rb

@@ -33,6 +33,7 @@ def push_without_retry!
 
       releases = platform_api.release.list(name)
       if releases.last["status"] == "failed"
+        commit! # An empty commit allows us to deploy again
         raise FailedReleaseError.new(self, "Buildpack: #{@buildpack.inspect}\nRepo: #{git_repo}", output: output)
       end
 

spec/hatchet/allow_failure_git_spec.rb

@@ -13,14 +13,29 @@
     }
 
     it "is marked as a failure if the release fails" do
+      app = Hatchet::GitApp.new("default_ruby", before_deploy: release_fail_proc, retries: 2)
+      def app.retry_error_message(*args); @test_attempts_count ||= 0; @test_attempts_count += 1; "" end
+      def app.test_attempts_count; @test_attempts_count ; end
+
       expect {
-        Hatchet::GitApp.new("default_ruby", before_deploy: release_fail_proc).deploy {}
-      }.to(raise_error(Hatchet::App::FailedReleaseError))
+        app.deploy {}
+      }.to raise_error { |error|
+        expect(error).to be_a(Hatchet::App::FailedReleaseError)
+        expect(error.message).to_not match("Everything up-to-date")
+      }
+
+      expect(app.test_attempts_count).to eq(2)
     end
 
     it "works when failure is allowed" do
-      Hatchet::GitApp.new("default_ruby", before_deploy: release_fail_proc, allow_failure: true).deploy do |app|
-        expect(app.output).to match("failing on release")
+      Hatchet::GitApp.new("default_ruby", before_deploy: release_fail_proc, allow_failure: true, retries: 3).tap do |app|
+        def app.retry_error_message(*args); @test_attempts_count ||= 0; @test_attempts_count += 1; "" end
+        def app.test_attempts_count; @test_attempts_count ; end
+
+        app.deploy do
+          expect(app.output).to match("failing on release")
+          expect(app.test_attempts_count).to eq(nil)
+        end
       end
     end
   end