Skip to content

Commit e971d63

Browse files
authored
Add WithFileBufferedOpener for file-backed daemon image buffering (#2214)
* Add WithFileBufferedOpener for file-backed daemon image buffering See: kubernetes/minikube#17785 See: kubernetes/minikube#21103 * Reduce duplicate code in image_test * Suppress G703 security warning in image.go Added a #nosec comment to suppress security warning for file creation. * Remove temp file using AddCleanup * Remove confusing comment * Code clean up * Remove unused io import
1 parent 8e2d6a6 commit e971d63

4 files changed

Lines changed: 218 additions & 22 deletions

File tree

pkg/v1/daemon/bench_memory_test.go

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
// Copyright 2026 Google LLC All Rights Reserved.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
//go:build bench_memory
16+
17+
package daemon
18+
19+
import (
20+
"testing"
21+
22+
"github.com/google/go-containerregistry/pkg/name"
23+
)
24+
25+
const benchImage = "elasticsearch:9.0.3"
26+
27+
func loadAndAccess(t *testing.T, opts ...Option) {
28+
t.Helper()
29+
ref, err := name.ParseReference(benchImage)
30+
if err != nil {
31+
t.Fatalf("ParseReference: %v", err)
32+
}
33+
34+
img, err := Image(ref, opts...)
35+
if err != nil {
36+
t.Fatalf("Image: %v", err)
37+
}
38+
39+
// Access layers to trigger the docker save and buffering.
40+
layers, err := img.Layers()
41+
if err != nil {
42+
t.Fatalf("Layers: %v", err)
43+
}
44+
t.Logf("loaded %d layers", len(layers))
45+
}
46+
47+
func TestBenchMemoryBacked(t *testing.T) {
48+
loadAndAccess(t, WithBufferedOpener())
49+
}
50+
51+
func TestBenchFileBacked(t *testing.T) {
52+
loadAndAccess(t, WithFileBufferedOpener())
53+
}

pkg/v1/daemon/image.go

Lines changed: 53 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ import (
1818
"bytes"
1919
"context"
2020
"io"
21+
"os"
22+
"runtime"
2123
"sync"
2224
"time"
2325

@@ -46,12 +48,13 @@ type imageOpener struct {
4648
ref name.Reference
4749
ctx context.Context
4850

49-
buffered bool
50-
client Client
51+
bufferMode bufferMode
52+
client Client
5153

52-
once sync.Once
53-
bytes []byte
54-
err error
54+
once sync.Once
55+
bytes []byte
56+
tmpPath string
57+
err error
5558
}
5659

5760
func (i *imageOpener) saveImage() (io.ReadCloser, error) {
@@ -76,13 +79,50 @@ func (i *imageOpener) bufferedOpener() (io.ReadCloser, error) {
7679
return io.NopCloser(bytes.NewReader(i.bytes)), i.err
7780
}
7881

82+
func (i *imageOpener) fileBackedOpener() (io.ReadCloser, error) {
83+
i.once.Do(func() {
84+
rc, err := i.saveImage()
85+
if err != nil {
86+
i.err = err
87+
return
88+
}
89+
defer rc.Close()
90+
91+
f, err := os.CreateTemp("", "go-containerregistry-*.tar")
92+
if err != nil {
93+
i.err = err
94+
return
95+
}
96+
97+
if _, err := io.Copy(f, rc); err != nil {
98+
f.Close()
99+
os.Remove(f.Name())
100+
i.err = err
101+
return
102+
}
103+
f.Close()
104+
i.tmpPath = f.Name()
105+
106+
runtime.AddCleanup(i, func(path string) {
107+
_ = os.Remove(path)
108+
}, i.tmpPath)
109+
})
110+
111+
if i.err != nil {
112+
return nil, i.err
113+
}
114+
return os.Open(i.tmpPath)
115+
}
116+
79117
func (i *imageOpener) opener() tarball.Opener {
80-
if i.buffered {
118+
switch i.bufferMode {
119+
case bufferMemory:
81120
return i.bufferedOpener
121+
case bufferFile:
122+
return i.fileBackedOpener
123+
default:
124+
return i.saveImage
82125
}
83-
84-
// To avoid storing the tarball in memory, do a save every time we need to access something.
85-
return i.saveImage
86126
}
87127

88128
// Image provides access to an image reference from the Docker daemon,
@@ -95,10 +135,10 @@ func Image(ref name.Reference, options ...Option) (v1.Image, error) {
95135
}
96136

97137
i := &imageOpener{
98-
ref: ref,
99-
buffered: o.buffered,
100-
client: o.client,
101-
ctx: o.ctx,
138+
ref: ref,
139+
bufferMode: o.bufferMode,
140+
client: o.client,
141+
ctx: o.ctx,
102142
}
103143

104144
img := &image{

pkg/v1/daemon/image_test.go

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,89 @@ func TestImage(t *testing.T) {
201201
}
202202
}
203203

204+
func TestImageFileBuffered(t *testing.T) {
205+
t.Run("success", func(t *testing.T) {
206+
mc := &MockClient{
207+
path: imagePath,
208+
inspectResp: inspectResp,
209+
}
210+
211+
tag, err := name.NewTag("unused", name.WeakValidation)
212+
if err != nil {
213+
t.Fatalf("error creating test name: %s", err)
214+
}
215+
216+
ref, err := tarball.ImageFromPath(imagePath, nil)
217+
if err != nil {
218+
t.Fatalf("error loading test image: %s", err)
219+
}
220+
221+
dmn, err := Image(tag, WithClient(mc), WithFileBufferedOpener())
222+
if err != nil {
223+
t.Fatalf("Image(): %v", err)
224+
}
225+
226+
if err := compare.Images(ref, dmn); err != nil {
227+
t.Errorf("compare.Images: %v", err)
228+
}
229+
if err := validate.Image(dmn); err != nil {
230+
t.Errorf("validate.Image: %v", err)
231+
}
232+
233+
// The concrete *image should have a temp file after access.
234+
img := dmn.(*image)
235+
if img.opener.tmpPath == "" {
236+
t.Fatal("expected tmpPath to be set after image access")
237+
}
238+
if _, err := os.Stat(img.opener.tmpPath); err != nil {
239+
t.Fatalf("temp file should exist: %v", err)
240+
}
241+
})
242+
243+
t.Run("save error", func(t *testing.T) {
244+
mc := &MockClient{
245+
saveBody: io.NopCloser(strings.NewReader("Loaded")),
246+
saveErr: fmt.Errorf("save failed"),
247+
inspectResp: inspectResp,
248+
}
249+
250+
tag, err := name.NewTag("unused", name.WeakValidation)
251+
if err != nil {
252+
t.Fatalf("error creating test name: %s", err)
253+
}
254+
255+
dmn, err := Image(tag, WithClient(mc), WithFileBufferedOpener())
256+
if err != nil {
257+
t.Fatalf("Image(): %v", err)
258+
}
259+
260+
if _, err := dmn.Layers(); err == nil || !strings.Contains(err.Error(), "save failed") {
261+
t.Errorf("expected save error, got: %v", err)
262+
}
263+
})
264+
265+
t.Run("read error", func(t *testing.T) {
266+
mc := &MockClient{
267+
inspectResp: inspectResp,
268+
saveBody: io.NopCloser(&errReader{fmt.Errorf("read failed")}),
269+
}
270+
271+
tag, err := name.NewTag("unused", name.WeakValidation)
272+
if err != nil {
273+
t.Fatalf("error creating test name: %s", err)
274+
}
275+
276+
dmn, err := Image(tag, WithClient(mc), WithFileBufferedOpener())
277+
if err != nil {
278+
t.Fatalf("Image(): %v", err)
279+
}
280+
281+
if _, err := dmn.Layers(); err == nil || !strings.Contains(err.Error(), "read failed") {
282+
t.Errorf("expected read error, got: %v", err)
283+
}
284+
})
285+
}
286+
204287
func TestImageDefaultClient(t *testing.T) {
205288
wantErr := fmt.Errorf("bad client")
206289
defaultClient = func() (Client, error) {

pkg/v1/daemon/options.go

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,19 @@ type ImageOption Option
2929
// Option is a functional option for daemon operations.
3030
type Option func(*options)
3131

32+
// bufferMode controls how the image tarball is buffered.
33+
type bufferMode int
34+
35+
const (
36+
bufferMemory bufferMode = iota // default: buffer entire image in memory
37+
bufferNone // no buffering: re-save on each access
38+
bufferFile // buffer to a temp file on disk
39+
)
40+
3241
type options struct {
33-
ctx context.Context
34-
client Client
35-
buffered bool
42+
ctx context.Context
43+
client Client
44+
bufferMode bufferMode
3645
}
3746

3847
var defaultClient = func() (Client, error) {
@@ -41,8 +50,8 @@ var defaultClient = func() (Client, error) {
4150

4251
func makeOptions(opts ...Option) (*options, error) {
4352
o := &options{
44-
buffered: true,
45-
ctx: context.Background(),
53+
bufferMode: bufferMemory,
54+
ctx: context.Background(),
4655
}
4756
for _, opt := range opts {
4857
opt(o)
@@ -60,17 +69,28 @@ func makeOptions(opts ...Option) (*options, error) {
6069
return o, nil
6170
}
6271

63-
// WithBufferedOpener buffers the image.
72+
// WithBufferedOpener buffers the entire image into memory.
6473
func WithBufferedOpener() Option {
6574
return func(o *options) {
66-
o.buffered = true
75+
o.bufferMode = bufferMemory
6776
}
6877
}
6978

70-
// WithUnbufferedOpener streams the image to avoid buffering.
79+
// WithUnbufferedOpener streams the image to avoid buffering it.
80+
// Each access triggers a new image save.
7181
func WithUnbufferedOpener() Option {
7282
return func(o *options) {
73-
o.buffered = false
83+
o.bufferMode = bufferNone
84+
}
85+
}
86+
87+
// WithFileBufferedOpener buffers the image to a temporary file on disk.
88+
// This avoids holding the entire image in memory while still only
89+
// performing a single image save. The temporary file is cleaned up via
90+
// runtime.AddCleanup on the imageOpener.
91+
func WithFileBufferedOpener() Option {
92+
return func(o *options) {
93+
o.bufferMode = bufferFile
7494
}
7595
}
7696

0 commit comments

Comments
 (0)