-
Notifications
You must be signed in to change notification settings - Fork 41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes for imgutil from recent refactor #267
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -216,24 +216,10 @@ func (s *Store) addImageToTar(tw *tar.Writer, image v1.Image, withName string) e | |
blankIdx int | ||
) | ||
for _, layer := range layers { | ||
var layerName string | ||
size, err := layer.Size() | ||
layerName, err := s.addLayerToTar(tw, layer, &blankIdx) | ||
if err != nil { | ||
return err | ||
} | ||
if size == -1 { // layer facade fronting empty layer | ||
layerName = fmt.Sprintf("blank_%d", blankIdx) | ||
blankIdx++ | ||
hdr := &tar.Header{Name: layerName, Mode: 0644, Size: 0} | ||
if err := tw.WriteHeader(hdr); err != nil { | ||
return err | ||
} | ||
Comment on lines
-224
to
-230
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved to |
||
} else { | ||
layerName, err = s.addLayerToTar(tw, layer) | ||
if err != nil { | ||
return err | ||
} | ||
} | ||
layerPaths = append(layerPaths, layerName) | ||
} | ||
|
||
|
@@ -250,32 +236,46 @@ func (s *Store) addImageToTar(tw *tar.Writer, image v1.Image, withName string) e | |
return addTextToTar(tw, manifestJSON, "manifest.json") | ||
} | ||
|
||
func (s *Store) addLayerToTar(tw *tar.Writer, layer v1.Layer) (string, error) { | ||
layerDiffID, err := layer.DiffID() | ||
func (s *Store) addLayerToTar(tw *tar.Writer, layer v1.Layer, blankIdx *int) (string, error) { | ||
// If the layer is a previous image layer that hasn't been downloaded yet, | ||
// cause ALL the previous image layers to be downloaded by grabbing the ReadCloser. | ||
layerReader, err := layer.Uncompressed() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Previously, we forced the layer to be downloaded by asking for its size. |
||
if err != nil { | ||
return "", err | ||
} | ||
withName := fmt.Sprintf("/%s.tar", layerDiffID.String()) | ||
defer layerReader.Close() | ||
|
||
uncompressedSize, err := s.getLayerSize(layer) | ||
var layerName string | ||
size, err := layer.Size() | ||
if err != nil { | ||
return "", err | ||
} | ||
hdr := &tar.Header{Name: withName, Mode: 0644, Size: uncompressedSize} | ||
if err = tw.WriteHeader(hdr); err != nil { | ||
if size == -1 { // it's a base (always empty) layer | ||
layerName = fmt.Sprintf("blank_%d", blankIdx) | ||
*blankIdx++ | ||
hdr := &tar.Header{Name: layerName, Mode: 0644, Size: 0} | ||
return layerName, tw.WriteHeader(hdr) | ||
} | ||
// it's a populated layer | ||
layerDiffID, err := layer.DiffID() | ||
if err != nil { | ||
return "", err | ||
} | ||
layerName = fmt.Sprintf("/%s.tar", layerDiffID.String()) | ||
|
||
layerReader, err := layer.Uncompressed() | ||
uncompressedSize, err := s.getLayerSize(layer) | ||
if err != nil { | ||
return "", err | ||
} | ||
defer layerReader.Close() | ||
hdr := &tar.Header{Name: layerName, Mode: 0644, Size: uncompressedSize} | ||
if err = tw.WriteHeader(hdr); err != nil { | ||
return "", err | ||
} | ||
if _, err = io.Copy(tw, layerReader); err != nil { | ||
return "", err | ||
} | ||
|
||
return withName, nil | ||
return layerName, nil | ||
} | ||
|
||
// getLayerSize returns the uncompressed layer size. | ||
|
@@ -405,7 +405,7 @@ func (s *Store) doDownloadLayersFor(identifier string) error { | |
|
||
imageReader, err := s.dockerClient.ImageSave(ctx, []string{identifier}) | ||
if err != nil { | ||
return fmt.Errorf("saving base image with ID %q from the docker daemon: %w", identifier, err) | ||
return fmt.Errorf("saving image with ID %q from the docker daemon: %w", identifier, err) | ||
} | ||
defer ensureReaderClosed(imageReader) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -182,9 +182,9 @@ func toV1Config(dockerCfg *container.Config) v1.Config { | |
var _ v1.Layer = &v1LayerFacade{} | ||
|
||
type v1LayerFacade struct { | ||
diffID v1.Hash | ||
uncompressed func() (io.ReadCloser, error) | ||
size func() (int64, error) | ||
diffID v1.Hash | ||
uncompressed func() (io.ReadCloser, error) | ||
uncompressedSize func() (int64, error) | ||
} | ||
|
||
func newEmptyLayer(diffID v1.Hash, store *Store) *v1LayerFacade { | ||
|
@@ -197,7 +197,7 @@ func newEmptyLayer(diffID v1.Hash, store *Store) *v1LayerFacade { | |
} | ||
return io.NopCloser(bytes.NewReader([]byte{})), nil | ||
}, | ||
size: func() (int64, error) { | ||
uncompressedSize: func() (int64, error) { | ||
layer, err := store.LayerByDiffID(diffID) | ||
if err == nil { | ||
return layer.Size() | ||
|
@@ -224,24 +224,17 @@ func newDownloadableEmptyLayer(diffID v1.Hash, store *Store, imageID string) *v1 | |
} | ||
return nil, err | ||
}, | ||
size: func() (int64, error) { | ||
uncompressedSize: func() (int64, error) { | ||
layer, err := store.LayerByDiffID(diffID) | ||
if err == nil { | ||
return layer.Size() | ||
} | ||
if err = store.downloadLayersFor(imageID); err != nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oops! We may ask a layer for its size just to construct a (missing stuff but good enough for our purposes) manifest. I didn't catch this in my earlier performance testing because the tests for WithPreviousImage (where this is relevant) use a hard-coded image, not the "runnable base image". |
||
return -1, err | ||
} | ||
layer, err = store.LayerByDiffID(diffID) | ||
if err == nil { | ||
return layer.Size() | ||
} | ||
return -1, err | ||
return -1, nil | ||
}, | ||
} | ||
} | ||
|
||
func newPopulatedLayer(diffID v1.Hash, fromPath string, sentinelSize int64) *v1LayerFacade { | ||
func newPopulatedLayer(diffID v1.Hash, fromPath string, uncompressedSize int64) *v1LayerFacade { | ||
return &v1LayerFacade{ | ||
diffID: diffID, | ||
uncompressed: func() (io.ReadCloser, error) { | ||
|
@@ -251,8 +244,8 @@ func newPopulatedLayer(diffID v1.Hash, fromPath string, sentinelSize int64) *v1L | |
} | ||
return f, nil | ||
}, | ||
size: func() (int64, error) { | ||
return sentinelSize, nil | ||
uncompressedSize: func() (int64, error) { | ||
return uncompressedSize, nil | ||
}, | ||
} | ||
} | ||
|
@@ -285,9 +278,10 @@ func (l *v1LayerFacade) Uncompressed() (io.ReadCloser, error) { | |
return l.uncompressed() | ||
} | ||
|
||
// Size returns a sentinel value indicating if the layer has data. | ||
// Size returns the uncompressed size. | ||
// If the layer is missing local data, it returns a sentinel value of -1. | ||
func (l *v1LayerFacade) Size() (int64, error) { | ||
return l.size() | ||
return l.uncompressedSize() | ||
} | ||
|
||
func (l *v1LayerFacade) MediaType() (v1types.MediaType, error) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was causing the analyzer not to ever find the previous image