deploy: fix progress bar math for containers-storage transport#2067
Conversation
When pulling from containers-storage, layers are stored uncompressed but the progress bar total was set from the manifest descriptor size (compressed). This caused the display to show transferred exceeding total, e.g. '2.66 GiB/1.14 GiB'. Update the byte progress bar length from LayerProgress.total on each update, which reflects the actual blob size for the transport. Also use the bar's actual length for completion accounting so that total_read and subtask bytes are consistent. This matches how ostree-ext's CLI handles the same progress (cli.rs). For registry pulls, LayerProgress.total equals the manifest descriptor size, so this is a no-op in that case. Closes: bootc-dev#2001 Signed-off-by: Andrew Dunn <andrew@dunn.dev>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses an issue where the progress bar for container image pulls from transports like containers-storage would show incorrect totals. This was caused by using the compressed layer size from the manifest, while the actual data transferred is uncompressed.
The changes fix this by updating the progress bar's total length with the actual blob size as progress is reported. The layer completion logic is also adjusted to use this corrected size for its final accounting, ensuring consistency.
The changes appear to correctly implement the fix for the described problem. The logic is self-contained within the progress handling function.
gursewak1997
left a comment
gursewak1997
left a comment
There was a problem hiding this comment.
Thanks for this fix.
lgtm
2 participants
When pulling from
containers-storage, layers are stored uncompressed but the progress bar total was set from the manifest descriptor size (compressed). This caused the display to show transferred exceeding total, e.g.2.66 GiB/1.14 GiB.The fix updates the byte progress bar length from
LayerProgress.totalon each progress update, which reflects the actual blob size for the current transport. For registry pulls this equals the manifest descriptor size (compressed), so the change is a no-op in that case. Forcontainers-storageanddocker-daemon, it uses the uncompressed size, matching whatProgressReaderactually tracks.Also updates the layer completion path to use the bar's actual length for
total_readand subtask accounting, so that completion events reportbytes == bytes_totalconsistently.This aligns with how
ostree-ext's CLI handles the same progress (cli.rsalready callspb.set_length(bytes.total)beforepb.set_position(bytes.fetched)).Closes #2001