From d8b9b695bb88b4779bb16f543bdbd1eb872e0859 Mon Sep 17 00:00:00 2001 From: seenmyfate Date: Fri, 11 Oct 2013 17:24:37 +0100 Subject: [PATCH 01/11] Allow configuration location to be configurable This change allows both the `deploy_config_path` and `stage_config_path` to be moved from the default locations of `config/deploy.rb` and `config/deploy` respectively. These values __must__ be set in the `Capfile` prior to `capistrano/setup` being called, for example: set :deploy_config_path, 'app/config/deploy.rb' set :stage_config_path, 'app/config/deploy' # Load DSL and Setup Up Stages require 'capistrano/setup' Fixes #610 --- features/configuration.feature | 15 +++++++ features/step_definitions/assertions.rb | 4 ++ features/step_definitions/cap_commands.rb | 6 ++- features/step_definitions/setup.rb | 3 ++ lib/capistrano/dsl/paths.rb | 8 ++++ lib/capistrano/dsl/stages.rb | 9 ++++- lib/capistrano/setup.rb | 4 +- spec/integration/dsl_spec.rb | 48 +++++++++++++++++++++++ spec/lib/capistrano/dsl_spec.rb | 11 ------ spec/support/test_app.rb | 33 +++++++++++++++- 10 files changed, 125 insertions(+), 16 deletions(-) create mode 100644 features/configuration.feature diff --git a/features/configuration.feature b/features/configuration.feature new file mode 100644 index 00000000..31dd59da --- /dev/null +++ b/features/configuration.feature @@ -0,0 +1,15 @@ +Feature: The path to the configuration can be changed, removing the need to + follow Ruby/Rails conventions + + Background: + Given a test app with the default configuration + And servers with the roles app and web + + Scenario: Deploying with configuration in default location + When I run "cap test" + Then the task is successful + + Scenario: Deploying with configuration in a custom location + But the configuration is in a custom location + When I run "cap test" + Then the task is successful diff --git a/features/step_definitions/assertions.rb b/features/step_definitions/assertions.rb index efa002cd..8b24040e 100644 --- a/features/step_definitions/assertions.rb +++ b/features/step_definitions/assertions.rb @@ -88,3 +88,7 @@ end Then(/^it will not recreate the file$/) do # end + +Then(/^the task is successful$/) do + expect(@success).to be_true +end diff --git a/features/step_definitions/cap_commands.rb b/features/step_definitions/cap_commands.rb index f5b5c14d..401e25f2 100644 --- a/features/step_definitions/cap_commands.rb +++ b/features/step_definitions/cap_commands.rb @@ -1,8 +1,12 @@ When(/^I run cap "(.*?)"$/) do |task| - TestApp.cap(task) + @success = TestApp.cap(task) end When(/^I run cap "(.*?)" as part of a release$/) do |task| TestApp.cap("deploy:new_release_path #{task}") end +When(/^I run "(.*?)"$/) do |command| + @success = TestApp.run(command) +end + diff --git a/features/step_definitions/setup.rb b/features/step_definitions/setup.rb index 4d16620a..7379deb4 100644 --- a/features/step_definitions/setup.rb +++ b/features/step_definitions/setup.rb @@ -23,3 +23,6 @@ Given(/^a custom task to generate a file$/) do TestApp.copy_task_to_test_app('spec/support/tasks/database.cap') end +Given(/^the configuration is in a custom location$/) do + TestApp.move_configuration_to_custom_location('app') +end diff --git a/lib/capistrano/dsl/paths.rb b/lib/capistrano/dsl/paths.rb index 020b2c7b..17e0a044 100644 --- a/lib/capistrano/dsl/paths.rb +++ b/lib/capistrano/dsl/paths.rb @@ -27,6 +27,14 @@ module Capistrano set(:release_path, releases_path.join(timestamp)) end + def stage_config_path + Pathname.new fetch(:stage_config_path, 'config/deploy') + end + + def deploy_config_path + Pathname.new fetch(:deploy_config_path, 'config/deploy.rb') + end + def repo_url require 'cgi' require 'uri' diff --git a/lib/capistrano/dsl/stages.rb b/lib/capistrano/dsl/stages.rb index 283421b0..3455a87a 100644 --- a/lib/capistrano/dsl/stages.rb +++ b/lib/capistrano/dsl/stages.rb @@ -3,7 +3,14 @@ module Capistrano module Stages def stages - Dir['config/deploy/*.rb'].map { |f| File.basename(f, '.rb') } + Dir[stage_definitions].map { |f| File.basename(f, '.rb') } + end + + def infer_stages_from_stage_files + end + + def stage_definitions + stage_config_path.join('*.rb') end def stage_set? diff --git a/lib/capistrano/setup.rb b/lib/capistrano/setup.rb index f08b470d..13c160f8 100644 --- a/lib/capistrano/setup.rb +++ b/lib/capistrano/setup.rb @@ -9,8 +9,8 @@ end stages.each do |stage| Rake::Task.define_task(stage) do invoke 'load:defaults' - load 'config/deploy.rb' - load "config/deploy/#{stage}.rb" + load deploy_config_path + load stage_config_path.join("#{stage}.rb") load "capistrano/#{fetch(:scm)}.rb" set(:stage, stage.to_sym) I18n.locale = fetch(:locale, :en) diff --git a/spec/integration/dsl_spec.rb b/spec/integration/dsl_spec.rb index d99a8657..ed5cadff 100644 --- a/spec/integration/dsl_spec.rb +++ b/spec/integration/dsl_spec.rb @@ -340,5 +340,53 @@ describe Capistrano::DSL do end end + describe 'setting deploy configuration path' do + subject { dsl.deploy_config_path.to_s } + + context 'where no config path is set' do + before do + dsl.delete(:deploy_config_path) + end + + it 'returns "config/deploy.rb"' do + expect(subject).to eq 'config/deploy.rb' + end + end + + context 'where a custom path is set' do + before do + dsl.set(:deploy_config_path, 'my/custom/path.rb') + end + + it 'returns the custom path' do + expect(subject).to eq 'my/custom/path.rb' + end + end + end + + describe 'setting stage configuration path' do + subject { dsl.stage_config_path.to_s } + + context 'where no config path is set' do + + before do + dsl.delete(:stage_config_path) + end + + it 'returns "config/deploy"' do + expect(subject).to eq 'config/deploy' + end + end + + context 'where a custom path is set' do + before do + dsl.set(:stage_config_path, 'my/custom/path') + end + + it 'returns the custom path' do + expect(subject).to eq 'my/custom/path' + end + end + end end end diff --git a/spec/lib/capistrano/dsl_spec.rb b/spec/lib/capistrano/dsl_spec.rb index 4157a996..9d244218 100644 --- a/spec/lib/capistrano/dsl_spec.rb +++ b/spec/lib/capistrano/dsl_spec.rb @@ -20,17 +20,6 @@ module Capistrano end end - describe '#stages' do - before do - Dir.expects(:[]).with('config/deploy/*.rb'). - returns(['config/deploy/staging.rb', 'config/deploy/production.rb']) - end - - it 'returns a list of defined stages' do - expect(dsl.stages).to eq %w{staging production} - end - end - describe '#stage_set?' do subject { dsl.stage_set? } diff --git a/spec/support/test_app.rb b/spec/support/test_app.rb index 5f18d1fd..3608426a 100644 --- a/spec/support/test_app.rb +++ b/spec/support/test_app.rb @@ -58,6 +58,14 @@ module TestApp end end + def prepend_to_capfile(config) + current_capfile = File.read(capfile) + File.open(capfile, 'w') do |file| + file.write config + file.write current_capfile + end + end + def create_shared_directory(path) FileUtils.mkdir_p(shared_path.join(path)) end @@ -67,9 +75,14 @@ module TestApp end def cap(task) + run "bundle exec cap #{stage} #{task}" + end + + def run(command) Dir.chdir(test_app_path) do - %x[bundle exec cap #{stage} #{task}] + %x[#{command}] end + $?.success? end def stage @@ -135,4 +148,22 @@ module TestApp def copy_task_to_test_app(source) FileUtils.cp(source, task_dir) end + + def config_path + test_app_path.join('config') + end + + def move_configuration_to_custom_location(location) + prepend_to_capfile( + %{ + set :stage_config_path, "app/config/deploy" + set :deploy_config_path, "app/config/deploy.rb" + } + ) + + location = test_app_path.join(location) + FileUtils.mkdir_p(location) + FileUtils.mv(config_path, location) + end + end From aae79dbd80960a08e2940c491729e427036e83e7 Mon Sep 17 00:00:00 2001 From: Kir Shatrov Date: Wed, 16 Oct 2013 23:54:51 +0200 Subject: [PATCH 02/11] Don't do :cleanup twice As it's already beeing called from https://github.com/capistrano/capistrano/blob/master/lib/capistrano/tasks/deploy.rake#L22 --- lib/capistrano/templates/deploy.rb.erb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/capistrano/templates/deploy.rb.erb b/lib/capistrano/templates/deploy.rb.erb index c3e88d26..461ca74c 100644 --- a/lib/capistrano/templates/deploy.rb.erb +++ b/lib/capistrano/templates/deploy.rb.erb @@ -35,6 +35,4 @@ namespace :deploy do end end - after :finishing, 'deploy:cleanup' - -end \ No newline at end of file +end From 7a05a77db90c355d87c4c932b1728226d34d6ad0 Mon Sep 17 00:00:00 2001 From: Nigel Ramsay Date: Thu, 17 Oct 2013 14:31:44 +1300 Subject: [PATCH 03/11] Update README.md add missing "bring" to example sentance --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index ee6da574..3f5e3fb4 100644 --- a/README.md +++ b/README.md @@ -126,7 +126,7 @@ This method is widely used. ``` ruby desc "Ask about breakfast" task :breakfast do - breakfast = ask(:breakfast, "What would you like your colleagues to you for breakfast?") + breakfast = ask(:breakfast, "What would you like your colleagues to bring you for breakfast?") on roles(:all) do |h| execute "echo \"$(whoami) wants #{breakfast} for breakfast!\" | wall" end From a9e71c804044791676ef856cd05d2b5d4a751c05 Mon Sep 17 00:00:00 2001 From: Kir Shatrov Date: Thu, 17 Oct 2013 12:14:32 +0200 Subject: [PATCH 04/11] Set stage at the beginning to be able to use it from config/deploy.rb --- lib/capistrano/setup.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/capistrano/setup.rb b/lib/capistrano/setup.rb index f08b470d..cf1f2c91 100644 --- a/lib/capistrano/setup.rb +++ b/lib/capistrano/setup.rb @@ -8,11 +8,12 @@ end stages.each do |stage| Rake::Task.define_task(stage) do + set(:stage, stage.to_sym) + invoke 'load:defaults' load 'config/deploy.rb' load "config/deploy/#{stage}.rb" load "capistrano/#{fetch(:scm)}.rb" - set(:stage, stage.to_sym) I18n.locale = fetch(:locale, :en) configure_backend end From 51d5d1ecd9231222c12cc3f18bae8579d400e96f Mon Sep 17 00:00:00 2001 From: Kir Shatrov Date: Thu, 17 Oct 2013 12:38:48 +0200 Subject: [PATCH 05/11] Warn developers that :all is a meta role Fix for #705 --- lib/capistrano/configuration.rb | 4 ++++ lib/capistrano/templates/stage.rb.erb | 1 + spec/integration/dsl_spec.rb | 8 ++++++++ 3 files changed, 13 insertions(+) diff --git a/lib/capistrano/configuration.rb b/lib/capistrano/configuration.rb index 40637737..979cff3c 100644 --- a/lib/capistrano/configuration.rb +++ b/lib/capistrano/configuration.rb @@ -34,6 +34,10 @@ module Capistrano end def role(name, hosts, options={}) + if name == :all + raise ArgumentError.new("#{name} reserved name for role. Please choose another name") + end + servers.add_role(name, hosts, options) end diff --git a/lib/capistrano/templates/stage.rb.erb b/lib/capistrano/templates/stage.rb.erb index 19fbcf7a..65b86dd3 100644 --- a/lib/capistrano/templates/stage.rb.erb +++ b/lib/capistrano/templates/stage.rb.erb @@ -5,6 +5,7 @@ set :stage, :<%= stage %> # Supports bulk-adding hosts to roles, the primary # server in each group is considered to be the first # unless any hosts have the primary property set. +# Don't declare `role :all`, it's a meta role role :app, %w{deploy@example.com} role :web, %w{deploy@example.com} role :db, %w{deploy@example.com} diff --git a/spec/integration/dsl_spec.rb b/spec/integration/dsl_spec.rb index d99a8657..36af5d5b 100644 --- a/spec/integration/dsl_spec.rb +++ b/spec/integration/dsl_spec.rb @@ -70,6 +70,14 @@ describe Capistrano::DSL do end + describe 'when defining role with reserved name' do + it 'fails with ArgumentError' do + expect { + dsl.role :all, %w{example1.com} + }.to raise_error(ArgumentError, "all reserved name for role. Please choose another name") + end + end + describe 'when defining hosts using the `role` syntax' do before do dsl.role :web, %w{example1.com example2.com example3.com} From 362fb86fffef8a9997c8f991f29edf8361b6314a Mon Sep 17 00:00:00 2001 From: Kir Shatrov Date: Thu, 17 Oct 2013 13:41:53 +0200 Subject: [PATCH 06/11] Default stage template cleanup Since `stage` is already set (https://github.com/capistrano/capistrano/pull/712), we don't need to declare it twice here. --- lib/capistrano/templates/stage.rb.erb | 2 -- spec/support/test_app.rb | 1 - 2 files changed, 3 deletions(-) diff --git a/lib/capistrano/templates/stage.rb.erb b/lib/capistrano/templates/stage.rb.erb index 65b86dd3..0bcd393e 100644 --- a/lib/capistrano/templates/stage.rb.erb +++ b/lib/capistrano/templates/stage.rb.erb @@ -1,5 +1,3 @@ -set :stage, :<%= stage %> - # Simple Role Syntax # ================== # Supports bulk-adding hosts to roles, the primary diff --git a/spec/support/test_app.rb b/spec/support/test_app.rb index 5f18d1fd..8e36c31b 100644 --- a/spec/support/test_app.rb +++ b/spec/support/test_app.rb @@ -8,7 +8,6 @@ module TestApp def default_config %{ - set :stage, :#{stage} set :deploy_to, '#{deploy_to}' set :repo_url, 'git://github.com/capistrano/capistrano.git' set :branch, 'v3' From 97d0ddf0ae843c2095fc4671f53714c3f2d6b601 Mon Sep 17 00:00:00 2001 From: seenmyfate Date: Fri, 18 Oct 2013 11:25:59 +0100 Subject: [PATCH 07/11] Introduce `no_release` server option In order to provide a way for a server to perform tasks as part of a deploy but without being involved in the standard deploy flow, all included tasks will now prefer `release_roles` over `roles`. For example: on release_roles :all do # end This behaviour is implemented using `exclude`, a new option when selecting roles. `release_roles` is equivalent to: on roles :all, exclude: :no_release do # end Any server defined with `no_release: true` will be excluded from these tasks: server 'localhost', roles: %w{db}, no_release: true `exclude` can also be used in user defined tasks against any attribute, for example: server 'localhost', roles: %w{app web}, inactive: true on roles :app, exclude: :inactive do # end This commit resolves #686 --- lib/capistrano/configuration/server.rb | 21 ++++++++- lib/capistrano/dsl/env.rb | 10 +++++ lib/capistrano/tasks/deploy.rake | 6 +-- lib/capistrano/tasks/git.rake | 10 ++--- lib/capistrano/tasks/hg.rake | 8 ++-- spec/integration/dsl_spec.rb | 45 ++++++++++++++++++- .../capistrano/configuration/server_spec.rb | 22 +++++++++ .../capistrano/configuration/servers_spec.rb | 29 ++++++++++++ spec/lib/capistrano/dsl/env_spec.rb | 10 ----- 9 files changed, 136 insertions(+), 25 deletions(-) delete mode 100644 spec/lib/capistrano/dsl/env_spec.rb diff --git a/lib/capistrano/configuration/server.rb b/lib/capistrano/configuration/server.rb index b81402db..1f151aa7 100644 --- a/lib/capistrano/configuration/server.rb +++ b/lib/capistrano/configuration/server.rb @@ -23,7 +23,7 @@ module Capistrano end def select?(options) - selector = Selector.new(options) + selector = Selector.for(options) selector.call(self) end @@ -103,6 +103,14 @@ module Capistrano @options = options end + def self.for(options) + if options.has_key?(:exclude) + Exclusive + else + self + end.new(options) + end + def callable if key.respond_to?(:call) key @@ -126,6 +134,17 @@ module Capistrano ->(server) { :all } end + class Exclusive < Selector + + def key + options[:exclude] + end + + def call(server) + !callable.call(server) + end + end + end end diff --git a/lib/capistrano/dsl/env.rb b/lib/capistrano/dsl/env.rb index a7bb858f..e6641344 100644 --- a/lib/capistrano/dsl/env.rb +++ b/lib/capistrano/dsl/env.rb @@ -43,6 +43,16 @@ module Capistrano env.roles_for(names) end + def release_roles(*names) + options = { exclude: :no_release } + if names.last.is_a? Hash + names.last.merge(options) + else + names << options + end + roles(*names) + end + def primary(role) env.primary(role) end diff --git a/lib/capistrano/tasks/deploy.rake b/lib/capistrano/tasks/deploy.rake index 101e5662..99d05071 100644 --- a/lib/capistrano/tasks/deploy.rake +++ b/lib/capistrano/tasks/deploy.rake @@ -42,7 +42,7 @@ namespace :deploy do namespace :check do desc 'Check shared and release directories exist' task :directories do - on roles :all do + on release_roles :all do execute :mkdir, '-pv', shared_path, releases_path end end @@ -80,7 +80,7 @@ namespace :deploy do namespace :symlink do desc 'Symlink release to current' task :release do - on roles :all do + on release_roles :all do execute :rm, '-rf', current_path execute :ln, '-s', release_path, current_path end @@ -133,7 +133,7 @@ namespace :deploy do desc 'Clean up old releases' task :cleanup do - on roles :all do |host| + on release_roles :all do |host| releases = capture(:ls, '-x', releases_path).split if releases.count >= fetch(:keep_releases) info t(:keeping_releases, host: host.to_s, keep_releases: fetch(:keep_releases), releases: releases.count) diff --git a/lib/capistrano/tasks/git.rake b/lib/capistrano/tasks/git.rake index a33cc20a..5e55b2c5 100644 --- a/lib/capistrano/tasks/git.rake +++ b/lib/capistrano/tasks/git.rake @@ -7,7 +7,7 @@ namespace :git do desc 'Upload the git wrapper script, this script guarantees that we can script git without getting an interactive prompt' task :wrapper do - on roles :all do + on release_roles :all do upload! StringIO.new("#!/bin/sh -e\nexec /usr/bin/ssh -o PasswordAuthentication=no -o StrictHostKeyChecking=no \"$@\"\n"), "#{fetch(:tmp_dir)}/git-ssh.sh" execute :chmod, "+x", "#{fetch(:tmp_dir)}/git-ssh.sh" end @@ -16,7 +16,7 @@ namespace :git do desc 'Check that the repository is reachable' task check: :'git:wrapper' do fetch(:branch) - on roles :all do + on release_roles :all do with git_environmental_variables do exit 1 unless test :git, :'ls-remote', repo_url end @@ -25,7 +25,7 @@ namespace :git do desc 'Clone the repo to the cache' task clone: :'git:wrapper' do - on roles :all do + on release_roles :all do if test " [ -f #{repo_path}/HEAD ] " info t(:mirror_exists, at: repo_path) else @@ -40,7 +40,7 @@ namespace :git do desc 'Update the repo mirror to reflect the origin state' task update: :'git:clone' do - on roles :all do + on release_roles :all do within repo_path do execute :git, :remote, :update end @@ -49,7 +49,7 @@ namespace :git do desc 'Copy repo to releases' task create_release: :'git:update' do - on roles :all do + on release_roles :all do with git_environmental_variables do within repo_path do execute :mkdir, '-p', release_path diff --git a/lib/capistrano/tasks/hg.rake b/lib/capistrano/tasks/hg.rake index 4018ef1a..138a75c7 100644 --- a/lib/capistrano/tasks/hg.rake +++ b/lib/capistrano/tasks/hg.rake @@ -1,14 +1,14 @@ namespace :hg do desc 'Check that the repo is reachable' task :check do - on roles :all do + on release_roles :all do execute "hg", "id", repo_url end end desc 'Clone the repo to the cache' task :clone do - on roles :all do + on release_roles :all do if test " [ -d #{repo_path}/.hg ] " info t(:mirror_exists, at: repo_path) else @@ -21,7 +21,7 @@ namespace :hg do desc 'Pull changes from the remote repo' task :update => :'hg:clone' do - on roles :all do + on release_roles :all do within repo_path do execute "hg", "pull" end @@ -30,7 +30,7 @@ namespace :hg do desc 'Copy repo to releases' task :create_release => :'hg:update' do - on roles :all do + on release_roles :all do within repo_path do execute "hg", "archive", release_path, "--rev", fetch(:branch) end diff --git a/spec/integration/dsl_spec.rb b/spec/integration/dsl_spec.rb index 36af5d5b..3dba85c9 100644 --- a/spec/integration/dsl_spec.rb +++ b/spec/integration/dsl_spec.rb @@ -11,13 +11,33 @@ describe Capistrano::DSL do dsl.server 'example2.com', roles: %w{web} dsl.server 'example3.com', roles: %w{app web}, active: true dsl.server 'example4.com', roles: %w{app}, primary: true + dsl.server 'example5.com', roles: %w{db}, no_release: true end describe 'fetching all servers' do subject { dsl.roles(:all) } it 'returns all servers' do - expect(subject.map(&:hostname)).to eq %w{example1.com example2.com example3.com example4.com} + expect(subject.map(&:hostname)).to eq %w{example1.com example2.com example3.com example4.com example5.com} + end + end + + describe 'fetching all release servers' do + + context 'with no additional options' do + subject { dsl.release_roles(:all) } + + it 'returns all release servers' do + expect(subject.map(&:hostname)).to eq %w{example1.com example2.com example3.com example4.com} + end + end + + context 'with filter options' do + subject { dsl.release_roles(:all, filter: :active) } + + it 'returns all release servers that match the filter' do + expect(subject.map(&:hostname)).to eq %w{example1.com example3.com} + end end end @@ -85,16 +105,37 @@ describe Capistrano::DSL do dsl.role :app, %w{example3.com example4.com} dsl.role :app, %w{example3.com}, active: true dsl.role :app, %w{example4.com}, primary: true + dsl.role :db, %w{example5.com}, no_release: true end describe 'fetching all servers' do subject { dsl.roles(:all) } it 'returns all servers' do - expect(subject.map(&:hostname)).to eq %w{example1.com example2.com example3.com example4.com} + expect(subject.map(&:hostname)).to eq %w{example1.com example2.com example3.com example4.com example5.com} end end + describe 'fetching all release servers' do + + context 'with no additional options' do + subject { dsl.release_roles(:all) } + + it 'returns all release servers' do + expect(subject.map(&:hostname)).to eq %w{example1.com example2.com example3.com example4.com} + end + end + + context 'with filter options' do + subject { dsl.release_roles(:all, filter: :active) } + + it 'returns all release servers that match the filter' do + expect(subject.map(&:hostname)).to eq %w{example1.com example3.com} + end + end + end + + describe 'fetching servers by role' do subject { dsl.roles(:app) } diff --git a/spec/lib/capistrano/configuration/server_spec.rb b/spec/lib/capistrano/configuration/server_spec.rb index 4d98cd50..b4bb032a 100644 --- a/spec/lib/capistrano/configuration/server_spec.rb +++ b/spec/lib/capistrano/configuration/server_spec.rb @@ -159,6 +159,11 @@ module Capistrano let(:options) { { select: :active }} it { should be_true } end + + context 'with :exclude' do + let(:options) { { exclude: :active }} + it { should be_false } + end end context 'value does not match server properly' do @@ -171,6 +176,11 @@ module Capistrano let(:options) { { select: :inactive }} it { should be_false } end + + context 'with :exclude' do + let(:options) { { exclude: :inactive }} + it { should be_true } + end end end @@ -186,6 +196,12 @@ module Capistrano let(:options) { { select: ->(s) { s.properties.active } } } it { should be_true } end + + context 'with :exclude' do + let(:options) { { exclude: ->(s) { s.properties.active } } } + it { should be_false } + end + end context 'value does not match server properly' do @@ -198,6 +214,12 @@ module Capistrano let(:options) { { select: ->(s) { s.properties.inactive } } } it { should be_false } end + + context 'with :exclude' do + let(:options) { { exclude: ->(s) { s.properties.inactive } } } + it { should be_true } + end + end end diff --git a/spec/lib/capistrano/configuration/servers_spec.rb b/spec/lib/capistrano/configuration/servers_spec.rb index f250593c..03677bf8 100644 --- a/spec/lib/capistrano/configuration/servers_spec.rb +++ b/spec/lib/capistrano/configuration/servers_spec.rb @@ -141,6 +141,35 @@ module Capistrano end + describe 'excluding by property' do + + before do + servers.add_host('1', roles: :app, active: true) + servers.add_host('2', roles: :app, active: true, no_release: true) + end + + it 'is empty if the filter would remove all matching hosts' do + hosts = servers.roles_for([:app, exclude: :active]) + expect(hosts.map(&:hostname)).to be_empty + end + + it 'returns the servers without the attributes specified' do + hosts = servers.roles_for([:app, exclude: :no_release]) + expect(hosts.map(&:hostname)).to eq %w{1} + end + + it 'can exclude hosts by properties on the host using a regular proc' do + hosts = servers.roles_for([:app, exclude: ->(h) { h.properties.no_release }]) + expect(hosts.map(&:hostname)).to eq %w{1} + end + + it 'is empty if the regular proc filter would remove all matching hosts' do + hosts = servers.roles_for([:app, exclude: ->(h) { h.properties.active }]) + expect(hosts.map(&:hostname)).to be_empty + end + + end + describe 'filtering roles' do before do diff --git a/spec/lib/capistrano/dsl/env_spec.rb b/spec/lib/capistrano/dsl/env_spec.rb deleted file mode 100644 index 61414cba..00000000 --- a/spec/lib/capistrano/dsl/env_spec.rb +++ /dev/null @@ -1,10 +0,0 @@ -require 'spec_helper' - -module Capistrano - module DSL - - describe Env do - - end - end -end From 4e6523e1f50707499cf75eb53dce37a89528a9b0 Mon Sep 17 00:00:00 2001 From: Kir Shatrov Date: Wed, 23 Oct 2013 16:18:56 +0200 Subject: [PATCH 08/11] Don't hardcode :restart task in deploy cycle Moving :restart into confing/deploy.rb example Fixed #692 --- lib/capistrano/i18n.rb | 1 - lib/capistrano/tasks/deploy.rake | 1 - lib/capistrano/templates/deploy.rb.erb | 1 + 3 files changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/capistrano/i18n.rb b/lib/capistrano/i18n.rb index 781ea2d9..e5dec4c5 100644 --- a/lib/capistrano/i18n.rb +++ b/lib/capistrano/i18n.rb @@ -7,7 +7,6 @@ en = { start: 'Start', update: 'Update', finalize: 'Finalise', - restart: 'Restart', finishing: 'Finishing', finished: 'Finished', stage_not_set: 'Stage not set, please call something such as `cap production deploy`, where production is a stage you have defined.', diff --git a/lib/capistrano/tasks/deploy.rake b/lib/capistrano/tasks/deploy.rake index 101e5662..e0746f50 100644 --- a/lib/capistrano/tasks/deploy.rake +++ b/lib/capistrano/tasks/deploy.rake @@ -15,7 +15,6 @@ namespace :deploy do task :publishing do invoke 'deploy:symlink:release' - invoke 'deploy:restart' end task :finishing do diff --git a/lib/capistrano/templates/deploy.rb.erb b/lib/capistrano/templates/deploy.rb.erb index 461ca74c..6f697d58 100644 --- a/lib/capistrano/templates/deploy.rb.erb +++ b/lib/capistrano/templates/deploy.rb.erb @@ -17,6 +17,7 @@ set :repo_url, 'git@example.com:me/my_repo.git' # set :keep_releases, 5 namespace :deploy do + after :publishing, :restart desc 'Restart application' task :restart do From 7432c710b05456d03f65a07934496bd44a0a0067 Mon Sep 17 00:00:00 2001 From: seenmyfate Date: Fri, 1 Nov 2013 11:59:13 +0000 Subject: [PATCH 09/11] Add hook for deploy failure If an error is raised during a deploy, the task `deploy:failed` will be triggered. Custom tasks can hook into this using `after`: after 'deploy:failed', :send_for_help do # end I've also taken the opportunity to provide a marginally more useful error message before triggering the task. By default, this 'deploy:failed' will only be triggered when running `cap deploy` - to trigger after individual tasks use `set :deploying, true` This closes #708 and replaces https://github.com/capistrano/capistrano/pull/720 --- features/deploy_failure.feature | 17 +++++++++++++++++ features/step_definitions/assertions.rb | 15 +++++++++++++++ features/step_definitions/setup.rb | 10 ++++++++++ features/support/remote_command_helpers.rb | 4 ++++ lib/capistrano/application.rb | 8 ++++++++ lib/capistrano/dsl/task_enhancements.rb | 10 ++++++++++ lib/capistrano/i18n.rb | 1 + lib/capistrano/tasks/deploy.rake | 2 ++ lib/capistrano/tasks/framework.rake | 1 + spec/support/tasks/fail.cap | 7 +++++++ spec/support/tasks/failed.cap | 5 +++++ 11 files changed, 80 insertions(+) create mode 100644 features/deploy_failure.feature create mode 100644 spec/support/tasks/fail.cap create mode 100644 spec/support/tasks/failed.cap diff --git a/features/deploy_failure.feature b/features/deploy_failure.feature new file mode 100644 index 00000000..5c754399 --- /dev/null +++ b/features/deploy_failure.feature @@ -0,0 +1,17 @@ +Feature: Deploy failure + + Background: + Given a test app with the default configuration + And a custom task that will simulate a failure + And a custom task to run in the event of a failure + And servers with the roles app and web + + Scenario: Triggering the custom task + When I run cap "deploy:starting" + But an error is raised + Then the failure task will not run + + Scenario: Triggering the custom task + When I run cap "deploy" + But an error is raised + Then the failure task will run diff --git a/features/step_definitions/assertions.rb b/features/step_definitions/assertions.rb index 8b24040e..ddc34db8 100644 --- a/features/step_definitions/assertions.rb +++ b/features/step_definitions/assertions.rb @@ -92,3 +92,18 @@ end Then(/^the task is successful$/) do expect(@success).to be_true end + +Then(/^the failure task will run$/) do + failed = TestApp.shared_path.join('failed') + run_vagrant_command(test_file_exists(failed)) +end + +Then(/^the failure task will not run$/) do + failed = TestApp.shared_path.join('failed') + !run_vagrant_command(test_file_exists(failed)) +end + +When(/^an error is raised$/) do + error = TestApp.shared_path.join('fail') + run_vagrant_command(test_file_exists(error)) +end diff --git a/features/step_definitions/setup.rb b/features/step_definitions/setup.rb index 7379deb4..16fabc2e 100644 --- a/features/step_definitions/setup.rb +++ b/features/step_definitions/setup.rb @@ -26,3 +26,13 @@ end Given(/^the configuration is in a custom location$/) do TestApp.move_configuration_to_custom_location('app') end + +Given(/^a custom task that will simulate a failure$/) do + safely_remove_file(TestApp.shared_path.join('failed')) + TestApp.copy_task_to_test_app('spec/support/tasks/fail.cap') +end + +Given(/^a custom task to run in the event of a failure$/) do + safely_remove_file(TestApp.shared_path.join('failed')) + TestApp.copy_task_to_test_app('spec/support/tasks/failed.cap') +end diff --git a/features/support/remote_command_helpers.rb b/features/support/remote_command_helpers.rb index 47b56ef8..7c045608 100644 --- a/features/support/remote_command_helpers.rb +++ b/features/support/remote_command_helpers.rb @@ -15,6 +15,10 @@ module RemoteCommandHelpers def exists?(type, path) %{[ -#{type} "#{path}" ] && echo "#{path} exists." || echo "Error: #{path} does not exist."} end + + def safely_remove_file(path) + run_vagrant_command("rm #{test_file}") rescue Vagrant::Errors::VagrantError + end end World(RemoteCommandHelpers) diff --git a/lib/capistrano/application.rb b/lib/capistrano/application.rb index 4f2a67bb..bf91f709 100644 --- a/lib/capistrano/application.rb +++ b/lib/capistrano/application.rb @@ -30,6 +30,14 @@ module Capistrano end end + def exit_because_of_exception(ex) + if deploying? + exit_deploy_because_of_exception(ex) + else + super + end + end + private # allows the `cap install` task to load without a capfile diff --git a/lib/capistrano/dsl/task_enhancements.rb b/lib/capistrano/dsl/task_enhancements.rb index 29090652..84f0a879 100644 --- a/lib/capistrano/dsl/task_enhancements.rb +++ b/lib/capistrano/dsl/task_enhancements.rb @@ -49,5 +49,15 @@ module Capistrano %w{install} end + def exit_deploy_because_of_exception(ex) + warn t(:deploy_failed, ex: ex.inspect) + invoke 'deploy:failed' + exit(false) + end + + def deploying? + fetch(:deploying, false) + end + end end diff --git a/lib/capistrano/i18n.rb b/lib/capistrano/i18n.rb index e5dec4c5..cb69e0df 100644 --- a/lib/capistrano/i18n.rb +++ b/lib/capistrano/i18n.rb @@ -18,6 +18,7 @@ en = { mirror_exists: "The repository mirror is at %{at}", revision_log_message: 'Branch %{branch} deployed as release %{release} by %{user}', rollback_log_message: '%{user} rolled back to release %{release}', + deploy_failed: 'The deploy has failed with an error: %{ex}', console: { welcome: 'capistrano console - enter command to execute on %{stage}', bye: 'bye' diff --git a/lib/capistrano/tasks/deploy.rake b/lib/capistrano/tasks/deploy.rake index 8312955e..25429bd6 100644 --- a/lib/capistrano/tasks/deploy.rake +++ b/lib/capistrano/tasks/deploy.rake @@ -200,4 +200,6 @@ namespace :deploy do end end + task :failed + end diff --git a/lib/capistrano/tasks/framework.rake b/lib/capistrano/tasks/framework.rake index 6ff02eef..e2d94052 100644 --- a/lib/capistrano/tasks/framework.rake +++ b/lib/capistrano/tasks/framework.rake @@ -57,6 +57,7 @@ end desc 'Deploy a new release.' task :deploy do + set(:deploying, true) %w{ starting started updating updated publishing published diff --git a/spec/support/tasks/fail.cap b/spec/support/tasks/fail.cap new file mode 100644 index 00000000..aec54a79 --- /dev/null +++ b/spec/support/tasks/fail.cap @@ -0,0 +1,7 @@ +set :fail, proc { fail } +before 'deploy:starting', :fail do + on roles :all do + execute :touch, shared_path.join('fail') + end + fetch(:fail) +end diff --git a/spec/support/tasks/failed.cap b/spec/support/tasks/failed.cap new file mode 100644 index 00000000..d1f6b58c --- /dev/null +++ b/spec/support/tasks/failed.cap @@ -0,0 +1,5 @@ +after 'deploy:failed', :failed do + on roles :all do + execute :touch, shared_path.join('failed') + end +end From 79ab495391d5ce1278b7d2ecc2325000e5aa3003 Mon Sep 17 00:00:00 2001 From: Kir Shatrov Date: Mon, 28 Oct 2013 16:37:46 +0400 Subject: [PATCH 10/11] Changelog entries for 3.1.x release --- CHANGELOG.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 81b7eaea..9da481ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,17 @@ Reverse Chronological Order: +## `3.1.0` (not released) +* + `deploy:restart` task is nomore required and run by default. + From this version, developers who restart the app on each deploy need to declare it in their deploy flow (@kirs) +* Fixed bug when `deploy:cleanup` was executed twice by default (@kirs) +* Config location can now be changed with `deploy_config_path` and `stage_config_path` options (@seenmyfate) +* `no_release` option is now available (@seenmyfate) +* Raise an error if developer tries to define `:all` role, which is reserved (@kirs) +* `deploy:fallback` hook was added to add some custom behaviour on failed deploy (@seenmyfate) + + ## `3.0.0` If you are coming here to wonder why your Capfile doesn't work anymore, please From a58bb3bca2b9f15b79eb9ca17791dff50a0c7413 Mon Sep 17 00:00:00 2001 From: seenmyfate Date: Fri, 1 Nov 2013 15:28:39 +0000 Subject: [PATCH 11/11] Correctly infer namespace in task enhancements Previously, defining a task using the `after` syntax within a namespace block ignored the namespace, now the following examples are equivalent: # within namespace namespace :deploy do task :my_task do # end after :starting, :my_task end # outside of namespace after 'deploy:starting', 'deploy:my_task' This resolves #541 --- lib/capistrano/dsl/task_enhancements.rb | 5 +++-- lib/capistrano/tasks/deploy.rake | 1 + lib/capistrano/templates/deploy.rb.erb | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/capistrano/dsl/task_enhancements.rb b/lib/capistrano/dsl/task_enhancements.rb index 84f0a879..c35caaba 100644 --- a/lib/capistrano/dsl/task_enhancements.rb +++ b/lib/capistrano/dsl/task_enhancements.rb @@ -6,9 +6,10 @@ module Capistrano end def after(task, post_task, *args, &block) - post_task = Rake::Task.define_task(post_task, *args, &block) if block_given? + Rake::Task.define_task(post_task, *args, &block) if block_given? + post_task = Rake::Task[post_task] Rake::Task[task].enhance do - invoke(post_task) + post_task.invoke end end diff --git a/lib/capistrano/tasks/deploy.rake b/lib/capistrano/tasks/deploy.rake index 25429bd6..eb1d8135 100644 --- a/lib/capistrano/tasks/deploy.rake +++ b/lib/capistrano/tasks/deploy.rake @@ -200,6 +200,7 @@ namespace :deploy do end end + task :restart task :failed end diff --git a/lib/capistrano/templates/deploy.rb.erb b/lib/capistrano/templates/deploy.rb.erb index 6f697d58..5f3e0455 100644 --- a/lib/capistrano/templates/deploy.rb.erb +++ b/lib/capistrano/templates/deploy.rb.erb @@ -17,7 +17,6 @@ set :repo_url, 'git@example.com:me/my_repo.git' # set :keep_releases, 5 namespace :deploy do - after :publishing, :restart desc 'Restart application' task :restart do @@ -27,6 +26,8 @@ namespace :deploy do end end + after :publishing, :restart + after :restart, :clear_cache do on roles(:web), in: :groups, limit: 3, wait: 10 do # Here we can do anything such as: