diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-08-25 09:11:32 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-08-25 09:11:32 +0300 |
commit | 92e4789eb069d5fe43025601c6b7839fddda3ad1 (patch) | |
tree | e4c4f4a20906af5bef930e10029f144e25c9b89a | |
parent | d14fca1c37889ced556802cb61f529dcab4cac60 (diff) |
Add latest changes from gitlab-org/gitlab@master
-rw-r--r-- | app/models/application_record.rb | 8 | ||||
-rw-r--r-- | app/models/ci/build.rb | 4 | ||||
-rw-r--r-- | app/models/concerns/ci/contextable.rb | 4 | ||||
-rw-r--r-- | config/feature_flags/development/env_vars_resource_group.yml | 8 | ||||
-rw-r--r-- | config/feature_flags/development/optimize_safe_find_or_create_by.yml | 8 | ||||
-rw-r--r-- | doc/api/graphql/reference/index.md | 2 | ||||
-rw-r--r-- | doc/ci/variables/where_variables_can_be_used.md | 2 | ||||
-rw-r--r-- | lib/gitlab/ci/pipeline/seed/build.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/ci/pipeline/seed/processable/resource_group.rb | 11 | ||||
-rw-r--r-- | qa/qa/resource/merge_request.rb | 2 | ||||
-rw-r--r-- | spec/lib/gitlab/ci/pipeline/seed/build_spec.rb | 11 | ||||
-rw-r--r-- | spec/models/application_record_spec.rb | 22 | ||||
-rw-r--r-- | spec/services/ci/create_pipeline_service/parent_child_pipeline_spec.rb | 26 | ||||
-rw-r--r-- | spec/services/ci/create_pipeline_service_spec.rb | 68 |
14 files changed, 135 insertions, 43 deletions
diff --git a/app/models/application_record.rb b/app/models/application_record.rb index 121c4bbfc32..9787e02643b 100644 --- a/app/models/application_record.rb +++ b/app/models/application_record.rb @@ -64,14 +64,6 @@ class ApplicationRecord < ActiveRecord::Base end def self.safe_find_or_create_by(*args, &block) - return optimized_safe_find_or_create_by(*args, &block) if Feature.enabled?(:optimize_safe_find_or_create_by, default_enabled: :yaml) - - safe_ensure_unique(retries: 1) do # rubocop:disable Performance/ActiveRecordSubtransactionMethods - find_or_create_by(*args, &block) - end - end - - def self.optimized_safe_find_or_create_by(*args, &block) record = find_by(*args) return record if record.present? diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 2df9f3bc75c..d7b93a616fc 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -90,6 +90,10 @@ module Ci end end + def persisted_environment=(environment) + strong_memoize(:persisted_environment) { environment } + end + serialize :options # rubocop:disable Cop/ActiveRecordSerialize serialize :yaml_variables, Gitlab::Serializer::Ci::Variables # rubocop:disable Cop/ActiveRecordSerialize diff --git a/app/models/concerns/ci/contextable.rb b/app/models/concerns/ci/contextable.rb index bdba2d3e251..27a704c1de0 100644 --- a/app/models/concerns/ci/contextable.rb +++ b/app/models/concerns/ci/contextable.rb @@ -33,13 +33,13 @@ module Ci # def simple_variables strong_memoize(:simple_variables) do - scoped_variables(environment: nil).to_runner_variables + scoped_variables(environment: nil) end end def simple_variables_without_dependencies strong_memoize(:variables_without_dependencies) do - scoped_variables(environment: nil, dependencies: false).to_runner_variables + scoped_variables(environment: nil, dependencies: false) end end diff --git a/config/feature_flags/development/env_vars_resource_group.yml b/config/feature_flags/development/env_vars_resource_group.yml new file mode 100644 index 00000000000..0b5fdb02da1 --- /dev/null +++ b/config/feature_flags/development/env_vars_resource_group.yml @@ -0,0 +1,8 @@ +--- +name: env_vars_resource_group +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/67876 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/339024 +milestone: '14.3' +type: development +group: group::release +default_enabled: false diff --git a/config/feature_flags/development/optimize_safe_find_or_create_by.yml b/config/feature_flags/development/optimize_safe_find_or_create_by.yml deleted file mode 100644 index 6adedb5cece..00000000000 --- a/config/feature_flags/development/optimize_safe_find_or_create_by.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: optimize_safe_find_or_create_by -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68458 -rollout_issue_url: -milestone: '14.3' -type: development -group: group::database -default_enabled: false diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 46506750e26..c081addb24a 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -7605,7 +7605,7 @@ Describes a rule for who can approve merge requests. | <a id="approvalruleapproved"></a>`approved` | [`Boolean`](#boolean) | Indicates if the rule is satisfied. | | <a id="approvalruleapprovedby"></a>`approvedBy` | [`UserCoreConnection`](#usercoreconnection) | List of users defined in the rule that approved the merge request. (see [Connections](#connections)) | | <a id="approvalrulecontainshiddengroups"></a>`containsHiddenGroups` | [`Boolean`](#boolean) | Indicates if the rule contains approvers from a hidden group. | -| <a id="approvalruleeligibleapprovers"></a>`eligibleApprovers` | [`UserCoreConnection`](#usercoreconnection) | List of all users eligible to approve the merge request (defined explicitly and from associated groups). (see [Connections](#connections)) | +| <a id="approvalruleeligibleapprovers"></a>`eligibleApprovers` | [`[UserCore!]`](#usercore) | List of all users eligible to approve the merge request (defined explicitly and from associated groups). | | <a id="approvalrulegroups"></a>`groups` | [`GroupConnection`](#groupconnection) | List of groups added as approvers for the rule. (see [Connections](#connections)) | | <a id="approvalruleid"></a>`id` | [`GlobalID!`](#globalid) | ID of the rule. | | <a id="approvalrulename"></a>`name` | [`String`](#string) | Name of the rule. | diff --git a/doc/ci/variables/where_variables_can_be_used.md b/doc/ci/variables/where_variables_can_be_used.md index 8009687dbca..c15e4c123dc 100644 --- a/doc/ci/variables/where_variables_can_be_used.md +++ b/doc/ci/variables/where_variables_can_be_used.md @@ -26,7 +26,7 @@ There are two places defined variables can be used. On the: |:-------------------------------------------|:-----------------|:-----------------------|:----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | `environment:url` | yes | GitLab | The variable expansion is made by the [internal variable expansion mechanism](#gitlab-internal-variable-expansion-mechanism) in GitLab.<br/><br/>Supported are all variables defined for a job (project/group variables, variables from `.gitlab-ci.yml`, variables from triggers, variables from pipeline schedules).<br/><br/>Not supported are variables defined in the GitLab Runner `config.toml` and variables created in the job's `script`. | | `environment:name` | yes | GitLab | Similar to `environment:url`, but the variables expansion doesn't support the following:<br/><br/>- Variables that are based on the environment's name (`CI_ENVIRONMENT_NAME`, `CI_ENVIRONMENT_SLUG`).<br/>- Any other variables related to environment (currently only `CI_ENVIRONMENT_URL`).<br/>- [Persisted variables](#persisted-variables). | -| `resource_group` | yes | GitLab | Similar to `environment:url`, but the variables expansion doesn't support the following:<br/><br/>- Variables that are based on the environment's name (`CI_ENVIRONMENT_NAME`, `CI_ENVIRONMENT_SLUG`).<br/>- Any other variables related to environment (currently only `CI_ENVIRONMENT_URL`).<br/>- [Persisted variables](#persisted-variables). | +| `resource_group` | yes | GitLab | Similar to `environment:url`, but the variables expansion doesn't support the following:<br/>- `CI_ENVIRONMENT_URL`<br/>- [Persisted variables](#persisted-variables) | | `include` | yes | GitLab | The variable expansion is made by the [internal variable expansion mechanism](#gitlab-internal-variable-expansion-mechanism) in GitLab. <br/><br/>Predefined project variables are supported: `GITLAB_FEATURES`, `CI_DEFAULT_BRANCH`, and all variables that start with `CI_PROJECT_` (for example `CI_PROJECT_NAME`). | | `variables` | yes | GitLab/Runner | The variable expansion is first made by the [internal variable expansion mechanism](#gitlab-internal-variable-expansion-mechanism) in GitLab, and then any unrecognized or unavailable variables are expanded by GitLab Runner's [internal variable expansion mechanism](#gitlab-runner-internal-variable-expansion-mechanism). | | `image` | yes | Runner | The variable expansion is made by GitLab Runner's [internal variable expansion mechanism](#gitlab-runner-internal-variable-expansion-mechanism) | diff --git a/lib/gitlab/ci/pipeline/seed/build.rb b/lib/gitlab/ci/pipeline/seed/build.rb index a8043882c1a..934bf22d8ad 100644 --- a/lib/gitlab/ci/pipeline/seed/build.rb +++ b/lib/gitlab/ci/pipeline/seed/build.rb @@ -118,6 +118,8 @@ module Gitlab return { environment: nil } end + build.persisted_environment = environment + { deployment: Seed::Deployment.new(build, environment).to_resource, metadata_attributes: { diff --git a/lib/gitlab/ci/pipeline/seed/processable/resource_group.rb b/lib/gitlab/ci/pipeline/seed/processable/resource_group.rb index f8ea6d4184c..d0ecfff4f34 100644 --- a/lib/gitlab/ci/pipeline/seed/processable/resource_group.rb +++ b/lib/gitlab/ci/pipeline/seed/processable/resource_group.rb @@ -28,7 +28,16 @@ module Gitlab def expanded_resource_group_key strong_memoize(:expanded_resource_group_key) do - ExpandVariables.expand(resource_group_key, -> { processable.simple_variables }) + ExpandVariables.expand(resource_group_key, -> { variables }) + end + end + + def variables + processable.simple_variables.tap do |variables| + # Adding persisted environment variables + if Feature.enabled?(:env_vars_resource_group) && processable.persisted_environment.present? + variables.concat(processable.persisted_environment.predefined_variables) + end end end end diff --git a/qa/qa/resource/merge_request.rb b/qa/qa/resource/merge_request.rb index 8c313f5d518..93b5fc3ef6b 100644 --- a/qa/qa/resource/merge_request.rb +++ b/qa/qa/resource/merge_request.rb @@ -25,7 +25,7 @@ module QA :state attribute :project do - Project.fabricate! do |resource| + Project.fabricate_via_api! do |resource| resource.name = 'project-with-merge-request' end end diff --git a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb index b211bc50f4b..0c28515b574 100644 --- a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb @@ -490,12 +490,21 @@ RSpec.describe Gitlab::Ci::Pipeline::Seed::Build do end context 'when job belongs to a resource group' do - let(:attributes) { { name: 'rspec', ref: 'master', resource_group_key: 'iOS' } } + let(:resource_group) { 'iOS' } + let(:attributes) { { name: 'rspec', ref: 'master', resource_group_key: resource_group, environment: 'production' }} it 'returns a job with resource group' do expect(subject.resource_group).not_to be_nil expect(subject.resource_group.key).to eq('iOS') end + + context 'when resource group has $CI_ENVIRONMENT_NAME in it' do + let(:resource_group) { 'test/$CI_ENVIRONMENT_NAME' } + + it 'expands environment name' do + expect(subject.resource_group.key).to eq('test/production') + end + end end end diff --git a/spec/models/application_record_spec.rb b/spec/models/application_record_spec.rb index 40e2aae0bad..7b9e70261fb 100644 --- a/spec/models/application_record_spec.rb +++ b/spec/models/application_record_spec.rb @@ -39,7 +39,7 @@ RSpec.describe ApplicationRecord do let(:suggestion_attributes) { attributes_for(:suggestion).merge!(note_id: note.id) } - shared_examples '.safe_find_or_create_by' do + describe '.safe_find_or_create_by' do it 'creates the suggestion avoiding race conditions' do existing_suggestion = double(:Suggestion) @@ -63,7 +63,7 @@ RSpec.describe ApplicationRecord do end end - shared_examples '.safe_find_or_create_by!' do + describe '.safe_find_or_create_by!' do it 'creates a record using safe_find_or_create_by' do expect(Suggestion.safe_find_or_create_by!(suggestion_attributes)) .to be_a(Suggestion) @@ -88,24 +88,6 @@ RSpec.describe ApplicationRecord do .to raise_error(ActiveRecord::RecordNotFound) end end - - context 'when optimized_safe_find_or_create_by is enabled' do - before do - stub_feature_flags(optimized_safe_find_or_create_by: true) - end - - it_behaves_like '.safe_find_or_create_by' - it_behaves_like '.safe_find_or_create_by!' - end - - context 'when optimized_safe_find_or_create_by is disabled' do - before do - stub_feature_flags(optimized_safe_find_or_create_by: false) - end - - it_behaves_like '.safe_find_or_create_by' - it_behaves_like '.safe_find_or_create_by!' - end end describe '.underscore' do diff --git a/spec/services/ci/create_pipeline_service/parent_child_pipeline_spec.rb b/spec/services/ci/create_pipeline_service/parent_child_pipeline_spec.rb index 6eb1315fff4..4326fa5533f 100644 --- a/spec/services/ci/create_pipeline_service/parent_child_pipeline_spec.rb +++ b/spec/services/ci/create_pipeline_service/parent_child_pipeline_spec.rb @@ -127,6 +127,32 @@ RSpec.describe Ci::CreatePipelineService, '#execute' do end end end + + context 'when resource group key includes a variable' do + let(:config) do + <<~YAML + instrumentation_test: + stage: test + resource_group: $CI_ENVIRONMENT_NAME + trigger: + include: path/to/child.yml + strategy: depend + YAML + end + + it 'ignores the resource group keyword because it fails to expand the variable', :aggregate_failures do + pipeline = create_pipeline! + Ci::InitialPipelineProcessWorker.new.perform(pipeline.id) + + test = pipeline.statuses.find_by(name: 'instrumentation_test') + expect(pipeline).to be_created_successfully + expect(pipeline.triggered_pipelines).not_to be_exist + expect(project.resource_groups.count).to eq(0) + expect(test).to be_a Ci::Bridge + expect(test).to be_pending + expect(test.resource_group).to be_nil + end + end end end diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index d5d65598589..4c9e7da83f9 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -991,6 +991,74 @@ RSpec.describe Ci::CreatePipelineService do end end + context 'when resource group is defined for review app deployment' do + before do + config = YAML.dump( + review_app: { + stage: 'test', + script: 'deploy', + environment: { + name: 'review/$CI_COMMIT_REF_SLUG', + on_stop: 'stop_review_app' + }, + resource_group: '$CI_ENVIRONMENT_NAME' + }, + stop_review_app: { + stage: 'test', + script: 'stop', + when: 'manual', + environment: { + name: 'review/$CI_COMMIT_REF_SLUG', + action: 'stop' + }, + resource_group: '$CI_ENVIRONMENT_NAME' + } + ) + + stub_ci_pipeline_yaml_file(config) + end + + it 'persists the association correctly' do + result = execute_service.payload + deploy_job = result.builds.find_by_name!(:review_app) + stop_job = result.builds.find_by_name!(:stop_review_app) + + expect(result).to be_persisted + expect(deploy_job.resource_group.key).to eq('review/master') + expect(stop_job.resource_group.key).to eq('review/master') + expect(project.resource_groups.count).to eq(1) + end + + it 'initializes scoped variables only once for each build' do + # Bypassing `stub_build` hack because it distrubs the expectations below. + allow_next_instances_of(Gitlab::Ci::Build::Context::Build, 2) do |build_context| + allow(build_context).to receive(:variables) { Gitlab::Ci::Variables::Collection.new } + end + + expect_next_instances_of(::Ci::Build, 2) do |ci_build| + expect(ci_build).to receive(:scoped_variables).once.and_call_original + end + + expect(execute_service.payload).to be_created_successfully + end + + context 'when the env_vars_resource_group feature flag is disabled' do + before do + stub_feature_flags(env_vars_resource_group: false) + end + + it 'does not create a resource group because its key contains an invalid character' do + result = execute_service.payload + deploy_job = result.builds.find_by_name!(:review_app) + stop_job = result.builds.find_by_name!(:stop_review_app) + expect(result).to be_persisted + expect(deploy_job.resource_group).to be_nil + expect(stop_job.resource_group).to be_nil + expect(project.resource_groups.count).to eq(0) + end + end + end + context 'with timeout' do context 'when builds with custom timeouts are configured' do before do |