Merge branch...

Merge branch '330315-disable-cleanup-policies-linked-to-no-container-images-post-migration' into 'master'

Disable cleanup policies linked to projects with no container images

See merge request gitlab-org/gitlab!61983

db/post_migrate/20210518074332_schedule_disable_expiration_policies_linked_to_no_container_images.rb

+# frozen_string_literal: true
+
+class ScheduleDisableExpirationPoliciesLinkedToNoContainerImages < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  BATCH_SIZE = 30_000
+  DELAY = 2.minutes.freeze
+  DOWNTIME = false
+  MIGRATION = 'DisableExpirationPoliciesLinkedToNoContainerImages'
+
+  disable_ddl_transaction!
+
+  def up
+    queue_background_migration_jobs_by_range_at_intervals(
+      define_batchable_model('container_expiration_policies').where(enabled: true),
+      MIGRATION,
+      DELAY,
+      batch_size: BATCH_SIZE,
+      track_jobs: false,
+      primary_column_name: :project_id
+    )
+  end
+
+  def down
+    # this migration is irreversible
+
+    # we can't accuretaly know which policies were previously enabled during the background migration
+  end
+end

db/schema_migrations/20210518074332

+9eb5e68b0d79863687530ff22cbe6a2bffd2e2d31237e919134b9ce77810b1a0
\ No newline at end of file

lib/gitlab/background_migration/disable_expiration_policies_linked_to_no_container_images.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module BackgroundMigration
+    BATCH_SIZE = 1000
+
+    # This background migration disables container expiration policies connected
+    # to a project that has no container repositories
+    class DisableExpirationPoliciesLinkedToNoContainerImages
+      # rubocop: disable Style/Documentation
+      class ContainerExpirationPolicy < ActiveRecord::Base
+        include EachBatch
+
+        self.table_name = 'container_expiration_policies'
+      end
+      # rubocop: enable Style/Documentation
+
+      def perform(from_id, to_id)
+        ContainerExpirationPolicy.where(enabled: true, project_id: from_id..to_id).each_batch(of: BATCH_SIZE) do |batch|
+          sql = <<-SQL
+            WITH batched_relation AS MATERIALIZED (#{batch.select(:project_id).limit(BATCH_SIZE).to_sql})
+            UPDATE container_expiration_policies
+            SET enabled = FALSE
+            FROM batched_relation
+            WHERE container_expiration_policies.project_id = batched_relation.project_id
+            AND NOT EXISTS (SELECT 1 FROM "container_repositories" WHERE container_repositories.project_id = container_expiration_policies.project_id)
+          SQL
+          execute(sql)
+        end
+      end
+
+      private
+
+      def execute(sql)
+        ActiveRecord::Base
+          .connection
+          .execute(sql)
+      end
+    end
+  end
+end

spec/lib/gitlab/background_migration/disable_expiration_policies_linked_to_no_container_images_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::BackgroundMigration::DisableExpirationPoliciesLinkedToNoContainerImages do
+  let_it_be(:projects) { table(:projects) }
+  let_it_be(:container_expiration_policies) { table(:container_expiration_policies) }
+  let_it_be(:container_repositories) { table(:container_repositories) }
+  let_it_be(:namespaces) { table(:namespaces) }
+
+  let!(:namespace) { namespaces.create!(name: 'test', path: 'test') }
+
+  let!(:policy1) { create_expiration_policy(project_id: 1, enabled: true) }
+  let!(:policy2) { create_expiration_policy(project_id: 2, enabled: false) }
+  let!(:policy3) { create_expiration_policy(project_id: 3, enabled: false) }
+  let!(:policy4) { create_expiration_policy(project_id: 4, enabled: true, with_images: true) }
+  let!(:policy5) { create_expiration_policy(project_id: 5, enabled: false, with_images: true) }
+  let!(:policy6) { create_expiration_policy(project_id: 6, enabled: false) }
+  let!(:policy7) { create_expiration_policy(project_id: 7, enabled: true) }
+  let!(:policy8) { create_expiration_policy(project_id: 8, enabled: true, with_images: true) }
+  let!(:policy9) { create_expiration_policy(project_id: 9, enabled: true) }
+
+  describe '#perform' do
+    subject { described_class.new.perform(from_id, to_id) }
+
+    shared_examples 'disabling policies with no images' do
+      it 'disables the proper policies' do
+        subject
+
+        rows = container_expiration_policies.order(:project_id).to_h do |row|
+          [row.project_id, row.enabled]
+        end
+        expect(rows).to eq(expected_rows)
+      end
+    end
+
+    context 'the whole range' do
+      let(:from_id) { 1 }
+      let(:to_id) { 9 }
+
+      it_behaves_like 'disabling policies with no images' do
+        let(:expected_rows) do
+          {
+            1 => false,
+            2 => false,
+            3 => false,
+            4 => true,
+            5 => false,
+            6 => false,
+            7 => false,
+            8 => true,
+            9 => false
+          }
+        end
+      end
+    end
+
+    context 'a range with no policies to disable' do
+      let(:from_id) { 2 }
+      let(:to_id) { 6 }
+
+      it_behaves_like 'disabling policies with no images' do
+        let(:expected_rows) do
+          {
+            1 => true,
+            2 => false,
+            3 => false,
+            4 => true,
+            5 => false,
+            6 => false,
+            7 => true,
+            8 => true,
+            9 => true
+          }
+        end
+      end
+    end
+
+    context 'a range with only images' do
+      let(:from_id) { 4 }
+      let(:to_id) { 5 }
+
+      it_behaves_like 'disabling policies with no images' do
+        let(:expected_rows) do
+          {
+            1 => true,
+            2 => false,
+            3 => false,
+            4 => true,
+            5 => false,
+            6 => false,
+            7 => true,
+            8 => true,
+            9 => true
+          }
+        end
+      end
+    end
+
+    context 'a range with a single element' do
+      let(:from_id) { 9 }
+      let(:to_id) { 9 }
+
+      it_behaves_like 'disabling policies with no images' do
+        let(:expected_rows) do
+          {
+            1 => true,
+            2 => false,
+            3 => false,
+            4 => true,
+            5 => false,
+            6 => false,
+            7 => true,
+            8 => true,
+            9 => false
+          }
+        end
+      end
+    end
+  end
+
+  def create_expiration_policy(project_id:, enabled:, with_images: false)
+    projects.create!(id: project_id, namespace_id: namespace.id, name: "gitlab-#{project_id}")
+
+    if with_images
+      container_repositories.create!(project_id: project_id, name: "image-#{project_id}")
+    end
+
+    container_expiration_policies.create!(
+      enabled: enabled,
+      project_id: project_id
+    )
+  end
+
+  def enabled_policies
+    container_expiration_policies.where(enabled: true)
+  end
+
+  def disabled_policies
+    container_expiration_policies.where(enabled: false)
+  end
+end

spec/migrations/schedule_disable_expiration_policies_linked_to_no_container_images_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+require Rails.root.join('db', 'post_migrate', '20210518074332_schedule_disable_expiration_policies_linked_to_no_container_images.rb')
+
+RSpec.describe ScheduleDisableExpirationPoliciesLinkedToNoContainerImages do
+  let_it_be(:projects) { table(:projects) }
+  let_it_be(:container_expiration_policies) { table(:container_expiration_policies) }
+  let_it_be(:container_repositories) { table(:container_repositories) }
+  let_it_be(:namespaces) { table(:namespaces) }
+  let_it_be(:namespace) { namespaces.create!(name: 'test', path: 'test') }
+
+  let_it_be(:policy1) { create_expiration_policy(id: 1, enabled: true) }
+  let_it_be(:policy2) { create_expiration_policy(id: 2, enabled: false) }
+  let_it_be(:policy3) { create_expiration_policy(id: 3, enabled: false) }
+  let_it_be(:policy4) { create_expiration_policy(id: 4, enabled: true) }
+  let_it_be(:policy5) { create_expiration_policy(id: 5, enabled: false) }
+  let_it_be(:policy6) { create_expiration_policy(id: 6, enabled: false) }
+  let_it_be(:policy7) { create_expiration_policy(id: 7, enabled: true) }
+  let_it_be(:policy8) { create_expiration_policy(id: 8, enabled: true) }
+  let_it_be(:policy9) { create_expiration_policy(id: 9, enabled: true) }
+
+  it 'schedules background migrations', :aggregate_failures do
+    stub_const("#{described_class}::BATCH_SIZE", 2)
+
+    Sidekiq::Testing.fake! do
+      freeze_time do
+        migrate!
+
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(2.minutes, 1, 4)
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(4.minutes, 7, 8)
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(6.minutes, 9, 9)
+
+        expect(BackgroundMigrationWorker.jobs.size).to eq(3)
+      end
+    end
+  end
+
+  def create_expiration_policy(id:, enabled:)
+    project = projects.create!(id: id, namespace_id: namespace.id, name: "gitlab-#{id}")
+    container_expiration_policies.create!(
+      enabled: enabled,
+      project_id: project.id
+    )
+  end
+end