progressui: handle active statuses for completed vertex

[tonistiigi]

There seems to be a bug currently on daemon side on writing progress while importing remote cache. It looks like the status messages appear while the vertex messages show the current vertex as complete.

@sipsma I think the issue is that the vertex messages only come when cache.Load is called but that function doesn't actually pull the data. Later when blobs are lazily loaded only the status events come. I can repro it by:

FROM alpine
RUN apk add curl
run dd if=/dev/urandom of=/foo bs=1M count=50
#run sleep 1

First build docker buildx build --cache-to type=local,dest=/tmp/cache . then prune cache, comment out the sleep line, and run docker buildx build --cache-from type=local,src=/tmp/cache --progress=plain ..

I think we should fix it in the daemon side as well but not very familiar with that code. The current fix is to handle it on client-side and not write the suffix for the vertex when there are active status rows.

Signed-off-by: Tonis Tiigi [email protected]

[sipsma]

I think the issue is that the vertex messages only come when cache.Load is called but that function doesn't actually pull the data. Later when blobs are lazily loaded only the status events come.

IIUC the vertex messages should come in when the blobs are lazily loaded. It should call this line, which will send a vertex start event.

The fix here should work but I think the more fundamental problem might be that I didn't make the right updates to printer.go in my change to support multiple start/stop intervals. It should be using isCompleted() instead of just checking Completed and a few other issues. I can make those updates quick and push a separate PR for you to try (I'm not sure how to quickly build buildx with these changes to test).

[tonistiigi]

It should call this line, which will send a vertex start event.

I did print out the events I got. It was a start event and then end event only some milliseconds later. Then status events started to come later. https://gist.github.com/tonistiigi/4f376db480cec9915c3b6798595fdeab Search 5978c53677b463577c

(I'm not sure how to quickly build buildx with these changes to test).

Same should be in buildctl as well, buildx was just easier to test for me. This change does require vendor update/replace. After that it is just a simple docker build (or make install).

[tonistiigi]

I think better to do this change anyway, at least temporarily. With only fixing the daemon side it would require the latest daemon to be used.

[sipsma]

I did print out the events I got. It was a start event and then end event only some milliseconds later. Then status events started to come later. https://gist.github.com/tonistiigi/4f376db480cec9915c3b6798595fdeab Search 5978c53677b463577c

Ah okay, agree there's probably something wrong daemon-side then.

I think better to do this change anyway, at least temporarily. With only fixing the daemon side it would require the latest daemon to be used.

SGTM.