From 2c2bd00a6c2f4484ac7ba522b8d6b5fb64304a42 Mon Sep 17 00:00:00 2001 From: mfluharty Date: Thu, 18 Apr 2019 17:47:03 -0600 Subject: [PATCH 1/4] Make canceled jobs not retryable See which tests break --- app/models/ci/build.rb | 2 +- ...anceling-the-pipeline-and-manually-running-later-jobs.yml | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/53064-bypassing-pipeline-jobs-by-canceling-the-pipeline-and-manually-running-later-jobs.yml diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 5cf9bb4979a..8db2b05de9a 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -353,7 +353,7 @@ module Ci end def retryable? - !archived? && (success? || failed? || canceled?) + !archived? && (success? || failed?) end def retries_count diff --git a/changelogs/unreleased/53064-bypassing-pipeline-jobs-by-canceling-the-pipeline-and-manually-running-later-jobs.yml b/changelogs/unreleased/53064-bypassing-pipeline-jobs-by-canceling-the-pipeline-and-manually-running-later-jobs.yml new file mode 100644 index 00000000000..48f0a668982 --- /dev/null +++ b/changelogs/unreleased/53064-bypassing-pipeline-jobs-by-canceling-the-pipeline-and-manually-running-later-jobs.yml @@ -0,0 +1,5 @@ +--- +title: Make canceled jobs not retryable +merge_request: 27503 +author: +type: changed -- GitLab From 6b7b07b7ec0bc434716236d18d34c9a4a4d2f2f4 Mon Sep 17 00:00:00 2001 From: mfluharty Date: Fri, 19 Apr 2019 13:19:59 -0600 Subject: [PATCH 2/4] Update specs that use retryable canceled jobs Specs that test canceled jobs now - expect them not to be retryable or playable - expect them not to show retry buttons Specs that test retryability now - use failed status instead of canceled status --- spec/features/projects/jobs_spec.rb | 6 +++--- spec/features/projects/pipelines/pipeline_spec.rb | 8 ++++---- spec/lib/gitlab/ci/status/build/factory_spec.rb | 8 ++++---- spec/models/ci/build_spec.rb | 4 ++-- spec/requests/api/jobs_spec.rb | 4 ++-- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/spec/features/projects/jobs_spec.rb b/spec/features/projects/jobs_spec.rb index 224375daf71..9cf04fe13b4 100644 --- a/spec/features/projects/jobs_spec.rb +++ b/spec/features/projects/jobs_spec.rb @@ -936,8 +936,8 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do find('.js-cancel-job').click end - it 'loads the page and shows all needed controls' do - expect(page).to have_content 'Retry' + it 'loads the page and shows no controls' do + expect(page).not_to have_content 'Retry' end end end @@ -946,7 +946,7 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do context "Job from project", :js do before do job.run! - job.cancel! + job.drop!(:script_failure) visit project_job_path(project, job) wait_for_requests diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb index cf334e1e4da..0ea82ba9363 100644 --- a/spec/features/projects/pipelines/pipeline_spec.rb +++ b/spec/features/projects/pipelines/pipeline_spec.rb @@ -115,11 +115,11 @@ describe 'Pipeline', :js do end end - it 'cancels the running build and shows retry button' do + it 'cancels the running build and does not show retry button' do find('#ci-badge-deploy .ci-action-icon-container').click page.within('#ci-badge-deploy') do - expect(page).to have_css('.js-icon-retry') + expect(page).not_to have_css('.js-icon-retry') end end end @@ -133,11 +133,11 @@ describe 'Pipeline', :js do end end - it 'cancels the preparing build and shows retry button' do + it 'cancels the preparing build and does not show retry button' do find('#ci-badge-deploy .ci-action-icon-container').click page.within('#ci-badge-deploy') do - expect(page).to have_css('.js-icon-retry') + expect(page).not_to have_css('.js-icon-retry') end end end diff --git a/spec/lib/gitlab/ci/status/build/factory_spec.rb b/spec/lib/gitlab/ci/status/build/factory_spec.rb index b6231510b91..025439f1b6e 100644 --- a/spec/lib/gitlab/ci/status/build/factory_spec.rb +++ b/spec/lib/gitlab/ci/status/build/factory_spec.rb @@ -163,11 +163,11 @@ describe Gitlab::Ci::Status::Build::Factory do it 'matches correct extended statuses' do expect(factory.extended_statuses) - .to eq [Gitlab::Ci::Status::Build::Canceled, Gitlab::Ci::Status::Build::Retryable] + .to eq [Gitlab::Ci::Status::Build::Canceled] end - it 'fabricates a retryable build status' do - expect(status).to be_a Gitlab::Ci::Status::Build::Retryable + it 'does not fabricate a retryable build status' do + expect(status).not_to be_a Gitlab::Ci::Status::Build::Retryable end it 'fabricates status with correct details' do @@ -177,7 +177,7 @@ describe Gitlab::Ci::Status::Build::Factory do expect(status.illustration).to include(:image, :size, :title) expect(status.label).to eq 'canceled' expect(status).to have_details - expect(status).to have_action + expect(status).not_to have_action end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 66be192ab21..7f53b7e7efc 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1425,7 +1425,7 @@ describe Ci::Build do build.cancel! end - it { is_expected.to be_retryable } + it { is_expected.not_to be_retryable } end end @@ -1955,7 +1955,7 @@ describe Ci::Build do context 'when build has been canceled' do subject { build_stubbed(:ci_build, :manual, status: :canceled) } - it { is_expected.to be_playable } + it { is_expected.not_to be_playable } end context 'when build is successful' do diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb index ed2ef4c730b..c14507de186 100644 --- a/spec/requests/api/jobs_spec.rb +++ b/spec/requests/api/jobs_spec.rb @@ -863,7 +863,7 @@ describe API::Jobs do end describe 'POST /projects/:id/jobs/:job_id/retry' do - let(:job) { create(:ci_build, :canceled, pipeline: pipeline) } + let(:job) { create(:ci_build, :failed, pipeline: pipeline) } before do post api("/projects/#{project.id}/jobs/#{job.id}/retry", api_user) @@ -873,7 +873,7 @@ describe API::Jobs do context 'user with :update_build permission' do it 'retries non-running job' do expect(response).to have_gitlab_http_status(201) - expect(project.builds.first.status).to eq('canceled') + expect(project.builds.first.status).to eq('failed') expect(json_response['status']).to eq('pending') end end -- GitLab From 9c6ef1db259abdb7313817c26182282874621f52 Mon Sep 17 00:00:00 2001 From: mfluharty Date: Tue, 23 Apr 2019 20:28:31 -0600 Subject: [PATCH 3/4] Update documentation to explain job retryability --- doc/ci/pipelines.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/doc/ci/pipelines.md b/doc/ci/pipelines.md index 2ffa3d4edc7..cb1fd43e179 100644 --- a/doc/ci/pipelines.md +++ b/doc/ci/pipelines.md @@ -266,6 +266,11 @@ Clicking on an individual job will show you its job trace, and allow you to: - Retry the job. - Erase the job trace. +>**Note:** +Once a job has been canceled, it cannot be retried in isolation. +This prevents jobs from being bypassed or run out of order. +To retry a canceled job, retry the pipeline it belongs to. + ### Seeing the failure reason for jobs > [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/17782) in GitLab 10.7. -- GitLab From d6cf6fd754842d67fe4bcc39e35a9a298ac60091 Mon Sep 17 00:00:00 2001 From: Miranda Fluharty Date: Wed, 1 May 2019 15:36:16 +0000 Subject: [PATCH 4/4] Apply suggestion to doc/ci/pipelines.md --- doc/ci/pipelines.md | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/doc/ci/pipelines.md b/doc/ci/pipelines.md index cb1fd43e179..07129eb4186 100644 --- a/doc/ci/pipelines.md +++ b/doc/ci/pipelines.md @@ -266,10 +266,8 @@ Clicking on an individual job will show you its job trace, and allow you to: - Retry the job. - Erase the job trace. ->**Note:** -Once a job has been canceled, it cannot be retried in isolation. -This prevents jobs from being bypassed or run out of order. -To retry a canceled job, retry the pipeline it belongs to. +NOTE: **Note:** +To prevent jobs from being bypassed or run out of order, canceled jobs can only be retried when the whole pipeline they belong to is retried. ### Seeing the failure reason for jobs -- GitLab