diff --git a/.gitlab/ci/frontend.gitlab-ci.yml b/.gitlab/ci/frontend.gitlab-ci.yml index 45a6a177943a954a47b3b091d7153df8f1c6d544..b3e7c85385f36f3b48ef62dd0e55910871ada3d1 100644 --- a/.gitlab/ci/frontend.gitlab-ci.yml +++ b/.gitlab/ci/frontend.gitlab-ci.yml @@ -19,7 +19,7 @@ dependencies: - setup-test-env services: - - docker:stable-dind + - docker:19.03.0-dind variables: NODE_ENV: "production" RAILS_ENV: "production" diff --git a/.gitlab/ci/review.gitlab-ci.yml b/.gitlab/ci/review.gitlab-ci.yml index 41d52c4e0954cc5dde2ea1e00fabf97d95af30a1..e3a77574d788a0c14c926a7e1a82485ff3d33624 100644 --- a/.gitlab/ci/review.gitlab-ci.yml +++ b/.gitlab/ci/review.gitlab-ci.yml @@ -35,7 +35,7 @@ <<: *review-base image: registry.gitlab.com/gitlab-org/gitlab-build-images:gitlab-qa-alpine services: - - docker:stable-dind + - docker:19.03.0-dind tags: - gitlab-org - docker @@ -261,6 +261,7 @@ danger-review: except: refs: - master + - /^[\d-]+-stable(-ee)?$/ variables: - $CI_COMMIT_REF_NAME =~ /^ce-to-ee-.*/ - $CI_COMMIT_REF_NAME =~ /.*-stable(-ee)?-prepare-.*/ diff --git a/app/assets/stylesheets/pages/builds.scss b/app/assets/stylesheets/pages/builds.scss index 6e98908eeeda68964f31fb13a7d5b8c875839ade..262c0bf5ed22f0dfefa435681c870f28141ccbd3 100644 --- a/app/assets/stylesheets/pages/builds.scss +++ b/app/assets/stylesheets/pages/builds.scss @@ -127,6 +127,7 @@ .section-header ~ .section.line { margin-left: $gl-padding; + display: block; } } diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 68e6e48fb7d6d5a16af473cae7dc9c37a123ad10..d800364e5446b20b294d7a160a0a743a8729e2d6 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -752,7 +752,7 @@ class MergeRequest < ApplicationRecord end def check_mergeability - MergeRequests::MergeabilityCheckService.new(self).execute + MergeRequests::MergeabilityCheckService.new(self).execute(retry_lease: false) end # rubocop: enable CodeReuse/ServiceClass diff --git a/app/services/merge_requests/mergeability_check_service.rb b/app/services/merge_requests/mergeability_check_service.rb index 9fa50c9448fbdf3606d9a955e7dfd2e5cd241b2c..962e2327b3ed10b22a346e36dd3cfbc0d9853e5c 100644 --- a/app/services/merge_requests/mergeability_check_service.rb +++ b/app/services/merge_requests/mergeability_check_service.rb @@ -3,6 +3,7 @@ module MergeRequests class MergeabilityCheckService < ::BaseService include Gitlab::Utils::StrongMemoize + include Gitlab::ExclusiveLeaseHelpers delegate :project, to: :@merge_request delegate :repository, to: :project @@ -21,13 +22,35 @@ module MergeRequests # where we need the current state of the merge ref in repository, the `recheck` # argument is required. # + # retry_lease - Concurrent calls wait for at least 10 seconds until the + # lease is granted (other process finishes running). Returns an error + # ServiceResponse if the lease is not granted during this time. + # # Returns a ServiceResponse indicating merge_status is/became can_be_merged # and the merge-ref is synced. Success in case of being/becoming mergeable, # error otherwise. - def execute(recheck: false) + def execute(recheck: false, retry_lease: true) return ServiceResponse.error(message: 'Invalid argument') unless merge_request return ServiceResponse.error(message: 'Unsupported operation') if Gitlab::Database.read_only? + return check_mergeability(recheck) unless merge_ref_auto_sync_lock_enabled? + + in_write_lock(retry_lease: retry_lease) do |retried| + # When multiple calls are waiting for the same lock (retry_lease), + # it's possible that when granted, the MR status was already updated for + # that object, therefore we reset if there was a lease retry. + merge_request.reset if retried + + check_mergeability(recheck) + end + rescue FailedToObtainLockError => error + ServiceResponse.error(message: error.message) + end + + private + + attr_reader :merge_request + def check_mergeability(recheck) recheck! if recheck update_merge_status @@ -46,9 +69,21 @@ module MergeRequests ServiceResponse.success(payload: payload) end - private + # It's possible for this service to send concurrent requests to Gitaly in order + # to "git update-ref" the same ref. Therefore we handle a light exclusive + # lease here. + # + def in_write_lock(retry_lease:, &block) + lease_key = "mergeability_check:#{merge_request.id}" - attr_reader :merge_request + lease_opts = { + ttl: 1.minute, + retries: retry_lease ? 10 : 0, + sleep_sec: retry_lease ? 1.second : 0 + } + + in_lock(lease_key, lease_opts, &block) + end def payload strong_memoize(:payload) do @@ -116,5 +151,9 @@ module MergeRequests def merge_ref_auto_sync_enabled? Feature.enabled?(:merge_ref_auto_sync, project, default_enabled: true) end + + def merge_ref_auto_sync_lock_enabled? + Feature.enabled?(:merge_ref_auto_sync_lock, project, default_enabled: true) + end end end diff --git a/app/views/shared/projects/_project.html.haml b/app/views/shared/projects/_project.html.haml index 71bd932059337653895b83cfef4d92fe0fe95987..5174472927559d6aa4bc35d46d2a2bdf9b71611c 100644 --- a/app/views/shared/projects/_project.html.haml +++ b/app/views/shared/projects/_project.html.haml @@ -89,4 +89,4 @@ %span.icon-wrapper.pipeline-status = render 'ci/status/icon', status: project.commit.last_pipeline.detailed_status(current_user), type: 'commit', tooltip_placement: 'top', path: pipeline_path .updated-note - %span Updated #{updated_tooltip} + %span #{_('Updated')} #{updated_tooltip} diff --git a/changelogs/unreleased/fix-i18n-updated-projects.yml b/changelogs/unreleased/fix-i18n-updated-projects.yml new file mode 100644 index 0000000000000000000000000000000000000000..408ee4384805c7f4a6da6bc29cdfd8369fc2a77f --- /dev/null +++ b/changelogs/unreleased/fix-i18n-updated-projects.yml @@ -0,0 +1,5 @@ +--- +title: Properly translate term in projects list +merge_request: 30958 +author: +type: fixed diff --git a/changelogs/unreleased/leipert-improve-ansi2html.yml b/changelogs/unreleased/leipert-improve-ansi2html.yml new file mode 100644 index 0000000000000000000000000000000000000000..dd3582b34340bf3b250e8e44430def450579a19c --- /dev/null +++ b/changelogs/unreleased/leipert-improve-ansi2html.yml @@ -0,0 +1,5 @@ +--- +title: Improve job log rendering performance +merge_request: 31262 +author: +type: performance diff --git a/changelogs/unreleased/osw-avoid-errors-due-to-concurrent-calls.yml b/changelogs/unreleased/osw-avoid-errors-due-to-concurrent-calls.yml new file mode 100644 index 0000000000000000000000000000000000000000..17ff1b012cf40809b99e0abb74fcc3acb912e91d --- /dev/null +++ b/changelogs/unreleased/osw-avoid-errors-due-to-concurrent-calls.yml @@ -0,0 +1,5 @@ +--- +title: Add exclusive lease to mergeability check process +merge_request: 31082 +author: +type: fixed diff --git a/changelogs/unreleased/patch-72.yml b/changelogs/unreleased/patch-72.yml new file mode 100644 index 0000000000000000000000000000000000000000..ff2bac2fc29004b064b2dec2f2bfbf308c71b63b --- /dev/null +++ b/changelogs/unreleased/patch-72.yml @@ -0,0 +1,5 @@ +--- +title: Fix Docker in Docker (DIND) listen port behavior change by adding DOCKER_TLS_CERTDIR in CI job templates. +merge_request: 31201 +author: Cameron Boulton +type: fixed diff --git a/doc/development/testing_guide/end_to_end/index.md b/doc/development/testing_guide/end_to_end/index.md index 2dc06ba10a5de4ff696bb34a8b4ec19c589b8bd6..882e2230636906cd821b6d39652de07338d17f4a 100644 --- a/doc/development/testing_guide/end_to_end/index.md +++ b/doc/development/testing_guide/end_to_end/index.md @@ -65,28 +65,24 @@ Below you can read more about how to use it and how does it work. Currently, we are using _multi-project pipeline_-like approach to run QA pipelines. -![QA on merge requests CI/CD architecture](../img/qa_on_merge_requests_cicd_architecture.png) - -
-Show mermaid source -
+```mermaid
 graph LR
     A1 -.->|1. Triggers an omnibus-gitlab pipeline and wait for it to be done| A2
-    B2[`Trigger-qa` stage
`Trigger:qa-test` job] -.->|2. Triggers a gitlab-qa pipeline and wait for it to be done| A3 + B2[`Trigger-qa` stage
`Trigger:qa-test` job] -.->|2. Triggers a gitlab-qa pipeline and wait for it to be done| A3 -subgraph gitlab-ce/ee pipeline - A1[`test` stage
`package-and-qa` job] +subgraph "gitlab-ce/ee pipeline" + A1[`test` stage
`package-and-qa` job] end -subgraph omnibus-gitlab pipeline - A2[`Trigger-docker` stage
`Trigger:gitlab-docker` job] -->|once done| B2 +subgraph "omnibus-gitlab pipeline" + A2[`Trigger-docker` stage
`Trigger:gitlab-docker` job] -->|once done| B2 end -subgraph gitlab-qa pipeline - A3>QA jobs run] -.->|3. Reports back the pipeline result to the `package-and-qa` job
and post the result on the original commit tested| A1 +subgraph "gitlab-qa pipeline" + A3>QA jobs run] -.->|3. Reports back the pipeline result to the `package-and-qa` job
and post the result on the original commit tested| A1 end -
-
+``` + 1. Developer triggers a manual action, that can be found in CE / EE merge requests. This starts a chain of pipelines in multiple projects. diff --git a/doc/development/testing_guide/img/qa_on_merge_requests_cicd_architecture.png b/doc/development/testing_guide/img/qa_on_merge_requests_cicd_architecture.png deleted file mode 100644 index 5b93a05db962e6b0f7dd8f16e06019799584a4f2..0000000000000000000000000000000000000000 Binary files a/doc/development/testing_guide/img/qa_on_merge_requests_cicd_architecture.png and /dev/null differ diff --git a/doc/development/testing_guide/img/review_apps_cicd_architecture.png b/doc/development/testing_guide/img/review_apps_cicd_architecture.png deleted file mode 100644 index 1ee28d3db917393109d9e9d874944d5c6599691b..0000000000000000000000000000000000000000 Binary files a/doc/development/testing_guide/img/review_apps_cicd_architecture.png and /dev/null differ diff --git a/doc/development/testing_guide/review_apps.md b/doc/development/testing_guide/review_apps.md index 96761622cfe5ffd6674324d0f001be531a6b5a20..d0cad7527b52163900ad0c96b40d69a02aeb7240 100644 --- a/doc/development/testing_guide/review_apps.md +++ b/doc/development/testing_guide/review_apps.md @@ -8,38 +8,33 @@ Review Apps are automatically deployed by each pipeline, both in ### CI/CD architecture diagram -![Review Apps CI/CD architecture](img/review_apps_cicd_architecture.png) - -
-Show mermaid source -
+```mermaid
 graph TD
     build-qa-image -.->|once the `prepare` stage is done| gitlab:assets:compile
     review-build-cng -->|triggers a CNG-mirror pipeline and wait for it to be done| CNG-mirror
     review-build-cng -.->|once the `test` stage is done| review-deploy
     review-deploy -.->|once the `review` stage is done| review-qa-smoke
 
-subgraph 1. gitlab-ce/ee `prepare` stage
+subgraph "1. gitlab-ce/ee `prepare` stage"
     build-qa-image
     end
 
-subgraph 2. gitlab-ce/ee `test` stage
+subgraph "2. gitlab-ce/ee `test` stage"
     gitlab:assets:compile -->|plays dependent job once done| review-build-cng
     end
 
-subgraph 3. gitlab-ce/ee `review` stage
-    review-deploy["review-deploy

Helm deploys the Review App using the Cloud
Native images built by the CNG-mirror pipeline.

Cloud Native images are deployed to the `review-apps-ce` or `review-apps-ee`
Kubernetes (GKE) cluster, in the GCP `gitlab-review-apps` project."] +subgraph "3. gitlab-ce/ee `review` stage" + review-deploy["review-deploy

Helm deploys the Review App using the Cloud
Native images built by the CNG-mirror pipeline.

Cloud Native images are deployed to the `review-apps-ce` or `review-apps-ee`
Kubernetes (GKE) cluster, in the GCP `gitlab-review-apps` project."] end -subgraph 4. gitlab-ce/ee `qa` stage - review-qa-smoke[review-qa-smoke

gitlab-qa runs the smoke suite against the Review App.] +subgraph "4. gitlab-ce/ee `qa` stage" + review-qa-smoke[review-qa-smoke

gitlab-qa runs the smoke suite against the Review App.] end -subgraph CNG-mirror pipeline +subgraph "CNG-mirror pipeline" CNG-mirror>Cloud Native images are built]; end -
-
+``` ### Detailed explanation diff --git a/doc/user/markdown.md b/doc/user/markdown.md index 96e74125801a0eb4411c2351fdb9a301735f2ee3..b83a466fd7f6b2a4f5cac3347ca57577d72020f1 100644 --- a/doc/user/markdown.md +++ b/doc/user/markdown.md @@ -185,6 +185,49 @@ graph TD; C-->D; ``` +#### Subgraphs + +NOTE: **Note:** GitLab 12.1 and up now [requires quotes around subgraph +titles that contain multiple words](https://github.com/knsv/mermaid/pull/845). + +Subgraphs can also be included: + +~~~ +```mermaid +graph TB + + SubGraph1 --> SubGraph1Flow + subgraph "SubGraph 1 Flow" + SubGraph1Flow(SubNode 1) + SubGraph1Flow -- Choice1 --> DoChoice1 + SubGraph1Flow -- Choice2 --> DoChoice2 + end + + subgraph "Main Graph" + Node1[Node 1] --> Node2[Node 2] + Node2 --> SubGraph1[Jump to SubGraph1] + SubGraph1 --> FinalThing[Final Thing] +end +``` +~~~ + +```mermaid +graph TB + + SubGraph1 --> SubGraph1Flow + subgraph "SubGraph 1 Flow" + SubGraph1Flow(SubNode 1) + SubGraph1Flow -- Choice1 --> DoChoice1 + SubGraph1Flow -- Choice2 --> DoChoice2 + end + + subgraph "Main Graph" + Node1[Node 1] --> Node2[Node 2] + Node2 --> SubGraph1[Jump to SubGraph1] + SubGraph1 --> FinalThing[Final Thing] +end +``` + ### Emoji > If this is not rendered correctly, [view it in GitLab itself](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/user/markdown.md#emoji). diff --git a/lib/gitlab/ci/ansi2html.rb b/lib/gitlab/ci/ansi2html.rb index fc3223e7442ed1a5e4603e107a02a9a1b7b89b41..7e348763e81bf9627112807800e39acba75408f1 100644 --- a/lib/gitlab/ci/ansi2html.rb +++ b/lib/gitlab/ci/ansi2html.rb @@ -194,16 +194,10 @@ module Gitlab end def handle_new_line - css_classes = [] - - if @sections.any? - css_classes = %w[section line] + sections.map { |section| "s_#{section}" } - end - write_in_tag %{
} - write_raw %{} if css_classes.any? + + close_open_tags if @sections.any? && @lineno_in_section == 0 @lineno_in_section += 1 - open_new_tag end def handle_section(scanner) @@ -310,11 +304,24 @@ module Gitlab if @sections.any? css_classes << "section" - css_classes << "js-section-header section-header" if @lineno_in_section == 0 + + css_classes << if @lineno_in_section == 0 + "js-section-header section-header" + else + "line" + end + css_classes += sections.map { |section| "js-s-#{section}" } end - @out << %{} + close_open_tags + + @out << if css_classes.any? + %{} + else + %{} + end + @n_open_tags += 1 end diff --git a/lib/gitlab/ci/templates/Security/Container-Scanning.gitlab-ci.yml b/lib/gitlab/ci/templates/Security/Container-Scanning.gitlab-ci.yml index 5ad624bb15f35b4128c8413ef88cf0189766388e..3e0061942366e6053b4729fb7bd93cd562977993 100644 --- a/lib/gitlab/ci/templates/Security/Container-Scanning.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Security/Container-Scanning.gitlab-ci.yml @@ -5,6 +5,7 @@ container_scanning: image: docker:stable variables: DOCKER_DRIVER: overlay2 + DOCKER_TLS_CERTDIR: "" # Defining two new variables based on GitLab's CI/CD predefined variables # https://docs.gitlab.com/ee/ci/variables/#predefined-environment-variables CI_APPLICATION_REPOSITORY: $CI_REGISTRY_IMAGE/$CI_COMMIT_REF_SLUG diff --git a/lib/gitlab/exclusive_lease_helpers.rb b/lib/gitlab/exclusive_lease_helpers.rb index 7961d4bbd6e328b102cb0a70989a2640c33d4ad7..61eb030563d267364f9a9d8c5be290a45f04cd4e 100644 --- a/lib/gitlab/exclusive_lease_helpers.rb +++ b/lib/gitlab/exclusive_lease_helpers.rb @@ -15,17 +15,18 @@ module Gitlab raise ArgumentError, 'Key needs to be specified' unless key lease = Gitlab::ExclusiveLease.new(key, timeout: ttl) + retried = false until uuid = lease.try_obtain # Keep trying until we obtain the lease. To prevent hammering Redis too # much we'll wait for a bit. sleep(sleep_sec) - break if (retries -= 1) < 0 + (retries -= 1) < 0 ? break : retried ||= true end raise FailedToObtainLockError, 'Failed to obtain a lock' unless uuid - yield + yield(retried) ensure Gitlab::ExclusiveLease.cancel(key, uuid) end diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index 901402aa5fd7be88a41adf19a730bdf7d0e6d8a0..71a14ac9da4c0c3192011c63ac29acf5a1051012 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -546,7 +546,7 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do expect(response).to have_gitlab_http_status(:ok) expect(json_response['id']).to eq job.id expect(json_response['status']).to eq job.status - expect(json_response['html']).to eq('BUILD TRACE') + expect(json_response['html']).to eq('BUILD TRACE') end end diff --git a/spec/lib/gitlab/ci/ansi2html_spec.rb b/spec/lib/gitlab/ci/ansi2html_spec.rb index 3d57ce431ab81016b93ca18cafc45c0ef24f9b58..679176f37440ac4aace4ee6ddbae1b205f983e36 100644 --- a/spec/lib/gitlab/ci/ansi2html_spec.rb +++ b/spec/lib/gitlab/ci/ansi2html_spec.rb @@ -4,11 +4,11 @@ describe Gitlab::Ci::Ansi2html do subject { described_class } it "prints non-ansi as-is" do - expect(convert_html("Hello")).to eq('Hello') + expect(convert_html("Hello")).to eq('Hello') end it "strips non-color-changing control sequences" do - expect(convert_html("Hello \e[2Kworld")).to eq('Hello world') + expect(convert_html("Hello \e[2Kworld")).to eq('Hello world') end it "prints simply red" do @@ -32,7 +32,7 @@ describe Gitlab::Ci::Ansi2html do end it "resets colors after red on blue" do - expect(convert_html("\e[31;44mHello\e[0m world")).to eq('Hello world') + expect(convert_html("\e[31;44mHello\e[0m world")).to eq('Hello world') end it "performs color change from red/blue to yellow/blue" do @@ -44,11 +44,11 @@ describe Gitlab::Ci::Ansi2html do end it "performs color change from red/blue to reset to yellow/green" do - expect(convert_html("\e[31;44mHello\e[0m \e[33;42mworld")).to eq('Hello world') + expect(convert_html("\e[31;44mHello\e[0m \e[33;42mworld")).to eq('Hello world') end it "ignores unsupported codes" do - expect(convert_html("\e[51mHello\e[0m")).to eq('Hello') + expect(convert_html("\e[51mHello\e[0m")).to eq('Hello') end it "prints light red" do @@ -72,8 +72,8 @@ describe Gitlab::Ci::Ansi2html do end it "resets bold text" do - expect(convert_html("\e[1mHello\e[21m world")).to eq('Hello world') - expect(convert_html("\e[1mHello\e[22m world")).to eq('Hello world') + expect(convert_html("\e[1mHello\e[21m world")).to eq('Hello world') + expect(convert_html("\e[1mHello\e[22m world")).to eq('Hello world') end it "prints italic text" do @@ -81,7 +81,7 @@ describe Gitlab::Ci::Ansi2html do end it "resets italic text" do - expect(convert_html("\e[3mHello\e[23m world")).to eq('Hello world') + expect(convert_html("\e[3mHello\e[23m world")).to eq('Hello world') end it "prints underlined text" do @@ -89,7 +89,7 @@ describe Gitlab::Ci::Ansi2html do end it "resets underlined text" do - expect(convert_html("\e[4mHello\e[24m world")).to eq('Hello world') + expect(convert_html("\e[4mHello\e[24m world")).to eq('Hello world') end it "prints concealed text" do @@ -97,7 +97,7 @@ describe Gitlab::Ci::Ansi2html do end it "resets concealed text" do - expect(convert_html("\e[8mHello\e[28m world")).to eq('Hello world') + expect(convert_html("\e[8mHello\e[28m world")).to eq('Hello world') end it "prints crossed-out text" do @@ -105,7 +105,7 @@ describe Gitlab::Ci::Ansi2html do end it "resets crossed-out text" do - expect(convert_html("\e[9mHello\e[29m world")).to eq('Hello world') + expect(convert_html("\e[9mHello\e[29m world")).to eq('Hello world') end it "can print 256 xterm fg colors" do @@ -137,15 +137,15 @@ describe Gitlab::Ci::Ansi2html do end it "prints <" do - expect(convert_html("<")).to eq('<') + expect(convert_html("<")).to eq('<') end it "replaces newlines with line break tags" do - expect(convert_html("\n")).to eq('
') + expect(convert_html("\n")).to eq('
') end it "groups carriage returns with newlines" do - expect(convert_html("\r\n")).to eq('
') + expect(convert_html("\r\n")).to eq('
') end describe "incremental update" do @@ -182,7 +182,7 @@ describe Gitlab::Ci::Ansi2html do context "with partial sequence" do let(:pre_text) { "Hello\e" } - let(:pre_html) { "Hello" } + let(:pre_html) { "Hello" } let(:text) { "[1m World" } let(:html) { " World" } @@ -191,9 +191,9 @@ describe Gitlab::Ci::Ansi2html do context 'with new line' do let(:pre_text) { "Hello\r" } - let(:pre_html) { "Hello\r" } + let(:pre_html) { "Hello\r" } let(:text) { "\nWorld" } - let(:html) { "
World
" } + let(:html) { "
World
" } it_behaves_like 'stateable converter' end @@ -220,7 +220,7 @@ describe Gitlab::Ci::Ansi2html do text = "#{section_start}Some text#{section_end}" class_name_start = section_start.gsub("\033[0K", '').gsub('<', '<') class_name_end = section_end.gsub("\033[0K", '').gsub('<', '<') - html = %{#{class_name_start}Some text#{class_name_end}} + html = %{#{class_name_start}Some text#{class_name_end}} expect(convert_html(text)).to eq(html) end @@ -230,12 +230,11 @@ describe Gitlab::Ci::Ansi2html do let(:text) { "#{section_start}Some text#{section_end}" } it 'prints light red' do - text = "#{section_start}\e[91mHello\e[0m\n#{section_end}" + text = "#{section_start}\e[91mHello\e[0m\nLine 1\nLine 2\nLine 3\n#{section_end}" header = %{Hello} line_break = %{
} - line = %{} - empty_line = %{} - html = "#{section_start_html}#{header}#{line_break}#{line}#{empty_line}#{section_end_html}" + output_line = %{Line 1
Line 2
Line 3
} + html = "#{section_start_html}#{header}#{line_break}#{output_line}#{section_end_html}" expect(convert_html(text)).to eq(html) end diff --git a/spec/lib/gitlab/ci/trace/stream_spec.rb b/spec/lib/gitlab/ci/trace/stream_spec.rb index 35250632e86742477094c2f9861c25e4017e2040..af519f4bae6a1638b8d356b7979d25a0897e1c5c 100644 --- a/spec/lib/gitlab/ci/trace/stream_spec.rb +++ b/spec/lib/gitlab/ci/trace/stream_spec.rb @@ -65,9 +65,9 @@ describe Gitlab::Ci::Trace::Stream, :clean_gitlab_redis_cache do result = stream.html expect(result).to eq( - "ヾ(´༎ຶД༎ຶ`)ノ
"\ - "許功蓋
"\ - "
") + "ヾ(´༎ຶД༎ຶ`)ノ
"\ + "許功蓋"\ + "
") expect(result.encoding).to eq(Encoding.default_external) end end @@ -253,7 +253,7 @@ describe Gitlab::Ci::Trace::Stream, :clean_gitlab_redis_cache do it 'returns html content with state' do result = stream.html_with_state - expect(result.html).to eq("1234") + expect(result.html).to eq("1234") end context 'follow-up state' do @@ -269,7 +269,7 @@ describe Gitlab::Ci::Trace::Stream, :clean_gitlab_redis_cache do result = stream.html_with_state(last_result.state) expect(result.append).to be_truthy - expect(result.html).to eq("5678") + expect(result.html).to eq("5678") end end end @@ -305,13 +305,11 @@ describe Gitlab::Ci::Trace::Stream, :clean_gitlab_redis_cache do describe '#html' do shared_examples_for 'htmls' do it "returns html" do - expect(stream.html).to eq( - "12
34
"\ - "56
") + expect(stream.html).to eq("12
34
56
") end it "returns html for last line only" do - expect(stream.html(last_lines: 1)).to eq("56") + expect(stream.html(last_lines: 1)).to eq("56") end end diff --git a/spec/lib/gitlab/exclusive_lease_helpers_spec.rb b/spec/lib/gitlab/exclusive_lease_helpers_spec.rb index 5107e1efbbda97a3d6c1087b4c1f48245336d1db..c3b706fc538490f08be29517fd3398b98ec8e70e 100644 --- a/spec/lib/gitlab/exclusive_lease_helpers_spec.rb +++ b/spec/lib/gitlab/exclusive_lease_helpers_spec.rb @@ -25,13 +25,13 @@ describe Gitlab::ExclusiveLeaseHelpers, :clean_gitlab_redis_shared_state do end it 'calls the given block' do - expect { |b| class_instance.in_lock(unique_key, &b) }.to yield_control.once + expect { |b| class_instance.in_lock(unique_key, &b) }.to yield_with_args(false) end it 'calls the given block continuously' do - expect { |b| class_instance.in_lock(unique_key, &b) }.to yield_control.once - expect { |b| class_instance.in_lock(unique_key, &b) }.to yield_control.once - expect { |b| class_instance.in_lock(unique_key, &b) }.to yield_control.once + expect { |b| class_instance.in_lock(unique_key, &b) }.to yield_with_args(false) + expect { |b| class_instance.in_lock(unique_key, &b) }.to yield_with_args(false) + expect { |b| class_instance.in_lock(unique_key, &b) }.to yield_with_args(false) end it 'cancels the exclusive lease after the block' do @@ -68,6 +68,15 @@ describe Gitlab::ExclusiveLeaseHelpers, :clean_gitlab_redis_shared_state do expect { subject }.to raise_error('Failed to obtain a lock') end + + context 'when lease is granted after retry' do + it 'yields block with true' do + expect(lease).to receive(:try_obtain).exactly(3).times { nil } + expect(lease).to receive(:try_obtain).once { unique_key } + + expect { |b| class_instance.in_lock(unique_key, &b) }.to yield_with_args(true) + end + end end context 'when sleep second is specified' do diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index ced853caab4790f005ba128561953f034e62c9ea..a8b3f05f182ac1b51c720a000e15e51c9b5c97d6 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -1571,7 +1571,7 @@ describe API::MergeRequests do end end - describe "GET /projects/:id/merge_requests/:merge_request_iid/merge_ref" do + describe "GET /projects/:id/merge_requests/:merge_request_iid/merge_ref", :clean_gitlab_redis_shared_state do before do merge_request.mark_as_unchecked! end diff --git a/spec/services/merge_requests/mergeability_check_service_spec.rb b/spec/services/merge_requests/mergeability_check_service_spec.rb index 6e827f2ea5bf475b7ad501f789d8a73c86025d77..a864da0a6fba076d91ec1e452840a9ac6b127b95 100644 --- a/spec/services/merge_requests/mergeability_check_service_spec.rb +++ b/spec/services/merge_requests/mergeability_check_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe MergeRequests::MergeabilityCheckService do +describe MergeRequests::MergeabilityCheckService, :clean_gitlab_redis_shared_state do shared_examples_for 'unmergeable merge request' do it 'updates or keeps merge status as cannot_be_merged' do subject @@ -60,24 +60,69 @@ describe MergeRequests::MergeabilityCheckService do subject { described_class.new(merge_request).execute } + def execute_within_threads(amount:, retry_lease: true) + threads = [] + + amount.times do + # Let's use a different object for each thread to get closer + # to a real world scenario. + mr = MergeRequest.find(merge_request.id) + + threads << Thread.new do + described_class.new(mr).execute(retry_lease: retry_lease) + end + end + + threads.each(&:join) + threads + end + before do project.add_developer(merge_request.author) end it_behaves_like 'mergeable merge request' - context 'when multiple calls to the service' do - it 'returns success' do - subject - result = subject + context 'when lock is disabled' do + before do + stub_feature_flags(merge_ref_auto_sync_lock: false) + end - expect(result).to be_a(ServiceResponse) - expect(result.success?).to be(true) + it_behaves_like 'mergeable merge request' + end + + context 'when concurrent calls' do + it 'waits first lock and returns "cached" result in subsequent calls' do + threads = execute_within_threads(amount: 3) + results = threads.map { |t| t.value.status } + + expect(results).to contain_exactly(:success, :success, :success) + end + + it 'writes the merge-ref once' do + service = instance_double(MergeRequests::MergeToRefService) + + expect(MergeRequests::MergeToRefService).to receive(:new).once { service } + expect(service).to receive(:execute).once.and_return(success: true) + + execute_within_threads(amount: 3) end - it 'second call does not change the merge-ref' do - expect { subject }.to change(merge_request, :merge_ref_head).from(nil) - expect { subject }.not_to change(merge_request.merge_ref_head, :id) + it 'resets one merge request upon execution' do + expect_any_instance_of(MergeRequest).to receive(:reset).once + + execute_within_threads(amount: 2) + end + + context 'when retry_lease flag is false' do + it 'the first call succeeds, subsequent concurrent calls get a lock error response' do + threads = execute_within_threads(amount: 3, retry_lease: false) + results = threads.map { |t| [t.value.status, t.value.message] } + + expect(results).to contain_exactly([:error, 'Failed to obtain a lock'], + [:error, 'Failed to obtain a lock'], + [:success, nil]) + end end end @@ -102,8 +147,7 @@ describe MergeRequests::MergeabilityCheckService do context 'when broken' do before do - allow(merge_request).to receive(:broken?) { true } - allow(project.repository).to receive(:can_be_merged?) { false } + expect(merge_request).to receive(:broken?) { true } end it_behaves_like 'unmergeable merge request' @@ -117,10 +161,13 @@ describe MergeRequests::MergeabilityCheckService do end end - context 'when it has conflicts' do - before do - allow(merge_request).to receive(:broken?) { false } - allow(project.repository).to receive(:can_be_merged?) { false } + context 'when it cannot be merged on git' do + let(:merge_request) do + create(:merge_request, + merge_status: :unchecked, + source_branch: 'conflict-resolvable', + source_project: project, + target_branch: 'conflict-start') end it_behaves_like 'unmergeable merge request' diff --git a/spec/support/shared_examples/ci_trace_shared_examples.rb b/spec/support/shared_examples/ci_trace_shared_examples.rb index ab0550e2613141f02b4a50d995b8680917d8536b..c13699faca02e288f9574fe491a0f99bff43f19b 100644 --- a/spec/support/shared_examples/ci_trace_shared_examples.rb +++ b/spec/support/shared_examples/ci_trace_shared_examples.rb @@ -5,11 +5,11 @@ shared_examples_for 'common trace features' do end it "returns formatted html" do - expect(trace.html).to eq("12
34
") + expect(trace.html).to eq("12
34
") end it "returns last line of formatted html" do - expect(trace.html(last_lines: 1)).to eq("34") + expect(trace.html(last_lines: 1)).to eq("34") end end