Skip to content

Return an error when deleting projects if the deletion of container repositories failed

Context

In #379021 (closed), we investigated projects being "successfully" deleted even if its container respositories were not deleted. There are several action points as seen below:

Problem

In Projects::DestroyService, when removing registry tags we have this (some code lines marked with Point X for discussion later):

def remove_registry_tags
  return true unless Gitlab.config.registry.enabled
  return false unless remove_legacy_registry_tags # Point A

  project.container_repositories.find_each do |container_repository|
    service = Projects::ContainerRepository::DestroyService.new(project, current_user)
    service.execute(container_repository)
    # Point B
  end

  true # Point C
end

In point A, when deleting legacy tags, we use ContainerRepository#delete_tags! which uses ContainerRepository#tags that has a fixed page size of 10 000 (added here) so if there are tags more than 10 000, they would not really be deleted.

Note: We should still keep the removal legacy tags: #379021 (comment 1197877273).

In point B, we simply iterate through the calls of ContainerRepository::DestroyService without checking if the service executed successfully or not. Currently, ContainerRepository::DestroyService can know if it failed or not but it does not return it yet.

In point C, we return true as soon as the iteration in finished (even if some operations in the iteration failed.)

Proposal

  1. In remove_legacy_registry_tags, use ContainerRepository::DestroyService instead of ContainerRepository#delete_tags!. #delete_tags! uses ContainerRepository#tags that uses a fixed page size of 10 000 (added here) so if there are tags more than 10 000, they would not really be deleted.

  2. Update Projects::ContainerRepository::DestroyService such that it returns a proper response. By proper response, we mean that from the return value, we can determine if the operation succeeded or not.

  3. Once (2.) above is done, we update #attempt_destroy by returning true/false depending if DestroyService for all containers succeeded.