Commit 01990196 authored by Kamil Trzciński's avatar Kamil Trzciński

Merge branch 'fix/sm/fix_pages_worker-ee' into 'master'

Port to EE: Reload stable object to prevent erase artifacts with old state

See merge request gitlab-org/gitlab-ee!4844
parents b89accf8 4790fb18
module Projects
class UpdatePagesService < BaseService
InvaildStateError = Class.new(StandardError)
FailedToExtractError = Class.new(StandardError)
BLOCK_SIZE = 32.kilobytes
MAX_SIZE = 1.terabyte
SITE_PATH = 'public/'.freeze
......@@ -11,13 +14,15 @@ module Projects
end
def execute
register_attempt
# Create status notifying the deployment of pages
@status = create_status
@status.enqueue!
@status.run!
raise 'missing pages artifacts' unless build.artifacts?
raise 'pages are outdated' unless latest?
raise InvaildStateError, 'missing pages artifacts' unless build.artifacts?
raise InvaildStateError, 'pages are outdated' unless latest?
# Create temporary directory in which we will extract the artifacts
FileUtils.mkdir_p(tmp_path)
......@@ -26,24 +31,22 @@ module Projects
# Check if we did extract public directory
archive_public_path = File.join(archive_path, 'public')
raise 'pages miss the public folder' unless Dir.exist?(archive_public_path)
raise 'pages are outdated' unless latest?
raise FailedToExtractError, 'pages miss the public folder' unless Dir.exist?(archive_public_path)
raise InvaildStateError, 'pages are outdated' unless latest?
deploy_page!(archive_public_path)
success
end
rescue => e
rescue InvaildStateError, FailedToExtractError => e
register_failure
error(e.message)
ensure
register_attempt
build.erase_artifacts! unless build.has_expiring_artifacts?
end
private
def success
@status.success
delete_artifact!
super
end
......@@ -52,6 +55,7 @@ module Projects
@status.allow_failure = !latest?
@status.description = message
@status.drop(:script_failure)
delete_artifact!
super
end
......@@ -72,7 +76,7 @@ module Projects
elsif artifacts_filename.ends_with?('.zip')
extract_zip_archive!(temp_path)
else
raise 'unsupported artifacts format'
raise FailedToExtractError, 'unsupported artifacts format'
end
end
......@@ -82,18 +86,18 @@ module Projects
%W(dd bs=#{BLOCK_SIZE} count=#{blocks}),
%W(tar -x -C #{temp_path} #{SITE_PATH}),
err: '/dev/null')
raise 'pages failed to extract' unless results.compact.all?(&:success?)
raise FailedToExtractError, 'pages failed to extract' unless results.compact.all?(&:success?)
end
end
def extract_zip_archive!(temp_path)
raise 'missing artifacts metadata' unless build.artifacts_metadata?
raise FailedToExtractError, 'missing artifacts metadata' unless build.artifacts_metadata?
# Calculate page size after extract
public_entry = build.artifacts_metadata_entry(SITE_PATH, recursive: true)
if public_entry.total_size > max_size
raise "artifacts for pages are too large: #{public_entry.total_size}"
raise FailedToExtractError, "artifacts for pages are too large: #{public_entry.total_size}"
end
# Requires UnZip at least 6.00 Info-ZIP.
......@@ -103,7 +107,7 @@ module Projects
site_path = File.join(SITE_PATH, '*')
build.artifacts_file.use_file do |artifacts_path|
unless system(*%W(unzip -qq -n #{artifacts_path} #{site_path} -d #{temp_path}))
raise 'pages failed to extract'
raise FailedToExtractError, 'pages failed to extract'
end
end
end
......@@ -167,6 +171,11 @@ module Projects
build.ref
end
def delete_artifact!
build.reload # Reload stable object to prevent erase artifacts with old state
build.erase_artifacts! unless build.has_expiring_artifacts?
end
def latest_sha
project.commit(build.ref).try(:sha).to_s
end
......
class PagesWorker
include ApplicationWorker
sidekiq_options retry: false
sidekiq_options retry: 3
def perform(action, *arg)
send(action, *arg) # rubocop:disable GitlabSecurity/PublicSend
......
---
title: Fix pages flaky failure by reloading stale object
merge_request: 17522
author:
type: fixed
......@@ -258,7 +258,7 @@ feature 'Pages' do
end
let(:ci_build) do
build(
create(
:ci_build,
project: project,
pipeline: pipeline,
......
......@@ -34,6 +34,7 @@ describe Projects::UpdatePagesService do
context 'with expiry date' do
before do
build.artifacts_expire_in = "2 days"
build.save!
end
it "doesn't delete artifacts" do
......@@ -105,6 +106,7 @@ describe Projects::UpdatePagesService do
context 'with expiry date' do
before do
build.artifacts_expire_in = "2 days"
build.save!
end
it "doesn't delete artifacts" do
......@@ -159,6 +161,20 @@ describe Projects::UpdatePagesService do
expect(execute).not_to eq(:success)
end
context 'when timeout happens by DNS error' do
before do
allow_any_instance_of(described_class)
.to receive(:extract_zip_archive!).and_raise(SocketError)
end
it 'raises an error' do
expect { execute }.to raise_error(SocketError)
build.reload
expect(build.artifacts?).to eq(true)
end
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