Commit 547bf581 authored by Giorgenes Gelatti's avatar Giorgenes Gelatti

Better handle docker errors

Add specs to dummy manifest upload errors
Add specs to dummy delete errors
Fix crash associated with docker error when deleting tags
parent 6bb277fc
...@@ -22,10 +22,18 @@ module Projects ...@@ -22,10 +22,18 @@ module Projects
def smart_delete(container_repository, tag_names) def smart_delete(container_repository, tag_names)
# generates the blobs for the dummy image # generates the blobs for the dummy image
dummy_manifest = container_repository.client.generate_empty_manifest(container_repository.path) dummy_manifest = container_repository.client.generate_empty_manifest(container_repository.path)
return error('could not generate manifest') if dummy_manifest.nil?
# update the manifests of the tags with the new dummy image # update the manifests of the tags with the new dummy image
tag_digests = tag_names.map do |name| deleted_tags = []
container_repository.client.put_tag(container_repository.path, name, dummy_manifest) tag_digests = []
tag_names.each do |name|
digest = container_repository.client.put_tag(container_repository.path, name, dummy_manifest)
next unless digest
deleted_tags << name
tag_digests << digest
end end
# make sure the digests are the same (it should always be) # make sure the digests are the same (it should always be)
...@@ -37,8 +45,8 @@ module Projects ...@@ -37,8 +45,8 @@ module Projects
# Deletes the dummy image # Deletes the dummy image
# All created tag digests are the same since they all have the same dummy image. # All created tag digests are the same since they all have the same dummy image.
# a single delete is sufficient to remove all tags with it # a single delete is sufficient to remove all tags with it
if container_repository.delete_tag_by_digest(tag_digests.first) if tag_digests.any? && container_repository.delete_tag_by_digest(tag_digests.first)
success(deleted: tag_names) success(deleted: deleted_tags)
else else
error('could not delete tags') error('could not delete tags')
end end
......
...@@ -51,7 +51,7 @@ module ContainerRegistry ...@@ -51,7 +51,7 @@ module ContainerRegistry
def upload_blob(name, content, digest) def upload_blob(name, content, digest)
upload = faraday.post("/v2/#{name}/blobs/uploads/") upload = faraday.post("/v2/#{name}/blobs/uploads/")
return unless upload.success? return upload unless upload.success?
location = URI(upload.headers['location']) location = URI(upload.headers['location'])
......
...@@ -99,8 +99,8 @@ describe ContainerRegistry::Client do ...@@ -99,8 +99,8 @@ describe ContainerRegistry::Client do
stub_upload('path', 'content', 'sha256:123', 400) stub_upload('path', 'content', 'sha256:123', 400)
end end
it 'returns nil' do it 'returns a failure' do
expect(subject).to be nil expect(subject).not_to be_success
end end
end end
end end
...@@ -125,6 +125,14 @@ describe ContainerRegistry::Client do ...@@ -125,6 +125,14 @@ describe ContainerRegistry::Client do
expect(subject).to eq(result_manifest) expect(subject).to eq(result_manifest)
end end
context 'when upload fails' do
before do
stub_upload('path', "{\n \"config\": {\n }\n}", 'sha256:4435000728ee66e6a80e55637fc22725c256b61de344a2ecdeaac6bdb36e8bc3', 500)
end
it { is_expected.to be nil }
end
end end
describe '#put_tag' do describe '#put_tag' do
......
...@@ -88,6 +88,33 @@ describe Projects::ContainerRepository::DeleteTagsService do ...@@ -88,6 +88,33 @@ describe Projects::ContainerRepository::DeleteTagsService do
is_expected.to include(status: :success) is_expected.to include(status: :success)
end end
context 'with failures' do
context 'when the dummy manifest generation fails' do
before do
stub_upload("{\n \"config\": {\n }\n}", 'sha256:4435000728ee66e6a80e55637fc22725c256b61de344a2ecdeaac6bdb36e8bc3', success: false)
end
it { is_expected.to include(status: :error) }
end
context 'when updating the tags fails' do
before do
stub_upload("{\n \"config\": {\n }\n}", 'sha256:4435000728ee66e6a80e55637fc22725c256b61de344a2ecdeaac6bdb36e8bc3')
stub_request(:put, "http://registry.gitlab/v2/#{repository.path}/manifests/A")
.to_return(status: 500, body: "", headers: { 'docker-content-digest' => 'sha256:dummy' })
stub_request(:put, "http://registry.gitlab/v2/#{repository.path}/manifests/Ba")
.to_return(status: 500, body: "", headers: { 'docker-content-digest' => 'sha256:dummy' })
stub_request(:delete, "http://registry.gitlab/v2/#{repository.path}/manifests/sha256:4435000728ee66e6a80e55637fc22725c256b61de344a2ecdeaac6bdb36e8bc3")
.to_return(status: 200, body: "", headers: {})
end
it { is_expected.to include(status: :error) }
end
end
end end
end end
end end
...@@ -107,10 +134,10 @@ describe Projects::ContainerRepository::DeleteTagsService do ...@@ -107,10 +134,10 @@ describe Projects::ContainerRepository::DeleteTagsService do
end end
end end
def stub_upload(content, digest) def stub_upload(content, digest, success: true)
expect_any_instance_of(ContainerRegistry::Client) expect_any_instance_of(ContainerRegistry::Client)
.to receive(:upload_blob) .to receive(:upload_blob)
.with(repository.path, content, digest) { double(success?: true ) } .with(repository.path, content, digest) { double(success?: success ) }
end end
def expect_delete_tag(digest) def expect_delete_tag(digest)
......
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