From 041ca8e74f392d81541b0e25f00297d40c9ae0c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20G=C3=B3rniak?= Date: Fri, 10 Jan 2025 00:51:20 +0100 Subject: [PATCH 1/6] Define a new "cleanup_safe_time_window" option with a default value --- lib/deploy_pin.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/deploy_pin.rb b/lib/deploy_pin.rb index ded978b..2927bdf 100644 --- a/lib/deploy_pin.rb +++ b/lib/deploy_pin.rb @@ -15,6 +15,7 @@ module DeployPin OPTIONS = %i[ + cleanup_safe_time_window deployment_state_transition fallback_group groups @@ -26,6 +27,7 @@ module DeployPin ].freeze DEFAULTS = { + cleanup_safe_time_window: -> { 1.year }, task_wrapper: ->(_task, task_runner) { task_runner.call } }.freeze From 5524982deb5c202ef74d21072c758eb832057d2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20G=C3=B3rniak?= Date: Fri, 10 Jan 2025 00:54:18 +0100 Subject: [PATCH 2/6] Define simple code for the new cleanup task --- lib/deploy_pin/collector.rb | 4 ++++ lib/deploy_pin/runner.rb | 4 ++++ lib/deploy_pin/task.rb | 11 +++++++++++ lib/tasks/deploy_pin_tasks.rake | 7 +++++++ test/lib/deploy_pin/collector_test.rb | 18 ++++++++++++++++-- test/lib/deploy_pin/runner_test.rb | 6 ++++++ test/lib/deploy_pin/task_test.rb | 21 +++++++++++++++++++++ 7 files changed, 69 insertions(+), 2 deletions(-) diff --git a/lib/deploy_pin/collector.rb b/lib/deploy_pin/collector.rb index 7b842ca..0a70ae0 100644 --- a/lib/deploy_pin/collector.rb +++ b/lib/deploy_pin/collector.rb @@ -26,6 +26,10 @@ def list end end + def cleanup + init_tasks.select(&:classified_for_cleanup?).each(&:remove) + end + def executable # cache tasks tasks = init_tasks diff --git a/lib/deploy_pin/runner.rb b/lib/deploy_pin/runner.rb index 6e2f6f6..9372429 100644 --- a/lib/deploy_pin/runner.rb +++ b/lib/deploy_pin/runner.rb @@ -11,6 +11,10 @@ def self.list(identifiers:) DeployPin::Collector.new(identifiers:).list end + def self.cleanup(identifiers:) + DeployPin::Collector.new(identifiers:).cleanup + end + def self.summary(identifiers:) # print summary self.print('======= Summary ========') diff --git a/lib/deploy_pin/task.rb b/lib/deploy_pin/task.rb index 3009685..45f65e4 100644 --- a/lib/deploy_pin/task.rb +++ b/lib/deploy_pin/task.rb @@ -31,6 +31,11 @@ def run eval(script) end + def remove + File.delete(file) if File.exist?(file) + record&.destroy + end + def record DeployPin::Record.find_by(uuid: identifier) end @@ -71,6 +76,12 @@ def under_timeout? !explicit_timeout? && !parallel? end + def classified_for_cleanup? + return false unless done? + + record.completed_at < DeployPin.cleanup_safe_time_window.call.ago + end + def parse each_line do |line| case line.strip diff --git a/lib/tasks/deploy_pin_tasks.rake b/lib/tasks/deploy_pin_tasks.rake index 6167c91..d2985d3 100644 --- a/lib/tasks/deploy_pin_tasks.rake +++ b/lib/tasks/deploy_pin_tasks.rake @@ -15,4 +15,11 @@ namespace :deploy_pin do DeployPin::Runner.list(**args) DeployPin::Runner.summary(**args) end + + desc 'Remove tasks codebase and DB records for a defined time window' + task cleanup: :environment do + args.with_defaults(identifiers: DeployPin.groups) + + DeployPin::Runner.cleanup(**args) + end end diff --git a/test/lib/deploy_pin/collector_test.rb b/test/lib/deploy_pin/collector_test.rb index 6f25c04..a2d98c5 100644 --- a/test/lib/deploy_pin/collector_test.rb +++ b/test/lib/deploy_pin/collector_test.rb @@ -10,8 +10,8 @@ class DeployPin::Collector::Test < ActiveSupport::TestCase ::FileUtils.cp 'test/support/files/task_same.rb', "#{DeployPin.tasks_path}3_task.rb" ::FileUtils.cp 'test/support/files/other_task.rb', "#{DeployPin.tasks_path}4_task.rb" - # create one completed record - DeployPin::Record.create(uuid: '75371573753754', completed_at: Time.current) + @completed_record = + DeployPin::Record.create(uuid: '75371573753754', completed_at: Time.current) @collector = DeployPin::Collector.new(identifiers: DeployPin.groups) @ids_collector = DeployPin::Collector.new(identifiers: ['75371573753753', '75371573753754!']) @@ -55,6 +55,20 @@ class DeployPin::Collector::Test < ActiveSupport::TestCase end end + test 'cleanup' do + assert_no_difference 'DeployPin::Record.count' do + @collector.cleanup + end + + assert File.exist?("#{DeployPin.tasks_path}4_task.rb") + + assert_difference 'DeployPin::Record.count', -1 do + @collector.cleanup + end + + refute File.exist?("#{DeployPin.tasks_path}4_task.rb") + end + test 'custom task wrapper' do DeployPin.setup do task_wrapper( diff --git a/test/lib/deploy_pin/runner_test.rb b/test/lib/deploy_pin/runner_test.rb index 6a9bd2a..f2d7fd2 100644 --- a/test/lib/deploy_pin/runner_test.rb +++ b/test/lib/deploy_pin/runner_test.rb @@ -33,4 +33,10 @@ class DeployPin::Runner::Test < ActiveSupport::TestCase DeployPin::Runner.list(identifiers: [DeployPin.fallback_group]) end end + + test 'cleanup' do + assert_nothing_raised do + DeployPin::Runner.cleanup(identifiers: DeployPin.groups) + end + end end diff --git a/test/lib/deploy_pin/task_test.rb b/test/lib/deploy_pin/task_test.rb index 8c2aec7..79bc402 100644 --- a/test/lib/deploy_pin/task_test.rb +++ b/test/lib/deploy_pin/task_test.rb @@ -48,6 +48,16 @@ class DeployPin::Task::Test < ActiveSupport::TestCase assert_equal @task.progress, 1 end + test 'remove' do + @task.prepare + + assert_difference 'DeployPin::Record.count', -1 do + assert_nothing_raised { @task.remove } + end + + refute File.exist?(@task_file) + end + test 'parse' do assert_nothing_raised { @task.parse } end @@ -84,6 +94,17 @@ class DeployPin::Task::Test < ActiveSupport::TestCase assert_equal parallel_task.under_timeout?, false end + test 'classified_for_cleanup?' do + @task.prepare + refute @task.classified_for_cleanup? + + @task.mark + refute @task.classified_for_cleanup? + + @task.record.update(completed_at: (DeployPin.cleanup_safe_time_window.call + 1.day).ago) + assert @task.classified_for_cleanup? + end + test 'group_index' do assert_nothing_raised do @task.send(:group_index) From 3f5e89d5279dc28132c4965feaee8a528533f9fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20G=C3=B3rniak?= Date: Fri, 10 Jan 2025 00:55:26 +0100 Subject: [PATCH 3/6] Improve tests a bit by adding built-in simpler assertions wherever it's possible --- test/lib/deploy_pin/task_test.rb | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test/lib/deploy_pin/task_test.rb b/test/lib/deploy_pin/task_test.rb index 79bc402..4bb1ca0 100644 --- a/test/lib/deploy_pin/task_test.rb +++ b/test/lib/deploy_pin/task_test.rb @@ -12,9 +12,9 @@ class DeployPin::Task::Test < ActiveSupport::TestCase end test 'prepare' do - assert_equal DeployPin::Record.where(uuid: @task.identifier).count, 0 - assert_nothing_raised { @task.prepare } - assert_equal DeployPin::Record.where(uuid: @task.identifier).count, 1 + assert_difference 'DeployPin::Record.count', 1 do + assert_nothing_raised { @task.prepare } + end end test 'mark' do @@ -25,9 +25,9 @@ class DeployPin::Task::Test < ActiveSupport::TestCase test 'done?' do @task.prepare assert_nothing_raised { @task.done? } - assert_equal @task.done?, false + refute @task.done? @task.mark - assert_equal @task.done?, true + assert @task.done? end test 'increment_progress!' do @@ -74,24 +74,24 @@ class DeployPin::Task::Test < ActiveSupport::TestCase test 'explicit_timeout?' do @task.parse - assert_equal @task.send(:explicit_timeout?), false + refute @task.send(:explicit_timeout?) task_with_timeout = DeployPin::Task.new('test/support/files/task_with_timeout.rb') task_with_timeout.parse - assert_equal task_with_timeout.send(:explicit_timeout?), true + assert task_with_timeout.send(:explicit_timeout?) end test 'under_timeout?' do @task.parse - assert_equal @task.under_timeout?, true + assert @task.under_timeout? task_with_timeout = DeployPin::Task.new('test/support/files/task_with_timeout.rb') task_with_timeout.parse - assert_equal task_with_timeout.under_timeout?, false + refute task_with_timeout.under_timeout? parallel_task = DeployPin::Task.new('test/support/files/parallel_task.rb') parallel_task.parse - assert_equal parallel_task.under_timeout?, false + refute parallel_task.under_timeout? end test 'classified_for_cleanup?' do From 6600fdd693cf9a4f9fd1e66bfc5cf3b325f1b7bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20G=C3=B3rniak?= Date: Fri, 10 Jan 2025 00:56:04 +0100 Subject: [PATCH 4/6] Fix an issue with removed deploy_pin task file when tests are executed --- test/lib/deploy_pin/task_test.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/lib/deploy_pin/task_test.rb b/test/lib/deploy_pin/task_test.rb index 4bb1ca0..6b4392d 100644 --- a/test/lib/deploy_pin/task_test.rb +++ b/test/lib/deploy_pin/task_test.rb @@ -4,7 +4,9 @@ class DeployPin::Task::Test < ActiveSupport::TestCase setup do - @task = DeployPin::Task.new('test/support/files/task.rb') + @task_file = "#{DeployPin.tasks_path}1_task.rb" + ::FileUtils.cp 'test/support/files/task.rb', @task_file + @task = DeployPin::Task.new(@task_file) end test 'run' do @@ -68,7 +70,7 @@ class DeployPin::Task::Test < ActiveSupport::TestCase test 'eql?' do assert_nothing_raised do - @task.eql?(DeployPin::Task.new('test/support/files/task.rb')) + @task.eql?(DeployPin::Task.new(@task_file)) end end @@ -113,7 +115,7 @@ class DeployPin::Task::Test < ActiveSupport::TestCase test '<=>' do assert_nothing_raised do - @task.send(:<=>, DeployPin::Task.new('test/support/files/task.rb')) + @task.send(:<=>, DeployPin::Task.new(@task_file)) end end end From 6f1e831bc7076e06637e314a9054af65e2a262f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20G=C3=B3rniak?= Date: Fri, 10 Jan 2025 00:56:23 +0100 Subject: [PATCH 5/6] Fix typo in "summary" --- test/lib/deploy_pin/runner_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/lib/deploy_pin/runner_test.rb b/test/lib/deploy_pin/runner_test.rb index f2d7fd2..d739c91 100644 --- a/test/lib/deploy_pin/runner_test.rb +++ b/test/lib/deploy_pin/runner_test.rb @@ -10,7 +10,7 @@ class DeployPin::Runner::Test < ActiveSupport::TestCase ::FileUtils.cp 'test/support/files/task_same.rb', "#{DeployPin.tasks_path}3_task.rb" end - test 'sumary' do + test 'summary' do assert_nothing_raised do DeployPin::Runner.summary(identifiers: [DeployPin.fallback_group]) end From a64145007ed5821d4a935a4a666a0d46676c1ba3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20G=C3=B3rniak?= Date: Fri, 10 Jan 2025 00:56:43 +0100 Subject: [PATCH 6/6] Add missing description for existing rake task --- lib/tasks/deploy_pin_tasks.rake | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/tasks/deploy_pin_tasks.rake b/lib/tasks/deploy_pin_tasks.rake index d2985d3..45ad137 100644 --- a/lib/tasks/deploy_pin_tasks.rake +++ b/lib/tasks/deploy_pin_tasks.rake @@ -1,7 +1,7 @@ # frozen_string_literal: true namespace :deploy_pin do - desc 'run pending tasks' + desc 'Run pending tasks' task :run, [:identifiers] => :environment do |_t, args| identifiers = args.identifiers attributes = identifiers.nil? ? DeployPin.groups : identifiers.split(/\s*,\s*/) @@ -9,6 +9,7 @@ namespace :deploy_pin do DeployPin::Runner.run(identifiers: attributes) end + desc 'List defined tasks' task :list, [:identifiers] => :environment do |_t, args| args.with_defaults(identifiers: DeployPin.groups)