From 8b2df268dc896c40336c949649f07a306d374b6d Mon Sep 17 00:00:00 2001
From: David Fernandez <dfernandez@gitlab.com>
Date: Tue, 26 Apr 2022 15:32:20 +0200
Subject: [PATCH 1/3] Add the packages cleanup policy model

The policy hosts the next_run_at timestamp and the first rule:
keep_n_duplicated_package_files.

Changelog: added
---
 app/models/packages/cleanup.rb                |  8 +++++++
 app/models/packages/cleanup/policy.rb         | 24 +++++++++++++++++++
 app/models/project.rb                         |  5 ++++
 db/docs/packages_cleanup_policies.yml         |  9 +++++++
 ...120604_create_packages_cleanup_policies.rb | 22 +++++++++++++++++
 db/schema_migrations/20220425120604           |  1 +
 db/structure.sql                              | 14 +++++++++++
 lib/gitlab/database/gitlab_schemas.yml        |  1 +
 spec/factories/packages/cleanup/policies.rb   | 16 +++++++++++++
 spec/lib/gitlab/import_export/all_models.yml  |  1 +
 spec/models/concerns/schedulable_spec.rb      | 10 ++++++++
 spec/models/packages/cleanup/policy_spec.rb   | 13 ++++++++++
 spec/models/project_spec.rb                   |  1 +
 13 files changed, 125 insertions(+)
 create mode 100644 app/models/packages/cleanup.rb
 create mode 100644 app/models/packages/cleanup/policy.rb
 create mode 100644 db/docs/packages_cleanup_policies.yml
 create mode 100644 db/migrate/20220425120604_create_packages_cleanup_policies.rb
 create mode 100644 db/schema_migrations/20220425120604
 create mode 100644 spec/factories/packages/cleanup/policies.rb
 create mode 100644 spec/models/packages/cleanup/policy_spec.rb

diff --git a/app/models/packages/cleanup.rb b/app/models/packages/cleanup.rb
new file mode 100644
index 00000000000000..16bba4f445d9e0
--- /dev/null
+++ b/app/models/packages/cleanup.rb
@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+module Packages
+  module Cleanup
+    def self.table_name_prefix
+      'packages_cleanup_'
+    end
+  end
+end
diff --git a/app/models/packages/cleanup/policy.rb b/app/models/packages/cleanup/policy.rb
new file mode 100644
index 00000000000000..a5760878470440
--- /dev/null
+++ b/app/models/packages/cleanup/policy.rb
@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+
+module Packages
+  module Cleanup
+    class Policy < ApplicationRecord
+      include Schedulable
+
+      self.primary_key = :project_id
+
+      belongs_to :project
+
+      validates :project, presence: true
+
+      enum keep_n_duplicated_package_files: { all: 0, '10': 1, '20': 2, '30': 3, '40': 4, '50': 5 }, _prefix: true
+
+      scope :runnable_schedules, -> { where("next_run_at < ?", Time.zone.now) }
+
+      def set_next_run_at
+        # fixed cadence of 12 hours
+        self.next_run_at = Time.zone.now + 12.hours
+      end
+    end
+  end
+end
diff --git a/app/models/project.rb b/app/models/project.rb
index 84db2b343e620b..568e06fa8d6c51 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -237,6 +237,7 @@ def self.integration_association_name(name)
   has_many :package_files, through: :packages, class_name: 'Packages::PackageFile'
   # debian_distributions and associated component_files must be destroyed by ruby code in order to properly remove carrierwave uploads
   has_many :debian_distributions, class_name: 'Packages::Debian::ProjectDistribution', dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
+  has_one :packages_cleanup_policy, class_name: 'Packages::Cleanup::Policy', inverse_of: :project
 
   has_one :import_state, autosave: true, class_name: 'ProjectImportState', inverse_of: :project
   has_one :import_export_upload, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
@@ -1013,6 +1014,10 @@ def has_packages?(package_type)
     packages.where(package_type: package_type).exists?
   end
 
+  def packages_cleanup_policy
+    super || build_packages_cleanup_policy
+  end
+
   def first_auto_devops_config
     return namespace.first_auto_devops_config if auto_devops&.enabled.nil?
 
diff --git a/db/docs/packages_cleanup_policies.yml b/db/docs/packages_cleanup_policies.yml
new file mode 100644
index 00000000000000..1221c7952a03d7
--- /dev/null
+++ b/db/docs/packages_cleanup_policies.yml
@@ -0,0 +1,9 @@
+---
+table_name: packages_cleanup_policies
+classes:
+- Packages::Cleanup::Policy
+feature_categories:
+- package_registry
+description: Cleanup policy parameters for packages.
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/85918
+milestone: '15.0'
diff --git a/db/migrate/20220425120604_create_packages_cleanup_policies.rb b/db/migrate/20220425120604_create_packages_cleanup_policies.rb
new file mode 100644
index 00000000000000..c30db98993396e
--- /dev/null
+++ b/db/migrate/20220425120604_create_packages_cleanup_policies.rb
@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+class CreatePackagesCleanupPolicies < Gitlab::Database::Migration[2.0]
+  enable_lock_retries!
+
+  def up
+    create_table :packages_cleanup_policies, id: false do |t|
+      t.timestamps_with_timezone null: false
+      t.references :project,
+                   primary_key: true,
+                   default: nil,
+                   index: false,
+                   foreign_key: { to_table: :projects, on_delete: :cascade }
+      t.datetime_with_timezone :next_run_at, null: true
+      t.integer :keep_n_duplicated_package_files, default: 0, null: false, limit: 2
+    end
+  end
+
+  def down
+    drop_table :packages_cleanup_policies
+  end
+end
diff --git a/db/schema_migrations/20220425120604 b/db/schema_migrations/20220425120604
new file mode 100644
index 00000000000000..dd68ec935d8cd9
--- /dev/null
+++ b/db/schema_migrations/20220425120604
@@ -0,0 +1 @@
+56ebfb7a97217af23f12db7f93c47104be30da7633a22caf9e74547d5a27d29b
\ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index 74ffa603508870..dff34fb957a4e2 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -17888,6 +17888,14 @@ CREATE SEQUENCE packages_build_infos_id_seq
 
 ALTER SEQUENCE packages_build_infos_id_seq OWNED BY packages_build_infos.id;
 
+CREATE TABLE packages_cleanup_policies (
+    created_at timestamp with time zone NOT NULL,
+    updated_at timestamp with time zone NOT NULL,
+    project_id bigint NOT NULL,
+    next_run_at timestamp with time zone,
+    keep_n_duplicated_package_files smallint DEFAULT 0 NOT NULL
+);
+
 CREATE TABLE packages_composer_cache_files (
     id bigint NOT NULL,
     created_at timestamp with time zone NOT NULL,
@@ -24950,6 +24958,9 @@ ALTER TABLE ONLY operations_user_lists
 ALTER TABLE ONLY packages_build_infos
     ADD CONSTRAINT packages_build_infos_pkey PRIMARY KEY (id);
 
+ALTER TABLE ONLY packages_cleanup_policies
+    ADD CONSTRAINT packages_cleanup_policies_pkey PRIMARY KEY (project_id);
+
 ALTER TABLE ONLY packages_composer_cache_files
     ADD CONSTRAINT packages_composer_cache_files_pkey PRIMARY KEY (id);
 
@@ -32346,6 +32357,9 @@ ALTER TABLE ONLY namespace_settings
 ALTER TABLE ONLY self_managed_prometheus_alert_events
     ADD CONSTRAINT fk_rails_3936dadc62 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY packages_cleanup_policies
+    ADD CONSTRAINT fk_rails_393ba98591 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY approval_project_rules_groups
     ADD CONSTRAINT fk_rails_396841e79e FOREIGN KEY (group_id) REFERENCES namespaces(id) ON DELETE CASCADE;
 
diff --git a/lib/gitlab/database/gitlab_schemas.yml b/lib/gitlab/database/gitlab_schemas.yml
index b1869574a2cfc6..f769201464fc76 100644
--- a/lib/gitlab/database/gitlab_schemas.yml
+++ b/lib/gitlab/database/gitlab_schemas.yml
@@ -348,6 +348,7 @@ operations_strategies: :gitlab_main
 operations_strategies_user_lists: :gitlab_main
 operations_user_lists: :gitlab_main
 packages_build_infos: :gitlab_main
+packages_cleanup_policies: :gitlab_main
 packages_composer_cache_files: :gitlab_main
 packages_composer_metadata: :gitlab_main
 packages_conan_file_metadata: :gitlab_main
diff --git a/spec/factories/packages/cleanup/policies.rb b/spec/factories/packages/cleanup/policies.rb
new file mode 100644
index 00000000000000..2c6f1578037daf
--- /dev/null
+++ b/spec/factories/packages/cleanup/policies.rb
@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :packages_cleanup_policy, class: 'Packages::Cleanup::Policy' do
+    project
+
+    keep_n_duplicated_package_files { :'10' }
+
+    trait :runnable do
+      after(:create) do |policy|
+        # next_run_at will be set before_save to Time.now + cadence, so this ensures the policy is active
+        policy.update_column(:next_run_at, Time.zone.now - 1.day)
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml
index 730f9035293df8..1546b6a26c86ae 100644
--- a/spec/lib/gitlab/import_export/all_models.yml
+++ b/spec/lib/gitlab/import_export/all_models.yml
@@ -550,6 +550,7 @@ project:
 - project_registry
 - packages
 - package_files
+- packages_cleanup_policy
 - tracing_setting
 - alerting_setting
 - project_setting
diff --git a/spec/models/concerns/schedulable_spec.rb b/spec/models/concerns/schedulable_spec.rb
index 62acd12e267770..b98dcf1c1747a4 100644
--- a/spec/models/concerns/schedulable_spec.rb
+++ b/spec/models/concerns/schedulable_spec.rb
@@ -57,6 +57,16 @@
     it_behaves_like '.runnable_schedules'
   end
 
+  context 'for a packages cleanup policy' do
+    # let! is used to reset the next_run_at value before each spec
+    let(:object) { create(:packages_cleanup_policy, :runnable) }
+    let(:non_runnable_object) { create(:packages_cleanup_policy) }
+
+    it_behaves_like '#schedule_next_run!'
+    it_behaves_like 'before_save callback'
+    it_behaves_like '.runnable_schedules'
+  end
+
   describe '#next_run_at' do
     let(:schedulable_instance) do
       Class.new(ActiveRecord::Base) do
diff --git a/spec/models/packages/cleanup/policy_spec.rb b/spec/models/packages/cleanup/policy_spec.rb
new file mode 100644
index 00000000000000..7a7c606273b2a4
--- /dev/null
+++ b/spec/models/packages/cleanup/policy_spec.rb
@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Packages::Cleanup::Policy, type: :model do
+  describe 'relationships' do
+    it { is_expected.to belong_to(:project) }
+  end
+
+  describe 'validations' do
+    it { is_expected.to validate_presence_of(:project) }
+  end
+end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 0bb584845c2a85..6066822f4f1a72 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -135,6 +135,7 @@
     it { is_expected.to have_many(:packages).class_name('Packages::Package') }
     it { is_expected.to have_many(:package_files).class_name('Packages::PackageFile') }
     it { is_expected.to have_many(:debian_distributions).class_name('Packages::Debian::ProjectDistribution').dependent(:destroy) }
+    it { is_expected.to have_one(:packages_cleanup_policy).class_name('Packages::Cleanup::Policy').inverse_of(:project) }
     it { is_expected.to have_many(:pipeline_artifacts).dependent(:restrict_with_error) }
     it { is_expected.to have_many(:terraform_states).class_name('Terraform::State').inverse_of(:project) }
     it { is_expected.to have_many(:timelogs) }
-- 
GitLab


From 44db3426f58eda8cd5a531854ef0a4431700b618 Mon Sep 17 00:00:00 2001
From: David Fernandez <dfernandez@gitlab.com>
Date: Mon, 2 May 2022 16:24:01 +0200
Subject: [PATCH 2/3] Switch to a text column

---
 app/models/packages/cleanup/policy.rb             | 14 +++++++++++---
 ...0425120604_create_packages_cleanup_policies.rb |  2 +-
 db/structure.sql                                  |  3 ++-
 spec/factories/packages/cleanup/policies.rb       |  2 +-
 spec/models/packages/cleanup/policy_spec.rb       | 15 +++++++++++++++
 5 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/app/models/packages/cleanup/policy.rb b/app/models/packages/cleanup/policy.rb
index a5760878470440..6b6aa6bad248d3 100644
--- a/app/models/packages/cleanup/policy.rb
+++ b/app/models/packages/cleanup/policy.rb
@@ -5,15 +5,23 @@ module Cleanup
     class Policy < ApplicationRecord
       include Schedulable
 
+      KEEP_N_DUPLICATED_PACKAGE_FILES_VALUES = %w[all 10 20 30 40 50].freeze
+
       self.primary_key = :project_id
 
       belongs_to :project
 
       validates :project, presence: true
+      validates :keep_n_duplicated_package_files,
+                inclusion: {
+                  in: KEEP_N_DUPLICATED_PACKAGE_FILES_VALUES,
+                  message: 'keep_n_duplicated_package_files is invalid'
+                }
 
-      enum keep_n_duplicated_package_files: { all: 0, '10': 1, '20': 2, '30': 3, '40': 4, '50': 5 }, _prefix: true
-
-      scope :runnable_schedules, -> { where("next_run_at < ?", Time.zone.now) }
+      # used by Schedulable
+      def self.active
+        where.not(keep_n_duplicated_package_files: 'all')
+      end
 
       def set_next_run_at
         # fixed cadence of 12 hours
diff --git a/db/migrate/20220425120604_create_packages_cleanup_policies.rb b/db/migrate/20220425120604_create_packages_cleanup_policies.rb
index c30db98993396e..0b04457235e534 100644
--- a/db/migrate/20220425120604_create_packages_cleanup_policies.rb
+++ b/db/migrate/20220425120604_create_packages_cleanup_policies.rb
@@ -12,7 +12,7 @@ def up
                    index: false,
                    foreign_key: { to_table: :projects, on_delete: :cascade }
       t.datetime_with_timezone :next_run_at, null: true
-      t.integer :keep_n_duplicated_package_files, default: 0, null: false, limit: 2
+      t.text :keep_n_duplicated_package_files, default: 'all', null: false, limit: 255
     end
   end
 
diff --git a/db/structure.sql b/db/structure.sql
index dff34fb957a4e2..1991e0f67e2c6a 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -17893,7 +17893,8 @@ CREATE TABLE packages_cleanup_policies (
     updated_at timestamp with time zone NOT NULL,
     project_id bigint NOT NULL,
     next_run_at timestamp with time zone,
-    keep_n_duplicated_package_files smallint DEFAULT 0 NOT NULL
+    keep_n_duplicated_package_files text DEFAULT 'all'::text NOT NULL,
+    CONSTRAINT check_e53f35ab7b CHECK ((char_length(keep_n_duplicated_package_files) <= 255))
 );
 
 CREATE TABLE packages_composer_cache_files (
diff --git a/spec/factories/packages/cleanup/policies.rb b/spec/factories/packages/cleanup/policies.rb
index 2c6f1578037daf..80baa2f78bd7e6 100644
--- a/spec/factories/packages/cleanup/policies.rb
+++ b/spec/factories/packages/cleanup/policies.rb
@@ -4,7 +4,7 @@
   factory :packages_cleanup_policy, class: 'Packages::Cleanup::Policy' do
     project
 
-    keep_n_duplicated_package_files { :'10' }
+    keep_n_duplicated_package_files { '10' }
 
     trait :runnable do
       after(:create) do |policy|
diff --git a/spec/models/packages/cleanup/policy_spec.rb b/spec/models/packages/cleanup/policy_spec.rb
index 7a7c606273b2a4..972071aa0ad64b 100644
--- a/spec/models/packages/cleanup/policy_spec.rb
+++ b/spec/models/packages/cleanup/policy_spec.rb
@@ -9,5 +9,20 @@
 
   describe 'validations' do
     it { is_expected.to validate_presence_of(:project) }
+    it do
+      is_expected
+        .to validate_inclusion_of(:keep_n_duplicated_package_files)
+              .in_array(described_class::KEEP_N_DUPLICATED_PACKAGE_FILES_VALUES)
+              .with_message('keep_n_duplicated_package_files is invalid')
+    end
+  end
+
+  describe '.active' do
+    let_it_be(:active_policy) { create(:packages_cleanup_policy) }
+    let_it_be(:inactive_policy) { create(:packages_cleanup_policy, keep_n_duplicated_package_files: 'all') }
+
+    subject { described_class.active }
+
+    it { is_expected.to contain_exactly(active_policy) }
   end
 end
-- 
GitLab


From 978becba437071c3083c13ed54431ce757cf02cb Mon Sep 17 00:00:00 2001
From: David Fernandez <dfernandez@gitlab.com>
Date: Wed, 4 May 2022 10:25:32 +0200
Subject: [PATCH 3/3] Add 1 to the possible values

---
 app/models/packages/cleanup/policy.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/packages/cleanup/policy.rb b/app/models/packages/cleanup/policy.rb
index 6b6aa6bad248d3..87c101cfb8ce29 100644
--- a/app/models/packages/cleanup/policy.rb
+++ b/app/models/packages/cleanup/policy.rb
@@ -5,7 +5,7 @@ module Cleanup
     class Policy < ApplicationRecord
       include Schedulable
 
-      KEEP_N_DUPLICATED_PACKAGE_FILES_VALUES = %w[all 10 20 30 40 50].freeze
+      KEEP_N_DUPLICATED_PACKAGE_FILES_VALUES = %w[all 1 10 20 30 40 50].freeze
 
       self.primary_key = :project_id
 
-- 
GitLab