fix: multiple fixes based on recent test failures (#3582)

* fix(monitoring): fix race condition in TestPopulateStorageMetrics

Write images to filesystem before starting controller to prevent GC
from reading index.json while it's being written.

See failure:
https://github.com/project-zot/zot/actions/runs/19633819097/job/56219606041

Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>

* fix(sync): fix TestSignatures assertion on referrers order

Search through all referrers to find the expected OCI ref digest instead
of assuming it's at index.Manifests[1], as the order is not guaranteed.

See https://github.com/project-zot/zot/actions/runs/19630382617/job/56208492397?pr=3579

Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>

* fix(redis): fix error handling in GetMultipleRepoMeta

The function was returning err at the end, which could be nil even when
HGetAll should have failed, because err gets reassigned in the loop.
Changed to return nil on success path, ensuring HGetAll errors are
properly propagated. Fixes TestRedisUnreachable failure.

See https://github.com/project-zot/zot/actions/runs/19634990856/job/56223414955?pr=3582

Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>

* refactor(sync): simplify sync test structure and restore file permissions

Remove unnecessary nested Convey blocks and integrate error tests into
parent blocks. Add defer statements to restore file permissions after
chmod operations to prevent side effects on subsequent tests.

Fix potential root cause of https://github.com/project-zot/zot/actions/runs/19636284485/job/56227682319?pr=3579

Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>

---------

Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
This commit is contained in:
Andrei Aaron
2025-11-24 19:23:52 +02:00
committed by GitHub
parent 6f0e05e676
commit a296968c1a
3 changed files with 46 additions and 27 deletions
+5 -5
View File
@@ -453,17 +453,17 @@ func TestPopulateStorageMetrics(t *testing.T) {
So(ctlr, ShouldNotBeNil)
ctlr.Log = log.NewLoggerWithWriter("debug", writers)
cm := test.NewControllerManager(ctlr)
cm.StartAndWait(port)
defer cm.StopServer()
// write a couple of images
// Write images before starting controller to avoid race condition with garbage collection
srcStorageCtlr := ociutils.GetDefaultStoreController(rootDir, ctlr.Log)
err = WriteImageToFileSystem(CreateDefaultImage(), "alpine", "0.0.1", srcStorageCtlr)
So(err, ShouldBeNil)
err = WriteImageToFileSystem(CreateDefaultImage(), "busybox", "0.0.1", srcStorageCtlr)
So(err, ShouldBeNil)
cm := test.NewControllerManager(ctlr)
cm.StartAndWait(port)
defer cm.StopServer()
metrics := monitoring.NewMetricsServer(true, ctlr.Log)
sch := scheduler.NewScheduler(conf, metrics, ctlr.Log)
sch.RunScheduler()
+40 -21
View File
@@ -1461,18 +1461,22 @@ func TestDockerImagesAreSkipped(t *testing.T) {
So(err, ShouldBeNil)
So(resp.StatusCode(), ShouldEqual, http.StatusNotFound)
Convey("trigger config blob upstream error", func() {
// remove synced image
err := os.RemoveAll(path.Join(destDir, testImage))
So(err, ShouldBeNil)
// trigger config blob upstream error
// remove synced image
err = os.RemoveAll(path.Join(destDir, testImage))
So(err, ShouldBeNil)
err = os.Chmod(path.Join(srcDir, testImage, "blobs/sha256", configBlobDigest.Encoded()), 0o000)
So(err, ShouldBeNil)
configBlobPath := path.Join(srcDir, testImage, "blobs/sha256", configBlobDigest.Encoded())
err = os.Chmod(configBlobPath, 0o000)
So(err, ShouldBeNil)
resp, err = resty.R().Get(destBaseURL + "/v2/" + testImage + "/manifests/" + testImageTag)
So(err, ShouldBeNil)
So(resp.StatusCode(), ShouldEqual, http.StatusNotFound)
})
defer func() {
_ = os.Chmod(configBlobPath, storageConstants.DefaultFilePerms)
}()
resp, err = resty.R().Get(destBaseURL + "/v2/" + testImage + "/manifests/" + testImageTag)
So(err, ShouldBeNil)
So(resp.StatusCode(), ShouldEqual, http.StatusNotFound)
})
Convey("skipping already synced multiarch docker image", func() {
@@ -1627,18 +1631,22 @@ func TestDockerImagesAreSkipped(t *testing.T) {
So(err, ShouldBeNil)
So(resp.StatusCode(), ShouldEqual, http.StatusNotFound)
Convey("trigger config blob upstream error", func() {
// remove synced image
err := os.RemoveAll(path.Join(destDir, indexRepoName))
So(err, ShouldBeNil)
// trigger config blob upstream error
// remove synced image
err = os.RemoveAll(path.Join(destDir, indexRepoName))
So(err, ShouldBeNil)
err = os.Chmod(path.Join(srcDir, indexRepoName, "blobs/sha256", configBlobDigest.Encoded()), 0o000)
So(err, ShouldBeNil)
configBlobPath := path.Join(srcDir, indexRepoName, "blobs/sha256", configBlobDigest.Encoded())
err = os.Chmod(configBlobPath, 0o000)
So(err, ShouldBeNil)
resp, err = resty.R().Get(destBaseURL + "/v2/" + indexRepoName + "/manifests/" + "latest")
So(err, ShouldBeNil)
So(resp.StatusCode(), ShouldEqual, http.StatusNotFound)
})
defer func() {
_ = os.Chmod(configBlobPath, storageConstants.DefaultFilePerms)
}()
resp, err = resty.R().Get(destBaseURL + "/v2/" + indexRepoName + "/manifests/" + "latest")
So(err, ShouldBeNil)
So(resp.StatusCode(), ShouldEqual, http.StatusNotFound)
})
})
}
@@ -4855,7 +4863,18 @@ func TestSignatures(t *testing.T) {
So(err, ShouldBeNil)
So(len(index.Manifests), ShouldEqual, 2)
So(index.Manifests[1].Digest, ShouldEqual, ociRefDigest)
foundOCIRef := false
for _, manifest := range index.Manifests {
if manifest.Digest == ociRefDigest {
foundOCIRef = true
break
}
}
So(foundOCIRef, ShouldBeTrue)
// test negative cases (trigger errors)
// test notary signatures errors
+1 -1
View File
@@ -1296,7 +1296,7 @@ func (rc *RedisDB) GetMultipleRepoMeta(ctx context.Context, filter func(repoMeta
}
}
return foundRepos, err
return foundRepos, nil
}
// AddManifestSignature adds signature metadata to a given manifest in the database.