kotovalexarian-likes-gitlab/gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Add latest changes from gitlab-org/gitlab@master

Browse source
This commit is contained in:
GitLab Bot 2021-12-31 18:16:55 +00:00
parent e218822bd5
commit 81617e0e06
13 changed files with 189 additions and 66 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
app/finders/ci/runners_finder.rb
View file

@ -53,9 +53,13 @@ module Ci
when :direct
Ci::Runner.belonging_to_group(@group.id)
when :descendants, nil
# Getting all runners from the group itself and all its descendant groups/projects
descendant_projects = Project.for_group_and_its_subgroups(@group)
Ci::Runner.belonging_to_group_or_project(@group.self_and_descendants, descendant_projects)
if ::Feature.enabled?(:ci_find_runners_by_ci_mirrors, @group, default_enabled: :yaml)
Ci::Runner.belonging_to_group_or_project_descendants(@group.id)
else
# Getting all runners from the group itself and all its descendant groups/projects
descendant_projects = Project.for_group_and_its_subgroups(@group)
Ci::Runner.legacy_belonging_to_group_or_project(@group.self_and_descendants, descendant_projects)
end
else
raise ArgumentError, 'Invalid membership filter'
end

2
app/models/ci/namespace_mirror.rb
View file

@ -10,6 +10,8 @@ module Ci
where('traversal_ids @> ARRAY[?]::int[]', id)
end
scope :by_namespace_id, -> (namespace_id) { where(namespace_id: namespace_id) }
class << self
def sync!(event)
namespace = event.namespace

3
app/models/ci/project_mirror.rb
View file

@ -6,6 +6,9 @@ module Ci
class ProjectMirror < ApplicationRecord
belongs_to :project
scope :by_namespace_id, -> (namespace_id) { where(namespace_id: namespace_id) }
scope :by_project_id, -> (project_id) { where(project_id: project_id) }
class << self
def sync!(event)
upsert({ project_id: event.project_id, namespace_id: event.project.namespace_id },

26
app/models/ci/runner.rb
View file

@ -95,7 +95,26 @@ module Ci
joins(:runner_projects).where(ci_runner_projects: { project_id: project_id })
}
scope :belonging_to_group, -> (group_id, include_ancestors: false) {
scope :belonging_to_group, -> (group_id) {
joins(:runner_namespaces)
.where(ci_runner_namespaces: { namespace_id: group_id })
}
scope :belonging_to_group_or_project_descendants, -> (group_id) {
group_ids = Ci::NamespaceMirror.contains_namespace(group_id).select(:namespace_id)
project_ids = Ci::ProjectMirror.by_namespace_id(group_ids).select(:project_id)
group_runners = joins(:runner_namespaces).where(ci_runner_namespaces: { namespace_id: group_ids })
project_runners = joins(:runner_projects).where(ci_runner_projects: { project_id: project_ids })
union_sql = ::Gitlab::SQL::Union.new([group_runners, project_runners]).to_sql
from("(#{union_sql}) #{table_name}")
}
# deprecated
# split this into: belonging_to_group & belonging_to_group_and_ancestors
scope :legacy_belonging_to_group, -> (group_id, include_ancestors: false) {
groups = ::Group.where(id: group_id)
groups = groups.self_and_ancestors if include_ancestors
@ -104,7 +123,8 @@ module Ci
.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/336433')
}
scope :belonging_to_group_or_project, -> (group_id, project_id) {
# deprecated
scope :legacy_belonging_to_group_or_project, -> (group_id, project_id) {
groups = ::Group.where(id: group_id)
group_runners = joins(:runner_namespaces).where(ci_runner_namespaces: { namespace_id: groups })
@ -116,6 +136,7 @@ module Ci
.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/336433')
}
# deprecated
scope :belonging_to_parent_group_of_project, -> (project_id) {
project_groups = ::Group.joins(:projects).where(projects: { id: project_id })
@ -124,6 +145,7 @@ module Ci
.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/336433')
}
# deprecated
scope :owned_or_instance_wide, -> (project_id) do
from_union(
[

8
config/feature_flags/development/ci_find_runners_by_ci_mirrors.yml Normal file
View file

@ -0,0 +1,8 @@
---
name: ci_find_runners_by_ci_mirrors
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/74900
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/347226
milestone: '14.7'
type: development
group: group::runner
default_enabled: false

2
lib/api/ci/runners.rb
View file

@ -229,7 +229,7 @@ module API
use :pagination
end
get ':id/runners' do
runners = ::Ci::Runner.belonging_to_group(user_group.id, include_ancestors: true)
runners = ::Ci::Runner.legacy_belonging_to_group(user_group.id, include_ancestors: true)
runners = apply_filter(runners, params)
present paginate(runners), with: Entities::Ci::Runner

8
spec/factories/namespaces.rb
View file

@ -11,6 +11,14 @@ FactoryBot.define do
owner { association(:user, strategy: :build, namespace: instance, username: path) }
after(:create) do |namespace, evaluator|
# simulating ::Namespaces::ProcessSyncEventsWorker because most tests don't run Sidekiq inline
# Note: we need to get refreshed `traversal_ids` it is updated via SQL query
# in `Namespaces::Traversal::Linear#sync_traversal_ids` (see the NOTE in that method).
# We cannot use `.reload` because it cleans other on-the-fly attributes.
namespace.create_ci_namespace_mirror!(traversal_ids: Namespace.find(namespace.id).traversal_ids) unless namespace.ci_namespace_mirror
end
trait :with_aggregation_schedule do
after(:create) do |namespace|
create(:namespace_aggregation_schedules, namespace: namespace)

3
spec/factories/projects.rb
View file

@ -101,6 +101,9 @@ FactoryBot.define do
import_state.last_error = evaluator.import_last_error
import_state.save!
end
# simulating ::Projects::ProcessSyncEventsWorker because most tests don't run Sidekiq inline
project.create_ci_project_mirror!(namespace_id: project.namespace_id) unless project.ci_project_mirror
end
trait :public do

12
spec/finders/ci/runners_finder_spec.rb
View file

@ -206,7 +206,7 @@ RSpec.describe Ci::RunnersFinder do
sub_group_4.runners << runner_sub_group_4
end
describe '#execute' do
shared_examples '#execute' do
subject { described_class.new(current_user: user, params: params).execute }
shared_examples 'membership equal to :descendants' do
@ -349,6 +349,16 @@ RSpec.describe Ci::RunnersFinder do
end
end
it_behaves_like '#execute'
context 'when the FF ci_find_runners_by_ci_mirrors is disabled' do
before do
stub_feature_flags(ci_find_runners_by_ci_mirrors: false)
end
it_behaves_like '#execute'
end
describe '#sort_key' do
subject { described_class.new(current_user: user, params: params.merge(group: group)).sort_key }

97
spec/models/ci/namespace_mirror_spec.rb
View file

@ -8,50 +8,77 @@ RSpec.describe Ci::NamespaceMirror do
let!(:group3) { create(:group, parent: group2) }
let!(:group4) { create(:group, parent: group3) }
describe '.sync!' do
let!(:event) { namespace.sync_events.create! }
before do
# refreshing ci mirrors according to the parent tree above
Namespaces::SyncEvent.find_each { |event| Ci::NamespaceMirror.sync!(event) }
subject(:sync) { described_class.sync!(event.reload) }
# checking initial situation. we need to reload to reflect the changes of event sync
expect(group1.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id])
expect(group2.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id, group2.id])
expect(group3.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id, group2.id, group3.id])
expect(group4.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id, group2.id, group3.id, group4.id])
end
context 'when namespace hierarchy does not exist in the first place' do
let(:namespace) { group3 }
context 'scopes' do
describe '.contains_namespace' do
let_it_be(:another_group) { create(:group) }
it 'creates the hierarchy' do
expect { sync }.to change { described_class.count }.from(0).to(1)
subject(:result) { described_class.contains_namespace(group2.id) }
expect(namespace.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id, group2.id, group3.id])
it 'returns groups having group2.id in traversal_ids' do
expect(result.pluck(:namespace_id)).to contain_exactly(group2.id, group3.id, group4.id)
end
end
context 'when namespace hierarchy does already exist' do
describe '.by_namespace_id' do
subject(:result) { described_class.by_namespace_id(group2.id) }
it 'returns namesapce mirrors of namespace id' do
expect(result).to contain_exactly(group2.ci_namespace_mirror)
end
end
end
describe '.sync!' do
subject(:sync) { described_class.sync!(Namespaces::SyncEvent.last) }
context 'when namespace mirror does not exist in the first place' do
let(:namespace) { group3 }
before do
described_class.create!(namespace: namespace, traversal_ids: [namespace.id])
namespace.ci_namespace_mirror.destroy!
namespace.sync_events.create!
end
it 'updates the hierarchy' do
expect { sync }.not_to change { described_class.count }
it 'creates the mirror' do
expect { sync }.to change { described_class.count }.from(3).to(4)
expect(namespace.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id, group2.id, group3.id])
expect(namespace.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id, group2.id, group3.id])
end
end
# I did not extract this context to a `shared_context` because the behavior will change
# after implementing the TODO in `Ci::NamespaceMirror.sync!`
context 'changing the middle namespace' do
context 'when namespace mirror does already exist' do
let(:namespace) { group3 }
before do
namespace.sync_events.create!
end
it 'updates the mirror' do
expect { sync }.not_to change { described_class.count }
expect(namespace.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id, group2.id, group3.id])
end
end
shared_context 'changing the middle namespace' do
let(:namespace) { group2 }
before do
described_class.create!(namespace_id: group1.id, traversal_ids: [group1.id])
described_class.create!(namespace_id: group2.id, traversal_ids: [group1.id, group2.id])
described_class.create!(namespace_id: group3.id, traversal_ids: [group1.id, group2.id, group3.id])
described_class.create!(namespace_id: group4.id, traversal_ids: [group1.id, group2.id, group3.id, group4.id])
group2.update!(parent: nil)
group2.update!(parent: nil) # creates a sync event
end
it 'updates hierarchies for the base but wait for events for the children' do
it 'updates traversal_ids for the base and descendants' do
expect { sync }.not_to change { described_class.count }
expect(group1.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id])
@ -61,6 +88,8 @@ RSpec.describe Ci::NamespaceMirror do
end
end
it_behaves_like 'changing the middle namespace'
context 'when the FFs sync_traversal_ids, use_traversal_ids and use_traversal_ids_for_ancestors are disabled' do
before do
stub_feature_flags(sync_traversal_ids: false,
@ -68,27 +97,7 @@ RSpec.describe Ci::NamespaceMirror do
use_traversal_ids_for_ancestors: false)
end
context 'changing the middle namespace' do
let(:namespace) { group2 }
before do
described_class.create!(namespace_id: group1.id, traversal_ids: [group1.id])
described_class.create!(namespace_id: group2.id, traversal_ids: [group1.id, group2.id])
described_class.create!(namespace_id: group3.id, traversal_ids: [group1.id, group2.id, group3.id])
described_class.create!(namespace_id: group4.id, traversal_ids: [group1.id, group2.id, group3.id, group4.id])
group2.update!(parent: nil)
end
it 'updates hierarchies for the base and descendants' do
expect { sync }.not_to change { described_class.count }
expect(group1.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group1.id])
expect(group2.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group2.id])
expect(group3.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group2.id, group3.id])
expect(group4.reload.ci_namespace_mirror).to have_attributes(traversal_ids: [group2.id, group3.id, group4.id])
end
end
it_behaves_like 'changing the middle namespace'
end
end
end

34
spec/models/ci/project_mirror_spec.rb
View file

@ -8,12 +8,36 @@ RSpec.describe Ci::ProjectMirror do
let!(:project) { create(:project, namespace: group2) }
context 'scopes' do
let_it_be(:another_project) { create(:project, namespace: group1) }
describe '.by_project_id' do
subject(:result) { described_class.by_project_id(project.id) }
it 'returns project mirrors of project' do
expect(result.pluck(:project_id)).to contain_exactly(project.id)
end
end
describe '.by_namespace_id' do
subject(:result) { described_class.by_namespace_id(group2.id) }
it 'returns project mirrors of namespace id' do
expect(result).to contain_exactly(project.ci_project_mirror)
end
end
end
describe '.sync!' do
let!(:event) { Projects::SyncEvent.create!(project: project) }
subject(:sync) { described_class.sync!(event.reload) }
subject(:sync) { described_class.sync!(event) }
context 'when project mirror does not exist in the first place' do
before do
project.ci_project_mirror.destroy!
end
context 'when project hierarchy does not exist in the first place' do
it 'creates a ci_projects record' do
expect { sync }.to change { described_class.count }.from(0).to(1)
@ -21,11 +45,7 @@ RSpec.describe Ci::ProjectMirror do
end
end
context 'when project hierarchy does already exist' do
before do
described_class.create!(project_id: project.id, namespace_id: group1.id)
end
context 'when project mirror does already exist' do
it 'updates the related ci_projects record' do
expect { sync }.not_to change { described_class.count }

42
spec/models/ci/runner_spec.rb
View file

@ -1358,7 +1358,7 @@ RSpec.describe Ci::Runner do
it { is_expected.to eq(contacted_at_stored) }
end
describe '.belonging_to_group' do
describe '.legacy_belonging_to_group' do
shared_examples 'returns group runners' do
it 'returns the specific group runner' do
group = create(:group)
@ -1366,7 +1366,7 @@ RSpec.describe Ci::Runner do
unrelated_group = create(:group)
create(:ci_runner, :group, groups: [unrelated_group])
expect(described_class.belonging_to_group(group.id)).to contain_exactly(runner)
expect(described_class.legacy_belonging_to_group(group.id)).to contain_exactly(runner)
end
context 'runner belonging to parent group' do
@ -1376,13 +1376,13 @@ RSpec.describe Ci::Runner do
context 'when include_parent option is passed' do
it 'returns the group runner from the parent group' do
expect(described_class.belonging_to_group(group.id, include_ancestors: true)).to contain_exactly(parent_runner)
expect(described_class.legacy_belonging_to_group(group.id, include_ancestors: true)).to contain_exactly(parent_runner)
end
end
context 'when include_parent option is not passed' do
it 'does not return the group runner from the parent group' do
expect(described_class.belonging_to_group(group.id)).to be_empty
expect(described_class.legacy_belonging_to_group(group.id)).to be_empty
end
end
end
@ -1398,4 +1398,38 @@ RSpec.describe Ci::Runner do
it_behaves_like 'returns group runners'
end
end
describe '.belonging_to_group' do
it 'returns the specific group runner' do
group = create(:group)
runner = create(:ci_runner, :group, groups: [group])
unrelated_group = create(:group)
create(:ci_runner, :group, groups: [unrelated_group])
expect(described_class.belonging_to_group(group.id)).to contain_exactly(runner)
end
end
describe '.belonging_to_group_or_project_descendants' do
it 'returns the specific group runners' do
group1 = create(:group)
group2 = create(:group, parent: group1)
group3 = create(:group)
project1 = create(:project, namespace: group1)
project2 = create(:project, namespace: group2)
project3 = create(:project, namespace: group3)
runner1 = create(:ci_runner, :group, groups: [group1])
runner2 = create(:ci_runner, :group, groups: [group2])
_runner3 = create(:ci_runner, :group, groups: [group3])
runner4 = create(:ci_runner, :project, projects: [project1])
runner5 = create(:ci_runner, :project, projects: [project2])
_runner6 = create(:ci_runner, :project, projects: [project3])
expect(described_class.belonging_to_group_or_project_descendants(group1.id)).to contain_exactly(
runner1, runner2, runner4, runner5
)
end
end
end

8
spec/services/ci/process_sync_events_service_spec.rb
View file

@ -28,10 +28,10 @@ RSpec.describe Ci::ProcessSyncEventsService do
it 'consumes events' do
expect { execute }.to change(Projects::SyncEvent, :count).from(2).to(0)
expect(project1.ci_project_mirror).to have_attributes(
expect(project1.reload.ci_project_mirror).to have_attributes(
namespace_id: parent_group_1.id
)
expect(project2.ci_project_mirror).to have_attributes(
expect(project2.reload.ci_project_mirror).to have_attributes(
namespace_id: parent_group_2.id
)
end
@ -88,10 +88,10 @@ RSpec.describe Ci::ProcessSyncEventsService do
it 'consumes events' do
expect { execute }.to change(Namespaces::SyncEvent, :count).from(2).to(0)
expect(group.ci_namespace_mirror).to have_attributes(
expect(group.reload.ci_namespace_mirror).to have_attributes(
traversal_ids: [parent_group_1.id, parent_group_2.id, group.id]
)
expect(parent_group_2.ci_namespace_mirror).to have_attributes(
expect(parent_group_2.reload.ci_namespace_mirror).to have_attributes(
traversal_ids: [parent_group_1.id, parent_group_2.id]
)
end