Introduce cache policies for CI jobs

changelogs/unreleased/33772-readonly-gitlab-ci-cache.yml

@@ -0,0 +1,4 @@
+---
+title: Introduce cache policies for CI jobs
+merge_request: 12483
+author:

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

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

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

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

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

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

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

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

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