Skip to content
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

Fix: imgutil/local: Add a warning message when saving a local image and containerd storage is enable #284

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions local/local_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,14 @@

var localTestRegistry *h.DockerRegistry

type TestLogger struct {
WarnCalled bool
}

func (logger *TestLogger) Warn(msg string) {

Check failure on line 34 in local/local_test.go

View workflow job for this annotation

GitHub Actions / test-and-build-windows

unused-parameter: parameter 'msg' seems to be unused, consider removing or renaming it as _ (revive)

Check failure on line 34 in local/local_test.go

View workflow job for this annotation

GitHub Actions / test-and-build-linux

unused-parameter: parameter 'msg' seems to be unused, consider removing or renaming it as _ (revive)
logger.WarnCalled = true
}

func TestLocal(t *testing.T) {
localTestRegistry = h.NewDockerRegistry()
localTestRegistry.Start(t)
Expand Down Expand Up @@ -2028,6 +2036,32 @@
h.AssertError(t, err, "daemon response")
})
})
when("logger is set", func() {
when("containerd is used", func() {
testLogger := &TestLogger{}
dockerClient := h.DockerCli(t)
imgName := "test-image"

img, err := local.NewImage(imgName, dockerClient, local.WithLogger(testLogger))
if err != nil {
t.Fatalf("Failed to create image: %v", err)
}
err = img.Save()
if err != nil {
t.Fatalf("Failed to save image: %v", err)
}

defer func() {
if err := h.DockerRmi(dockerClient, imgName); err != nil {
t.Errorf("Failed to remove image %s: %v", imgName, err)
}
}()

if !testLogger.WarnCalled {
t.Error("Expected warning to be logged,but it was not.")
}
})
})
})

when("#SaveFile", func() {
Expand Down
6 changes: 5 additions & 1 deletion local/new.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,11 @@ func NewImage(repoName string, dockerClient DockerClient, ops ...imgutil.ImageOp
baseIdentifier = baseImage.identifier
store = baseImage.layerStore
} else {
store = NewStore(dockerClient)
if options.Logger != nil {
store = NewStore(dockerClient, WithStoreLogger(options.Logger))
} else {
store = NewStore(dockerClient)
}
}

cnbImage, err := imgutil.NewCNBImage(*options)
Expand Down
12 changes: 12 additions & 0 deletions local/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,15 @@ func WithMediaTypes(m imgutil.MediaTypes) func(*imgutil.ImageOptions) {
func WithPreviousImage(name string) func(*imgutil.ImageOptions) {
return imgutil.WithPreviousImage(name)
}

func WithLogger(logger imgutil.Logger) func(*imgutil.ImageOptions) {
return imgutil.WithLogger(logger)
}

type StoreOption func(*Store)

func WithStoreLogger(logger imgutil.Logger) StoreOption {
return func(s *Store) {
s.Logger = logger
}
}
14 changes: 12 additions & 2 deletions local/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ type Store struct {
// optional
downloadOnce *sync.Once
onDiskLayersByDiffID map[v1.Hash]annotatedLayer
Logger imgutil.Logger
}

// DockerClient is subset of client.CommonAPIClient required by this package.
Expand All @@ -53,12 +54,18 @@ type annotatedLayer struct {
uncompressedSize int64
}

func NewStore(dockerClient DockerClient) *Store {
return &Store{
func NewStore(dockerClient DockerClient, ops ...StoreOption) *Store {
store := &Store{
dockerClient: dockerClient,
downloadOnce: &sync.Once{},
onDiskLayersByDiffID: make(map[v1.Hash]annotatedLayer),
Logger: nil,
}
for _, op := range ops {
op(store)
}

return store
}

// images
Expand Down Expand Up @@ -95,6 +102,9 @@ func (s *Store) Save(image *Image, withName string, withAdditionalNames ...strin
inspect, err = s.doSave(image, withName)
}
if !canOmitBaseLayers || err != nil {
if s.Logger != nil && !canOmitBaseLayers {
s.Logger.Warn("Containerd is enabled, which will make saving your work more expensive.")
}
if err = image.ensureLayers(); err != nil {
return "", err
}
Expand Down
12 changes: 12 additions & 0 deletions options.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ type ImageOptions struct {
BaseImageRepoName string
PreviousImageRepoName string
Config *v1.Config
Logger Logger
CreatedAt time.Time
MediaTypes MediaTypes
Platform Platform
Expand All @@ -43,6 +44,10 @@ type RegistrySetting struct {
Insecure bool
}

type Logger interface {
Warn(msg string)
}

// FromBaseImage loads the provided image as the manifest, config, and layers for the working image.
// If the image is not found, it does nothing.
func FromBaseImage(name string) func(*ImageOptions) {
Expand Down Expand Up @@ -99,6 +104,13 @@ func WithPreviousImage(name string) func(*ImageOptions) {
}
}

// WithLogger if provided will check if contained is used.
func WithLogger(logger Logger) func(*ImageOptions) {
return func(o *ImageOptions) {
o.Logger = logger
}
}

type IndexOption func(options *IndexOptions) error

type IndexOptions struct {
Expand Down
Loading