Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixes Bug #1810 removal of old releases #1811

Merged
merged 4 commits into from Dec 7, 2016
Merged

Fixes Bug #1810 removal of old releases #1811

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b2ab6f8
Fixes Bug #1810 removal of old releases
Dec 5, 2016
d04c1e3
Corrected style issue
Dec 5, 2016
3ee9536
Added Changelog entry
Dec 7, 2016
42a6ee1
Added your contribution here back to CHANGELOG
Dec 7, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Expand Up @@ -8,6 +8,8 @@ https://github.com/capistrano/capistrano/compare/v3.7.0.beta1...HEAD

* Your contribution here!

* Fix the removal of old releases `deploy:cleanup`. Logic is changed because of unreliable modification times on folders. Removal of directories is now decided by sorting on folder names (name is generated from current datetime format YmdHis). Cleanup is skipped, and a warning is given when a folder name is in a different format.

## `3.7.0.beta1` (2016-11-02)

https://github.com/capistrano/capistrano/compare/v3.6.1...v3.7.0.beta1
Expand Down
1 change: 1 addition & 0 deletions lib/capistrano/i18n.rb
View file
Open in desktop
Expand Up @@ -12,6 +12,7 @@
written_file: "create %{file}",
question: "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)",
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",
Expand Down
6 changes: 4 additions & 2 deletions lib/capistrano/tasks/deploy.rake
View file
Open in desktop
Expand Up @@ -148,8 +148,10 @@ namespace :deploy do
desc "Clean up old releases"
task :cleanup do
on release_roles :all do |host|
releases = capture(:ls, "-xtr", releases_path).split
if releases.count >= fetch(:keep_releases)
releases = capture(:ls, "-x", releases_path).split
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you have removed the -r flag, doesn't that mean the releases are no longer in reverse order? If so, the Ruby code for choosing which releases to delete needs to be flipped, right?

Copy link
Contributor Author

@markri markri Dec 6, 2016 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At first I didn't understand as well. But as it seems. ls -t sorts in another direction than expected (at least for me it is) see: http://man7.org/linux/man-pages/man1/ls.1.html

So newest items will be listed first, thats why the r is involved, just because the t flag was used.

In the new situation we're sorting on folder names in alphabetical order which results in the newest folders to be in the end of the array, but just based on title

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you're right. Makes sense.

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)))
if directories.any?
Expand Down