diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index 16ccf8ad78a278518eb1318f50f3376520fc1777..339938245a0c2466c2145b4830704c04794f9b4a 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -273,6 +273,7 @@ def visible_attributes :housekeeping_full_repack_period, :housekeeping_gc_period, :housekeeping_incremental_repack_period, + :housekeeping_optimize_repository_period, :html_emails_enabled, :import_sources, :in_product_marketing_emails_enabled, diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 4bc6d531beb356eb315cb2592082efb9ab81a7a5..59ad0650eb33e2213634cec136126e299ea79fd8 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -35,6 +35,7 @@ class ApplicationSetting < ApplicationRecord belongs_to :instance_group, class_name: "Group", foreign_key: 'instance_administrators_group_id' alias_attribute :instance_group_id, :instance_administrators_group_id alias_attribute :instance_administrators_group, :instance_group + alias_attribute :housekeeping_optimize_repository_period, :housekeeping_incremental_repack_period sanitizes! :default_branch_name @@ -256,18 +257,10 @@ def self.kroki_formats_attributes presence: { message: 'Domain denylist cannot be empty if denylist is enabled.' }, if: :domain_denylist_enabled? - validates :housekeeping_incremental_repack_period, + validates :housekeeping_optimize_repository_period, presence: true, numericality: { only_integer: true, greater_than: 0 } - validates :housekeeping_full_repack_period, - presence: true, - numericality: { only_integer: true, greater_than_or_equal_to: :housekeeping_incremental_repack_period } - - validates :housekeeping_gc_period, - presence: true, - numericality: { only_integer: true, greater_than_or_equal_to: :housekeeping_full_repack_period } - validates :terminal_max_session_time, presence: true, numericality: { only_integer: true, greater_than_or_equal_to: 0 } diff --git a/app/views/admin/application_settings/_repository_check.html.haml b/app/views/admin/application_settings/_repository_check.html.haml index 477ac5ca0d68968cbbf9843d9f717f91974467cb..b67cc29f296bf030531852d1adca3d4bf664ccdf 100644 --- a/app/views/admin/application_settings/_repository_check.html.haml +++ b/app/views/admin/application_settings/_repository_check.html.haml @@ -24,8 +24,8 @@ _("Enable automatic repository housekeeping"), help_text: '%{help_text} %{help_link}'.html_safe % { help_text: help_text, help_link: help_link } .form-group - = f.label :housekeeping_incremental_repack_period, _('Optimize repository period'), class: 'label-bold' - = f.number_field :housekeeping_incremental_repack_period, class: 'form-control gl-form-input' + = f.label :housekeeping_optimize_repository_period, _('Optimize repository period'), class: 'label-bold' + = f.number_field :housekeeping_optimize_repository_period, class: 'form-control gl-form-input' .form-text.text-muted = _('Number of Git pushes after which Gitaly is asked to optimize a repository.') .sub-section diff --git a/doc/api/settings.md b/doc/api/settings.md index 77f425fa13fe9d784324a637cc27317d271eba37..671db520c23e041406a0c6dc5d5fa1c741c28e82 100644 --- a/doc/api/settings.md +++ b/doc/api/settings.md @@ -378,10 +378,11 @@ listed in the descriptions of the relevant settings. | `hide_third_party_offers` | boolean | no | Do not display offers from third parties in GitLab. | | `home_page_url` | string | no | Redirect to this URL when not logged in. | | `housekeeping_bitmaps_enabled` | boolean | no | Git pack file bitmap creation is always enabled and cannot be changed via API and UI. This API field is deprecated and always returns `true`. | -| `housekeeping_enabled` | boolean | no | (**If enabled, requires:** `housekeeping_bitmaps_enabled`, `housekeeping_full_repack_period`, `housekeeping_gc_period`, and `housekeeping_incremental_repack_period`) Enable or disable Git housekeeping. | -| `housekeeping_full_repack_period` | integer | required by: `housekeeping_enabled` | Number of Git pushes after which an incremental `git repack` is run. | -| `housekeeping_gc_period` | integer | required by: `housekeeping_enabled` | Number of Git pushes after which `git gc` is run. | -| `housekeeping_incremental_repack_period` | integer | required by: `housekeeping_enabled` | Number of Git pushes after which an incremental `git repack` is run. | +| `housekeeping_enabled` | boolean | no | (**If enabled, requires either:** `housekeeping_bitmaps_enabled`, `housekeeping_full_repack_period`, `housekeeping_gc_period`, and `housekeeping_incremental_repack_period` **or** `housekeeping_optimize_repository_period`) Enable or disable Git housekeeping. | +| `housekeeping_full_repack_period` | integer | no | **Deprecated** (use `housekeeping_optimize_repository_period` instead): Number of Git pushes after which an incremental `git repack` is run. | +| `housekeeping_gc_period` | integer | no | **Deprecated** (use `housekeeping_optimize_repository_period` instead): Number of Git pushes after which `git gc` is run. | +| `housekeeping_incremental_repack_period` | integer | no | **Deprecated** (use `housekeeping_optimize_repository_period` instead): Number of Git pushes after which an incremental `git repack` is run. | +| `housekeeping_optimize_repository_period`| integer | no | Number of Git pushes after which an incremental `git repack` is run. | | `html_emails_enabled` | boolean | no | Enable HTML emails. | | `import_sources` | array of strings | no | Sources to allow project import from, possible values: `github`, `bitbucket`, `bitbucket_server`, `gitlab`, `fogbugz`, `git`, `gitlab_project`, `gitea`, `manifest`, and `phabricator`. | | `in_product_marketing_emails_enabled` | boolean | no | Enable [in-product marketing emails](../user/profile/notifications.md#global-notification-settings). Enabled by default. | diff --git a/lib/api/entities/application_setting.rb b/lib/api/entities/application_setting.rb index db51d4380d08e742ea807daac11ca03de19878af..8aace9126d645a4e22f9b62f8204f87feb96a47b 100644 --- a/lib/api/entities/application_setting.rb +++ b/lib/api/entities/application_setting.rb @@ -43,6 +43,11 @@ def self.exposed_attributes # This field is deprecated and always returns true expose(:housekeeping_bitmaps_enabled) { |_settings, _options| true } + + # These fields are deprecated and always returns housekeeping_optimize_repository_period value + expose(:housekeeping_full_repack_period) { |settings, _options| settings.housekeeping_optimize_repository_period } + expose(:housekeeping_gc_period) { |settings, _options| settings.housekeeping_optimize_repository_period } + expose(:housekeeping_incremental_repack_period) { |settings, _options| settings.housekeeping_optimize_repository_period } end end end diff --git a/lib/api/settings.rb b/lib/api/settings.rb index 96b900c420a822e7c216ef9c7bde9de577e81096..06b576a982bbe8c77823110c991c571129708b10 100644 --- a/lib/api/settings.rb +++ b/lib/api/settings.rb @@ -85,9 +85,15 @@ def filter_attributes_using_license(attrs) optional :home_page_url, type: String, desc: 'We will redirect non-logged in users to this page' optional :housekeeping_enabled, type: Boolean, desc: 'Enable automatic repository housekeeping (git repack, git gc)' given housekeeping_enabled: ->(val) { val } do - requires :housekeeping_full_repack_period, type: Integer, desc: "Number of Git pushes after which a full 'git repack' is run." - requires :housekeeping_gc_period, type: Integer, desc: "Number of Git pushes after which 'git gc' is run." - requires :housekeeping_incremental_repack_period, type: Integer, desc: "Number of Git pushes after which an incremental 'git repack' is run." + optional :housekeeping_full_repack_period, type: Integer, desc: "Number of Git pushes after which a full 'git repack' is run." + optional :housekeeping_gc_period, type: Integer, desc: "Number of Git pushes after which 'git gc' is run." + optional :housekeeping_incremental_repack_period, type: Integer, desc: "Number of Git pushes after which an incremental 'git repack' is run." + + optional :housekeeping_optimize_repository_period, type: Integer, desc: "Number of Git pushes after which Gitaly is asked to optimize a repository." + + # Requires either all three deprecated attributes (housekeeping_full_repack_period, housekeeping_gc_period, housekeeping_incremental_repack_period) or housekeeping_optimize_repository_period + all_or_none_of :housekeeping_full_repack_period, :housekeeping_gc_period, :housekeeping_incremental_repack_period + exactly_one_of :housekeeping_incremental_repack_period, :housekeeping_optimize_repository_period end optional :html_emails_enabled, type: Boolean, desc: 'By default GitLab sends emails in HTML and plain text formats so mail clients can choose what format to use. Disable this option if you only want to send emails in plain text format.' optional :import_sources, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce, diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index a35a903a13b20c298029caf99de6f1f3ed986070..5b99c68ec80d0c675020b120d5108c0a86780d11 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -725,35 +725,7 @@ def expect_invalid end context 'housekeeping settings' do - it { is_expected.not_to allow_value(0).for(:housekeeping_incremental_repack_period) } - - it 'wants the full repack period to be at least the incremental repack period' do - subject.housekeeping_incremental_repack_period = 2 - subject.housekeeping_full_repack_period = 1 - - expect(subject).not_to be_valid - end - - it 'wants the gc period to be at least the full repack period' do - subject.housekeeping_full_repack_period = 100 - subject.housekeeping_gc_period = 90 - - expect(subject).not_to be_valid - end - - it 'allows the same period for incremental repack and full repack, effectively skipping incremental repack' do - subject.housekeeping_incremental_repack_period = 2 - subject.housekeeping_full_repack_period = 2 - - expect(subject).to be_valid - end - - it 'allows the same period for full repack and gc, effectively skipping full repack' do - subject.housekeeping_full_repack_period = 100 - subject.housekeeping_gc_period = 100 - - expect(subject).to be_valid - end + it { is_expected.not_to allow_value(0).for(:housekeeping_optimize_repository_period) } end context 'gitaly timeouts' do diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index e89988893c27e4e497ad3dc13c9d6bbef8c4b448..4d85849cff3237826108430ba506222aa2d460e7 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -806,5 +806,62 @@ .to include(a_string_matching('is not a number')) end end + + context 'with housekeeping enabled' do + it 'at least one of housekeeping_incremental_repack_period or housekeeping_optimize_repository_period is required' do + put api("/application/settings", admin), params: { + housekeeping_enabled: true + } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq( + "housekeeping_incremental_repack_period, housekeeping_optimize_repository_period are missing, exactly one parameter must be provided" + ) + end + + context 'when housekeeping_incremental_repack_period is specified' do + it 'requires all three housekeeping settings' do + put api("/application/settings", admin), params: { + housekeeping_enabled: true, + housekeeping_incremental_repack_period: 10 + } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq( + "housekeeping_full_repack_period, housekeeping_gc_period, housekeeping_incremental_repack_period provide all or none of parameters" + ) + end + + it 'returns housekeeping_optimize_repository_period value for all housekeeping settings attributes' do + put api("/application/settings", admin), params: { + housekeeping_enabled: true, + housekeeping_gc_period: 150, + housekeeping_full_repack_period: 125, + housekeeping_incremental_repack_period: 100 + } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['housekeeping_optimize_repository_period']).to eq(100) + expect(json_response['housekeeping_full_repack_period']).to eq(100) + expect(json_response['housekeeping_gc_period']).to eq(100) + expect(json_response['housekeeping_incremental_repack_period']).to eq(100) + end + end + + context 'when housekeeping_optimize_repository_period is specified' do + it 'returns housekeeping_optimize_repository_period value for all housekeeping settings attributes' do + put api("/application/settings", admin), params: { + housekeeping_enabled: true, + housekeeping_optimize_repository_period: 100 + } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['housekeeping_optimize_repository_period']).to eq(100) + expect(json_response['housekeeping_full_repack_period']).to eq(100) + expect(json_response['housekeeping_gc_period']).to eq(100) + expect(json_response['housekeeping_incremental_repack_period']).to eq(100) + end + end + end end end