Skip to content

Conversation

dmcgowan
Copy link
Member

Currently image references end up being stored in a
random order due to the way maps are iterated through
in Go. This leads to inconsistent identifiers being
resolved when a single reference is needed to identify
an image and the ordering of the references is used for
the selection.

Sort references in a consistent and ranked manner,
from higher information formats to lower.

Note: A name + tag reference is considered higher
information than a name + digest reference since a
registry may be used to resolve the digest from a
name + tag reference.

Original Issue:
References is created using https://github.com/containerd/containerd/blob/master/pkg/cri/util/strings.go#L46 which does not guarantee any order

The first item of this array then gets used to set the image name when creating a container at https://github.com/containerd/containerd/blob/master/pkg/cri/server/container_create.go#L158

Currently image references end up being stored in a
random order due to the way maps are iterated through
in Go. This leads to inconsistent identifiers being
resolved when a single reference is needed to identify
an image and the ordering of the references is used for
the selection.

Sort references in a consistent and ranked manner,
from higher information formats to lower.

Note: A `name + tag` reference is considered higher
information than a `name + digest` reference since a
registry may be used to resolve the digest from a
`name + tag` reference.

Signed-off-by: Derek McGowan <derek@mcg.dev>
Copy link
Member

@crosbymichael crosbymichael left a comment

Choose a reason for hiding this comment

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

LGTM

@crosbymichael
Copy link
Member

I think the ordering makes sense the way you have it.

@mxpv mxpv merged commit 181e2d4 into containerd:master Mar 23, 2021
@dmcgowan dmcgowan deleted the cri-fix-reference-ordering branch March 23, 2022 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants