diff options
author | Stan Hu <stanhu@gmail.com> | 2019-04-12 12:29:47 -0700 |
---|---|---|
committer | Stan Hu <stanhu@gmail.com> | 2019-04-15 08:10:10 -0700 |
commit | 16259796539f19f7b04ad078a28aa38c5b50912f (patch) | |
tree | d3596f714417223feb8b788a8f037ce2406e4531 /app | |
parent | 0a99e0220d9371423039f05f700af3675b26624f (diff) | |
download | gitlab-ce-16259796539f19f7b04ad078a28aa38c5b50912f.tar.gz |
Properly expire all pipeline caches when pipeline is deletedsh-fix-pipeline-delete-caching
When deleting a pipeline, only some of the cache structures were being
expired, but not the full pipeline list. We have to synchronously
schedule a pipeline cache expiration because the pipeline will be
deleted if the Sidekiq expiration job picks it up. To do this, properly
extract all the logic buried in the Sidekiq worker into a service, and
then call the service.
Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/60469
Diffstat (limited to 'app')
-rw-r--r-- | app/services/ci/destroy_pipeline_service.rb | 4 | ||||
-rw-r--r-- | app/services/ci/expire_pipeline_cache_service.rb | 62 | ||||
-rw-r--r-- | app/workers/expire_pipeline_cache_worker.rb | 51 |
3 files changed, 65 insertions, 52 deletions
diff --git a/app/services/ci/destroy_pipeline_service.rb b/app/services/ci/destroy_pipeline_service.rb index 2292ec42b16..9aea20c45f7 100644 --- a/app/services/ci/destroy_pipeline_service.rb +++ b/app/services/ci/destroy_pipeline_service.rb @@ -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 diff --git a/app/services/ci/expire_pipeline_cache_service.rb b/app/services/ci/expire_pipeline_cache_service.rb new file mode 100644 index 00000000000..d8d38128af6 --- /dev/null +++ b/app/services/ci/expire_pipeline_cache_service.rb @@ -0,0 +1,62 @@ +# 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 diff --git a/app/workers/expire_pipeline_cache_worker.rb b/app/workers/expire_pipeline_cache_worker.rb index 2c070d482a1..78e68d7bf46 100644 --- a/app/workers/expire_pipeline_cache_worker.rb +++ b/app/workers/expire_pipeline_cache_worker.rb @@ -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 |