Rollup output for tasks in terminal states

PR Shopify#1133 changed how Tasks store their output. Previously, output chunks
were store to a table in the RDBMS of the host application. Now, the
application's Redis store houses these "output chunks". Periodically
shipit-engine will attempt to consolidate these "output chunks" into a
gzipped record of the "output" into the Task's table.

Our shipit-engine's Redis volume recently reached 100% capacity -
bringing CD service down. We observed that the majority of the volume
was consumed by un-"rolled up" task logs. We were able to manually
cleanup these orphaned output chunks and restore service.

The cron task only schedules ChunkRollupJobs for a subset of "finished"
tasks - `error`, `failed`, `aborted`, etc tasks will never "rollup"
their log output and eventually fill the Redis data store with task
output logs. When output chunks were stored in the RDBMS it was probably
OK that these records remained in the output_chunks table, but since
critical services of shipit-engine - namely Sidekiq - rely on a
functioning Redis instance - leaving these records in Redis seems harmful.

This changes the "status" check of the `due_for_rollup` scope to include
all terminal statuses.

References
----------

- Shopify#1133

app/models/shipit/task.rb

@@ -39,7 +39,7 @@ class Task < Record
     scope :last_seven_days, -> { where("created_at > ?", 7.days.ago) }
     scope :previous_seven_days, -> { where(created_at: 14.days.ago..7.days.ago) }
 
-    scope :due_for_rollup, -> { completed.where(rolled_up: false).where('created_at <= ?', 1.hour.ago) }
+    scope :due_for_rollup, -> { not_active.where(rolled_up: false).where('created_at <= ?', 1.hour.ago) }
 
     after_save :record_status_change
     after_create :prevent_concurrency, unless: :allow_concurrency?

test/models/tasks_test.rb

@@ -88,5 +88,27 @@ class TasksTest < ActiveSupport::TestCase
       task_with_zero_retries = shipit_tasks(:shipit_restart)
       refute_predicate task_with_zero_retries, :retries_configured?
     end
+
+    test ".due_for_rollup includes tasks in successful terminal states" do
+      task = shipit_tasks(:shipit)
+      task.update(
+        rolled_up: false,
+        created_at: (60 + 1).minutes.ago.to_s(:db),
+        status: "success",
+      )
+
+      assert_includes Shipit::Task.due_for_rollup, task
+    end
+
+    test ".due_for_rollup includes tasks in unsuccessful terminal states" do
+      task = shipit_tasks(:shipit)
+      task.update(
+        rolled_up: false,
+        created_at: (60 + 1).minutes.ago.to_s(:db),
+        status: "error",
+      )
+
+      assert_includes Shipit::Task.due_for_rollup, task
+    end
   end
 end