cephadm: normalize repo digests #40577
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
src/cephadm/cephadm
Outdated
| # docker.io/ubuntu -> no change | ||
| bits = digest.split('/') | ||
| if '.' not in bits[0] or len(bits) < 3: | ||
| digest = 'docker.io/' + digest |
There was a problem hiding this comment.
this is actually not totally true. this depends on the unqualified-search-registries parameter of the registries.conf and might vary between hosts.
There was a problem hiding this comment.
Hmm. When would it not be docker.io though? On my dev host, for instance, selenium/standalone-chrome:3.141 has repo digest selenium/standalone-chrome@sha256:8d0915d29d5fa495256bda78371cb5adb70fd16ec9e4b4aaae72a0181682f5f0. But registries.conf has
unqualified-search-registries = ["registry.fedoraproject.org", "registry.access.redhat.com", "registry.centos.org", "docker.io"]
It seems like we could only have an ambiguous situation come up if we have an unqualified image name in one of the search registries. Perhaps we should, in addition to this patch, also require fully-qualified image names for the ceph orch upgrade start' command, and/or canonicalize the image name that we get from container_image so that we'll always have the docker.io prefix (or some other registry).
I'm not sure if it's possible, though, that we pull something from registry foo (not docker.io) and inspect it and see a RepoDigest that is unqualified (and therefore incorrectly prepend docker.io). We could instead canonicalize using the registry we just pulled from... maybe that is the safest thing?
jmolmo
reviewed
Apr 9, 2021
jmolmo
reviewed
Apr 9, 2021
jmolmo
approved these changes
Apr 9, 2021
|
updated, along with the commit messages! |
If we get an unqualified target image, assume it's docker.io. This ensures that we're passing a fully-qualified target to docker|podman on the various hosts and don't end up with something different based on the per-host search path for unqualified image names. Signed-off-by: Sage Weil <sage@newdream.net>
A RepoDigests returned by docker|podman image inspect can either include the docker.io/ prefix or not. For reasons that aren't entirely clear, this may vary between hosts in a cluster. However, ceph/ceph@sha256:abc... is the same thing as docker.io/ceph/ceph@sha256:abc..., and should be treated as such. Otherwise, upgrade can get into a loop where it pulls the image on a new host, finds the other variant of the repodigests, sees no overlap, updates target_digests, and restarts. (It will then find the first variant again on the first host and loop.) Avoid this by normalizing any docker.io digests by always including the docker.io/ prefix. Note that it is technically possible that this assumption is wrong: it may be that the image that already exists on the local host is from a different registry in registries.conf's unqualified-search-registries. However, we don't know which, since this is a search list. In practice, it should be exceeding rare that an image that *we* are installing using a fully-qualified image name will end up having an unqualified repodigest in the local registry. Hopefully! Fixes: https://tracker.ceph.com/issues/50114 Signed-off-by: Sage Weil <sage@newdream.net>
|
jenkins test docs |
1 similar comment
|
jenkins test docs |
tserong
added a commit
to SUSE/ceph
that referenced
this pull request
Jun 1, 2022
For SUSE downstream we never want to attempt to use docker.io images, which is what normalize_image_digest() falls back to when presented with an unqualified image name (ceph/ceph, ceph/daemon, ceph/daemon-base). Beyond that though, having read ceph#40577, ceph#44306 and the related tracker issues, I've become extremely dubious about allowing unqualified image names at all, so I figured the safest thing to do is not try to normalize these, but instead raise an error if one is somehow specified. Signed-off-by: Tim Serong <tserong@suse.com>
mgfritch
pushed a commit
to SUSE/ceph
that referenced
this pull request
Jun 1, 2022
For SUSE downstream we never want to attempt to use docker.io images, which is what normalize_image_digest() falls back to when presented with an unqualified image name (ceph/ceph, ceph/daemon, ceph/daemon-base). Beyond that though, having read ceph#40577, ceph#44306 and the related tracker issues, I've become extremely dubious about allowing unqualified image names at all, so I figured the safest thing to do is not try to normalize these, but instead raise an error if one is somehow specified. Signed-off-by: Tim Serong <tserong@suse.com>
mgfritch
pushed a commit
to SUSE/ceph
that referenced
this pull request
Jun 7, 2022
For SUSE downstream we never want to attempt to use docker.io images, which is what normalize_image_digest() falls back to when presented with an unqualified image name (ceph/ceph, ceph/daemon, ceph/daemon-base). Beyond that though, having read ceph#40577, ceph#44306 and the related tracker issues, I've become extremely dubious about allowing unqualified image names at all, so I figured the safest thing to do is not try to normalize these, but instead raise an error if one is somehow specified. Signed-off-by: Tim Serong <tserong@suse.com>
mgfritch
pushed a commit
to SUSE/ceph
that referenced
this pull request
Jul 12, 2022
For SUSE downstream we never want to attempt to use docker.io images, which is what normalize_image_digest() falls back to when presented with an unqualified image name (ceph/ceph, ceph/daemon, ceph/daemon-base). Beyond that though, having read ceph#40577, ceph#44306 and the related tracker issues, I've become extremely dubious about allowing unqualified image names at all, so I figured the safest thing to do is not try to normalize these, but instead raise an error if one is somehow specified. Signed-off-by: Tim Serong <tserong@suse.com>
mgfritch
pushed a commit
to SUSE/ceph
that referenced
this pull request
Jul 12, 2022
For SUSE downstream we never want to attempt to use docker.io images, which is what normalize_image_digest() falls back to when presented with an unqualified image name (ceph/ceph, ceph/daemon, ceph/daemon-base). Beyond that though, having read ceph#40577, ceph#44306 and the related tracker issues, I've become extremely dubious about allowing unqualified image names at all, so I figured the safest thing to do is not try to normalize these, but instead raise an error if one is somehow specified. Signed-off-by: Tim Serong <tserong@suse.com>
mgfritch
pushed a commit
to SUSE/ceph
that referenced
this pull request
Jul 13, 2022
For SUSE downstream we never want to attempt to use docker.io images, which is what normalize_image_digest() falls back to when presented with an unqualified image name (ceph/ceph, ceph/daemon, ceph/daemon-base). Beyond that though, having read ceph#40577, ceph#44306 and the related tracker issues, I've become extremely dubious about allowing unqualified image names at all, so I figured the safest thing to do is not try to normalize these, but instead raise an error if one is somehow specified. Signed-off-by: Tim Serong <tserong@suse.com>
mgfritch
pushed a commit
to SUSE/ceph
that referenced
this pull request
Aug 10, 2022
For SUSE downstream we never want to attempt to use docker.io images, which is what normalize_image_digest() falls back to when presented with an unqualified image name (ceph/ceph, ceph/daemon, ceph/daemon-base). Beyond that though, having read ceph#40577, ceph#44306 and the related tracker issues, I've become extremely dubious about allowing unqualified image names at all, so I figured the safest thing to do is not try to normalize these, but instead raise an error if one is somehow specified. Signed-off-by: Tim Serong <tserong@suse.com>
mgfritch
pushed a commit
to SUSE/ceph
that referenced
this pull request
Aug 11, 2022
For SUSE downstream we never want to attempt to use docker.io images, which is what normalize_image_digest() falls back to when presented with an unqualified image name (ceph/ceph, ceph/daemon, ceph/daemon-base). Beyond that though, having read ceph#40577, ceph#44306 and the related tracker issues, I've become extremely dubious about allowing unqualified image names at all, so I figured the safest thing to do is not try to normalize these, but instead raise an error if one is somehow specified. Signed-off-by: Tim Serong <tserong@suse.com>
mgfritch
pushed a commit
to SUSE/ceph
that referenced
this pull request
Oct 10, 2022
For SUSE downstream we never want to attempt to use docker.io images, which is what normalize_image_digest() falls back to when presented with an unqualified image name (ceph/ceph, ceph/daemon, ceph/daemon-base). Beyond that though, having read ceph#40577, ceph#44306 and the related tracker issues, I've become extremely dubious about allowing unqualified image names at all, so I figured the safest thing to do is not try to normalize these, but instead raise an error if one is somehow specified. Signed-off-by: Tim Serong <tserong@suse.com>
mgfritch
pushed a commit
to SUSE/ceph
that referenced
this pull request
Oct 11, 2022
For SUSE downstream we never want to attempt to use docker.io images, which is what normalize_image_digest() falls back to when presented with an unqualified image name (ceph/ceph, ceph/daemon, ceph/daemon-base). Beyond that though, having read ceph#40577, ceph#44306 and the related tracker issues, I've become extremely dubious about allowing unqualified image names at all, so I figured the safest thing to do is not try to normalize these, but instead raise an error if one is somehow specified. Signed-off-by: Tim Serong <tserong@suse.com>
tserong
added a commit
to SUSE/ceph
that referenced
this pull request
Dec 14, 2022
For SUSE downstream we never want to attempt to use docker.io images, which is what normalize_image_digest() falls back to when presented with an unqualified image name (ceph/ceph, ceph/daemon, ceph/daemon-base). Beyond that though, having read ceph#40577, ceph#44306 and the related tracker issues, I've become extremely dubious about allowing unqualified image names at all, so I figured the safest thing to do is not try to normalize these, but instead raise an error if one is somehow specified. Signed-off-by: Tim Serong <tserong@suse.com>
mgfritch
pushed a commit
to SUSE/ceph
that referenced
this pull request
Jan 11, 2023
For SUSE downstream we never want to attempt to use docker.io images, which is what normalize_image_digest() falls back to when presented with an unqualified image name (ceph/ceph, ceph/daemon, ceph/daemon-base). Beyond that though, having read ceph#40577, ceph#44306 and the related tracker issues, I've become extremely dubious about allowing unqualified image names at all, so I figured the safest thing to do is not try to normalize these, but instead raise an error if one is somehow specified. Signed-off-by: Tim Serong <tserong@suse.com>
mgfritch
pushed a commit
to SUSE/ceph
that referenced
this pull request
Jan 27, 2023
For SUSE downstream we never want to attempt to use docker.io images, which is what normalize_image_digest() falls back to when presented with an unqualified image name (ceph/ceph, ceph/daemon, ceph/daemon-base). Beyond that though, having read ceph#40577, ceph#44306 and the related tracker issues, I've become extremely dubious about allowing unqualified image names at all, so I figured the safest thing to do is not try to normalize these, but instead raise an error if one is somehow specified. Signed-off-by: Tim Serong <tserong@suse.com>
mgfritch
pushed a commit
to SUSE/ceph
that referenced
this pull request
Apr 19, 2023
For SUSE downstream we never want to attempt to use docker.io images, which is what normalize_image_digest() falls back to when presented with an unqualified image name (ceph/ceph, ceph/daemon, ceph/daemon-base). Beyond that though, having read ceph#40577, ceph#44306 and the related tracker issues, I've become extremely dubious about allowing unqualified image names at all, so I figured the safest thing to do is not try to normalize these, but instead raise an error if one is somehow specified. Signed-off-by: Tim Serong <tserong@suse.com>
mgfritch
pushed a commit
to SUSE/ceph
that referenced
this pull request
May 10, 2023
For SUSE downstream we never want to attempt to use docker.io images, which is what normalize_image_digest() falls back to when presented with an unqualified image name (ceph/ceph, ceph/daemon, ceph/daemon-base). Beyond that though, having read ceph#40577, ceph#44306 and the related tracker issues, I've become extremely dubious about allowing unqualified image names at all, so I figured the safest thing to do is not try to normalize these, but instead raise an error if one is somehow specified. Signed-off-by: Tim Serong <tserong@suse.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A RepoDigests returned by docker|podman image inspect can either include
the docker.io/ prefix or not. For reasons that aren't entirely clear,
this may vary between hosts in a cluster. However, ceph/ceph@sha256:abc...
is the same thing as docker.io/ceph/ceph@sha256:abc..., and should be
treated as such. Otherwise, upgrade can get into a loop where it pulls
the image on a new host, finds the other variant of the repodigests,
sees no overlap, updates target_digests, and restarts. (It will then
find the first variant again on the first host and loop.)
Avoid this by normalizing any docker.io digests by always including the
docker.io/ prefix.
Fixes: https://tracker.ceph.com/issues/50114
Checklist
Show available Jenkins commands
jenkins retest this pleasejenkins test classic perfjenkins test crimson perfjenkins test signedjenkins test make checkjenkins test make check arm64jenkins test submodulesjenkins test dashboardjenkins test apijenkins test docsjenkins render docsjenkins test ceph-volume alljenkins test ceph-volume tox