Skip to content

fix image size#310

Merged
AkihiroSuda merged 1 commit into
containerd:masterfrom
fahedouch:fix-images-size
Aug 18, 2021
Merged

fix image size#310
AkihiroSuda merged 1 commit into
containerd:masterfrom
fahedouch:fix-images-size

Conversation

@fahedouch

Copy link
Copy Markdown
Member

the actual image size is equal to the ChainID size.

this PR use the Walk API and Usage on each parent of ChainID to calculate the total usage of unpacked image layers

related issue #304

Signed-off-by: fahed dorgaa fahed.dorgaa@gmail.com

@AkihiroSuda AkihiroSuda requested a review from ktock August 8, 2021 15:37

@ktock ktock left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overall LGTM but minor nits

Comment thread cmd/nerdctl/images.go Outdated
Comment thread cmd/nerdctl/images.go Outdated
Comment thread cmd/nerdctl/images.go Outdated
@fahedouch fahedouch force-pushed the fix-images-size branch 3 times, most recently from 6cdffbb to 7048508 Compare August 9, 2021 09:47
@fahedouch

Copy link
Copy Markdown
Member Author

@ktock done

Comment thread cmd/nerdctl/images.go Outdated
Comment on lines +179 to +189
walkFn := func(ctx context.Context, info snapshots.Info) error {
keys[info.Name] = info.Parent
return nil
}
if err := s.Walk(ctx, walkFn); err != nil {
return 0, err
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does it scan all snapshots? Can we walk snapshots only in the chain? I think we can do this by following parents from the topmost snapshot using Stat API.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ktock thank you, This is what I was looking for ==> Stat API . I made modification.

Signed-off-by: fahed dorgaa <fahed.dorgaa@gmail.com>
@AkihiroSuda

Copy link
Copy Markdown
Member

CI wasn't triggered?

Comment thread cmd/nerdctl/images.go
}

func unpackedImageSize(ctx context.Context, clicontext *cli.Context, client *containerd.Client, i images.Image) (int64, error) {
type snapshotKey string

@AkihiroSuda AkihiroSuda Aug 12, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why define new type just for a string?

@fahedouch fahedouch Aug 12, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to define new local type to use it as a receiver to the add method. We cannot define the add method on non-local type string

@AkihiroSuda AkihiroSuda reopened this Aug 12, 2021
@AkihiroSuda AkihiroSuda added this to the v0.12.0 milestone Aug 12, 2021

@AkihiroSuda AkihiroSuda left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants