Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement soft deletes of manifests and blobs through the Registry HTTP API #461

Closed
stevvooe opened this issue Apr 28, 2015 · 8 comments
Closed
Assignees
Labels
Milestone

Comments

@stevvooe
Copy link
Collaborator

Per the Registry API spec, implement soft deletes of blobs and manifests. To be specific, these are objects identified by blob reference. The deletes only have to remove the internal references from the repository under which the delete is issued. Deletes by manifest tag are not required by the specification.

@stevvooe
Copy link
Collaborator Author

cc @ncdc

@RichardScothern
Copy link
Contributor

@stevvooe : some thoughts.

TLDR: use tombstone to implement soft delete for manifest and registry cache flushing.

To implement delete as per the spec the following postconditions should apply:

  1. The delete returns true
  2. Accessing the manifest (/v2/<name>/manifests/<reference>) returns ErrManifestUnknown
  3. Listing tags for a manifest (/v2/<name>/tags/list) returns ErrManifestUnknown

With the current path based layout, satisfying this requirement means that multiple link files must be deleted to fulfill this. A manifest with one tag (i.e. latest) will require at 8 IOs [1] and will be much higher when additional tags are added due to the high write fan-out when tagging a manifest.

There are two concerns here; the lesser being performance. Deleting a manifest is an infrequent operation but given an S3 latencies of ~1000ms for a private registry not hosted on EC2, waiting for double digit sequential IOs to complete is not a good experience.

The main concern is consistency. If part of the operation completes, but fails before all files are removed then not all postconditions can be met. e.g., delete manifest link succeeds, but a failure occurs removing the tag hierarchy. The delete returns error, but postcondition 3 does not hold, as some remnants of the tag hierarchy will cause (*tagStore) tags() to return a name. The possibility of failure is low, but it does exist.

An alternative to removing link files is to use tombstones. A simple approach is to place a file in the root of the repository directory when delete is called on a repository. The file will contain the date of the deletion. A single file creation on S3 and posix (and I presume azure) is atomic. If the file can't be created, delete returns error. In order to feign deletedness any manifest operations (get manifest, list tags) in the registry would check for the presence of this file and return ErrManifestUnknown.

While this doesn't strictly delete any data, the tombstone model is fairly common and I think enough to appease users that want to delete manifests.

[1] 1 - delete the manifest link file + 7 (files consisting of the tags/latest directory structure)

@stevvooe
Copy link
Collaborator Author

stevvooe commented May 2, 2015

@RichardScothern This approach sounds fine. Tombstone deletes are appropriate and we'll effectively get us a list of deleted items, if required. The specification requires supporting deletes to manifests and blobs, both keyed by digest. As clarification for your design, deletes cannot be issued to the repository (we may allow this in the future, but not for this iteration).

Right now, the path layout for each repository is as follows:

_uploads
_manifests
_layers

Since manifests and layers are both effectively blobs, I propose we add a fourth item, called '_deleted'. Upon deletion of a layer or manifest from the repository, a file should be written with the following format:

<root>/v2/repositories/<name>/_deleted/<algorithm>/<hex digest>/link

If a manifest or layer is accessed in the repository, if it is present here, it should be treated as absent. Asynchronously, the registry may attempt to remove the corresponding links.

Note that on writing new blobs or manifests, the registry must clear the deleted file.

@DreadPirateShawn
Copy link
Contributor

Two questions, with apologies if I'm not reading the details correctly:

1) Can a single tag be deleted?

The api doc defines DELETE for a specific "name and reference where reference can be a tag or digest", and postcondition 2 also includes <name> and <reference>, but postcondition 3 seems to imply that there will be no other tags remaining to be listed... does postcondition 3 only apply perhaps when the last reference of an image name is deleted?

The use case would be:

GET /v2/shawn/project/tags/list
{
  "name": "shawn/project",
  "tags": [ "latest", "release", "releasetypodammit" ]
}

DELETE /v2/shawn/project/manifests/releasetypodammit

2) Are there any timing issues if a deleted tag or image is recreated / repushed to the registry?

Use case would be accidentally deleting an image that other Dockerfiles are using, and attempting to re-add the image right away. (Or realizing it days later, after possible cleanup, and attempting to re-add the image at that time.)

@stevvooe
Copy link
Collaborator Author

@DreadPirateShawn

  1. For the reasons you've outlined, it doesn't really make a whole lot of sense to delete a manifest by tag. The [api specification] explicitly calls this out:

    For deletes, reference must be a digest or the delete will fail.

    Otherwise, the registry must keep all sorts of history about what manifest was push when and which manifest gets the new tag. This could result in some terrible surprises. Perhaps, the API call detail section needs to make this more clear (please file an issue or a PR if you think this needs to be clarified).

    There may be a way to specify this more completely, but we are going to separate tags from manifests (doc/spec: tags as a first class object #173) rather than avoid the muddy behavior around this point.

  2. This is always going to be an issue with deletes. This is part the reason why deletes are so hard to get right in any data management system. If you delete data, it is gone. Pushing a new version should make that new image available, as before, possibly removing tombstones files in the process.

    The feature covered here makes this less problematic: one can add it back by removing the tombstones. Data isn't really deleted so much as it is made unavailable.

@DreadPirateShawn
Copy link
Contributor

Thanks!

Re: 1 -- Yeah, the API definitely needs to be more clear... if you look a few lines lower in the Method / Path / Entity / Description table, DELETE says that the reference can be a tag or a digest. So yes, the [api specification] explicitly calls that out... but it calls it out twice and contradicts itself. :-P Filed #549.

@EugenMayer
Copy link

Fetching the digest https://github.com/EugenMayer/docker_registry_cli/blob/master/DockerRegistryRequest.rb#L92 getting a string like sha256:XXXX
Using this to feed into delete https://github.com/EugenMayer/docker_registry_cli/blob/master/DockerRegistryRequest.rb#L83
i get notfied, that the digest is wrong.

Using the current latest registry:2 (sha256:20f5d95004b71fe14dbe7468eff33f18ee7fa52502423c5d107d4fb0abb05c1d).

Is this a bug / fixed already?

@stevvooe
Copy link
Collaborator Author

@EugenMayer This issue has been closed since May 2015. If you think this is still an issue or need help using the library, please open another issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment