-
Notifications
You must be signed in to change notification settings - Fork 205
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
conformance test: tear down manifests before blobs #156
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@majewsky please see this issue for some context: #134
I think modifying this order will break the test for most other registries, so I think that either:
- You should set necessary env vars prevent pull setup/teardown (please see https://github.com/opencontainers/distribution-spec/tree/master/conformance#pull)
- This PR should be modified to switch the order based on the presence of some env var
My own registry implementation, Keppel [1] forbids deleting blobs that are still referenced by manifests. To do a proper cleanup in this case, the manifest needs to be deleted first. [1] https://github.com/sapcc/keppel Signed-off-by: Stefan Majewsky <[email protected]>
1d786d5
to
567865a
Compare
Done. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@majewsky looks good - either in this or a follow up PR, we should document OCI_DELETE_MANIFEST_BEFORE_BLOBS
in the readme
Signed-off-by: Stefan Majewsky <[email protected]>
Documentation added. |
@majewsky looks good. Last thing is the DCO check failed on your last commit. Can you amend the commit to contain signoff then we can merge? |
Signed-off-by: Stefan Majewsky <[email protected]>
9620234
to
4e45fd3
Compare
@majewsky - thanks for you patience, looks like the final commit had a different email in signed-off-by which might be causing an error: https://pullapprove.com/opencontainers/distribution-spec/pull-request/156/requirements/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
see Vince's #170 rebase PR
As explained in the added comments, "some registries" (i.e. my own implementation, Keppel https://github.com/sapcc/keppel) forbid deleting blobs that are still referenced by manifests. To do a proper cleanup in this case, the manifest needs to be deleted first.
The diff looks a bit weird, but I really just swapped the order of some paragraphs, pulling the manifest deletion to the top.