From 46cbb0ab6aeacb25ebbce7d9883f3179e536e49d Mon Sep 17 00:00:00 2001 From: wuhenggongzi <1305678586@qq.com> Date: Sun, 10 Nov 2024 17:43:22 +0800 Subject: [PATCH 1/3] imgutil: add logger options Signed-off-by: wuhenggongzi <1305678586@qq.com> --- options.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/options.go b/options.go index 99dd9aeb..dd3f149f 100644 --- a/options.go +++ b/options.go @@ -17,6 +17,7 @@ type ImageOptions struct { BaseImageRepoName string PreviousImageRepoName string Config *v1.Config + Logger Logger CreatedAt time.Time MediaTypes MediaTypes Platform Platform @@ -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) { @@ -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 { From 77ee93a635370ec4d9fbf1fb518af7c89b8366a5 Mon Sep 17 00:00:00 2001 From: wuhenggongzi <1305678586@qq.com> Date: Sun, 10 Nov 2024 18:30:06 +0800 Subject: [PATCH 2/3] imgutil(local): Add Logger functionality(#282) This commit enhances the Logger functionality by integrating it with the existing automatic detection of the storage driver type in the NewStore function. The Logger interface has been added to log warnings related to the containerd storage driver. Test cases have been created to ensure that warnings are logged correctly when using containerd. Signed-off-by: wuhenggongzi <1305678586@qq.com> --- local/local_test.go | 34 ++++++++++++++++++++++++++++++++++ local/new.go | 6 +++++- local/options.go | 12 ++++++++++++ local/store.go | 14 ++++++++++++-- 4 files changed, 63 insertions(+), 3 deletions(-) diff --git a/local/local_test.go b/local/local_test.go index 134248fa..d09f781e 100644 --- a/local/local_test.go +++ b/local/local_test.go @@ -27,6 +27,14 @@ const someSHA = "sha256:aec070645fe53ee3b3763059376134f058cc337247c978add178b6cc var localTestRegistry *h.DockerRegistry +type TestLogger struct { + WarnCalled bool +} + +func (logger *TestLogger) Warn(msg string) { + logger.WarnCalled = true +} + func TestLocal(t *testing.T) { localTestRegistry = h.NewDockerRegistry() localTestRegistry.Start(t) @@ -2028,6 +2036,32 @@ func testImage(t *testing.T, when spec.G, it spec.S) { 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() { diff --git a/local/new.go b/local/new.go index 72c636fd..dc9e0148 100644 --- a/local/new.go +++ b/local/new.go @@ -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) diff --git a/local/options.go b/local/options.go index 82740066..4b9b0ebe 100644 --- a/local/options.go +++ b/local/options.go @@ -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 + } +} diff --git a/local/store.go b/local/store.go index 106d3a93..f1e110c5 100644 --- a/local/store.go +++ b/local/store.go @@ -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. @@ -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 @@ -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 } From 9acf4783661ce57953a4ca8a1b77a08f91f1ed97 Mon Sep 17 00:00:00 2001 From: wuhenggongzi <1305678586@qq.com> Date: Mon, 11 Nov 2024 18:37:05 +0800 Subject: [PATCH 3/3] imgutil(local): Fix the logger test function The message parameter was not handled as specified before, but now the message parameter is printed. Signed-off-by: wuhenggongzi <1305678586@qq.com> --- local/local_test.go | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/local/local_test.go b/local/local_test.go index d09f781e..c6bea4c8 100644 --- a/local/local_test.go +++ b/local/local_test.go @@ -29,10 +29,12 @@ var localTestRegistry *h.DockerRegistry type TestLogger struct { WarnCalled bool + Message string } func (logger *TestLogger) Warn(msg string) { logger.WarnCalled = true + logger.Message = msg } func TestLocal(t *testing.T) { @@ -2057,8 +2059,28 @@ func testImage(t *testing.T, when spec.G, it spec.S) { } }() - if !testLogger.WarnCalled { - t.Error("Expected warning to be logged,but it was not.") + isContainerd := func(cli local.DockerClient) bool { + info, err := cli.Info(context.Background()) + if err != nil { + return false + } + for _, driverStatus := range info.DriverStatus { + if driverStatus[0] == "driver-type" && driverStatus[1] == "io.containerd.snapshotter.v1" { + return true + } + } + return false + }(dockerClient) + if isContainerd { + if !testLogger.WarnCalled { + t.Error("Expected warning to be logged,but it was not.") + t.Errorf(testLogger.Message) + } + } else { + if testLogger.WarnCalled { + t.Error("did not expected warning to be logged,but it was.") + t.Errorf(testLogger.Message) + } } }) })