Skip to content
Projects
Groups
Snippets
Help
Loading...
Help
Support
Keyboard shortcuts
?
Submit feedback
Contribute to GitLab
Sign in / Register
Toggle navigation
G
gitlab-ce
Project overview
Project overview
Details
Activity
Releases
Repository
Repository
Files
Commits
Branches
Tags
Contributors
Graph
Compare
Issues
0
Issues
0
List
Boards
Labels
Milestones
Merge Requests
1
Merge Requests
1
Analytics
Analytics
Repository
Value Stream
Wiki
Wiki
Snippets
Snippets
Members
Members
Collapse sidebar
Close sidebar
Activity
Graph
Create a new issue
Commits
Issue Boards
Open sidebar
nexedi
gitlab-ce
Commits
2998c15b
Commit
2998c15b
authored
Jan 28, 2022
by
Marius Bobin
Browse files
Options
Browse Files
Download
Email Patches
Plain Diff
Reduce Redis calls for instance level variables
Changelog: performance
parent
f013396b
Changes
10
Show whitespace changes
Inline
Side-by-side
Showing
10 changed files
with
155 additions
and
65 deletions
+155
-65
app/models/ci/pipeline.rb
app/models/ci/pipeline.rb
+4
-0
app/models/project.rb
app/models/project.rb
+0
-8
lib/gitlab/ci/config.rb
lib/gitlab/ci/config.rb
+9
-1
lib/gitlab/ci/variables/builder.rb
lib/gitlab/ci/variables/builder.rb
+14
-3
lib/gitlab/ci/variables/builder/instance.rb
lib/gitlab/ci/variables/builder/instance.rb
+23
-0
spec/lib/gitlab/ci/config_spec.rb
spec/lib/gitlab/ci/config_spec.rb
+2
-2
spec/lib/gitlab/ci/variables/builder/instance_spec.rb
spec/lib/gitlab/ci/variables/builder/instance_spec.rb
+39
-0
spec/lib/gitlab/ci/variables/builder_spec.rb
spec/lib/gitlab/ci/variables/builder_spec.rb
+27
-12
spec/models/ci/pipeline_spec.rb
spec/models/ci/pipeline_spec.rb
+37
-0
spec/models/project_spec.rb
spec/models/project_spec.rb
+0
-39
No files found.
app/models/ci/pipeline.rb
View file @
2998c15b
...
@@ -40,6 +40,10 @@ module Ci
...
@@ -40,6 +40,10 @@ module Ci
# https://gitlab.com/gitlab-org/gitlab/-/issues/259010
# https://gitlab.com/gitlab-org/gitlab/-/issues/259010
attr_accessor
:merged_yaml
attr_accessor
:merged_yaml
# This is used to retain access to the method defined by `Ci::HasRef`
# before being overridden in this class.
alias_method
:jobs_git_ref
,
:git_ref
belongs_to
:project
,
inverse_of: :all_pipelines
belongs_to
:project
,
inverse_of: :all_pipelines
belongs_to
:user
belongs_to
:user
belongs_to
:auto_canceled_by
,
class_name:
'Ci::Pipeline'
belongs_to
:auto_canceled_by
,
class_name:
'Ci::Pipeline'
...
...
app/models/project.rb
View file @
2998c15b
...
@@ -2187,14 +2187,6 @@ class Project < ApplicationRecord
...
@@ -2187,14 +2187,6 @@ class Project < ApplicationRecord
end
end
end
end
def
ci_instance_variables_for
(
ref
:)
if
protected_for?
(
ref
)
Ci
::
InstanceVariable
.
all_cached
else
Ci
::
InstanceVariable
.
unprotected_cached
end
end
def
protected_for?
(
ref
)
def
protected_for?
(
ref
)
raise
Repository
::
AmbiguousRefError
if
repository
.
ambiguous_ref?
(
ref
)
raise
Repository
::
AmbiguousRefError
if
repository
.
ambiguous_ref?
(
ref
)
...
...
lib/gitlab/ci/config.rb
View file @
2998c15b
...
@@ -158,7 +158,7 @@ module Gitlab
...
@@ -158,7 +158,7 @@ module Gitlab
# See more detail in the docs: https://docs.gitlab.com/ee/ci/variables/#cicd-variable-precedence
# See more detail in the docs: https://docs.gitlab.com/ee/ci/variables/#cicd-variable-precedence
variables
.
concat
(
project
.
predefined_variables
)
variables
.
concat
(
project
.
predefined_variables
)
variables
.
concat
(
pipeline
.
predefined_variables
)
if
pipeline
variables
.
concat
(
pipeline
.
predefined_variables
)
if
pipeline
variables
.
concat
(
project
.
ci_instance_variables_for
(
ref:
source_ref_path
))
variables
.
concat
(
secret_variables
(
project:
project
,
pipeline:
pipeline
))
variables
.
concat
(
project
.
group
.
ci_variables_for
(
source_ref_path
,
project
))
if
project
.
group
variables
.
concat
(
project
.
group
.
ci_variables_for
(
source_ref_path
,
project
))
if
project
.
group
variables
.
concat
(
project
.
ci_variables_for
(
ref:
source_ref_path
))
variables
.
concat
(
project
.
ci_variables_for
(
ref:
source_ref_path
))
variables
.
concat
(
pipeline
.
variables
)
if
pipeline
variables
.
concat
(
pipeline
.
variables
)
if
pipeline
...
@@ -166,6 +166,14 @@ module Gitlab
...
@@ -166,6 +166,14 @@ module Gitlab
end
end
end
end
def
secret_variables
(
project
:,
pipeline
:)
if
pipeline
pipeline
.
variables_builder
.
secret_instance_variables
else
Gitlab
::
Ci
::
Variables
::
Builder
::
Instance
.
new
.
secret_variables
end
end
def
track_and_raise_for_dev_exception
(
error
)
def
track_and_raise_for_dev_exception
(
error
)
Gitlab
::
ErrorTracking
.
track_and_raise_for_dev_exception
(
error
,
@context
.
sentry_payload
)
Gitlab
::
ErrorTracking
.
track_and_raise_for_dev_exception
(
error
,
@context
.
sentry_payload
)
end
end
...
...
lib/gitlab/ci/variables/builder.rb
View file @
2998c15b
...
@@ -8,6 +8,7 @@ module Gitlab
...
@@ -8,6 +8,7 @@ module Gitlab
def
initialize
(
pipeline
)
def
initialize
(
pipeline
)
@pipeline
=
pipeline
@pipeline
=
pipeline
@instance_variables_builder
=
Builder
::
Instance
.
new
end
end
def
scoped_variables
(
job
,
environment
:,
dependencies
:)
def
scoped_variables
(
job
,
environment
:,
dependencies
:)
...
@@ -21,7 +22,7 @@ module Gitlab
...
@@ -21,7 +22,7 @@ module Gitlab
variables
.
concat
(
job
.
yaml_variables
)
variables
.
concat
(
job
.
yaml_variables
)
variables
.
concat
(
user_variables
(
job
.
user
))
variables
.
concat
(
user_variables
(
job
.
user
))
variables
.
concat
(
job
.
dependency_variables
)
if
dependencies
variables
.
concat
(
job
.
dependency_variables
)
if
dependencies
variables
.
concat
(
secret_instance_variables
(
ref:
job
.
git_ref
)
)
variables
.
concat
(
secret_instance_variables
)
variables
.
concat
(
secret_group_variables
(
environment:
environment
,
ref:
job
.
git_ref
))
variables
.
concat
(
secret_group_variables
(
environment:
environment
,
ref:
job
.
git_ref
))
variables
.
concat
(
secret_project_variables
(
environment:
environment
,
ref:
job
.
git_ref
))
variables
.
concat
(
secret_project_variables
(
environment:
environment
,
ref:
job
.
git_ref
))
variables
.
concat
(
job
.
trigger_request
.
user_variables
)
if
job
.
trigger_request
variables
.
concat
(
job
.
trigger_request
.
user_variables
)
if
job
.
trigger_request
...
@@ -62,8 +63,11 @@ module Gitlab
...
@@ -62,8 +63,11 @@ module Gitlab
end
end
end
end
def
secret_instance_variables
(
ref
:)
def
secret_instance_variables
project
.
ci_instance_variables_for
(
ref:
ref
)
strong_memoize
(
:secret_instance_variables
)
do
instance_variables_builder
.
secret_variables
(
protected_ref:
protected_ref?
)
end
end
end
def
secret_group_variables
(
environment
:,
ref
:)
def
secret_group_variables
(
environment
:,
ref
:)
...
@@ -79,6 +83,7 @@ module Gitlab
...
@@ -79,6 +83,7 @@ module Gitlab
private
private
attr_reader
:pipeline
attr_reader
:pipeline
attr_reader
:instance_variables_builder
delegate
:project
,
to: :pipeline
delegate
:project
,
to: :pipeline
def
predefined_variables
(
job
)
def
predefined_variables
(
job
)
...
@@ -104,6 +109,12 @@ module Gitlab
...
@@ -104,6 +109,12 @@ module Gitlab
parallel
=
parallel
.
dig
(
:total
)
if
parallel
.
is_a?
(
Hash
)
parallel
=
parallel
.
dig
(
:total
)
if
parallel
.
is_a?
(
Hash
)
parallel
||
1
parallel
||
1
end
end
def
protected_ref?
strong_memoize
(
:protected_ref
)
do
project
.
protected_for?
(
pipeline
.
jobs_git_ref
)
end
end
end
end
end
end
end
end
...
...
lib/gitlab/ci/variables/builder/instance.rb
0 → 100644
View file @
2998c15b
# frozen_string_literal: true
module
Gitlab
module
Ci
module
Variables
class
Builder
class
Instance
include
Gitlab
::
Utils
::
StrongMemoize
def
secret_variables
(
protected_ref:
false
)
variables
=
if
protected_ref
::
Ci
::
InstanceVariable
.
all_cached
else
::
Ci
::
InstanceVariable
.
unprotected_cached
end
Gitlab
::
Ci
::
Variables
::
Collection
.
new
(
variables
)
end
end
end
end
end
end
spec/lib/gitlab/ci/config_spec.rb
View file @
2998c15b
...
@@ -462,7 +462,7 @@ RSpec.describe Gitlab::Ci::Config do
...
@@ -462,7 +462,7 @@ RSpec.describe Gitlab::Ci::Config do
expect
(
project
.
repository
).
to
receive
(
:blob_data_at
)
expect
(
project
.
repository
).
to
receive
(
:blob_data_at
)
.
with
(
'eeff1122'
,
local_location
)
.
with
(
'eeff1122'
,
local_location
)
described_class
.
new
(
gitlab_ci_yml
,
project:
project
,
sha:
'eeff1122'
,
user:
user
)
described_class
.
new
(
gitlab_ci_yml
,
project:
project
,
sha:
'eeff1122'
,
user:
user
,
pipeline:
pipeline
)
end
end
end
end
...
@@ -470,7 +470,7 @@ RSpec.describe Gitlab::Ci::Config do
...
@@ -470,7 +470,7 @@ RSpec.describe Gitlab::Ci::Config do
it
'is using latest SHA on the default branch'
do
it
'is using latest SHA on the default branch'
do
expect
(
project
.
repository
).
to
receive
(
:root_ref_sha
)
expect
(
project
.
repository
).
to
receive
(
:root_ref_sha
)
described_class
.
new
(
gitlab_ci_yml
,
project:
project
,
sha:
nil
,
user:
user
)
described_class
.
new
(
gitlab_ci_yml
,
project:
project
,
sha:
nil
,
user:
user
,
pipeline:
pipeline
)
end
end
end
end
end
end
...
...
spec/lib/gitlab/ci/variables/builder/instance_spec.rb
0 → 100644
View file @
2998c15b
# frozen_string_literal: true
require
'spec_helper'
RSpec
.
describe
Gitlab
::
Ci
::
Variables
::
Builder
::
Instance
do
let_it_be
(
:variable
)
{
create
(
:ci_instance_variable
,
protected:
false
)
}
let_it_be
(
:protected_variable
)
{
create
(
:ci_instance_variable
,
protected:
true
)
}
let
(
:builder
)
{
described_class
.
new
}
describe
'#secret_variables'
do
let
(
:variable_item
)
{
item
(
variable
)
}
let
(
:protected_variable_item
)
{
item
(
protected_variable
)
}
subject
do
builder
.
secret_variables
(
protected_ref:
protected_ref
)
end
context
'when the ref is protected'
do
let
(
:protected_ref
)
{
true
}
it
'contains all the variables'
do
is_expected
.
to
contain_exactly
(
variable_item
,
protected_variable_item
)
end
end
context
'when the ref is not protected'
do
let
(
:protected_ref
)
{
false
}
it
'contains only unprotected variables'
do
is_expected
.
to
contain_exactly
(
variable_item
)
end
end
end
def
item
(
variable
)
Gitlab
::
Ci
::
Variables
::
Collection
::
Item
.
fabricate
(
variable
)
end
end
spec/lib/gitlab/ci/variables/builder_spec.rb
View file @
2998c15b
...
@@ -7,7 +7,7 @@ RSpec.describe Gitlab::Ci::Variables::Builder do
...
@@ -7,7 +7,7 @@ RSpec.describe Gitlab::Ci::Variables::Builder do
let_it_be
(
:project
)
{
create
(
:project
,
:repository
,
namespace:
group
)
}
let_it_be
(
:project
)
{
create
(
:project
,
:repository
,
namespace:
group
)
}
let_it_be_with_reload
(
:pipeline
)
{
create
(
:ci_pipeline
,
project:
project
)
}
let_it_be_with_reload
(
:pipeline
)
{
create
(
:ci_pipeline
,
project:
project
)
}
let_it_be
(
:user
)
{
create
(
:user
)
}
let_it_be
(
:user
)
{
create
(
:user
)
}
let_it_be
(
:job
)
do
let_it_be
_with_reload
(
:job
)
do
create
(
:ci_build
,
create
(
:ci_build
,
pipeline:
pipeline
,
pipeline:
pipeline
,
user:
user
,
user:
user
,
...
@@ -276,27 +276,30 @@ RSpec.describe Gitlab::Ci::Variables::Builder do
...
@@ -276,27 +276,30 @@ RSpec.describe Gitlab::Ci::Variables::Builder do
create
(
:protected_branch
,
:developers_can_merge
,
name:
job
.
ref
,
project:
project
)
create
(
:protected_branch
,
:developers_can_merge
,
name:
job
.
ref
,
project:
project
)
end
end
it
{
is_expected
.
to
include
(
variable
)
}
it
{
is_expected
.
to
contain_exactly
(
protected_variable_item
,
unprotected_variable_item
)
}
end
end
context
'when ref is not protected'
do
context
'when ref is not protected'
do
it
{
is_expected
.
not_to
include
(
variable
)
}
it
{
is_expected
.
to
contain_exactly
(
unprotected_variable_item
)
}
end
end
end
end
context
'when ref is tag'
do
context
'when ref is tag'
do
let_it_be
(
:job
)
{
create
(
:ci_build
,
ref:
'v1.1.0'
,
tag:
true
,
pipeline:
pipeline
)
}
before
do
job
.
update!
(
ref:
'v1.1.0'
,
tag:
true
)
pipeline
.
update!
(
ref:
'v1.1.0'
,
tag:
true
)
end
context
'when ref is protected'
do
context
'when ref is protected'
do
before
do
before
do
create
(
:protected_tag
,
project:
project
,
name:
'v*'
)
create
(
:protected_tag
,
project:
project
,
name:
'v*'
)
end
end
it
{
is_expected
.
to
include
(
variable
)
}
it
{
is_expected
.
to
contain_exactly
(
protected_variable_item
,
unprotected_variable_item
)
}
end
end
context
'when ref is not protected'
do
context
'when ref is not protected'
do
it
{
is_expected
.
not_to
include
(
variable
)
}
it
{
is_expected
.
to
contain_exactly
(
unprotected_variable_item
)
}
end
end
end
end
...
@@ -311,20 +314,24 @@ RSpec.describe Gitlab::Ci::Variables::Builder do
...
@@ -311,20 +314,24 @@ RSpec.describe Gitlab::Ci::Variables::Builder do
end
end
it
'does not return protected variables as it is not supported for merge request pipelines'
do
it
'does not return protected variables as it is not supported for merge request pipelines'
do
is_expected
.
not_to
include
(
variable
)
is_expected
.
to
contain_exactly
(
unprotected_variable_item
)
end
end
end
end
context
'when ref is not protected'
do
context
'when ref is not protected'
do
it
{
is_expected
.
not_to
include
(
variable
)
}
it
{
is_expected
.
to
contain_exactly
(
unprotected_variable_item
)
}
end
end
end
end
end
end
describe
'#secret_instance_variables'
do
describe
'#secret_instance_variables'
do
subject
{
builder
.
secret_instance_variables
(
ref:
job
.
git_ref
)
}
subject
{
builder
.
secret_instance_variables
}
let_it_be
(
:protected_variable
)
{
create
(
:ci_instance_variable
,
protected:
true
)
}
let_it_be
(
:unprotected_variable
)
{
create
(
:ci_instance_variable
,
protected:
false
)
}
let_it_be
(
:variable
)
{
create
(
:ci_instance_variable
,
protected:
true
)
}
let
(
:protected_variable_item
)
{
Gitlab
::
Ci
::
Variables
::
Collection
::
Item
.
fabricate
(
protected_variable
)
}
let
(
:unprotected_variable_item
)
{
Gitlab
::
Ci
::
Variables
::
Collection
::
Item
.
fabricate
(
unprotected_variable
)
}
include_examples
"secret CI variables"
include_examples
"secret CI variables"
end
end
...
@@ -332,7 +339,11 @@ RSpec.describe Gitlab::Ci::Variables::Builder do
...
@@ -332,7 +339,11 @@ RSpec.describe Gitlab::Ci::Variables::Builder do
describe
'#secret_group_variables'
do
describe
'#secret_group_variables'
do
subject
{
builder
.
secret_group_variables
(
ref:
job
.
git_ref
,
environment:
job
.
expanded_environment_name
)
}
subject
{
builder
.
secret_group_variables
(
ref:
job
.
git_ref
,
environment:
job
.
expanded_environment_name
)
}
let_it_be
(
:variable
)
{
create
(
:ci_group_variable
,
protected:
true
,
group:
group
)
}
let_it_be
(
:protected_variable
)
{
create
(
:ci_group_variable
,
protected:
true
,
group:
group
)
}
let_it_be
(
:unprotected_variable
)
{
create
(
:ci_group_variable
,
protected:
false
,
group:
group
)
}
let
(
:protected_variable_item
)
{
protected_variable
}
let
(
:unprotected_variable_item
)
{
unprotected_variable
}
include_examples
"secret CI variables"
include_examples
"secret CI variables"
end
end
...
@@ -340,7 +351,11 @@ RSpec.describe Gitlab::Ci::Variables::Builder do
...
@@ -340,7 +351,11 @@ RSpec.describe Gitlab::Ci::Variables::Builder do
describe
'#secret_project_variables'
do
describe
'#secret_project_variables'
do
subject
{
builder
.
secret_project_variables
(
ref:
job
.
git_ref
,
environment:
job
.
expanded_environment_name
)
}
subject
{
builder
.
secret_project_variables
(
ref:
job
.
git_ref
,
environment:
job
.
expanded_environment_name
)
}
let_it_be
(
:variable
)
{
create
(
:ci_variable
,
protected:
true
,
project:
project
)
}
let_it_be
(
:protected_variable
)
{
create
(
:ci_variable
,
protected:
true
,
project:
project
)
}
let_it_be
(
:unprotected_variable
)
{
create
(
:ci_variable
,
protected:
false
,
project:
project
)
}
let
(
:protected_variable_item
)
{
protected_variable
}
let
(
:unprotected_variable_item
)
{
unprotected_variable
}
include_examples
"secret CI variables"
include_examples
"secret CI variables"
end
end
...
...
spec/models/ci/pipeline_spec.rb
View file @
2998c15b
...
@@ -4738,4 +4738,41 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
...
@@ -4738,4 +4738,41 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
let!
(
:model
)
{
create
(
:ci_pipeline
,
project:
parent
)
}
let!
(
:model
)
{
create
(
:ci_pipeline
,
project:
parent
)
}
end
end
end
end
describe
'#jobs_git_ref'
do
subject
{
pipeline
.
jobs_git_ref
}
context
'when tag is true'
do
let
(
:pipeline
)
{
build
(
:ci_pipeline
,
tag:
true
)
}
it
'returns a tag ref'
do
is_expected
.
to
start_with
(
Gitlab
::
Git
::
TAG_REF_PREFIX
)
end
end
context
'when tag is false'
do
let
(
:pipeline
)
{
build
(
:ci_pipeline
,
tag:
false
)
}
it
'returns a branch ref'
do
is_expected
.
to
start_with
(
Gitlab
::
Git
::
BRANCH_REF_PREFIX
)
end
end
context
'when tag is nil'
do
let
(
:pipeline
)
{
build
(
:ci_pipeline
,
tag:
nil
)
}
it
'returns a branch ref'
do
is_expected
.
to
start_with
(
Gitlab
::
Git
::
BRANCH_REF_PREFIX
)
end
end
context
'when it is triggered by a merge request'
do
let
(
:merge_request
)
{
create
(
:merge_request
,
:with_detached_merge_request_pipeline
)
}
let
(
:pipeline
)
{
merge_request
.
pipelines_for_merge_request
.
first
}
it
'returns nil'
do
is_expected
.
to
be_nil
end
end
end
end
end
spec/models/project_spec.rb
View file @
2998c15b
...
@@ -3871,45 +3871,6 @@ RSpec.describe Project, factory_default: :keep do
...
@@ -3871,45 +3871,6 @@ RSpec.describe Project, factory_default: :keep do
end
end
end
end
describe
'#ci_instance_variables_for'
do
let
(
:project
)
{
build_stubbed
(
:project
)
}
let!
(
:instance_variable
)
do
create
(
:ci_instance_variable
,
value:
'secret'
)
end
let!
(
:protected_instance_variable
)
do
create
(
:ci_instance_variable
,
:protected
,
value:
'protected'
)
end
subject
{
project
.
ci_instance_variables_for
(
ref:
'ref'
)
}
before
do
stub_application_setting
(
default_branch_protection:
Gitlab
::
Access
::
PROTECTION_NONE
)
end
context
'when the ref is not protected'
do
before
do
allow
(
project
).
to
receive
(
:protected_for?
).
with
(
'ref'
).
and_return
(
false
)
end
it
'contains only the CI variables'
do
is_expected
.
to
contain_exactly
(
instance_variable
)
end
end
context
'when the ref is protected'
do
before
do
allow
(
project
).
to
receive
(
:protected_for?
).
with
(
'ref'
).
and_return
(
true
)
end
it
'contains all the variables'
do
is_expected
.
to
contain_exactly
(
instance_variable
,
protected_instance_variable
)
end
end
end
describe
'#any_lfs_file_locks?'
,
:request_store
do
describe
'#any_lfs_file_locks?'
,
:request_store
do
let_it_be
(
:project
)
{
create
(
:project
)
}
let_it_be
(
:project
)
{
create
(
:project
)
}
...
...
Write
Preview
Markdown
is supported
0%
Try again
or
attach a new file
Attach a file
Cancel
You are about to add
0
people
to the discussion. Proceed with caution.
Finish editing this message first!
Cancel
Please
register
or
sign in
to comment