fix: remove misleading error messages on successful syncs (#3619)

See: https://github.com/project-zot/zot/issues/3560#issuecomment-3594856118
What happens is:
- syncRef skips the image ("skipping image because it's already synced")
- syncReferrers doesn't sync anything
- CommitAll is still called even though nothing was synced
- The temp directory exists but is empty (no index.json, no blobs)
- CommitAll fails because index.json is missing

Let's ensure we properly check for errors, and skip the log messages if some of the cases.

Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
This commit is contained in:
Andrei Aaron
2025-12-09 08:11:28 +02:00
committed by GitHub
parent ba8ab49502
commit 4628080fa1
2 changed files with 107 additions and 2 deletions
+25 -2
View File
@@ -92,13 +92,36 @@ func (registry *DestinationRegistry) CommitAll(repo string, imageReference ref.R
defer os.RemoveAll(tempImageStore.RootDir())
registry.log.Info().Str("syncTempDir", path.Join(tempImageStore.RootDir(), repo)).Str("repository", repo).
repoDir := path.Join(tempImageStore.RootDir(), repo)
// Check if directory is empty before attempting to get index
entries, err := os.ReadDir(repoDir)
if err != nil {
if errors.Is(err, os.ErrNotExist) {
// Directory doesn't exist - nothing to commit (image was skipped)
return nil
}
// Other directory read errors should be reported
registry.log.Error().Str("errorType", common.TypeOf(err)).
Err(err).Str("dir", repoDir).Str("repo", repo).
Msg("failed to read temp sync dir")
return err
}
// If directory is empty, nothing was synced (e.g., image was skipped)
if len(entries) == 0 {
return nil
}
registry.log.Info().Str("syncTempDir", repoDir).Str("repository", repo).
Msg("pushing synced local image to local registry")
index, err := storageCommon.GetIndex(tempImageStore, repo, registry.log)
if err != nil {
registry.log.Error().Str("errorType", common.TypeOf(err)).
Err(err).Str("dir", path.Join(tempImageStore.RootDir(), repo)).Str("repo", repo).
Err(err).Str("dir", repoDir).Str("repo", repo).
Msg("failed to get repo index from temp sync dir")
return err
+82
View File
@@ -11,6 +11,7 @@ import (
"net/http"
"net/http/httptest"
"os"
"path"
"strings"
"testing"
"time"
@@ -995,6 +996,87 @@ func TestDestinationRegistry(t *testing.T) {
So(err, ShouldBeNil)
})
})
Convey("CommitAll with non-existent directory", func() {
// Create a registry and get an image reference
registry := NewDestinationRegistry(storeController, storeController, nil, log)
imageReference, err := registry.GetImageReference("nonexistent-repo", "1.0")
So(err, ShouldBeNil)
// Remove the directory to simulate it not existing
tempImageStore := getImageStoreFromImageReference("nonexistent-repo", imageReference, log)
repoDir := path.Join(tempImageStore.RootDir(), "nonexistent-repo")
err = os.RemoveAll(repoDir)
So(err, ShouldBeNil)
// CommitAll should return nil when directory doesn't exist (image was skipped)
err = registry.CommitAll("nonexistent-repo", imageReference)
So(err, ShouldBeNil)
})
Convey("CommitAll with empty directory", func() {
// Create a registry and get an image reference
registry := NewDestinationRegistry(storeController, storeController, nil, log)
imageReference, err := registry.GetImageReference("empty-repo", "1.0")
So(err, ShouldBeNil)
// Create an empty directory (no index.json, no blobs)
tempImageStore := getImageStoreFromImageReference("empty-repo", imageReference, log)
repoDir := path.Join(tempImageStore.RootDir(), "empty-repo")
err = os.MkdirAll(repoDir, 0o755)
So(err, ShouldBeNil)
// CommitAll should return nil when directory is empty (image was skipped)
err = registry.CommitAll("empty-repo", imageReference)
So(err, ShouldBeNil)
})
Convey("CommitAll with directory containing files but no index.json", func() {
// Create a registry and get an image reference
registry := NewDestinationRegistry(storeController, storeController, nil, log)
imageReference, err := registry.GetImageReference("inconsistent-repo", "1.0")
So(err, ShouldBeNil)
// Create a directory with some files but no index.json (inconsistent state)
tempImageStore := getImageStoreFromImageReference("inconsistent-repo", imageReference, log)
repoDir := path.Join(tempImageStore.RootDir(), "inconsistent-repo")
err = os.MkdirAll(repoDir, 0o755)
So(err, ShouldBeNil)
// Create a dummy file to make directory non-empty
dummyFile := path.Join(repoDir, "dummy.txt")
err = os.WriteFile(dummyFile, []byte("dummy"), 0o644)
So(err, ShouldBeNil)
// CommitAll should return an error when directory is not empty but index.json is missing
err = registry.CommitAll("inconsistent-repo", imageReference)
So(err, ShouldNotBeNil)
So(errors.Is(err, zerr.ErrRepoNotFound), ShouldBeTrue)
})
Convey("CommitAll with ReadDir error (non-ErrNotExist)", func() {
// Create a registry and get an image reference
registry := NewDestinationRegistry(storeController, storeController, nil, log)
imageReference, err := registry.GetImageReference("error-repo", "1.0")
So(err, ShouldBeNil)
// Get the repo directory path
tempImageStore := getImageStoreFromImageReference("error-repo", imageReference, log)
repoDir := path.Join(tempImageStore.RootDir(), "error-repo")
// Create a file at the repoDir path instead of a directory
// This will cause os.ReadDir to fail with an error that is NOT os.ErrNotExist
err = os.MkdirAll(path.Dir(repoDir), 0o755)
So(err, ShouldBeNil)
err = os.WriteFile(repoDir, []byte("not a directory"), 0o644)
So(err, ShouldBeNil)
// CommitAll should return the ReadDir error (not ErrNotExist)
err = registry.CommitAll("error-repo", imageReference)
So(err, ShouldNotBeNil)
So(errors.Is(err, os.ErrNotExist), ShouldBeFalse)
})
})
}