Skip to content

importer: refactor and fix GC#1880

Merged
dmcgowan merged 1 commit intocontainerd:masterfrom
AkihiroSuda:refactor-importer
Dec 5, 2017
Merged

importer: refactor and fix GC#1880
dmcgowan merged 1 commit intocontainerd:masterfrom
AkihiroSuda:refactor-importer

Conversation

@AkihiroSuda
Copy link
Copy Markdown
Member

@AkihiroSuda AkihiroSuda commented Dec 5, 2017 •
edited
Loading

Cherry-pick non-docker part from #1602

For usage, please refer to ctr images import --help.

$ ctr images import --help            
NAME:                             
   ctr images import - import images                                

USAGE:                            
   ctr images import [command options] [flags] <in>                 

DESCRIPTION:                      
   Import images from a tar stream.                                 
Implemented formats:              
- oci.v1     (default)                          


For oci.v1 format, you need to specify --oci-name because an OCI archive contains image refs (tags)                                     
but does not contain the base image name.                           

e.g.                              
  $ ctr images import --format oci.v1 --oci-name foo/bar foobar.tar 

If foobar.tar contains an OCI ref named "latest" and anonymous ref "sha256:deadbeef", the command will create                           
"foo/bar:latest" and "foo/bar@sha256:deadbeef" images in the containerd store.                                                          

OPTIONS:                          
   --format value       image format. See DESCRIPTION. (default: "oci.v1")                                                              
   --oci-name value     prefix added to either oci.v1 ref annotation or digest (default: "unknown/unknown")                             
   --snapshotter value  snapshotter name. Empty value stands for the daemon default value. (default: "overlayfs")

Signed-off-by: Akihiro Suda [email protected]

No impact on the server API, but since this contains fix for GC, I suggest setting milestone of this to v1.0.0

@AkihiroSuda
importer: refactor
6340197 
- Use lease API (previoisly, GC was not supported)
- Refactored interfaces for ease of future Docker v1 importer support

For usage, please refer to `ctr images import --help`.

Signed-off-by: Akihiro Suda <[email protected]>
@AkihiroSuda AkihiroSuda added this to the 1.0.0 milestone Dec 5, 2017
@AkihiroSuda AkihiroSuda changed the title importer: refactor importer: refactor and fix GC Dec 5, 2017
AkihiroSuda
AkihiroSuda commented Dec 5, 2017
View reviewed changes
Comment thread images/oci/importer.go
return nil, nil
}

func setGCRefContentLabels(ctx context.Context, store content.Store, desc ocispec.Descriptor) error {
Copy link
Copy Markdown
Member Author

@AkihiroSuda AkihiroSuda Dec 5, 2017

Choose a reason for hiding this comment

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

The most significant change is that this function was added for fixing the GC issue.
Other lines are just interface changes.

Copy link
Copy Markdown
Member

@dmcgowan dmcgowan Dec 5, 2017

Choose a reason for hiding this comment

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

Where are the blobs being leased while this is running though?

Copy link
Copy Markdown
Member Author

@AkihiroSuda AkihiroSuda Dec 5, 2017

Choose a reason for hiding this comment

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

@dmcgowan WithLease() in client.go: Import()
https://github.com/containerd/containerd/pull/1880/files#diff-d0b7f86cea13f90d3cc0cc2338da8d5fR521

@stevvooe
Copy link
Copy Markdown
Member

stevvooe commented Dec 5, 2017

LGTM

1 similar comment
@dmcgowan
Copy link
Copy Markdown
Member

dmcgowan commented Dec 5, 2017

LGTM

@dmcgowan dmcgowan merged commit 40c67fd into containerd:master Dec 5, 2017
@AkihiroSuda AkihiroSuda mentioned this pull request Dec 5, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dmcgowan dmcgowan dmcgowan left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

1.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

@AkihiroSuda @stevvooe @dmcgowan