diff --git a/changelogs/unreleased/33772-readonly-gitlab-ci-cache.yml b/changelogs/unreleased/33772-readonly-gitlab-ci-cache.yml new file mode 100644 index 00000000000..c2bce368a58 --- /dev/null +++ b/changelogs/unreleased/33772-readonly-gitlab-ci-cache.yml @@ -0,0 +1,4 @@ +--- +title: Introduce cache policies for CI jobs +merge_request: 12483 +author: diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 8a0662db6fd..724843a4d56 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -306,6 +306,53 @@ cache: untracked: true ``` +### cache:policy + +> Introduced in GitLab 9.4. + +The default behaviour of a caching job is to download the files at the start of +execution, and to re-upload them at the end. This allows any changes made by the +job to be persisted for future runs, and is known as the `pull-push` cache +policy. + +If you know the job doesn't alter the cached files, you can skip the upload step +by setting `policy: pull` in the job specification. Typically, this would be +twinned with an ordinary cache job at an earlier stage to ensure the cache +is updated from time to time: + +```yaml +stages: + - setup + - test + +prepare: + stage: setup + cache: + key: gems + paths: + - vendor/bundle + script: + - bundle install --deployment + +rspec: + stage: test + cache: + key: gems + paths: + - vendor/bundle + policy: pull + script: + - bundle exec rspec ... +``` + +This helps to speed up job execution and reduce load on the cache server, +especially when you have a large number of cache-using jobs executing in +parallel. + +Additionally, if you have a job that unconditionally recreates the cache without +reference to its previous contents, you can use `policy: push` in that job to +skip the download step. + ## Jobs `.gitlab-ci.yml` allows you to specify an unlimited number of jobs. Each job diff --git a/lib/api/entities.rb b/lib/api/entities.rb index cef5a0abe12..698e2ab3e8e 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -831,7 +831,7 @@ module API end class Cache < Grape::Entity - expose :key, :untracked, :paths + expose :key, :untracked, :paths, :policy end class Credentials < Grape::Entity diff --git a/lib/gitlab/ci/config/entry/cache.rb b/lib/gitlab/ci/config/entry/cache.rb index f074df9c7a1..d7e09acbbf3 100644 --- a/lib/gitlab/ci/config/entry/cache.rb +++ b/lib/gitlab/ci/config/entry/cache.rb @@ -7,11 +7,14 @@ module Gitlab # class Cache < Node include Configurable + include Attributable - ALLOWED_KEYS = %i[key untracked paths].freeze + ALLOWED_KEYS = %i[key untracked paths policy].freeze + DEFAULT_POLICY = 'pull-push'.freeze validations do validates :config, allowed_keys: ALLOWED_KEYS + validates :policy, inclusion: { in: %w[pull-push push pull], message: 'should be pull-push, push, or pull' }, allow_blank: true end entry :key, Entry::Key, @@ -25,8 +28,15 @@ module Gitlab helpers :key + attributes :policy + def value - super.merge(key: key_value) + result = super + + result[:key] = key_value + result[:policy] = policy || DEFAULT_POLICY + + result end end end diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index 0cc498f0ce9..a77f01ecb00 100644 --- a/spec/factories/ci/builds.rb +++ b/spec/factories/ci/builds.rb @@ -207,7 +207,8 @@ FactoryGirl.define do cache: { key: 'cache_key', untracked: false, - paths: ['vendor/*'] + paths: ['vendor/*'], + policy: 'pull-push' } } end diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb index 482f03aa0cc..ef58ef1b0cd 100644 --- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb +++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb @@ -878,7 +878,8 @@ module Ci expect(config_processor.builds_for_stage_and_ref("test", "master").first[:options][:cache]).to eq( paths: ["logs/", "binaries/"], untracked: true, - key: 'key' + key: 'key', + policy: 'pull-push' ) end @@ -896,7 +897,8 @@ module Ci expect(config_processor.builds_for_stage_and_ref("test", "master").first[:options][:cache]).to eq( paths: ["logs/", "binaries/"], untracked: true, - key: 'key' + key: 'key', + policy: 'pull-push' ) end @@ -915,7 +917,8 @@ module Ci expect(config_processor.builds_for_stage_and_ref("test", "master").first[:options][:cache]).to eq( paths: ["test/"], untracked: false, - key: 'local' + key: 'local', + policy: 'pull-push' ) end end diff --git a/spec/lib/gitlab/ci/config/entry/cache_spec.rb b/spec/lib/gitlab/ci/config/entry/cache_spec.rb index 878b1d6b862..8f711e02f9b 100644 --- a/spec/lib/gitlab/ci/config/entry/cache_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/cache_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Gitlab::Ci::Config::Entry::Cache do - let(:entry) { described_class.new(config) } + subject(:entry) { described_class.new(config) } describe 'validations' do before do @@ -9,22 +9,44 @@ describe Gitlab::Ci::Config::Entry::Cache do end context 'when entry config value is correct' do + let(:policy) { nil } + let(:config) do { key: 'some key', untracked: true, - paths: ['some/path/'] } + paths: ['some/path/'], + policy: policy } end describe '#value' do it 'returns hash value' do - expect(entry.value).to eq config + expect(entry.value).to eq(key: 'some key', untracked: true, paths: ['some/path/'], policy: 'pull-push') end end describe '#valid?' do - it 'is valid' do - expect(entry).to be_valid - end + it { is_expected.to be_valid } + end + + context 'policy is pull-push' do + let(:policy) { 'pull-push' } + + it { is_expected.to be_valid } + it { expect(entry.value).to include(policy: 'pull-push') } + end + + context 'policy is push' do + let(:policy) { 'push' } + + it { is_expected.to be_valid } + it { expect(entry.value).to include(policy: 'push') } + end + + context 'policy is pull' do + let(:policy) { 'pull' } + + it { is_expected.to be_valid } + it { expect(entry.value).to include(policy: 'pull') } end context 'when key is missing' do @@ -44,12 +66,20 @@ describe Gitlab::Ci::Config::Entry::Cache do context 'when entry value is not correct' do describe '#errors' do + subject { entry.errors } context 'when is not a hash' do let(:config) { 'ls' } it 'reports errors with config value' do - expect(entry.errors) - .to include 'cache config should be a hash' + is_expected.to include 'cache config should be a hash' + end + end + + context 'when policy is unknown' do + let(:config) { { policy: "unknown" } } + + it 'reports error' do + is_expected.to include('cache policy should be pull-push, push, or pull') end end @@ -57,8 +87,7 @@ describe Gitlab::Ci::Config::Entry::Cache do let(:config) { { key: 1 } } it 'reports error with descendants' do - expect(entry.errors) - .to include 'key config should be a string or symbol' + is_expected.to include 'key config should be a string or symbol' end end @@ -66,8 +95,7 @@ describe Gitlab::Ci::Config::Entry::Cache do let(:config) { { invalid: true } } it 'reports error with descendants' do - expect(entry.errors) - .to include 'cache config contains unknown keys: invalid' + is_expected.to include 'cache config contains unknown keys: invalid' end end end diff --git a/spec/lib/gitlab/ci/config/entry/global_spec.rb b/spec/lib/gitlab/ci/config/entry/global_spec.rb index 293f112b2b0..1860ed79bfd 100644 --- a/spec/lib/gitlab/ci/config/entry/global_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/global_spec.rb @@ -143,7 +143,7 @@ describe Gitlab::Ci::Config::Entry::Global do describe '#cache_value' do it 'returns cache configuration' do expect(global.cache_value) - .to eq(key: 'k', untracked: true, paths: ['public/']) + .to eq(key: 'k', untracked: true, paths: ['public/'], policy: 'pull-push') end end @@ -157,7 +157,7 @@ describe Gitlab::Ci::Config::Entry::Global do image: { name: 'ruby:2.2' }, services: [{ name: 'postgres:9.1' }, { name: 'mysql:5.5' }], stage: 'test', - cache: { key: 'k', untracked: true, paths: ['public/'] }, + cache: { key: 'k', untracked: true, paths: ['public/'], policy: 'pull-push' }, variables: { 'VAR' => 'value' }, ignore: false, after_script: ['make clean'] }, @@ -168,7 +168,7 @@ describe Gitlab::Ci::Config::Entry::Global do image: { name: 'ruby:2.2' }, services: [{ name: 'postgres:9.1' }, { name: 'mysql:5.5' }], stage: 'test', - cache: { key: 'k', untracked: true, paths: ['public/'] }, + cache: { key: 'k', untracked: true, paths: ['public/'], policy: 'pull-push' }, variables: {}, ignore: false, after_script: ['make clean'] } @@ -212,7 +212,7 @@ describe Gitlab::Ci::Config::Entry::Global do describe '#cache_value' do it 'returns correct cache definition' do - expect(global.cache_value).to eq(key: 'a') + expect(global.cache_value).to eq(key: 'a', policy: 'pull-push') end end end diff --git a/spec/lib/gitlab/ci/config/entry/job_spec.rb b/spec/lib/gitlab/ci/config/entry/job_spec.rb index 92cba689f47..c5cad887b64 100644 --- a/spec/lib/gitlab/ci/config/entry/job_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/job_spec.rb @@ -109,7 +109,7 @@ describe Gitlab::Ci::Config::Entry::Job do it 'overrides global config' do expect(entry[:image].value).to eq(name: 'some_image') - expect(entry[:cache].value).to eq(key: 'test') + expect(entry[:cache].value).to eq(key: 'test', policy: 'pull-push') end end @@ -123,7 +123,7 @@ describe Gitlab::Ci::Config::Entry::Job do it 'uses config from global entry' do expect(entry[:image].value).to eq 'specified' - expect(entry[:cache].value).to eq(key: 'test') + expect(entry[:cache].value).to eq(key: 'test', policy: 'pull-push') end end end diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index 339a57a1f20..ca5d98c78ef 100644 --- a/spec/requests/api/runner_spec.rb +++ b/spec/requests/api/runner_spec.rb @@ -351,7 +351,8 @@ describe API::Runner do let(:expected_cache) do [{ 'key' => 'cache_key', 'untracked' => false, - 'paths' => ['vendor/*'] }] + 'paths' => ['vendor/*'], + 'policy' => 'pull-push' }] end it 'picks a job' do