summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/models/user.rb2
-rw-r--r--app/services/users/destroy_service.rb2
-rw-r--r--app/services/users/migrate_to_ghost_user_service.rb13
-rw-r--r--config.ru10
-rw-r--r--config/initializers/7_prometheus_metrics.rb26
-rw-r--r--config/initializers/zz_metrics.rb4
-rw-r--r--doc/user/project/description_templates.md10
-rw-r--r--doc/user/project/settings/index.md6
-rw-r--r--lib/gitlab/cleanup/orphan_job_artifact_files.rb3
-rw-r--r--lib/gitlab/cluster/lifecycle_events.rb19
-rw-r--r--package.json2
-rw-r--r--spec/lib/gitlab/cleanup/orphan_job_artifact_files_spec.rb27
-rw-r--r--spec/services/users/destroy_service_spec.rb14
-rw-r--r--spec/services/users/migrate_to_ghost_user_service_spec.rb7
-rw-r--r--spec/support/services/migrate_to_ghost_user_service_shared_examples.rb12
-rw-r--r--yarn.lock18
16 files changed, 111 insertions, 64 deletions
diff --git a/app/models/user.rb b/app/models/user.rb
index 74832bff9ac..9cd238904ff 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -390,7 +390,7 @@ class User < ApplicationRecord
# rubocop: disable CodeReuse/ServiceClass
# Ideally we should not call a service object here but user.block
- # is also bcalled by Users::MigrateToGhostUserService which references
+ # is also called by Users::MigrateToGhostUserService which references
# this state transition object in order to do a rollback.
# For this reason the tradeoff is to disable this cop.
after_transition any => :blocked do |user|
diff --git a/app/services/users/destroy_service.rb b/app/services/users/destroy_service.rb
index 4ec875098fa..46eec082125 100644
--- a/app/services/users/destroy_service.rb
+++ b/app/services/users/destroy_service.rb
@@ -54,7 +54,7 @@ module Users
yield(user) if block_given?
- MigrateToGhostUserService.new(user).execute unless options[:hard_delete]
+ MigrateToGhostUserService.new(user).execute(hard_delete: options[:hard_delete])
response = Snippets::BulkDestroyService.new(current_user, user.snippets).execute(options)
raise DestroyError, response.message if response.error?
diff --git a/app/services/users/migrate_to_ghost_user_service.rb b/app/services/users/migrate_to_ghost_user_service.rb
index 515d7821416..575614e8743 100644
--- a/app/services/users/migrate_to_ghost_user_service.rb
+++ b/app/services/users/migrate_to_ghost_user_service.rb
@@ -10,14 +10,21 @@ module Users
class MigrateToGhostUserService
extend ActiveSupport::Concern
- attr_reader :ghost_user, :user
+ attr_reader :ghost_user, :user, :hard_delete
def initialize(user)
@user = user
@ghost_user = User.ghost
end
- def execute
+ # If an admin attempts to hard delete a user, in some cases associated
+ # records may have a NOT NULL constraint on the user ID that prevent that record
+ # from being destroyed. In such situations we must assign the record to the ghost user.
+ # Passing in `hard_delete: true` will ensure these records get assigned to
+ # the ghost user before the user is destroyed. Other associated records will be destroyed.
+ # letting the other associated records be destroyed.
+ def execute(hard_delete: false)
+ @hard_delete = hard_delete
transition = user.block_transition
# Block the user before moving records to prevent a data race.
@@ -46,6 +53,8 @@ module Users
private
def migrate_records
+ return if hard_delete
+
migrate_issues
migrate_merge_requests
migrate_notes
diff --git a/config.ru b/config.ru
index c74a49cd0e2..f07c25f503a 100644
--- a/config.ru
+++ b/config.ru
@@ -4,17 +4,7 @@
require ::File.expand_path('../config/environment', __FILE__)
-def master_process?
- Prometheus::PidProvider.worker_id == 'puma_master'
-end
-
warmup do |app|
- # The following is necessary to ensure stale Prometheus metrics don't accumulate over time.
- # It needs to be done as early as here to ensure metrics files aren't deleted.
- # After we hit our app in `warmup`, first metrics and corresponding files already being created,
- # for example in `lib/gitlab/metrics/requests_rack_middleware.rb`.
- Prometheus::CleanupMultiprocDirService.new.execute if master_process?
-
client = Rack::MockRequest.new(app)
client.get('/')
end
diff --git a/config/initializers/7_prometheus_metrics.rb b/config/initializers/7_prometheus_metrics.rb
index 17ce2a30d66..c55b074f9c8 100644
--- a/config/initializers/7_prometheus_metrics.rb
+++ b/config/initializers/7_prometheus_metrics.rb
@@ -13,6 +13,24 @@ def prometheus_default_multiproc_dir
end
end
+def puma_metrics_server_process?
+ Prometheus::PidProvider.worker_id == 'puma_master'
+end
+
+def sidekiq_metrics_server_process?
+ Gitlab::Runtime.sidekiq? && (!ENV['SIDEKIQ_WORKER_ID'] || ENV['SIDEKIQ_WORKER_ID'] == '0')
+end
+
+if puma_metrics_server_process? || sidekiq_metrics_server_process?
+ # The following is necessary to ensure stale Prometheus metrics don't accumulate over time.
+ # It needs to be done as early as here to ensure metrics files aren't deleted.
+ # After we hit our app in `warmup`, first metrics and corresponding files already being created,
+ # for example in `lib/gitlab/metrics/requests_rack_middleware.rb`.
+ Prometheus::CleanupMultiprocDirService.new.execute
+
+ ::Prometheus::Client.reinitialize_on_pid_change(force: true)
+end
+
::Prometheus::Client.configure do |config|
config.logger = Gitlab::AppLogger
@@ -47,21 +65,13 @@ if Gitlab::Runtime.sidekiq? && (!ENV['SIDEKIQ_WORKER_ID'] || ENV['SIDEKIQ_WORKER
end
if !Rails.env.test? && Gitlab::Metrics.prometheus_metrics_enabled?
- # When running Puma in a Single mode, `on_master_start` and `on_worker_start` are the same.
- # Thus, we order these events to run `reinitialize_on_pid_change` with `force: true` first.
Gitlab::Cluster::LifecycleEvents.on_master_start do
- ::Prometheus::Client.reinitialize_on_pid_change(force: true)
-
if Gitlab::Runtime.puma?
Gitlab::Metrics::Samplers::PumaSampler.instance.start
end
Gitlab::Metrics.gauge(:deployments, 'GitLab Version', {}, :max).set({ version: Gitlab::VERSION, revision: Gitlab.revision }, 1)
- if Gitlab::Runtime.web_server?
- Gitlab::Metrics::RequestsRackMiddleware.initialize_metrics
- end
-
Gitlab::Ci::Parsers.instrument!
rescue IOError => e
Gitlab::ErrorTracking.track_exception(e)
diff --git a/config/initializers/zz_metrics.rb b/config/initializers/zz_metrics.rb
index 7fa71225aae..88469d2cdef 100644
--- a/config/initializers/zz_metrics.rb
+++ b/config/initializers/zz_metrics.rb
@@ -36,6 +36,10 @@ if Gitlab::Metrics.enabled? && !Rails.env.test? && !(Rails.env.development? && d
config.middleware.use(Gitlab::Metrics::ElasticsearchRackMiddleware)
end
+ if Gitlab::Runtime.puma?
+ Gitlab::Metrics::RequestsRackMiddleware.initialize_metrics
+ end
+
GC::Profiler.enable
module TrackNewRedisConnections
diff --git a/doc/user/project/description_templates.md b/doc/user/project/description_templates.md
index 539f5230063..4f8cfad1444 100644
--- a/doc/user/project/description_templates.md
+++ b/doc/user/project/description_templates.md
@@ -129,8 +129,9 @@ Prerequisites:
To set a default description template for merge requests, either:
-- [Create a merge request template](#create-a-merge-request-template) named `Default.md` and save it in `.gitlab/merge_request_templates/`.
- This will not overwrite the default template if one has been set in the project settings.
+- [Create a merge request template](#create-a-merge-request-template) named `Default.md` or `default.md`
+ and save it in `.gitlab/merge_request_templates/`.
+ This doesn't overwrite the default template if one has been set in the project settings.
- Users on GitLab Premium and higher: set the default template in project settings:
1. On the top bar, select **Menu > Projects** and find your project.
@@ -141,8 +142,9 @@ To set a default description template for merge requests, either:
To set a default description template for issues, either:
-- [Create an issue template](#create-an-issue-template) named `Default.md` and save it in `.gitlab/issue_templates/`.
- This will not overwrite the default template if one has been set in the project settings.
+- [Create an issue template](#create-an-issue-template) named `Default.md` or `default.md`
+ and save it in `.gitlab/issue_templates/`.
+ This doesn't overwrite the default template if one has been set in the project settings.
- Users on GitLab Premium and higher: set the default template in project settings:
1. On the top bar, select **Menu > Projects** and find your project.
diff --git a/doc/user/project/settings/index.md b/doc/user/project/settings/index.md
index 4c6be3511f2..12d75551aac 100644
--- a/doc/user/project/settings/index.md
+++ b/doc/user/project/settings/index.md
@@ -217,7 +217,11 @@ cannot change them:
This ensures that your job uses the settings you intend and that they are not overridden by
project-level pipelines.
-##### Avoid parent and child pipelines
+##### Avoid parent and child pipelines in GitLab 14.7 and earlier
+
+NOTE:
+This advice does not apply to GitLab 14.8 and later because [a fix](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/78878) added
+compatibility for combining compliance pipelines, and parent and child pipelines.
Compliance pipelines start on the run of _every_ pipeline in a relevant project. This means that if a pipeline in the relevant project
triggers a child pipeline, the compliance pipeline runs first. This can trigger the parent pipeline, instead of the child pipeline.
diff --git a/lib/gitlab/cleanup/orphan_job_artifact_files.rb b/lib/gitlab/cleanup/orphan_job_artifact_files.rb
index 05dfdcd4486..90123b9d000 100644
--- a/lib/gitlab/cleanup/orphan_job_artifact_files.rb
+++ b/lib/gitlab/cleanup/orphan_job_artifact_files.rb
@@ -99,6 +99,9 @@ module Gitlab
# ^--+--+- components of hashed storage project path
cmd += %w[-mindepth 6 -maxdepth 6]
+ # Intentionally exclude pipeline artifacts which match the same path
+ cmd += %w[-not -path */pipelines/*]
+
# Artifact directories are named on their ID
cmd += %w[-type d]
diff --git a/lib/gitlab/cluster/lifecycle_events.rb b/lib/gitlab/cluster/lifecycle_events.rb
index 6159fb0a811..e423d1f17da 100644
--- a/lib/gitlab/cluster/lifecycle_events.rb
+++ b/lib/gitlab/cluster/lifecycle_events.rb
@@ -70,7 +70,7 @@ module Gitlab
# Hook registration methods (called from initializers)
#
def on_worker_start(&block)
- if in_clustered_environment?
+ if in_clustered_puma?
# Defer block execution
(@worker_start_hooks ||= []) << block
else
@@ -101,7 +101,7 @@ module Gitlab
end
def on_master_start(&block)
- if in_clustered_environment?
+ if in_clustered_puma?
on_before_fork(&block)
else
on_worker_start(&block)
@@ -158,21 +158,8 @@ module Gitlab
end
end
- def in_clustered_environment?
- # Sidekiq doesn't fork
- return false if Gitlab::Runtime.sidekiq?
-
- # Puma sometimes forks
- return true if in_clustered_puma?
-
- # Default assumption is that we don't fork
- false
- end
-
def in_clustered_puma?
- return false unless Gitlab::Runtime.puma?
-
- @puma_options && @puma_options[:workers] && @puma_options[:workers] > 0
+ Gitlab::Runtime.puma? && @puma_options && @puma_options[:workers] && @puma_options[:workers] > 0
end
end
end
diff --git a/package.json b/package.json
index 582d86454b8..ddaf4f60af7 100644
--- a/package.json
+++ b/package.json
@@ -155,6 +155,7 @@
"pikaday": "^1.8.0",
"popper.js": "^1.16.1",
"portal-vue": "^2.1.7",
+ "postcss": "8.4.5",
"prismjs": "^1.21.0",
"prosemirror-markdown": "1.6.0",
"prosemirror-model": "^1.16.1",
@@ -241,7 +242,6 @@
"miragejs": "^0.1.40",
"mock-apollo-client": "1.2.0",
"nodemon": "^2.0.4",
- "postcss": "^8.4.5",
"prettier": "2.2.1",
"prosemirror-schema-basic": "^1.1.2",
"prosemirror-schema-list": "^1.1.6",
diff --git a/spec/lib/gitlab/cleanup/orphan_job_artifact_files_spec.rb b/spec/lib/gitlab/cleanup/orphan_job_artifact_files_spec.rb
index b5adb603dab..e6ef2d8a541 100644
--- a/spec/lib/gitlab/cleanup/orphan_job_artifact_files_spec.rb
+++ b/spec/lib/gitlab/cleanup/orphan_job_artifact_files_spec.rb
@@ -34,10 +34,33 @@ RSpec.describe Gitlab::Cleanup::OrphanJobArtifactFiles do
cleanup.run!
end
- it 'finds artifacts on disk' do
+ it 'finds job artifacts on disk' do
artifact = create(:ci_job_artifact, :archive)
+ artifact_directory = artifact.file.relative_path.to_s.split('/')[0...6].join('/')
+
+ cleaned = []
+
+ expect(cleanup).to receive(:find_artifacts).and_wrap_original do |original_method, *args, &block|
+ original_method.call(*args) { |dir| cleaned << dir }
+ end
+
+ cleanup.run!
+
+ expect(cleaned).to include(/#{artifact_directory}/)
+ end
+
+ it 'does not find pipeline artifacts on disk' do
+ artifact = create(:ci_pipeline_artifact, :with_coverage_report)
+ # using 0...6 to match the -min/maxdepth 6 strictly, since this is one directory
+ # deeper than job artifacts, and .dirname would not match
+ artifact_directory = artifact.file.relative_path.to_s.split('/')[0...6].join('/')
+
+ expect(cleanup).to receive(:find_artifacts).and_wrap_original do |original_method, *args, &block|
+ # this can either _not_ yield at all, or yield with any other file
+ # except the one that we're explicitly excluding
+ original_method.call(*args) { |path| expect(path).not_to match(artifact_directory) }
+ end
- expect(cleanup).to receive(:find_artifacts).and_yield(artifact.file.path)
cleanup.run!
end
diff --git a/spec/services/users/destroy_service_spec.rb b/spec/services/users/destroy_service_spec.rb
index 76b84e3b4ab..602db66dba1 100644
--- a/spec/services/users/destroy_service_spec.rb
+++ b/spec/services/users/destroy_service_spec.rb
@@ -215,8 +215,8 @@ RSpec.describe Users::DestroyService do
end
end
- context "migrating associated records" do
- let!(:issue) { create(:issue, author: user) }
+ context 'migrating associated records' do
+ let!(:issue) { create(:issue, author: user) }
it 'delegates to the `MigrateToGhostUser` service to move associated records to the ghost user' do
expect_any_instance_of(Users::MigrateToGhostUserService).to receive(:execute).once.and_call_original
@@ -226,12 +226,14 @@ RSpec.describe Users::DestroyService do
expect(issue.reload.author).to be_ghost
end
- it 'does not run `MigrateToGhostUser` if hard_delete option is given' do
- expect_any_instance_of(Users::MigrateToGhostUserService).not_to receive(:execute)
+ context 'when hard_delete option is given' do
+ it 'will not ghost certain records' do
+ expect_any_instance_of(Users::MigrateToGhostUserService).to receive(:execute).once.and_call_original
- service.execute(user, hard_delete: true)
+ service.execute(user, hard_delete: true)
- expect(Issue.exists?(issue.id)).to be_falsy
+ expect(Issue.exists?(issue.id)).to be_falsy
+ end
end
end
diff --git a/spec/services/users/migrate_to_ghost_user_service_spec.rb b/spec/services/users/migrate_to_ghost_user_service_spec.rb
index c36889f20ec..073ebaae5b0 100644
--- a/spec/services/users/migrate_to_ghost_user_service_spec.rb
+++ b/spec/services/users/migrate_to_ghost_user_service_spec.rb
@@ -3,9 +3,10 @@
require 'spec_helper'
RSpec.describe Users::MigrateToGhostUserService do
- let!(:user) { create(:user) }
- let!(:project) { create(:project, :repository) }
- let(:service) { described_class.new(user) }
+ let!(:user) { create(:user) }
+ let!(:project) { create(:project, :repository) }
+ let(:service) { described_class.new(user) }
+ let(:always_ghost) { false }
context "migrating a user's associated records to the ghost user" do
context 'issues' do
diff --git a/spec/support/services/migrate_to_ghost_user_service_shared_examples.rb b/spec/support/services/migrate_to_ghost_user_service_shared_examples.rb
index 2fbc01a9195..1e291a90163 100644
--- a/spec/support/services/migrate_to_ghost_user_service_shared_examples.rb
+++ b/spec/support/services/migrate_to_ghost_user_service_shared_examples.rb
@@ -42,6 +42,18 @@ RSpec.shared_examples "migrating a deleted user's associated records to the ghos
end
end
+ it 'will only migrate specific records during a hard_delete' do
+ service.execute(hard_delete: true)
+
+ migrated_record = record_class.find_by_id(record.id)
+
+ check_user = always_ghost ? User.ghost : user
+
+ migrated_fields.each do |field|
+ expect(migrated_record.public_send(field)).to eq(check_user)
+ end
+ end
+
context "race conditions" do
context "when #{record_class_name} migration fails and is rolled back" do
before do
diff --git a/yarn.lock b/yarn.lock
index dc2080f9bfb..b05b46afdc5 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -9470,15 +9470,7 @@ postcss-value-parser@^4.1.0, postcss-value-parser@^4.2.0:
resolved "https://registry.yarnpkg.com/postcss-value-parser/-/postcss-value-parser-4.2.0.tgz#723c09920836ba6d3e5af019f92bc0971c02e514"
integrity sha512-1NNCs6uurfkVbeXG4S8JFT9t19m45ICnif8zWLd5oPSZ50QnwMfK+H3jv408d4jw/7Bttv5axS5IiHoLaVNHeQ==
-postcss@^7.0.14, postcss@^7.0.5, postcss@^7.0.6:
- version "7.0.39"
- resolved "https://registry.yarnpkg.com/postcss/-/postcss-7.0.39.tgz#9624375d965630e2e1f2c02a935c82a59cb48309"
- integrity sha512-yioayjNbHn6z1/Bywyb2Y4s3yvDAeXGOyxqD+LnVOinq6Mdmd++SW2wUNVzavyyHxd6+DxzWGIuosg6P1Rj8uA==
- dependencies:
- picocolors "^0.2.1"
- source-map "^0.6.1"
-
-postcss@^8.2.1, postcss@^8.4.5:
+postcss@8.4.5, postcss@^8.2.1, postcss@^8.4.5:
version "8.4.5"
resolved "https://registry.yarnpkg.com/postcss/-/postcss-8.4.5.tgz#bae665764dfd4c6fcc24dc0fdf7e7aa00cc77f95"
integrity sha512-jBDboWM8qpaqwkMwItqTQTiFikhs/67OYVvblFFTM7MrZjt6yMKd6r2kgXizEbTTljacm4NldIlZnhbjr84QYg==
@@ -9487,6 +9479,14 @@ postcss@^8.2.1, postcss@^8.4.5:
picocolors "^1.0.0"
source-map-js "^1.0.1"
+postcss@^7.0.14, postcss@^7.0.5, postcss@^7.0.6:
+ version "7.0.39"
+ resolved "https://registry.yarnpkg.com/postcss/-/postcss-7.0.39.tgz#9624375d965630e2e1f2c02a935c82a59cb48309"
+ integrity sha512-yioayjNbHn6z1/Bywyb2Y4s3yvDAeXGOyxqD+LnVOinq6Mdmd++SW2wUNVzavyyHxd6+DxzWGIuosg6P1Rj8uA==
+ dependencies:
+ picocolors "^0.2.1"
+ source-map "^0.6.1"
+
prelude-ls@^1.2.1:
version "1.2.1"
resolved "https://registry.yarnpkg.com/prelude-ls/-/prelude-ls-1.2.1.tgz#debc6489d7a6e6b0e7611888cec880337d316396"
View Lorry Controller Status