diff --git a/app/models/ci/bridge.rb b/app/models/ci/bridge.rb index f1e514f4a45efb4801478fdd0a7167277d09a937..5183e149617a896e32dccd921d50981dd9d1c38c 100644 --- a/app/models/ci/bridge.rb +++ b/app/models/ci/bridge.rb @@ -90,7 +90,7 @@ def self.with_preloads def self.clone_accessors %i[pipeline project ref tag options name - allow_failure stage stage_idx + allow_failure stage_idx yaml_variables when environment description needs_attributes scheduling_type ci_stage partition_id].freeze end diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 100ea7be12f05b66c3f4b0ef4574cd5558dae8be..547eaae43ee9d08313c5c1e71ed7f7edf3e8c5bb 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -165,7 +165,7 @@ class Build < Ci::Processable scope :eager_load_for_archiving_trace, -> { preload(:project, :pending_state) } scope :eager_load_for_api, -> do preload( - :job_artifacts_archive, :job_artifacts, :runner, :tags, :runner_manager, :metadata, + :job_artifacts_archive, :ci_stage, :job_artifacts, :runner, :tags, :runner_manager, :metadata, pipeline: :project, user: [:user_preference, :user_detail, :followees] ) @@ -263,7 +263,7 @@ def with_preloads def clone_accessors %i[pipeline project ref tag options name - allow_failure stage stage_idx trigger_request + allow_failure stage_idx trigger_request yaml_variables when environment coverage_regex description tag_list protected needs_attributes job_variables_attributes resource_group scheduling_type diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index eb6462d50e1fa4635ad9e352cedf2ba00864a78e..06880ad3f2cf5fbc2f129fcab25f656bda814dcf 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -608,7 +608,7 @@ def uses_needs? end def stages_count - statuses.select(:stage).distinct.count + stages.count end def total_size @@ -624,8 +624,7 @@ def distinct_tags_count end def stages_names - statuses.order(:stage_idx).distinct - .pluck(:stage, :stage_idx).map(&:first) + stages.order(:position).pluck(:name) end def ref_exists? diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index 1f4af83c37dd15ede266101e0e89920a99c565b7..63962a5b557f89b18779470f85140836c4d1176a 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -9,6 +9,8 @@ class CommitStatus < Ci::ApplicationRecord include BulkInsertableAssociations include TaggableQueries + ignore_columns :stage, remove_with: '17.10', remove_after: '2025-03-14' + self.table_name = :p_ci_builds self.sequence_name = :ci_builds_id_seq self.primary_key = :id @@ -344,6 +346,9 @@ def stage_name ci_stage&.name end + # TODO: Temporary technical debt so we can ignore `stage`: https://gitlab.com/gitlab-org/gitlab/-/issues/507579 + alias_method :stage, :stage_name + # Handled only by ci_build def exit_code=(value); end diff --git a/app/services/ci/create_commit_status_service.rb b/app/services/ci/create_commit_status_service.rb index 21725efb82f0234e037adbc6e87a1895421b7126..3c8c0fa1a71f52282c76af7f3c6b31bc24eafd18 100644 --- a/app/services/ci/create_commit_status_service.rb +++ b/app/services/ci/create_commit_status_service.rb @@ -109,7 +109,6 @@ def find_or_build_external_commit_status protected: project.protected_for?(ref), ci_stage: stage, stage_idx: stage.position, - stage: 'external', partition_id: pipeline.partition_id ).tap do |new_commit_status| new_commit_status.assign_attributes(optional_commit_status_params) diff --git a/app/services/projects/update_pages_service.rb b/app/services/projects/update_pages_service.rb index 9b25c891a1cdac0e8697c68ddc71f2b51c8c150e..88055b66e160a08093b612f3c7654fd72205a80b 100644 --- a/app/services/projects/update_pages_service.rb +++ b/app/services/projects/update_pages_service.rb @@ -77,7 +77,6 @@ def commit_status user: build.user, ci_stage: stage, name: 'pages:deploy', - stage: 'deploy', stage_idx: stage.position ) end diff --git a/ee/app/services/click_house/data_ingestion/ci_finished_builds_sync_service.rb b/ee/app/services/click_house/data_ingestion/ci_finished_builds_sync_service.rb index 79c60b8553be7bf1464f4211a7c2ffb93962e3f1..b4390e5ca5cd6faf7d794fd12459646c9a9cd8d7 100644 --- a/ee/app/services/click_house/data_ingestion/ci_finished_builds_sync_service.rb +++ b/ee/app/services/click_house/data_ingestion/ci_finished_builds_sync_service.rb @@ -118,7 +118,8 @@ def yield_builds(events_batch, records_yielder) build_ids = events_batch.to_a.pluck(:build_id) # rubocop: disable CodeReuse/ActiveRecord Ci::Build.id_in(build_ids) - .left_outer_joins(:runner_manager, runner: :owner_runner_namespace, project_mirror: :namespace_mirror) + .left_outer_joins(:runner_manager, runner: :owner_runner_namespace, + project_mirror: :namespace_mirror) .select(:finished_at, *finished_build_projections) .each { |build| records_yielder << build } @@ -138,7 +139,7 @@ def finished_build_projections end strong_memoize_attr :finished_build_projections - BUILD_FIELD_NAMES = %i[id project_id pipeline_id status name stage runner_id].freeze + BUILD_FIELD_NAMES = %i[id project_id pipeline_id status name runner_id].freeze BUILD_EPOCH_FIELD_NAMES = %i[created_at queued_at started_at finished_at].freeze BUILD_COMPUTED_FIELD_NAMES = %i[root_namespace_id runner_owner_namespace_id].freeze RUNNER_FIELD_NAMES = %i[run_untagged type].freeze diff --git a/ee/spec/models/ci/build_spec.rb b/ee/spec/models/ci/build_spec.rb index 07a9bcffad6a737eaa3153620c70e2c17272c763..b3587671b1d1659149498585be6c1c8436c0f243 100644 --- a/ee/spec/models/ci/build_spec.rb +++ b/ee/spec/models/ci/build_spec.rb @@ -19,7 +19,7 @@ ) end - let(:stage) { build(:ci_stage) } + let(:stage) { create(:ci_stage) } let(:job) { create(:ci_build, pipeline: pipeline) } let(:artifact) { create(:ee_ci_job_artifact, :sast, job: job, project: job.project) } let_it_be(:valid_secrets) do @@ -328,24 +328,32 @@ stub_pages_setting(enabled: true) end - where(:options, :ci_pages_hostname_value, :ci_pages_url_value) do - { pages: {} } | 'group1.example.com' | 'http://group1.example.com/project-1' - { pages: { path_prefix: 'foo' } } | 'group1.example.com' | 'http://group1.example.com/project-1/foo' - { pages: { path_prefix: nil } } | 'group1.example.com' | 'http://group1.example.com/project-1' - { pages: { path_prefix: '$CI_COMMIT_BRANCH' } } | 'group1.example.com' | 'http://group1.example.com/project-1/master' - end + it "includes CI_PAGES_* variables" do + build1 = create(:ci_build, pipeline: pipeline, options: { pages: {} }) + build2 = create(:ci_build, pipeline: pipeline, options: { pages: { path_prefix: 'foo' } }) + build3 = create(:ci_build, pipeline: pipeline, options: { pages: { path_prefix: nil } }) + build4 = create(:ci_build, pipeline: pipeline, options: { pages: { path_prefix: '$CI_COMMIT_BRANCH' } }) - with_them do - let(:job) do - create(:ci_build, pipeline: pipeline, options: options) - end + project_namespace, _, project_path = project.full_path.downcase.partition('/') + ci_pages_hostname = "#{project_namespace}.example.com" + ci_pages_url = "http://#{ci_pages_hostname}/#{project_path}" - it "includes CI_PAGES_* variables" do - expect(subject.to_runner_variables).to include( - { key: 'CI_PAGES_HOSTNAME', value: ci_pages_hostname_value, public: true, masked: false }, - { key: 'CI_PAGES_URL', value: ci_pages_url_value, public: true, masked: false } - ) - end + expect(build1.variables.to_runner_variables).to include( + { key: 'CI_PAGES_HOSTNAME', value: ci_pages_hostname, public: true, masked: false }, + { key: 'CI_PAGES_URL', value: ci_pages_url, public: true, masked: false } + ) + expect(build2.variables.to_runner_variables).to include( + { key: 'CI_PAGES_HOSTNAME', value: ci_pages_hostname, public: true, masked: false }, + { key: 'CI_PAGES_URL', value: "#{ci_pages_url}/foo", public: true, masked: false } + ) + expect(build3.variables.to_runner_variables).to include( + { key: 'CI_PAGES_HOSTNAME', value: ci_pages_hostname, public: true, masked: false }, + { key: 'CI_PAGES_URL', value: ci_pages_url, public: true, masked: false } + ) + expect(build4.variables.to_runner_variables).to include( + { key: 'CI_PAGES_HOSTNAME', value: ci_pages_hostname, public: true, masked: false }, + { key: 'CI_PAGES_URL', value: "#{ci_pages_url}/master", public: true, masked: false } + ) end context 'when it is not a pages job' do diff --git a/ee/spec/services/click_house/data_ingestion/ci_finished_builds_sync_service_spec.rb b/ee/spec/services/click_house/data_ingestion/ci_finished_builds_sync_service_spec.rb index 259f95411ef7e37436dcef6f0fa65da85d08d851..b351ff8a86b793219fc1efd377f4f02caa5c8c8c 100644 --- a/ee/spec/services/click_house/data_ingestion/ci_finished_builds_sync_service_spec.rb +++ b/ee/spec/services/click_house/data_ingestion/ci_finished_builds_sync_service_spec.rb @@ -19,7 +19,7 @@ let_it_be(:build1) { create(:ci_build, :success, runner_manager: runner_manager1) } let_it_be(:build2) { create(:ci_build, :canceled) } - let_it_be(:build3) { create(:ci_build, :failed, runner: group_runner, stage: 'test') } + let_it_be(:build3) { create(:ci_build, :failed, runner: group_runner) } let_it_be(:build4) { create(:ci_build, :pending) } before_all do @@ -255,7 +255,7 @@ def ci_finished_builds .map(&:symbolize_keys) end - # rubocop:disable Metrics/CyclomaticComplexity -- the method is straightforward, just a lot of || + # -- the method is straightforward, just a lot of || def expected_build_attributes(build) runner = build.runner runner_manager = build.runner_manager @@ -268,7 +268,7 @@ def expected_build_attributes(build) finished_at: a_value_within(1.second).of(build.finished_at), date: build.finished_at.beginning_of_month, name: build.name || '', - stage: build.stage || '', + stage: '', root_namespace_id: build.project.root_namespace.id, runner_id: runner&.id || 0, runner_type: Ci::Runner.runner_types.fetch(runner&.runner_type, 0), @@ -281,7 +281,6 @@ def expected_build_attributes(build) runner_manager_architecture: runner_manager&.architecture || '' } end - # rubocop:enable Metrics/CyclomaticComplexity def contain_exactly_builds(*builds) expected_builds = builds.map do |build| diff --git a/ee/spec/services/ee/ci/create_pipeline_service_spec.rb b/ee/spec/services/ee/ci/create_pipeline_service_spec.rb index a08ad6142a41d0bc098f36d40301196e22359e48..c3e50949005a5fa7e2634d86f3c5f8176d07bc79 100644 --- a/ee/spec/services/ee/ci/create_pipeline_service_spec.rb +++ b/ee/spec/services/ee/ci/create_pipeline_service_spec.rb @@ -78,7 +78,7 @@ expect(pipeline).to be_persisted expect(test).to be_a Ci::Build expect(bridge).to be_a Ci::Bridge - expect(bridge.stage).to eq 'deploy' + expect(bridge.ci_stage.name).to eq 'deploy' expect(pipeline.statuses).to match_array [test, bridge] expect(bridge.options).to eq(trigger: { project: 'my/project' }) expect(bridge.yaml_variables) diff --git a/lib/api/ci/pipelines.rb b/lib/api/ci/pipelines.rb index 2ddb63fbbd8b2a7cb0cb9b5d9f4cc5e1c9c9fa7c..267798dd80d0ca29e4d7682e5a260ba8ab16c120 100644 --- a/lib/api/ci/pipelines.rb +++ b/lib/api/ci/pipelines.rb @@ -163,7 +163,7 @@ class Pipelines < ::API::Base .new(current_user: current_user, pipeline: pipeline, params: params) .execute - builds = builds.with_preloads.preload(:metadata, :runner_manager) # rubocop:disable CodeReuse/ActiveRecord -- preload job.archived? + builds = builds.with_preloads.preload(:metadata, :runner_manager, :ci_stage) # rubocop:disable CodeReuse/ActiveRecord -- preload job.archived? present paginate(builds), with: Entities::Ci::Job end @@ -191,7 +191,9 @@ class Pipelines < ::API::Base bridges = ::Ci::JobsFinder .new(current_user: current_user, pipeline: pipeline, params: params, type: ::Ci::Bridge) .execute - bridges = bridges.with_preloads + # rubocop:disable CodeReuse/ActiveRecord -- Preload is only related to this endpoint + bridges = bridges.with_preloads.preload(:ci_stage) + # rubocop:enable CodeReuse/ActiveRecord present paginate(bridges), with: Entities::Ci::Bridge end diff --git a/lib/api/ci/runners.rb b/lib/api/ci/runners.rb index 40fee71e328ea576f3fc8c08261e63f72f9dbce9..4c7ec486c91dd5382ac29db714f592e8cb9051a0 100644 --- a/lib/api/ci/runners.rb +++ b/lib/api/ci/runners.rb @@ -98,6 +98,7 @@ def authenticate_list_runners_jobs!(runner) def preload_job_associations(jobs) jobs.preload( # rubocop: disable CodeReuse/ActiveRecord -- this preload is tightly related to the endpoint + :ci_stage, :user, { pipeline: { project: [:route, { namespace: :route }] } }, { project: [:route, { namespace: :route }, :ci_cd_settings] } diff --git a/lib/gitlab/ci/pipeline/seed/build.rb b/lib/gitlab/ci/pipeline/seed/build.rb index 5d472850f23a34dbc8f206b43333182cf011b565..117acbf4943cdcf7279f05a6b3a519f77d3c4805 100644 --- a/lib/gitlab/ci/pipeline/seed/build.rb +++ b/lib/gitlab/ci/pipeline/seed/build.rb @@ -73,6 +73,7 @@ def attributes .deep_merge(runner_tags) .deep_merge(build_execution_config_attribute) .deep_merge(scoped_user_id_attribute) + .except(:stage) end def bridge? diff --git a/lib/gitlab/import_export/project/import_export.yml b/lib/gitlab/import_export/project/import_export.yml index 87f6ce89ac29d48f43405cb2767bead4dd0d9ec8..3b9c620c257d1d547d47ef37f9066db839850d29 100644 --- a/lib/gitlab/import_export/project/import_export.yml +++ b/lib/gitlab/import_export/project/import_export.yml @@ -550,7 +550,6 @@ included_attributes: - :name - :options - :allow_failure - - :stage - :stage_idx - :tag - :ref diff --git a/spec/factories/ci/processable.rb b/spec/factories/ci/processable.rb index 221abcadc459113823dc1e7847915f2faf9a8a63..d5a149b670cee06112638acdd5fb94794082f4fd 100644 --- a/spec/factories/ci/processable.rb +++ b/spec/factories/ci/processable.rb @@ -27,7 +27,6 @@ after(:build) do |processable, evaluator| next if processable.ci_stage - processable.stage = evaluator.stage pipeline = processable.pipeline existing_stage = diff --git a/spec/factories/commit_statuses.rb b/spec/factories/commit_statuses.rb index bf436c1ceb28a91cebffd030d4b084418357c84d..04c0ba97ecaa0864a13827ca66a6c1475ed39bb0 100644 --- a/spec/factories/commit_statuses.rb +++ b/spec/factories/commit_statuses.rb @@ -10,7 +10,10 @@ started_at { 'Tue, 26 Jan 2016 08:21:42 +0100' } finished_at { 'Tue, 26 Jan 2016 08:23:42 +0100' } partition_id { pipeline&.partition_id } - stage { 'test' } + + transient do + stage { 'test' } + end before(:create) do |commit_status, evaluator| next if commit_status.ci_stage diff --git a/spec/features/projects/commit/mini_pipeline_graph_spec.rb b/spec/features/projects/commit/mini_pipeline_graph_spec.rb index aad9b76e0495aae6fb54e27eda2c6e8c7c3ea986..7f5e34c11507f8301754cfd6ed72f7ae5af5ac87 100644 --- a/spec/features/projects/commit/mini_pipeline_graph_spec.rb +++ b/spec/features/projects/commit/mini_pipeline_graph_spec.rb @@ -16,7 +16,8 @@ ) end - let(:build) { create(:ci_build, pipeline: pipeline, status: :running) } + let(:ci_stage) { create(:ci_stage, pipeline: pipeline, name: 'test') } + let(:build) { create(:ci_build, pipeline: pipeline, status: :running, ci_stage: ci_stage) } before do build.run diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 63346caa1e1c257ec590a69384284324ad9cf9b7..a0a2155647d3545d3313e5e62644a0fb37ca1bb4 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -558,6 +558,7 @@ it { expect(eager_load_for_api.last.association(:job_artifacts)).to be_loaded } it { expect(eager_load_for_api.last.association(:runner)).to be_loaded } it { expect(eager_load_for_api.last.association(:tags)).to be_loaded } + it { expect(eager_load_for_api.last.association(:ci_stage)).to be_loaded } it { expect(eager_load_for_api.last.association(:pipeline)).to be_loaded } it { expect(eager_load_for_api.last.pipeline.association(:project)).to be_loaded } end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index d56a124a22d02e26d0f35afc7bb5141df5c3f32e..afaef2c97223c3cadb04118144311a46f1cc2f78 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -1452,89 +1452,55 @@ def create_build(name, status) describe 'pipeline stages' do let(:pipeline) { build(:ci_empty_pipeline, :created) } - describe 'legacy stages' do - before do - create( - :commit_status, - pipeline: pipeline, - stage: 'build', - name: 'linux', - stage_idx: 0, - status: 'success' - ) - - create( - :commit_status, - pipeline: pipeline, - stage: 'build', - name: 'mac', - stage_idx: 0, - status: 'failed' - ) - - create( - :commit_status, - pipeline: pipeline, - stage: 'deploy', - name: 'staging', - stage_idx: 2, - status: 'running' - ) - - create( - :commit_status, - pipeline: pipeline, - stage: 'test', - name: 'rspec', - stage_idx: 1, - status: 'success' - ) - end + before do + create(:ci_stage, name: 'test', pipeline: pipeline, position: 1) + create(:ci_stage, name: 'build', pipeline: pipeline, position: 0) + create(:ci_stage, name: 'deploy', pipeline: pipeline, position: 2) + end - describe '#stages_count' do - it 'returns a valid number of stages' do - expect(pipeline.stages_count).to eq(3) - end + describe '#stages_count' do + it 'returns a valid number of stages' do + expect(pipeline.stages_count).to eq(3) end + end - describe '#stages_names' do - it 'returns a valid names of stages' do - expect(pipeline.stages_names).to eq(%w[build test deploy]) - end + describe '#stages_names' do + it 'returns a valid names of stages' do + expect(pipeline.stages_names).to eq(%w[build test deploy]) end end + end - describe '#stages' do - let(:pipeline) { build(:ci_empty_pipeline, :created) } + describe '#stages' do + let(:pipeline) { build(:ci_empty_pipeline, :created) } - before do - create(:ci_stage, project: project, pipeline: pipeline, position: 4, name: 'deploy') - create(:ci_build, project: project, pipeline: pipeline, stage: 'test', stage_idx: 3, name: 'test') - create(:ci_build, project: project, pipeline: pipeline, stage: 'build', stage_idx: 2, name: 'build') - create(:ci_stage, project: project, pipeline: pipeline, position: 1, name: 'sanity') - create(:ci_stage, project: project, pipeline: pipeline, position: 5, name: 'cleanup') - end + before do + create(:ci_stage, project: project, pipeline: pipeline, position: 4, name: 'deploy') + create(:ci_build, project: project, pipeline: pipeline, stage: 'test', stage_idx: 3, name: 'test') + create(:ci_build, project: project, pipeline: pipeline, stage: 'build', stage_idx: 2, name: 'build') + create(:ci_stage, project: project, pipeline: pipeline, position: 1, name: 'sanity') + create(:ci_stage, project: project, pipeline: pipeline, position: 5, name: 'cleanup') + end - subject { pipeline.stages } + subject { pipeline.stages } - context 'when pipelines is not complete' do - it 'returns stages in valid order' do - expect(subject).to all(be_a Ci::Stage) - expect(subject.map(&:name)) - .to eq %w[sanity build test deploy cleanup] - end + context 'when pipelines is not complete' do + it 'returns stages in valid order' do + expect(subject).to all(be_a Ci::Stage) + expect(subject.map(&:name)) + .to eq %w[sanity build test deploy cleanup] end + end - context 'when pipeline is complete' do - before do - pipeline.succeed! - end + context 'when pipeline is complete' do + before do + pipeline.succeed! + end - it 'returns stages in valid order' do - expect(subject).to all(be_a Ci::Stage) - expect(subject.map(&:name)) - .to eq %w[sanity build test deploy cleanup] - end + it 'returns stages in valid order' do + expect(subject).to all(be_a Ci::Stage) + expect(subject.map(&:name)) + .to eq %w[sanity build test deploy cleanup] end end end diff --git a/spec/models/ci/processable_spec.rb b/spec/models/ci/processable_spec.rb index 6ed20fcee71045d435d7649d6b7e2e728b7f705b..b705a3dfd4d97b3088abb0af453c42d0b86b1e5b 100644 --- a/spec/models/ci/processable_spec.rb +++ b/spec/models/ci/processable_spec.rb @@ -52,7 +52,7 @@ create( :ci_build, :failed, :picked, :expired, :erased, :queued, :coverage, :tags, :allowed_to_fail, :on_tag, :triggered, :teardown_environment, :resource_group, - description: 'my-job', stage: 'test', stage_id: stage.id, + description: 'my-job', stage_id: stage.id, pipeline: pipeline, auto_canceled_by: another_pipeline, scheduled_at: 10.seconds.since ) @@ -61,7 +61,7 @@ let_it_be(:internal_job_variable) { create(:ci_job_variable, job: processable) } let(:clone_accessors) do - %i[pipeline project ref tag options name allow_failure stage stage_idx trigger_request yaml_variables + %i[pipeline project ref tag options name allow_failure stage_idx trigger_request yaml_variables when environment coverage_regex description tag_list protected needs_attributes job_variables_attributes resource_group scheduling_type ci_stage partition_id id_tokens interruptible] end @@ -121,7 +121,7 @@ shared_examples_for 'clones the processable' do before_all do - processable.assign_attributes(stage: 'test', stage_id: stage.id, interruptible: true) + processable.assign_attributes(stage_id: stage.id, interruptible: true) processable.save! create(:ci_build_need, build: processable)