From a76b0766b040db7c61bbde65825c440c49749463 Mon Sep 17 00:00:00 2001 From: Darwin D Wu Date: Sat, 22 Jul 2017 16:15:44 -0700 Subject: [PATCH] deploy:cleanup should skip only the invalid releases and continue the rotation of releases (#1899) * deploy:cleanup should skip only the invalid releases and continue the rotation of releases * Add cucumber tests for deploy:cleanup * PR feedback * rebase on master * add ruby 2.4.1 to specs * vagrant works with bundler 1.15.3 * remove unnecessary command --- .travis.yml | 1 + CHANGELOG.md | 2 ++ DEVELOPMENT.md | 5 ++--- features/deploy.feature | 9 +++++++++ features/step_definitions/assertions.rb | 12 ++++++++++++ features/step_definitions/setup.rb | 13 +++++++++++++ lib/capistrano/i18n.rb | 2 +- lib/capistrano/tasks/deploy.rake | 12 +++++++----- spec/support/test_app.rb | 8 ++++---- 9 files changed, 51 insertions(+), 13 deletions(-) diff --git a/.travis.yml b/.travis.yml index f8ea17b7..022a50f4 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,5 +1,6 @@ language: ruby rvm: + - 2.4.1 - 2.3.1 - 2.2 - 2.1 diff --git a/CHANGELOG.md b/CHANGELOG.md index dd0d07f9..582c2231 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,8 @@ https://github.com/capistrano/capistrano/compare/v3.8.1...HEAD * Your contribution here! +* Updated `deploy:cleanup` to continue rotating the releases and skip the invalid directory names instead of skipping the whole rotation of releases. The warning message has changed slightly due to the change of behavior. + ## `3.8.2` (2017-06-16) https://github.com/capistrano/capistrano/compare/v3.8.1...v3.8.2 diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index 36f02bc2..44e1ebc9 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -28,11 +28,10 @@ Capistrano is a Ruby project, so we expect you to have a functioning Ruby enviro Make sure to install: -* [Bundler](https://bundler.io/) 1.10.5 (see note) +* [Bundler](https://bundler.io/) * [Vagrant](https://www.vagrantup.com/) * [VirtualBox](https://www.virtualbox.org/wiki/Downloads) (or another [Vagrant-supported](https://docs.vagrantup.com/v2/getting-started/providers.html) VM host) -*Note: As of this writing (December 2015), Vagrant does not work with Bundler > 1.10.5. If you have multiple versions of Bundler installed, use the special underscore command-line argument to force a compatible version, like this: `bundle _1.10.5_ exec rake features`.* ### Running tests @@ -45,7 +44,7 @@ $ bundle install # Run the RSpec suite $ bundle exec rake spec -# Run the Cucumber suite (requires Bundler 1.10.5) +# Run the Cucumber suite $ bundle exec rake features # Run the Cucumber suite and leave the VM running (faster for subsequent runs) diff --git a/features/deploy.feature b/features/deploy.feature index 5f585fec..58e49538 100644 --- a/features/deploy.feature +++ b/features/deploy.feature @@ -52,6 +52,14 @@ Feature: Deploy When I run cap "deploy:symlink:release" Then the current directory will be a symlink to the release + Scenario: Cleanup + Given config stage file has line "set :keep_releases, 3" + And 5 valid existing releases + And an invalid release named "new" + When I run cap "deploy:cleanup" + Then 3 valid releases are kept + And the invalid "new" release is ignored + Scenario: Rolling Back Given I make 2 deployments When I run cap "deploy:rollback" @@ -61,3 +69,4 @@ Feature: Deploy Given I make 3 deployments When I rollback to a specific release Then the current symlink points to that specific release + diff --git a/features/step_definitions/assertions.rb b/features/step_definitions/assertions.rb index 0ff483af..1e27185a 100644 --- a/features/step_definitions/assertions.rb +++ b/features/step_definitions/assertions.rb @@ -19,6 +19,18 @@ Then(/^the releases path is created$/) do run_vagrant_command(test_dir_exists(TestApp.releases_path)) end +Then(/^(\d+) valid releases are kept/) do |num| + test = %Q([ $(ls -g #{TestApp.releases_path} | grep -E '[0-9]{14}' | wc -l) == "#{num}" ]) + _, _, status = vagrant_cli_command("ssh -c #{test.shellescape}") + expect(status).to be_success +end + +Then(/^the invalid (.+) release is ignored$/) do |filename| + test = "ls -g #{TestApp.releases_path} | grep #{filename}" + _, _, status = vagrant_cli_command("ssh -c #{test.shellescape}") + expect(status).to be_success +end + Then(/^directories in :linked_dirs are created in shared$/) do TestApp.linked_dirs.each do |dir| run_vagrant_command(test_dir_exists(TestApp.shared_path.join(dir))) diff --git a/features/step_definitions/setup.rb b/features/step_definitions/setup.rb index dcf21dc7..6cd7a9ce 100644 --- a/features/step_definitions/setup.rb +++ b/features/step_definitions/setup.rb @@ -77,3 +77,16 @@ Given(/^I make (\d+) deployments$/) do |count| stdout.strip end end + +Given(/^(\d+) valid existing releases$/) do |num| + a_day = 86_400 # in seconds + offset = -(a_day * num.to_i) + num.to_i.times do + run_vagrant_command("mkdir -p #{TestApp.release_path(TestApp.timestamp(offset))}") + offset += a_day + end +end + +Given(/^an invalid release named "(.+)"$/) do |filename| + run_vagrant_command("mkdir -p #{TestApp.release_path(filename)}") +end diff --git a/lib/capistrano/i18n.rb b/lib/capistrano/i18n.rb index d7e6d407..59b8a91a 100644 --- a/lib/capistrano/i18n.rb +++ b/lib/capistrano/i18n.rb @@ -13,7 +13,7 @@ en = { question: "Please enter %{key}: ", question_default: "Please enter %{key} (%{default_value}): ", keeping_releases: "Keeping %{keep_releases} of %{releases} deployed releases on %{host}", - skip_cleanup: "Skipping cleanup of old releases on %{host}; unexpected foldername found (should be timestamp)", + skip_cleanup: "Skipping cleanup of invalid releases on %{host}; unexpected foldername found (should be timestamp)", no_old_releases: "No old releases (keeping newest %{keep_releases}) on %{host}", linked_file_does_not_exist: "linked file %{file} does not exist on %{host}", cannot_rollback: "There are no older releases to rollback to", diff --git a/lib/capistrano/tasks/deploy.rake b/lib/capistrano/tasks/deploy.rake index 0d79ea26..1153b78f 100644 --- a/lib/capistrano/tasks/deploy.rake +++ b/lib/capistrano/tasks/deploy.rake @@ -149,11 +149,13 @@ namespace :deploy do task :cleanup do on release_roles :all do |host| releases = capture(:ls, "-x", releases_path).split - if !(releases.all? { |e| /^\d{14}$/ =~ e }) - warn t(:skip_cleanup, host: host.to_s) - elsif releases.count >= fetch(:keep_releases) - info t(:keeping_releases, host: host.to_s, keep_releases: fetch(:keep_releases), releases: releases.count) - directories = (releases - releases.last(fetch(:keep_releases))) + valid, invalid = releases.partition { |e| /^\d{14}$/ =~ e } + + warn t(:skip_cleanup, host: host.to_s) if invalid.any? + + if valid.count >= fetch(:keep_releases) + info t(:keeping_releases, host: host.to_s, keep_releases: fetch(:keep_releases), releases: valid.count) + directories = (valid - valid.last(fetch(:keep_releases))) if directories.any? directories_str = directories.map do |release| releases_path.join(release) diff --git a/spec/support/test_app.rb b/spec/support/test_app.rb index 2bdf37ba..8d0eabcd 100644 --- a/spec/support/test_app.rb +++ b/spec/support/test_app.rb @@ -132,12 +132,12 @@ module TestApp deploy_to.join("releases") end - def release_path - releases_path.join(timestamp) + def release_path(t=timestamp) + releases_path.join(t) end - def timestamp - Time.now.utc.strftime("%Y%m%d%H%M%S") + def timestamp(offset=0) + (Time.now.utc + offset).strftime("%Y%m%d%H%M%S") end def repo_path