Skip to content
This repository has been archived by the owner on Dec 7, 2022. It is now read-only.

Add a unit for the new Docker metadata type. #76

Closed
wants to merge 1 commit into from

Conversation

bowlofeggs
Copy link
Contributor

The Docker v2 API introduces a new Image Manifest type, described here:

https://github.com/docker/distribution/blob/release/2.0/docs/spec/manifest-v2-1.md

https://pulp.plan.io/issues/967

re #967

@mhrivnak
Copy link
Contributor

mhrivnak commented Jul 3, 2015

Using the list of layers as a unit key member is interesting. I'm hesitant about it, but I'm not entirely sure why. It looks like mongo can index an array field, but if we switch to postgres some day, can it?

My suggestion was to use the "digest" instead. The manifest API references the digest quite a bit, and here are some docs about what it should be: https://docs.docker.com/registry/spec/api/#content-digests

It's weird that some calls, like deleting a manifest, only allow you to reference the manifest by its digest, BUT the API does not require a specific digest algorithm. I'm not sure what behavior a registry should exhibit if a request uses a digest with an algorithm that particular registry doesn't support.

In any case, this was my basic idea: when you get a manifest from an upstream registry, it returns the digest in a header. Based on the docs, I expect that to use sha256 until further notice. We can just store that, and worry later about how to calculate the digest ourselves. This is why doing sync from a registry is a different story in redmine from doing a sync from static files, which would lack the digest header.

Using the layers for uniqueness would certainly be simpler, but it seems the API spec really wants to use the digest to establish uniqueness. What do you think?

digest = 'sha256:6c3c624b58dbbcd3c0dd82b4c53f04194d1247c6eebdaab7c610cf7d66709b3b'
fs_layers = [{'layer_1': 'rsum:jsf'}]
history = [{'v1Compatibility': 'not sure what goes here but something does'}]
schema_version = math.pi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering why you would need to import math. Now I see that you didn't, and all is clear. ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol

@mhrivnak mhrivnak added the LGTM label Jul 10, 2015
@mhrivnak mhrivnak assigned mhrivnak and bowlofeggs and unassigned mhrivnak Jul 10, 2015
@bowlofeggs
Copy link
Contributor Author

This was LGTM'd, but we decided not to merge this against the 1.0-* branches. Instead, I've started a new docker_v2_api feature branch that contains this commit and is based on 1.0-release. This way downstream users can take this patch and apply it to pulp-docker-1.0.x releases cleanly, but upstream will only release it in >= 1.1.

This commit has also been merged to master.

@bowlofeggs bowlofeggs closed this Jul 13, 2015
@bowlofeggs bowlofeggs deleted the 967 branch July 13, 2015 20:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants