diff --git a/app/assets/javascripts/ci/constants.js b/app/assets/javascripts/ci/constants.js index 07bf90910f6e67edced84b3431948758de9485e9..9d3131d594a051907117a73bc912a563df7329fb 100644 --- a/app/assets/javascripts/ci/constants.js +++ b/app/assets/javascripts/ci/constants.js @@ -21,6 +21,8 @@ export const ICONS = { SUCCESS: 'success', }; +export const BRIDGE_KIND = 'BRIDGE'; + export const FAILED_STATUS = 'failed'; export const PASSED_STATUS = 'passed'; export const MANUAL_STATUS = 'manual'; diff --git a/app/assets/javascripts/ci/jobs_page/components/job_cells/job_cell.vue b/app/assets/javascripts/ci/jobs_page/components/job_cells/job_cell.vue index af7b011ba06eede26510858a971d62f064807cd5..ec66c2052e0da35cef7e16e5726ff6ff798ea0d3 100644 --- a/app/assets/javascripts/ci/jobs_page/components/job_cells/job_cell.vue +++ b/app/assets/javascripts/ci/jobs_page/components/job_cells/job_cell.vue @@ -3,7 +3,7 @@ import { GlBadge, GlIcon, GlLink, GlTooltipDirective } from '@gitlab/ui'; import { getIdFromGraphQLId } from '~/graphql_shared/utils'; import { s__ } from '~/locale'; import LinkCell from '~/ci/runner/components/cells/link_cell.vue'; -import { SUCCESS_STATUS } from '../../../constants'; +import { SUCCESS_STATUS, BRIDGE_KIND } from '../../../constants'; export default { iconSize: 12, @@ -72,6 +72,9 @@ export default { jobStuck() { return this.job?.stuck; }, + isTriggerJob() { + return this.job?.kind === BRIDGE_KIND; + }, }, }; @@ -150,6 +153,9 @@ export default { {{ s__('Job|manual') }} + + {{ s__('Job|trigger job') }} + diff --git a/app/assets/javascripts/ci/jobs_page/graphql/queries/get_jobs.query.graphql b/app/assets/javascripts/ci/jobs_page/graphql/queries/get_jobs.query.graphql index 36cf33a712509db49b52af1ddcf5e39f544cebd7..486298c6fc84ffed948e1937acdb382927b26327 100644 --- a/app/assets/javascripts/ci/jobs_page/graphql/queries/get_jobs.query.graphql +++ b/app/assets/javascripts/ci/jobs_page/graphql/queries/get_jobs.query.graphql @@ -91,6 +91,7 @@ query getJobs( updateBuild cancelBuild } + kind } } } diff --git a/app/finders/ci/jobs_finder.rb b/app/finders/ci/jobs_finder.rb index c46311a40c06a1985ce067059afecf2fd7705858..bfdb9f30420b0d69905a64a3e3a746d0f3a508a4 100644 --- a/app/finders/ci/jobs_finder.rb +++ b/app/finders/ci/jobs_finder.rb @@ -11,7 +11,13 @@ def initialize(current_user:, pipeline: nil, project: nil, runner: nil, params: @runner = runner @params = params @type = type - raise ArgumentError 'type must be a subclass of Ci::Processable' unless type < ::Ci::Processable + + if Feature.enabled?(:return_bridges_and_builds, project) + raise ArgumentError, 'type must be Ci::Processable or subclass' unless type <= ::Ci::Processable + raise ArgumentError, 'runner parameter can only be used with Ci::Build type' if runner && type != ::Ci::Build + else + raise ArgumentError, 'type must be a subclass of Ci::Processable' unless type < ::Ci::Processable + end end def execute @@ -96,7 +102,7 @@ def filter_by_scope(builds) def filter_by_runner_types(builds) return builds unless use_runner_type_filter? - builds.with_runner_type(params[:runner_type]) + builds.try(:with_runner_type, params[:runner_type]) || builds end # Overridden in EE @@ -105,9 +111,9 @@ def use_runner_type_filter? end def filter_by_with_artifacts(builds) - return builds.with_any_artifacts if params[:with_artifacts] + return builds unless params[:with_artifacts] - builds + builds.try(:with_any_artifacts) || builds end def filter_by_statuses!(builds) @@ -118,6 +124,29 @@ def filter_by_statuses!(builds) end def jobs_by_type(relation, type) + if Feature.enabled?(:return_bridges_and_builds, project) + jobs_by_type_with_processables(relation, type) + else + jobs_by_type_without_processables(relation, type) + end + end + + def jobs_by_type_with_processables(relation, type) + return relation.builds if params[:with_artifacts] + + case type.name + when ::Ci::Build.name + relation.builds + when ::Ci::Bridge.name + relation.bridges + when ::Ci::Processable.name + relation.processables + else + raise ArgumentError, "finder does not support #{type} type" + end + end + + def jobs_by_type_without_processables(relation, type) case type.name when ::Ci::Build.name relation.builds diff --git a/app/graphql/resolvers/project_jobs_resolver.rb b/app/graphql/resolvers/project_jobs_resolver.rb index adc4fbac3b9231137ee35b15dfa33008a3aa935b..1717e8522fd07c66f1b2d325a9ed28fd754c5691 100644 --- a/app/graphql/resolvers/project_jobs_resolver.rb +++ b/app/graphql/resolvers/project_jobs_resolver.rb @@ -34,12 +34,21 @@ def resolve_with_lookahead(**args) filter_by_name = Feature.enabled?(:populate_and_use_build_names_table, project) && args[:name].to_s.present? filter_by_sources = args[:sources].present? - jobs = ::Ci::JobsFinder.new( - current_user: current_user, project: project, params: { - scope: args[:statuses], with_artifacts: args[:with_artifacts], - skip_ordering: filter_by_sources - } - ).execute + # Determine the correct job type based on filtering needs + job_type = if Feature.enabled?(:return_bridges_and_builds, project) + filter_by_name || filter_by_sources ? ::Ci::Build : ::Ci::Processable + else + ::Ci::Build + end + + job_params = { + scope: args[:statuses], with_artifacts: args[:with_artifacts], + skip_ordering: filter_by_sources + } + + jobs = ::Ci::JobsFinder + .new(current_user: current_user, project: project, params: job_params, type: job_type) + .execute # These job filters are currently exclusive with each other if filter_by_name @@ -66,8 +75,8 @@ def resolve_with_lookahead(**args) def preloads { previous_stage_jobs_or_needs: [:needs, :pipeline], - artifacts: [:job_artifacts], pipeline: [:user], + artifacts: [:job_artifacts], build_source: [:source] } end diff --git a/app/models/project.rb b/app/models/project.rb index 8b3608a2e5085084f40a13254645cf9a25120c46..0d5ef0c73a0075998f81a2b32c1973a10b99461c 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -453,6 +453,7 @@ def with_developer_access has_many :pending_builds, class_name: 'Ci::PendingBuild' has_many :builds, class_name: 'Ci::Build', inverse_of: :project has_many :processables, class_name: 'Ci::Processable', inverse_of: :project + has_many :bridges, class_name: 'Ci::Bridge', inverse_of: :project has_many :build_trace_chunks, class_name: 'Ci::BuildTraceChunk', through: :builds, source: :trace_chunks, dependent: :restrict_with_error has_many :build_report_results, class_name: 'Ci::BuildReportResult', inverse_of: :project has_many :job_artifacts, class_name: 'Ci::JobArtifact', dependent: :restrict_with_error diff --git a/config/feature_flags/gitlab_com_derisk/return_bridges_and_builds.yml b/config/feature_flags/gitlab_com_derisk/return_bridges_and_builds.yml new file mode 100644 index 0000000000000000000000000000000000000000..2d47bf63895bed8c09c67f9c64c58bcf1839b003 --- /dev/null +++ b/config/feature_flags/gitlab_com_derisk/return_bridges_and_builds.yml @@ -0,0 +1,10 @@ +--- +name: return_bridges_and_builds +description: +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/379141 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/195869 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/553943 +milestone: '18.2' +group: group::pipeline execution +type: gitlab_com_derisk +default_enabled: false diff --git a/ee/app/finders/ee/ci/jobs_finder.rb b/ee/app/finders/ee/ci/jobs_finder.rb index 377bdc82adee1019922384cf061578e0860472c6..30f506a19d459de744c226ab7946fcce18274062 100644 --- a/ee/app/finders/ee/ci/jobs_finder.rb +++ b/ee/app/finders/ee/ci/jobs_finder.rb @@ -24,7 +24,8 @@ def use_runner_type_filter? def filter_by_failure_reason(builds) return builds unless use_failure_reason_filter? - builds.recently_failed_on_instance_runner(params[:failure_reason]) # currently limited to instance runners + # currently limited to instance runners + builds.try(:recently_failed_on_instance_runner, params[:failure_reason]) || builds end def use_failure_reason_filter? diff --git a/ee/spec/finders/ee/ci/jobs_finder_spec.rb b/ee/spec/finders/ee/ci/jobs_finder_spec.rb index f538f821b8fb3c6f25f29df724bd25d75393aa1c..e409aab61bbff4096542b5353374b0914cf2b365 100644 --- a/ee/spec/finders/ee/ci/jobs_finder_spec.rb +++ b/ee/spec/finders/ee/ci/jobs_finder_spec.rb @@ -130,6 +130,18 @@ end end end + + context 'when recently_failed_on_instance_runner method is not available' do + let(:params) { { failure_reason: :runner_system_failure, runner_type: %w[instance_type] } } + let(:runner_performance_insights) { true } + + it 'falls back to original builds' do + expect(::Ci::Build).to receive(:recently_failed_on_instance_runner) + .with(:runner_system_failure).and_return(nil) + + expect(finder_execute.ids).to contain_exactly(job1.id, job2.id, job7.id, job3.id, job4.id, job5.id, job6.id) + end + end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 22f14a680154f98c793b4244682dbb12bf96f31b..0d92c2572917210b2dcac2898832fec273c2f53b 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -35636,6 +35636,9 @@ msgstr "" msgid "Job|manual" msgstr "" +msgid "Job|trigger job" +msgstr "" + msgid "Job|trigger token" msgstr "" diff --git a/spec/finders/ci/jobs_finder_spec.rb b/spec/finders/ci/jobs_finder_spec.rb index c69d32f679d1cd0798217d4f9fe03c9c1d2f0cb1..a1333f40771102f381d5c880308214ada3402b12 100644 --- a/spec/finders/ci/jobs_finder_spec.rb +++ b/spec/finders/ci/jobs_finder_spec.rb @@ -178,8 +178,22 @@ project.add_maintainer(user) end - it 'returns jobs for the specified project' do - expect(subject).to match_array([successful_job]) + context 'when type is invalid' do + let(:unsupported_type) do + Class.new(::Ci::Processable) do + def self.name + "UnsupportedCustomType" + end + end + end + + subject do + described_class.new(current_user: user, project: project, params: params, type: unsupported_type) + end + + it 'raises an error' do + expect { subject.execute }.to raise_error(ArgumentError, /finder does not support/) + end end context 'when artifacts are present for some jobs' do @@ -464,4 +478,50 @@ end end end + + context 'when type is Ci::Processable' do + let_it_be(:bridge_job) { create(:ci_bridge, :success, pipeline: pipeline, name: 'bridge') } + let_it_be(:build_job_with_artifacts) do + create(:ci_build, :success, pipeline: pipeline, name: 'build_with_artifacts') + end + + let_it_be(:artifact) { create(:ci_job_artifact, job: build_job_with_artifacts) } + + subject do + described_class.new(current_user: user, project: project, params: params, type: ::Ci::Processable).execute + end + + context 'with user being project maintainer' do + before do + project.add_maintainer(user) + end + + context 'when with_artifacts is true' do + let(:params) { { with_artifacts: true } } + + it 'returns only builds with artifacts, not bridges' do + expect(subject).to match_array([build_job_with_artifacts]) + expect(subject).not_to include(bridge_job) + end + end + + context 'when with_artifacts is false' do + let(:params) { { with_artifacts: false } } + + it 'returns both builds and bridges' do + expect(subject).to include(bridge_job, build_job_with_artifacts) + end + end + end + + context 'with user being project guest' do + before do + project.add_guest(user) + end + + it 'returns no jobs' do + expect(subject).to be_empty + end + end + end end diff --git a/spec/frontend/ci/jobs_page/components/job_cells/job_cell_spec.js b/spec/frontend/ci/jobs_page/components/job_cells/job_cell_spec.js index 9665e25e2ace442f402f27704d82fcb12045bbc0..507e4f78313491d63b64d5603ca580e8e2d3f9c0 100644 --- a/spec/frontend/ci/jobs_page/components/job_cells/job_cell_spec.js +++ b/spec/frontend/ci/jobs_page/components/job_cells/job_cell_spec.js @@ -136,12 +136,14 @@ describe('Job Cell', () => { ${'trigger-token-job-badge'} | ${'trigger token'} ${'fail-job-badge'} | ${'allowed to fail'} ${'delayed-job-badge'} | ${'delayed'} + ${'trigger-job-badge'} | ${'trigger job'} `('displays the static $text badge', ({ testId, text }) => { createComponent({ manualJob: true, triggered: true, allowFailure: true, scheduledAt: '2021-03-09T14:58:50+00:00', + kind: 'BRIDGE', }); expect(findBadgeById(testId).exists()).toBe(true); diff --git a/spec/graphql/resolvers/project_jobs_resolver_spec.rb b/spec/graphql/resolvers/project_jobs_resolver_spec.rb index f1f24be4129593e95e4762172c3cd7de808f6ca6..189e9773595b5394e5c50da380aa4f6a04338db0 100644 --- a/spec/graphql/resolvers/project_jobs_resolver_spec.rb +++ b/spec/graphql/resolvers/project_jobs_resolver_spec.rb @@ -85,6 +85,13 @@ expect(parsed_response).to match_array([a_graphql_entity_for(pending_build)]) end end + + context 'with bridge' do + let_it_be(:bridge_job) { create(:ci_bridge, :success, name: 'Bridge Job', pipeline: pipeline) } + + it { is_expected.to include(successful_build, pending_build) } + it { is_expected.not_to include(bridge_job) } + end end context 'when filtering by build name' do @@ -131,6 +138,65 @@ end end end + + context 'with bridge' do + let_it_be(:bridge_with_same_name) { create(:ci_bridge, :success, name: 'Build Three', pipeline: pipeline) } + + it { is_expected.to include(failed_build, pending_build) } + it { is_expected.not_to include(bridge_with_same_name) } + end + end + + context 'when filtering by artifacts' do + let_it_be(:build_with_artifacts) { create(:ci_build, :success, :artifacts, name: 'Build With Artifacts', pipeline: pipeline) } + let_it_be(:build_without_artifacts) { create(:ci_build, :success, name: 'Build Without Artifacts', pipeline: pipeline) } + let_it_be(:bridge_job) { create(:ci_bridge, :success, name: 'Bridge Job', pipeline: pipeline) } + + context 'with with_artifacts: true' do + let(:args) { { with_artifacts: true } } + + it { is_expected.to include(build_with_artifacts) } + it { is_expected.not_to include(build_without_artifacts, bridge_job) } + end + + context 'with with_artifacts: false' do + let(:args) { { with_artifacts: false } } + + it { is_expected.to include(build_with_artifacts, build_without_artifacts, bridge_job) } + end + end + + context 'when returning both builds and bridges' do + let_it_be(:successful_bridge) { create(:ci_bridge, :success, name: 'Bridge Job', pipeline: pipeline) } + let_it_be(:failed_bridge) { create(:ci_bridge, :failed, name: 'Failed Bridge', pipeline: pipeline) } + + context 'without any filters' do + it { is_expected.to include(successful_build, successful_build_two, failed_build, pending_build, successful_bridge, failed_bridge) } + end + + context 'with status filter' do + let(:args) { { statuses: [Types::Ci::JobStatusEnum.coerce_isolated_input('SUCCESS')] } } + + it { is_expected.to include(successful_build, successful_build_two, successful_bridge) } + it { is_expected.not_to include(failed_build, pending_build, failed_bridge) } + end + end + + context 'when returning just builds' do + before do + stub_feature_flags(return_bridges_and_builds: false) + end + + context 'without any filters' do + it { is_expected.to include(successful_build, successful_build_two, failed_build, pending_build) } + end + + context 'with status filter' do + let(:args) { { statuses: [Types::Ci::JobStatusEnum.coerce_isolated_input('SUCCESS')] } } + + it { is_expected.to include(successful_build, successful_build_two) } + it { is_expected.not_to include(failed_build, pending_build) } + end end end diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index c9e06d98091b9082c2c6d05d9d6c8e5eb61a42be..896245ded90066be429f258f8f165eca0902d88c 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -573,6 +573,7 @@ container_repositories: - project - name project: +- bridges - instance_runner_monthly_usages - hosted_runner_monthly_usages - catalog_resource