Commit e5301b65 authored by Lin Jen-Shin's avatar Lin Jen-Shin 🔴

Merge branch 'sh-fix-pipeline-delete-caching' into 'master'

Properly expire all pipeline caches when pipeline is deleted

Closes #60469

See merge request gitlab-org/gitlab-ce!27334
parents 57cdf56a 16259796
Pipeline #56906328 passed with stages
in 48 minutes and 45 seconds
......@@ -5,9 +5,9 @@ module Ci
def execute(pipeline)
raise Gitlab::Access::AccessDeniedError unless can?(current_user, :destroy_pipeline, pipeline)
pipeline.destroy!
Ci::ExpirePipelineCacheService.new.execute(pipeline, delete: true)
Gitlab::Cache::Ci::ProjectPipelineStatus.new(pipeline.project).delete_from_cache
pipeline.destroy!
end
end
end
# frozen_string_literal: true
module Ci
class ExpirePipelineCacheService
def execute(pipeline, delete: false)
store = Gitlab::EtagCaching::Store.new
update_etag_cache(pipeline, store)
if delete
Gitlab::Cache::Ci::ProjectPipelineStatus.new(pipeline.project).delete_from_cache
else
Gitlab::Cache::Ci::ProjectPipelineStatus.update_for_pipeline(pipeline)
end
end
private
def project_pipelines_path(project)
Gitlab::Routing.url_helpers.project_pipelines_path(project, format: :json)
end
def project_pipeline_path(project, pipeline)
Gitlab::Routing.url_helpers.project_pipeline_path(project, pipeline, format: :json)
end
def commit_pipelines_path(project, commit)
Gitlab::Routing.url_helpers.pipelines_project_commit_path(project, commit.id, format: :json)
end
def new_merge_request_pipelines_path(project)
Gitlab::Routing.url_helpers.project_new_merge_request_path(project, format: :json)
end
def each_pipelines_merge_request_path(pipeline)
pipeline.all_merge_requests.each do |merge_request|
path = Gitlab::Routing.url_helpers.pipelines_project_merge_request_path(merge_request.target_project, merge_request, format: :json)
yield(path)
end
end
# Updates ETag caches of a pipeline.
#
# This logic resides in a separate method so that EE can more easily extend
# it.
#
# @param [Ci::Pipeline] pipeline
# @param [Gitlab::EtagCaching::Store] store
def update_etag_cache(pipeline, store)
project = pipeline.project
store.touch(project_pipelines_path(project))
store.touch(project_pipeline_path(project, pipeline))
store.touch(commit_pipelines_path(project, pipeline.commit)) unless pipeline.commit.nil?
store.touch(new_merge_request_pipelines_path(project))
each_pipelines_merge_request_path(pipeline) do |path|
store.touch(path)
end
end
end
end
......@@ -11,56 +11,7 @@ class ExpirePipelineCacheWorker
pipeline = Ci::Pipeline.find_by(id: pipeline_id)
return unless pipeline
store = Gitlab::EtagCaching::Store.new
update_etag_cache(pipeline, store)
Gitlab::Cache::Ci::ProjectPipelineStatus.update_for_pipeline(pipeline)
Ci::ExpirePipelineCacheService.new.execute(pipeline)
end
# rubocop: enable CodeReuse/ActiveRecord
private
def project_pipelines_path(project)
Gitlab::Routing.url_helpers.project_pipelines_path(project, format: :json)
end
def project_pipeline_path(project, pipeline)
Gitlab::Routing.url_helpers.project_pipeline_path(project, pipeline, format: :json)
end
def commit_pipelines_path(project, commit)
Gitlab::Routing.url_helpers.pipelines_project_commit_path(project, commit.id, format: :json)
end
def new_merge_request_pipelines_path(project)
Gitlab::Routing.url_helpers.project_new_merge_request_path(project, format: :json)
end
def each_pipelines_merge_request_path(pipeline)
pipeline.all_merge_requests.each do |merge_request|
path = Gitlab::Routing.url_helpers.pipelines_project_merge_request_path(merge_request.target_project, merge_request, format: :json)
yield(path)
end
end
# Updates ETag caches of a pipeline.
#
# This logic resides in a separate method so that EE can more easily extend
# it.
#
# @param [Ci::Pipeline] pipeline
# @param [Gitlab::EtagCaching::Store] store
def update_etag_cache(pipeline, store)
project = pipeline.project
store.touch(project_pipelines_path(project))
store.touch(project_pipeline_path(project, pipeline))
store.touch(commit_pipelines_path(project, pipeline.commit)) unless pipeline.commit.nil?
store.touch(new_merge_request_pipelines_path(project))
each_pipelines_merge_request_path(pipeline) do |path|
store.touch(path)
end
end
end
---
title: Properly expire all pipeline caches when pipeline is deleted
merge_request: 27334
author:
type: fixed
# frozen_string_literal: true
require 'spec_helper'
describe Ci::ExpirePipelineCacheService do
set(:user) { create(:user) }
set(:project) { create(:project) }
set(:pipeline) { create(:ci_pipeline, project: project) }
subject { described_class.new }
describe '#execute' do
it 'invalidates Etag caching for project pipelines path' do
pipelines_path = "/#{project.full_path}/pipelines.json"
new_mr_pipelines_path = "/#{project.full_path}/merge_requests/new.json"
pipeline_path = "/#{project.full_path}/pipelines/#{pipeline.id}.json"
expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(pipelines_path)
expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(new_mr_pipelines_path)
expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(pipeline_path)
subject.execute(pipeline)
end
it 'invalidates Etag caching for merge request pipelines if pipeline runs on any commit of that source branch' do
pipeline = create(:ci_empty_pipeline, status: 'created', project: project, ref: 'master')
merge_request = create(:merge_request, source_project: project, source_branch: pipeline.ref)
merge_request_pipelines_path = "/#{project.full_path}/merge_requests/#{merge_request.iid}/pipelines.json"
allow_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch)
expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(merge_request_pipelines_path)
subject.execute(pipeline)
end
it 'updates the cached status for a project' do
expect(Gitlab::Cache::Ci::ProjectPipelineStatus).to receive(:update_for_pipeline)
.with(pipeline)
subject.execute(pipeline)
end
context 'destroyed pipeline' do
let(:project_with_repo) { create(:project, :repository) }
let!(:pipeline_with_commit) { create(:ci_pipeline, :success, project: project_with_repo, sha: project_with_repo.commit.id) }
it 'clears the cache', :use_clean_rails_memory_store_caching do
create(:commit_status, :success, pipeline: pipeline_with_commit, ref: pipeline_with_commit.ref)
# Sanity check
expect(project_with_repo.pipeline_status.has_status?).to be_truthy
subject.execute(pipeline_with_commit, delete: true)
pipeline_with_commit.destroy!
# Need to use find to avoid memoization
expect(Project.find(project_with_repo.id).pipeline_status.has_status?).to be_falsey
end
end
end
end
......@@ -9,40 +9,17 @@ describe ExpirePipelineCacheWorker do
subject { described_class.new }
describe '#perform' do
it 'invalidates Etag caching for project pipelines path' do
pipelines_path = "/#{project.full_path}/pipelines.json"
new_mr_pipelines_path = "/#{project.full_path}/merge_requests/new.json"
pipeline_path = "/#{project.full_path}/pipelines/#{pipeline.id}.json"
expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(pipelines_path)
expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(new_mr_pipelines_path)
expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(pipeline_path)
subject.perform(pipeline.id)
end
it 'invalidates Etag caching for merge request pipelines if pipeline runs on any commit of that source branch' do
pipeline = create(:ci_empty_pipeline, status: 'created', project: project, ref: 'master')
merge_request = create(:merge_request, source_project: project, source_branch: pipeline.ref)
merge_request_pipelines_path = "/#{project.full_path}/merge_requests/#{merge_request.iid}/pipelines.json"
allow_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch)
expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(merge_request_pipelines_path)
it 'executes the service' do
expect_any_instance_of(Ci::ExpirePipelineCacheService).to receive(:execute).with(pipeline).and_call_original
subject.perform(pipeline.id)
end
it "doesn't do anything if the pipeline not exist" do
expect_any_instance_of(Ci::ExpirePipelineCacheService).not_to receive(:execute)
expect_any_instance_of(Gitlab::EtagCaching::Store).not_to receive(:touch)
subject.perform(617748)
end
it 'updates the cached status for a project' do
expect(Gitlab::Cache::Ci::ProjectPipelineStatus).to receive(:update_for_pipeline)
.with(pipeline)
subject.perform(pipeline.id)
end
end
end
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment