Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support progress channel for partial pulls #2585

Open
giuseppe opened this issue Sep 27, 2024 · 2 comments
Open

support progress channel for partial pulls #2585

giuseppe opened this issue Sep 27, 2024 · 2 comments
Labels
jira kind/feature A request for, or a PR adding, new functionality

Comments

@giuseppe
Copy link
Member

currently we do not support progress channel for partial pulls. The entire logic implemented in copy/progress_channel.go is not used when a partial pull is used.

It causes an issue with CRI-O as it uses the notifications from the progress channel to detect whether the pull is still alive, and in case it aborts the operation after 10 seconds.

Since there are some actions that might take more than 10 seconds even when there is no data incoming, e.g. converting to composefs, we should have a mechanism to keep alive the channel (maybe reports dummy events or new kind of notifications?).

@giuseppe giuseppe added the jira label Sep 27, 2024
@giuseppe
Copy link
Member Author

@mtrmac PTAL, this is an issue with CRI-O since it fails to pull images that take more than 10 seconds

kwilczynski added a commit to kwilczynski/cri-o that referenced this issue Sep 27, 2024
See: containers/image#2585

Signed-off-by: Krzysztof Wilczyński <kwilczynski@redhat.com>
kwilczynski added a commit to kwilczynski/cri-o that referenced this issue Sep 27, 2024
See: containers/image#2585

Signed-off-by: Krzysztof Wilczyński <kwilczynski@redhat.com>
@mtrmac
Copy link
Collaborator

mtrmac commented Sep 27, 2024

Thanks, that’s an interesting interaction. I have commented on that for CRI-O in https://github.com/cri-o/cri-o/pull/8619/files#r1778887578 , perhaps it should be elevated to a separate issue.

Notably, there’s also the “commit” phase of storageImageDestination, which actually extracts the layer tarballs into individual files (or finishes that work if it started concurrently to layer copies), and that can take a very long time, and is not integrated into the progress mechanism.

See also the various “This can take quite some time, and should ideally be cancellable” comments — even in the ordinary layer pull path, where we do generally report progress, we can stop for an arbitrarily-long time in putBlobToPendingFile when extracting a large layer.

So, partial pulls or not, it seems to me that this CRI-O logic is making assumptions on things c/image never promised, and I think short-term CRI-O might need to just give on that.


That image pulls “happen very quickly and then hang after the ’Storing signatures’ message” (= in the commit phases) is a long-standing problem. For a long time, that was ~hard to fix without breaking the API of Image{Source,Destination}, and we might have become accustomed to that oddity. Now that we have internal/private, this is all certainly possible to fix.

(Ideally, we’d somehow ~combine copy/progress_bars.go and copy/progress_channels.go into a single progress facility that can be consistently triggered from transports; compare also containers/skopeo#658 and #1532 . But that’s a much larger problem space and we should not block on a perfect solution; I’m just mentioning that to keep that in mind while designing the smaller parts.)

For this specific restricted issue as filed, we we already have blobChunkAccessorProxy, so just to do something similar to what the existing progress channel does, without dealing with the more general issue, we could either extend that proxy, or add one more like it, to send updates to the progress channel.

@mtrmac mtrmac added the kind/feature A request for, or a PR adding, new functionality label Sep 27, 2024
kwilczynski added a commit to kwilczynski/cri-o that referenced this issue Sep 30, 2024
See: containers/image#2585

Signed-off-by: Krzysztof Wilczyński <kwilczynski@redhat.com>
kwilczynski added a commit to kwilczynski/cri-o that referenced this issue Oct 10, 2024
See: containers/image#2585

Signed-off-by: Krzysztof Wilczyński <kwilczynski@redhat.com>
kwilczynski added a commit to kwilczynski/cri-o that referenced this issue Oct 10, 2024
See: containers/image#2585

Signed-off-by: Krzysztof Wilczyński <kwilczynski@redhat.com>
kwilczynski added a commit to kwilczynski/cri-o that referenced this issue Oct 15, 2024
See: containers/image#2585

Signed-off-by: Krzysztof Wilczyński <kwilczynski@redhat.com>
kwilczynski added a commit to kwilczynski/cri-o that referenced this issue Oct 15, 2024
See: containers/image#2585

Signed-off-by: Krzysztof Wilczyński <kwilczynski@redhat.com>
kwilczynski added a commit to kwilczynski/cri-o that referenced this issue Oct 16, 2024
See: containers/image#2585

Signed-off-by: Krzysztof Wilczyński <kwilczynski@redhat.com>
kwilczynski added a commit to kwilczynski/cri-o that referenced this issue Oct 21, 2024
See: containers/image#2585

Signed-off-by: Krzysztof Wilczyński <kwilczynski@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jira kind/feature A request for, or a PR adding, new functionality
Projects
None yet
Development

No branches or pull requests

2 participants