containerd integration: compute containers layer size#44934
containerd integration: compute containers layer size#44934thaJeztah merged 1 commit intomoby:masterfrom
Conversation
rumpl
added
area/images
laurazard
force-pushed
the
c8d-layer-size
branch
from
ce2ecde to
e8c9230
Compare
laurazard
force-pushed
the
c8d-layer-size
branch
2 times, most recently
from
918d65c to
4bd1444
Compare
|
Rebased on top of #44958, which includes the necessary refactors to address @neersighted's comment's here – #44934 (comment). Only the last commit includes actual relevant changes for this PR, the others come from #44958 |
laurazard
force-pushed
the
c8d-layer-size
branch
from
4bd1444 to
96c826a
Compare
laurazard
force-pushed
the
c8d-layer-size
branch
from
96c826a to
e6eac3e
Compare
neersighted
left a comment
neersighted
left a comment
There was a problem hiding this comment.
Overall LGTM, some nits/questions.
laurazard
force-pushed
the
c8d-layer-size
branch
from
e6eac3e to
aca81c2
Compare
laurazard
force-pushed
the
c8d-layer-size
branch
from
aca81c2 to
731763f
Compare
neersighted
left a comment
neersighted
left a comment
There was a problem hiding this comment.
I think the return to usage.Size is actually quite helpful here as it adds symmetry with snapshotSizeFn -- thanks for letting me pick your brain; LGTM with the tweaks.
neersighted
requested review from
AkihiroSuda,
cpuguy83,
rumpl,
thaJeztah,
tianon and
vvoland
laurazard
force-pushed
the
c8d-layer-size
branch
from
731763f to
a64380b
Compare
thaJeztah
left a comment
thaJeztah
left a comment
There was a problem hiding this comment.
Thanks! Sorry for the delay; code overall looks good to me. While looking at it locally I saw some variables shadowing imports, and some suggestions on moving some code closer to where it's used.
Not necessarily a blocker, but let me know what you think 😅
cpuguy83
left a comment
cpuguy83
left a comment
There was a problem hiding this comment.
LGTM other than existing comments.
Co-authored-by: Sebastiaan van Stijn <[email protected]> Signed-off-by: Laura Brehm <[email protected]>
thaJeztah
force-pushed
the
c8d-layer-size
branch
from
a64380b to
45ee4d7
Compare
|
@laurazard is traveling today, so I just pushed my suggestion from above; https://github.com/moby/moby/compare/a64380b47f70a7335d08cc8aae9af8e4f619e99a..45ee4d7c78d4d128366f372c4657911880571528 @cpuguy83 @neersighted ptal if those LGTY (then I think we can merge this one) |
5 participants
- What I did
Implement
GetContainerLayerSizeusing the containerd snapshotterUpstreams:
docker system dfrumpl/moby#46- How I did it
- How to verify it
After #44804 is merged, run:
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)