Skip to content
This repository has been archived by the owner on Aug 14, 2020. It is now read-only.

repository2: fix manifest v2.2 layer ordering. #190

Merged
merged 1 commit into from
Aug 4, 2016

Conversation

sgotti
Copy link
Member

@sgotti sgotti commented Aug 3, 2016

With docker manifest v2.2 and oci the layers are ordered from the base layer to
the topmost layer.

This is just a quick fix for review. Next steps will be to add tests and remove duplicated layers.

Fixes #189

for i := len(layerIDs) - 1; i > 0; i-- {
log.Debug("Generating layer ACI...")

log.Debug("Generating layer ACI...")
Copy link
Member

Choose a reason for hiding this comment

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

why did you move this outside the loop?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because if there's only one layer it won't be written.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, I noticed that it should be written for every layer, will fix. Thanks!

With docker manifest v2.2 and oci the layers are ordered from the base layer to
the topmost layer.
@lucab
Copy link
Contributor

lucab commented Aug 4, 2016

I confirm this fixes troublesome containers that we've got reported so far, and LGTM (but I don't have any super-power here).

@jonboulle jonboulle merged commit 606a7fe into appc:master Aug 4, 2016
@lucab
Copy link
Contributor

lucab commented Aug 4, 2016

wrt duplicate layers, @sgotti privately pointed out that distribution/distribution#1527 is a relevant reference.

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.

Layer ordering is somehow wrong on API v2.2
4 participants