Don't use skopeo inspect for canonical (digested) pull specs#51
Merged
cgwalters merged 1 commit intoopenshift:masterfrom Apr 25, 2019
Merged
Conversation
openshift-ci-robot
added
the
size/M
miabbott
reviewed
Apr 24, 2019
Member
|
Is there a double-negative issue in the PR/commit subject? I think we want to skip |
Member
|
Is there somewhere we can/should document the fact that with this PR |
jlebon
reviewed
Apr 24, 2019
`skopeo inspect` hits the `/tags` API which can be expensive for registries to compute. Quay.io recently changed how they do rate limiting, and we started hitting this in OpenShift CI jobs. Since the machine-config-operator always provides canonical/digested pull specs, the non-digested support is just for local developer convenience. This PR changes things so we don't use `skopeo` at all anymore, rather we `podman inspect` after we pull. Basically this PR ends up in a place where the non-canonical path only loses out on the optimization of avoiding pulling the image in the case where we already have that target.
cgwalters
force-pushed
the
no-inspect-fully-qualified
branch
from
3821542 to
fb4271d
Compare
Member
Author
Not sure why that'd matter? The few people who should care are all reading this PR.
Since as noted above this isn't a hard error anymore, all we're avoiding is re-pulling the image and only when the sha256 would match, and if it does, why are you asking |
Member
|
Ahh right, forgot about coreos/rpm-ostree#1807. /lgtm |
Member
|
Commit message is fixed, but PR subject still has the double-negative. |
cgwalters
changed the title
skopeo inspect for non-canonical pull specsskopeo inspect for canonical (digested) pull specs
cgwalters
added a commit
to cgwalters/installer
that referenced
this pull request
Apr 25, 2019
This includes openshift/pivot#51
cgwalters
added a commit
to cgwalters/coreos-assembler
that referenced
this pull request
Jun 2, 2020
A while ago, we were using `skopeo inspect` as part of `pivot` for OpenShift 4 OS upgrades, and it turns out that enumerates all tags, which is expensive and rate limited by quay.io: openshift/pivot#51 The RHCOS oscontainer builds have been failing in `skopeo inspect` and I suspect it's related to this. It looks like nowadays podman has `--digestfile` which is exactly what we want here, so let's use it and drop the `skopeo`. This simplifies the code too.
cgwalters
added a commit
to cgwalters/coreos-assembler
that referenced
this pull request
Jun 3, 2020
A while ago, we were using `skopeo inspect` as part of `pivot` for OpenShift 4 OS upgrades, and it turns out that enumerates all tags, which is expensive and rate limited by quay.io: openshift/pivot#51 The RHCOS oscontainer builds have been failing in `skopeo inspect` and I suspect it's related to this. It looks like nowadays podman has `--digestfile` which is exactly what we want here, so let's use it and drop the `skopeo`. This simplifies the code too.
openshift-merge-robot
pushed a commit
to coreos/coreos-assembler
that referenced
this pull request
Jun 3, 2020
A while ago, we were using `skopeo inspect` as part of `pivot` for OpenShift 4 OS upgrades, and it turns out that enumerates all tags, which is expensive and rate limited by quay.io: openshift/pivot#51 The RHCOS oscontainer builds have been failing in `skopeo inspect` and I suspect it's related to this. It looks like nowadays podman has `--digestfile` which is exactly what we want here, so let's use it and drop the `skopeo`. This simplifies the code too.
cgwalters
added a commit
to cgwalters/coreos-assembler
that referenced
this pull request
Jun 3, 2020
A while ago, we were using `skopeo inspect` as part of `pivot` for OpenShift 4 OS upgrades, and it turns out that enumerates all tags, which is expensive and rate limited by quay.io: openshift/pivot#51 The RHCOS oscontainer builds have been failing in `skopeo inspect` and I suspect it's related to this. It looks like nowadays podman has `--digestfile` which is exactly what we want here, so let's use it and drop the `skopeo`. This simplifies the code too. (cherry picked from commit b574572)
cgwalters
added a commit
to cgwalters/coreos-assembler
that referenced
this pull request
Jun 3, 2020
A while ago, we were using `skopeo inspect` as part of `pivot` for OpenShift 4 OS upgrades, and it turns out that enumerates all tags, which is expensive and rate limited by quay.io: openshift/pivot#51 The RHCOS oscontainer builds have been failing in `skopeo inspect` and I suspect it's related to this. It looks like nowadays podman has `--digestfile` which is exactly what we want here, so let's use it and drop the `skopeo`. This simplifies the code too. (cherry picked from commit b574572)
cgwalters
added a commit
to coreos/coreos-assembler
that referenced
this pull request
Jun 4, 2020
A while ago, we were using `skopeo inspect` as part of `pivot` for OpenShift 4 OS upgrades, and it turns out that enumerates all tags, which is expensive and rate limited by quay.io: openshift/pivot#51 The RHCOS oscontainer builds have been failing in `skopeo inspect` and I suspect it's related to this. It looks like nowadays podman has `--digestfile` which is exactly what we want here, so let's use it and drop the `skopeo`. This simplifies the code too. (cherry picked from commit b574572)
ashcrow
pushed a commit
to coreos/coreos-assembler
that referenced
this pull request
Jun 4, 2020
A while ago, we were using `skopeo inspect` as part of `pivot` for OpenShift 4 OS upgrades, and it turns out that enumerates all tags, which is expensive and rate limited by quay.io: openshift/pivot#51 The RHCOS oscontainer builds have been failing in `skopeo inspect` and I suspect it's related to this. It looks like nowadays podman has `--digestfile` which is exactly what we want here, so let's use it and drop the `skopeo`. This simplifies the code too. (cherry picked from commit b574572)
sinnykumari
added a commit
to sinnykumari/machine-config-operator
that referenced
this pull request
Jul 31, 2020
Calling image inspect directly pulls in RepoTags as well which can add up some extra time. See openshift/pivot#51 Since we are already vendoring containers/image in MCO, we are now calling necessary functions directly needed to get the OSTree version and commit information for the OSContainer. Thanks to Colin Walters and Miloslav Trmač for the idea of using container/image stack directly and reviewing the implementation.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
skopeo inspecthits the/tagsAPI which can be expensivefor registries to compute. Quay.io recently changed how
they do rate limiting, and we started hitting this in OpenShift CI
jobs.
Since the machine-config-operator always provides canonical/digested
pull specs, the non-digested support is just for local developer
convenience.
And anyways, what we really should do is pull the image unconditionally
and then inspect it afterwards, we don't need all of the tag info.
Basically this PR ends up in a place where the non-canonical
path only loses out on the optimization of avoiding pulling the
image in the case where we already have that target.