From d2aa016cdbb688d6eae289c9a05d7a9c61dcfc90 Mon Sep 17 00:00:00 2001 From: Ramkumar Chinchani <rchincha@cisco.com> Date: Fri, 21 Jan 2022 04:11:44 +0000 Subject: [PATCH] storage: flush/sync contents to disk on file close Behavior controlled by configuration (default=off) It is a trade-off between performance and consistency. References: [1] https://github.com/golang/go/issues/20599 Signed-off-by: Ramkumar Chinchani <rchincha@cisco.com> --- Makefile | 4 +- examples/config-commit.json | 15 + pkg/api/config/config.go | 4 +- pkg/api/controller.go | 8 +- pkg/api/controller_test.go | 456 ++++++++++++++++++-- pkg/api/routes.go | 7 +- pkg/extensions/search/common/common_test.go | 6 +- pkg/extensions/search/cve/cve_test.go | 8 +- pkg/extensions/search/digest/digest_test.go | 4 +- pkg/extensions/sync/sync_internal_test.go | 4 +- pkg/extensions/sync/utils.go | 2 +- pkg/log/log.go | 6 +- pkg/storage/s3/s3_test.go | 4 +- pkg/storage/s3/storage.go | 2 +- pkg/storage/scrub_test.go | 2 +- pkg/storage/storage_fs.go | 78 +++- pkg/storage/storage_fs_test.go | 71 ++- pkg/storage/storage_test.go | 10 +- pkg/test/dev.go | 38 +- pkg/test/inject_test.go | 5 + 20 files changed, 621 insertions(+), 113 deletions(-) create mode 100644 examples/config-commit.json diff --git a/Makefile b/Makefile index 4719b684..4dc61c99 100644 --- a/Makefile +++ b/Makefile @@ -69,8 +69,8 @@ test: check-skopeo $(NOTATION) go test -tags extended,containers_image_openpgp -v -trimpath -race -timeout 15m -cover -coverpkg ./... -coverprofile=coverage-extended.txt -covermode=atomic ./... go test -tags minimal,containers_image_openpgp -v -trimpath -race -cover -coverpkg ./... -coverprofile=coverage-minimal.txt -covermode=atomic ./... # development-mode unit tests possibly using failure injection - go test -tags dev,extended,containers_image_openpgp -v -trimpath -race -timeout 15m -cover -coverpkg ./... -coverprofile=coverage-dev-extended.txt -covermode=atomic ./pkg/api/... ./pkg/test/... - go test -tags dev,minimal,containers_image_openpgp -v -trimpath -race -cover -coverpkg ./... -coverprofile=coverage-dev-minimal.txt -covermode=atomic ./pkg/api/... ./pkg/test/... + go test -tags dev,extended,containers_image_openpgp -v -trimpath -race -timeout 15m -cover -coverpkg ./... -coverprofile=coverage-dev-extended.txt -covermode=atomic ./pkg/test/... ./pkg/storage/... + go test -tags dev,minimal,containers_image_openpgp -v -trimpath -race -cover -coverpkg ./... -coverprofile=coverage-dev-minimal.txt -covermode=atomic ./pkg/test/... ./pkg/storage/... .PHONY: run-bench run-bench: binary bench diff --git a/examples/config-commit.json b/examples/config-commit.json new file mode 100644 index 00000000..4b44c170 --- /dev/null +++ b/examples/config-commit.json @@ -0,0 +1,15 @@ +{ + "version": "0.1.0-dev", + "storage": { + "rootDirectory": "/tmp/zot", + "commit": true + }, + "http": { + "address": "127.0.0.1", + "port": "8080", + "ReadOnly": false + }, + "log": { + "level": "debug" + } +} diff --git a/pkg/api/config/config.go b/pkg/api/config/config.go index d786778e..b5530568 100644 --- a/pkg/api/config/config.go +++ b/pkg/api/config/config.go @@ -21,6 +21,7 @@ type StorageConfig struct { RootDirectory string GC bool Dedupe bool + Commit bool StorageDriver map[string]interface{} `mapstructure:",omitempty"` } @@ -90,9 +91,10 @@ type LogConfig struct { } type GlobalStorageConfig struct { - RootDirectory string Dedupe bool GC bool + Commit bool + RootDirectory string StorageDriver map[string]interface{} `mapstructure:",omitempty"` SubPaths map[string]StorageConfig } diff --git a/pkg/api/controller.go b/pkg/api/controller.go index 455c8aec..8c22f5f4 100644 --- a/pkg/api/controller.go +++ b/pkg/api/controller.go @@ -214,7 +214,7 @@ func (c *Controller) InitImageStore() error { var defaultStore storage.ImageStore if len(c.Config.Storage.StorageDriver) == 0 { defaultStore = storage.NewImageStore(c.Config.Storage.RootDirectory, - c.Config.Storage.GC, c.Config.Storage.Dedupe, c.Log, c.Metrics) + c.Config.Storage.GC, c.Config.Storage.Dedupe, c.Config.Storage.Commit, c.Log, c.Metrics) } else { storeName := fmt.Sprintf("%v", c.Config.Storage.StorageDriver["name"]) if storeName != storage.S3StorageDriverName { @@ -230,7 +230,7 @@ func (c *Controller) InitImageStore() error { } defaultStore = s3.NewImageStore(c.Config.Storage.RootDirectory, - c.Config.Storage.GC, c.Config.Storage.Dedupe, c.Log, c.Metrics, store) + c.Config.Storage.GC, c.Config.Storage.Dedupe, c.Config.Storage.Commit, c.Log, c.Metrics, store) } c.StoreController.DefaultStore = defaultStore @@ -266,7 +266,7 @@ func (c *Controller) InitImageStore() error { if len(storageConfig.StorageDriver) == 0 { subImageStore[route] = storage.NewImageStore(storageConfig.RootDirectory, - storageConfig.GC, storageConfig.Dedupe, c.Log, c.Metrics) + storageConfig.GC, storageConfig.Dedupe, storageConfig.Commit, c.Log, c.Metrics) } else { storeName := fmt.Sprintf("%v", storageConfig.StorageDriver["name"]) if storeName != storage.S3StorageDriverName { @@ -282,7 +282,7 @@ func (c *Controller) InitImageStore() error { } subImageStore[route] = s3.NewImageStore(storageConfig.RootDirectory, - storageConfig.GC, storageConfig.Dedupe, c.Log, c.Metrics, store) + storageConfig.GC, storageConfig.Dedupe, storageConfig.Commit, c.Log, c.Metrics, store) } // Enable extensions if extension config is provided diff --git a/pkg/api/controller_test.go b/pkg/api/controller_test.go index 29a9a26c..4aaf3cfe 100644 --- a/pkg/api/controller_test.go +++ b/pkg/api/controller_test.go @@ -25,6 +25,7 @@ import ( "time" "github.com/chartmuseum/auth" + "github.com/gorilla/mux" "github.com/mitchellh/mapstructure" vldap "github.com/nmcclain/ldap" notreg "github.com/notaryproject/notation/pkg/registry" @@ -2089,23 +2090,6 @@ func TestAuthorizationWithBasicAuth(t *testing.T) { So(resp, ShouldNotBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusOK) - Convey("Hard to reach cases", func() { - injected := test.InjectFailure(0) - - // get tags with read access should get 200 - conf.AccessControl.Repositories[AuthorizationNamespace].Policies[0].Actions = - append(conf.AccessControl.Repositories[AuthorizationNamespace].Policies[0].Actions, "read") - resp, err = resty.R().SetBasicAuth(username, passphrase). - Get(baseURL + "/v2/" + AuthorizationNamespace + "/tags/list") - So(err, ShouldBeNil) - So(resp, ShouldNotBeNil) - if injected { - So(resp.StatusCode(), ShouldEqual, http.StatusNotFound) - } else { - So(resp.StatusCode(), ShouldEqual, http.StatusOK) - } - }) - // head blob should get 200 now resp, err = resty.R().SetBasicAuth(username, passphrase). Head(baseURL + "/v2/" + AuthorizationNamespace + "/blobs/" + digest) @@ -2818,16 +2802,28 @@ func TestParallelRequests(t *testing.T) { panic(err) } + t.Cleanup(func() { + os.RemoveAll(dir) + }) + firstSubDir, err := ioutil.TempDir("", "oci-sub-dir") if err != nil { panic(err) } + t.Cleanup(func() { + os.RemoveAll(firstSubDir) + }) + secondSubDir, err := ioutil.TempDir("", "oci-sub-dir") if err != nil { panic(err) } + t.Cleanup(func() { + os.RemoveAll(secondSubDir) + }) + subPaths := make(map[string]config.StorageConfig) subPaths["/a"] = config.StorageConfig{RootDirectory: firstSubDir} @@ -2861,24 +2857,6 @@ func TestParallelRequests(t *testing.T) { assert.Equal(t, err, nil, "Error should be nil") assert.Equal(t, headResponse.StatusCode(), http.StatusNotFound, "response status code should return 404") - Convey("Hard to reach cases", t, func() { - _ = test.InjectFailure(0) - - headResponse, err := client.R().SetBasicAuth(username, passphrase). - Head(baseURL + "/v2/" + testcase.destImageName + "/manifests/test:1.0") - assert.Equal(t, err, nil, "Error should be nil") - assert.Equal(t, headResponse.StatusCode(), http.StatusNotFound, "response status code should return 404") - }) - - Convey("Hard to reach cases", t, func() { - _ = test.InjectFailure(1) - - headResponse, err := client.R().SetBasicAuth(username, passphrase). - Head(baseURL + "/v2/" + testcase.destImageName + "/manifests/test:1.0") - assert.Equal(t, err, nil, "Error should be nil") - assert.Equal(t, headResponse.StatusCode(), http.StatusNotFound, "response status code should return 404") - }) - getResponse, err := client.R().SetBasicAuth(username, passphrase). Get(baseURL + "/v2/" + testcase.destImageName + "/manifests/" + manifest) assert.Equal(t, err, nil, "Error should be nil") @@ -3158,6 +3136,7 @@ func TestImageSignatures(t *testing.T) { content := []byte("this is a blob") digest := godigest.FromBytes(content) So(digest, ShouldNotBeNil) + // monolithic blob upload: success resp, err = resty.R().SetQueryParam("digest", digest.String()). SetHeader("Content-Type", "application/octet-stream").SetBody(content).Put(loc) @@ -3447,6 +3426,413 @@ func TestImageSignatures(t *testing.T) { }) } +func TestRouteFailures(t *testing.T) { + Convey("Make a new controller", t, func() { + port := test.GetFreePort() + baseURL := test.GetBaseURL(port) + conf := config.New() + conf.HTTP.Port = port + + ctlr := api.NewController(conf) + dir, err := ioutil.TempDir("", "oci-repo-test") + if err != nil { + panic(err) + } + defer os.RemoveAll(dir) + ctlr.Config.Storage.RootDirectory = dir + ctlr.Config.Storage.Commit = true + + go startServer(ctlr) + defer stopServer(ctlr) + test.WaitTillServerReady(baseURL) + + rthdlr := api.NewRouteHandler(ctlr) + + Convey("List tags", func() { + request, _ := http.NewRequestWithContext(context.TODO(), "GET", baseURL+"/v2/foo/tags/list", nil) + mux.SetURLVars(request, map[string]string{}) + response := httptest.NewRecorder() + + rthdlr.ListTags(response, request) + + resp := response.Result() + defer resp.Body.Close() + So(resp, ShouldNotBeNil) + So(resp.StatusCode, ShouldEqual, http.StatusNotFound) + + request, _ = http.NewRequestWithContext(context.TODO(), "GET", baseURL+"/v2/foo/tags/list", nil) + request = mux.SetURLVars(request, map[string]string{"name": "foo"}) + response = httptest.NewRecorder() + + rthdlr.ListTags(response, request) + + resp = response.Result() + defer resp.Body.Close() + So(resp, ShouldNotBeNil) + So(resp.StatusCode, ShouldEqual, http.StatusNotFound) + + request, _ = http.NewRequestWithContext(context.TODO(), "GET", baseURL+"/v2/foo/tags/list", nil) + request = mux.SetURLVars(request, map[string]string{"name": "foo"}) + qparm := request.URL.Query() + qparm.Add("n", "a") + request.URL.RawQuery = qparm.Encode() + response = httptest.NewRecorder() + + rthdlr.ListTags(response, request) + + resp = response.Result() + defer resp.Body.Close() + So(resp, ShouldNotBeNil) + So(resp.StatusCode, ShouldEqual, http.StatusBadRequest) + + request, _ = http.NewRequestWithContext(context.TODO(), "GET", baseURL+"/v2/foo/tags/list", nil) + request = mux.SetURLVars(request, map[string]string{"name": "foo"}) + qparm = request.URL.Query() + qparm.Add("n", "abc") + request.URL.RawQuery = qparm.Encode() + response = httptest.NewRecorder() + + rthdlr.ListTags(response, request) + + resp = response.Result() + defer resp.Body.Close() + So(resp, ShouldNotBeNil) + So(resp.StatusCode, ShouldEqual, http.StatusBadRequest) + + request, _ = http.NewRequestWithContext(context.TODO(), "GET", baseURL+"/v2/foo/tags/list", nil) + request = mux.SetURLVars(request, map[string]string{"name": "foo"}) + qparm = request.URL.Query() + qparm.Add("n", "a") + qparm.Add("n", "abc") + request.URL.RawQuery = qparm.Encode() + response = httptest.NewRecorder() + + rthdlr.ListTags(response, request) + + resp = response.Result() + defer resp.Body.Close() + So(resp, ShouldNotBeNil) + So(resp.StatusCode, ShouldEqual, http.StatusBadRequest) + + request, _ = http.NewRequestWithContext(context.TODO(), "GET", baseURL+"/v2/foo/tags/list", nil) + request = mux.SetURLVars(request, map[string]string{"name": "foo"}) + qparm = request.URL.Query() + qparm.Add("n", "0") + request.URL.RawQuery = qparm.Encode() + response = httptest.NewRecorder() + + rthdlr.ListTags(response, request) + + resp = response.Result() + defer resp.Body.Close() + So(resp, ShouldNotBeNil) + So(resp.StatusCode, ShouldEqual, http.StatusNotFound) + + request, _ = http.NewRequestWithContext(context.TODO(), "GET", baseURL+"/v2/foo/tags/list", nil) + request = mux.SetURLVars(request, map[string]string{"name": "foo"}) + qparm = request.URL.Query() + qparm.Add("n", "1") + qparm.Add("last", "") + request.URL.RawQuery = qparm.Encode() + response = httptest.NewRecorder() + + rthdlr.ListTags(response, request) + + resp = response.Result() + defer resp.Body.Close() + So(resp, ShouldNotBeNil) + So(resp.StatusCode, ShouldEqual, http.StatusNotFound) + + request, _ = http.NewRequestWithContext(context.TODO(), "GET", baseURL+"/v2/foo/tags/list", nil) + request = mux.SetURLVars(request, map[string]string{"name": "foo"}) + qparm = request.URL.Query() + qparm.Add("n", "1") + qparm.Add("last", "a") + request.URL.RawQuery = qparm.Encode() + response = httptest.NewRecorder() + + rthdlr.ListTags(response, request) + + resp = response.Result() + defer resp.Body.Close() + So(resp, ShouldNotBeNil) + So(resp.StatusCode, ShouldEqual, http.StatusNotFound) + + request, _ = http.NewRequestWithContext(context.TODO(), "GET", baseURL+"/v2/foo/tags/list", nil) + request = mux.SetURLVars(request, map[string]string{"name": "foo"}) + qparm = request.URL.Query() + qparm.Add("n", "1") + qparm.Add("last", "a") + qparm.Add("last", "abc") + request.URL.RawQuery = qparm.Encode() + response = httptest.NewRecorder() + + rthdlr.ListTags(response, request) + + resp = response.Result() + defer resp.Body.Close() + So(resp, ShouldNotBeNil) + So(resp.StatusCode, ShouldEqual, http.StatusBadRequest) + }) + + Convey("Check manifest", func() { + request, _ := http.NewRequestWithContext(context.TODO(), "HEAD", baseURL+"/v2/foo/manifests/test:1.0", nil) + request = mux.SetURLVars(request, map[string]string{}) + response := httptest.NewRecorder() + + rthdlr.CheckManifest(response, request) + + resp := response.Result() + defer resp.Body.Close() + So(resp, ShouldNotBeNil) + So(resp.StatusCode, ShouldEqual, http.StatusNotFound) + + request, _ = http.NewRequestWithContext(context.TODO(), "HEAD", baseURL+"/v2/foo/manifests/test:1.0", nil) + request = mux.SetURLVars(request, map[string]string{"name": "foo"}) + response = httptest.NewRecorder() + + rthdlr.CheckManifest(response, request) + + resp = response.Result() + defer resp.Body.Close() + So(resp, ShouldNotBeNil) + So(resp.StatusCode, ShouldEqual, http.StatusNotFound) + + request, _ = http.NewRequestWithContext(context.TODO(), "HEAD", baseURL+"/v2/foo/manifests/test:1.0", nil) + request = mux.SetURLVars(request, map[string]string{"name": "foo", "reference": ""}) + response = httptest.NewRecorder() + + rthdlr.CheckManifest(response, request) + + resp = response.Result() + defer resp.Body.Close() + So(resp, ShouldNotBeNil) + So(resp.StatusCode, ShouldEqual, http.StatusNotFound) + }) + }) +} + +func TestStorageCommit(t *testing.T) { + Convey("Make a new controller", t, func() { + port := test.GetFreePort() + baseURL := test.GetBaseURL(port) + conf := config.New() + conf.HTTP.Port = port + + ctlr := api.NewController(conf) + dir, err := ioutil.TempDir("", "oci-repo-test") + if err != nil { + panic(err) + } + defer os.RemoveAll(dir) + ctlr.Config.Storage.RootDirectory = dir + ctlr.Config.Storage.Commit = true + + go startServer(ctlr) + defer stopServer(ctlr) + test.WaitTillServerReady(baseURL) + + Convey("Manifests", func() { + _, _ = Print("\nManifests") + // create a blob/layer + resp, err := resty.R().Post(baseURL + "/v2/repo7/blobs/uploads/") + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusAccepted) + loc := test.Location(baseURL, resp) + So(loc, ShouldNotBeEmpty) + + // since we are not specifying any prefix i.e provided in config while starting server, + // so it should store repo7 to global root dir + _, err = os.Stat(path.Join(dir, "repo7")) + So(err, ShouldBeNil) + + resp, err = resty.R().Get(loc) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusNoContent) + content := []byte("this is a blob5") + digest := godigest.FromBytes(content) + So(digest, ShouldNotBeNil) + // monolithic blob upload: success + resp, err = resty.R().SetQueryParam("digest", digest.String()). + SetHeader("Content-Type", "application/octet-stream").SetBody(content).Put(loc) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusCreated) + blobLoc := resp.Header().Get("Location") + So(blobLoc, ShouldNotBeEmpty) + So(resp.Header().Get("Content-Length"), ShouldEqual, "0") + So(resp.Header().Get(api.DistContentDigestKey), ShouldNotBeEmpty) + + // check a non-existent manifest + resp, err = resty.R().SetHeader("Content-Type", "application/vnd.oci.image.manifest.v1+json"). + SetBody(content).Head(baseURL + "/v2/unknown/manifests/test:1.0") + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusNotFound) + + // upload image config blob + resp, err = resty.R().Post(baseURL + "/v2/repo7/blobs/uploads/") + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusAccepted) + loc = test.Location(baseURL, resp) + cblob, cdigest := test.GetRandomImageConfig() + + resp, err = resty.R(). + SetContentLength(true). + SetHeader("Content-Length", fmt.Sprintf("%d", len(cblob))). + SetHeader("Content-Type", "application/octet-stream"). + SetQueryParam("digest", cdigest.String()). + SetBody(cblob). + Put(loc) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusCreated) + + // create a manifest + manifest := ispec.Manifest{ + Config: ispec.Descriptor{ + MediaType: "application/vnd.oci.image.config.v1+json", + Digest: cdigest, + Size: int64(len(cblob)), + }, + Layers: []ispec.Descriptor{ + { + MediaType: "application/vnd.oci.image.layer.v1.tar", + Digest: digest, + Size: int64(len(content)), + }, + }, + } + manifest.SchemaVersion = 2 + content, err = json.Marshal(manifest) + So(err, ShouldBeNil) + digest = godigest.FromBytes(content) + So(digest, ShouldNotBeNil) + resp, err = resty.R().SetHeader("Content-Type", "application/vnd.oci.image.manifest.v1+json"). + SetBody(content).Put(baseURL + "/v2/repo7/manifests/test:1.0") + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusCreated) + digestHdr := resp.Header().Get(api.DistContentDigestKey) + So(digestHdr, ShouldNotBeEmpty) + So(digestHdr, ShouldEqual, digest.String()) + + resp, err = resty.R().SetHeader("Content-Type", "application/vnd.oci.image.manifest.v1+json"). + SetBody(content).Put(baseURL + "/v2/repo7/manifests/test:1.0.1") + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusCreated) + digestHdr = resp.Header().Get(api.DistContentDigestKey) + So(digestHdr, ShouldNotBeEmpty) + So(digestHdr, ShouldEqual, digest.String()) + + content = []byte("this is a blob5") + digest = godigest.FromBytes(content) + So(digest, ShouldNotBeNil) + + // upload image config blob + resp, err = resty.R().Post(baseURL + "/v2/repo7/blobs/uploads/") + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusAccepted) + loc = test.Location(baseURL, resp) + cblob, cdigest = test.GetRandomImageConfig() + + resp, err = resty.R(). + SetContentLength(true). + SetHeader("Content-Length", fmt.Sprintf("%d", len(cblob))). + SetHeader("Content-Type", "application/octet-stream"). + SetQueryParam("digest", cdigest.String()). + SetBody(cblob). + Put(loc) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusCreated) + + // create a manifest with same blob but a different tag + manifest = ispec.Manifest{ + Config: ispec.Descriptor{ + MediaType: "application/vnd.oci.image.config.v1+json", + Digest: cdigest, + Size: int64(len(cblob)), + }, + Layers: []ispec.Descriptor{ + { + MediaType: "application/vnd.oci.image.layer.v1.tar", + Digest: digest, + Size: int64(len(content)), + }, + }, + } + manifest.SchemaVersion = 2 + content, err = json.Marshal(manifest) + So(err, ShouldBeNil) + digest = godigest.FromBytes(content) + So(digest, ShouldNotBeNil) + resp, err = resty.R().SetHeader("Content-Type", "application/vnd.oci.image.manifest.v1+json"). + SetBody(content).Put(baseURL + "/v2/repo7/manifests/test:2.0") + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusCreated) + digestHdr = resp.Header().Get(api.DistContentDigestKey) + So(digestHdr, ShouldNotBeEmpty) + So(digestHdr, ShouldEqual, digest.String()) + + // check/get by tag + resp, err = resty.R().Head(baseURL + "/v2/repo7/manifests/test:1.0") + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + So(resp.Header().Get("Content-Type"), ShouldNotBeEmpty) + resp, err = resty.R().Get(baseURL + "/v2/repo7/manifests/test:1.0") + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + So(resp.Body(), ShouldNotBeEmpty) + // check/get by reference + resp, err = resty.R().Head(baseURL + "/v2/repo7/manifests/" + digest.String()) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + So(resp.Header().Get("Content-Type"), ShouldNotBeEmpty) + resp, err = resty.R().Get(baseURL + "/v2/repo7/manifests/" + digest.String()) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusOK) + So(resp.Body(), ShouldNotBeEmpty) + + // delete manifest by tag should pass + resp, err = resty.R().Delete(baseURL + "/v2/repo7/manifests/test:1.0") + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusAccepted) + // delete manifest by digest (1.0 deleted but 1.0.1 has same reference) + resp, err = resty.R().Delete(baseURL + "/v2/repo7/manifests/" + digest.String()) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusAccepted) + // delete manifest by digest + resp, err = resty.R().Delete(baseURL + "/v2/repo7/manifests/" + digest.String()) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusNotFound) + // delete again should fail + resp, err = resty.R().Delete(baseURL + "/v2/repo7/manifests/" + digest.String()) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusNotFound) + + // check/get by tag + resp, err = resty.R().Head(baseURL + "/v2/repo7/manifests/test:1.0") + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusNotFound) + resp, err = resty.R().Get(baseURL + "/v2/repo7/manifests/test:1.0") + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusNotFound) + So(resp.Body(), ShouldNotBeEmpty) + resp, err = resty.R().Head(baseURL + "/v2/repo7/manifests/test:2.0") + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusNotFound) + resp, err = resty.R().Get(baseURL + "/v2/repo7/manifests/test:2.0") + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusNotFound) + So(resp.Body(), ShouldNotBeEmpty) + // check/get by reference + resp, err = resty.R().Head(baseURL + "/v2/repo7/manifests/" + digest.String()) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusNotFound) + resp, err = resty.R().Get(baseURL + "/v2/repo7/manifests/" + digest.String()) + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusNotFound) + So(resp.Body(), ShouldNotBeEmpty) + }) + }) +} + func getAllBlobs(imagePath string) []string { blobList := make([]string, 0) diff --git a/pkg/api/routes.go b/pkg/api/routes.go index 6176a9c9..f9e03aba 100644 --- a/pkg/api/routes.go +++ b/pkg/api/routes.go @@ -32,7 +32,6 @@ import ( ext "zotregistry.io/zot/pkg/extensions" "zotregistry.io/zot/pkg/log" "zotregistry.io/zot/pkg/storage" - "zotregistry.io/zot/pkg/test" // as required by swaggo. _ "zotregistry.io/zot/swagger" @@ -166,7 +165,7 @@ func (rh *RouteHandler) ListTags(response http.ResponseWriter, request *http.Req name, ok := vars["name"] - if !test.Ok(ok) || name == "" { + if !ok || name == "" { response.WriteHeader(http.StatusNotFound) return @@ -288,7 +287,7 @@ func (rh *RouteHandler) CheckManifest(response http.ResponseWriter, request *htt vars := mux.Vars(request) name, ok := vars["name"] - if !test.Ok(ok) || name == "" { + if !ok || name == "" { response.WriteHeader(http.StatusNotFound) return @@ -297,7 +296,7 @@ func (rh *RouteHandler) CheckManifest(response http.ResponseWriter, request *htt imgStore := rh.getImageStore(name) reference, ok := vars["reference"] - if !test.Ok(ok) || reference == "" { + if !ok || reference == "" { WriteJSON(response, http.StatusNotFound, NewErrorList(NewError(MANIFEST_INVALID, map[string]string{"reference": reference}))) diff --git a/pkg/extensions/search/common/common_test.go b/pkg/extensions/search/common/common_test.go index 2dd7643b..6d32e996 100644 --- a/pkg/extensions/search/common/common_test.go +++ b/pkg/extensions/search/common/common_test.go @@ -123,7 +123,7 @@ func TestImageFormat(t *testing.T) { dbDir := "../../../../test/data" metrics := monitoring.NewMetricsServer(false, log) - defaultStore := storage.NewImageStore(dbDir, false, false, log, metrics) + defaultStore := storage.NewImageStore(dbDir, false, false, false, log, metrics) storeController := storage.StoreController{DefaultStore: defaultStore} olu := common.NewOciLayoutUtils(storeController, log) @@ -411,9 +411,9 @@ func TestUtilsMethod(t *testing.T) { defer os.RemoveAll(subRootDir) metrics := monitoring.NewMetricsServer(false, log) - defaultStore := storage.NewImageStore(rootDir, false, false, log, metrics) + defaultStore := storage.NewImageStore(rootDir, false, false, false, log, metrics) - subStore := storage.NewImageStore(subRootDir, false, false, log, metrics) + subStore := storage.NewImageStore(subRootDir, false, false, false, log, metrics) subStoreMap := make(map[string]storage.ImageStore) diff --git a/pkg/extensions/search/cve/cve_test.go b/pkg/extensions/search/cve/cve_test.go index 74eff7fa..3edf7607 100644 --- a/pkg/extensions/search/cve/cve_test.go +++ b/pkg/extensions/search/cve/cve_test.go @@ -90,7 +90,7 @@ func testSetup() error { log := log.NewLogger("debug", "") metrics := monitoring.NewMetricsServer(false, log) - storeController := storage.StoreController{DefaultStore: storage.NewImageStore(dir, false, false, log, metrics)} + storeController := storage.StoreController{DefaultStore: storage.NewImageStore(dir, false, false, false, log, metrics)} layoutUtils := common.NewOciLayoutUtils(storeController, log) @@ -347,11 +347,11 @@ func TestMultipleStoragePath(t *testing.T) { metrics := monitoring.NewMetricsServer(false, log) // Create ImageStore - firstStore := storage.NewImageStore(firstRootDir, false, false, log, metrics) + firstStore := storage.NewImageStore(firstRootDir, false, false, false, log, metrics) - secondStore := storage.NewImageStore(secondRootDir, false, false, log, metrics) + secondStore := storage.NewImageStore(secondRootDir, false, false, false, log, metrics) - thirdStore := storage.NewImageStore(thirdRootDir, false, false, log, metrics) + thirdStore := storage.NewImageStore(thirdRootDir, false, false, false, log, metrics) storeController := storage.StoreController{} diff --git a/pkg/extensions/search/digest/digest_test.go b/pkg/extensions/search/digest/digest_test.go index 64ef2bc2..402d2477 100644 --- a/pkg/extensions/search/digest/digest_test.go +++ b/pkg/extensions/search/digest/digest_test.go @@ -97,7 +97,9 @@ func testSetup() error { log := log.NewLogger("debug", "") metrics := monitoring.NewMetricsServer(false, log) - storeController := storage.StoreController{DefaultStore: storage.NewImageStore(rootDir, false, false, log, metrics)} + storeController := storage.StoreController{ + DefaultStore: storage.NewImageStore(rootDir, false, false, false, log, metrics), + } digestInfo = digestinfo.NewDigestInfo(storeController, log) diff --git a/pkg/extensions/sync/sync_internal_test.go b/pkg/extensions/sync/sync_internal_test.go index 06c44a02..890c345c 100644 --- a/pkg/extensions/sync/sync_internal_test.go +++ b/pkg/extensions/sync/sync_internal_test.go @@ -268,7 +268,7 @@ func TestSyncInternal(t *testing.T) { log := log.Logger{Logger: zerolog.New(os.Stdout)} metrics := monitoring.NewMetricsServer(false, log) - imageStore := storage.NewImageStore(storageDir, false, false, log, metrics) + imageStore := storage.NewImageStore(storageDir, false, false, false, log, metrics) storeController := storage.StoreController{} storeController.DefaultStore = imageStore @@ -289,7 +289,7 @@ func TestSyncInternal(t *testing.T) { panic(err) } - testImageStore := storage.NewImageStore(testRootDir, false, false, log, metrics) + testImageStore := storage.NewImageStore(testRootDir, false, false, false, log, metrics) manifestContent, _, _, err := testImageStore.GetImageManifest(testImage, testImageTag) So(err, ShouldBeNil) diff --git a/pkg/extensions/sync/utils.go b/pkg/extensions/sync/utils.go index 8221d0d8..7fb1689d 100644 --- a/pkg/extensions/sync/utils.go +++ b/pkg/extensions/sync/utils.go @@ -442,7 +442,7 @@ func pushSyncedLocalImage(repo, tag, localCachePath string, imageStore := storeController.GetImageStore(repo) metrics := monitoring.NewMetricsServer(false, log) - cacheImageStore := storage.NewImageStore(localCachePath, false, false, log, metrics) + cacheImageStore := storage.NewImageStore(localCachePath, false, false, false, log, metrics) manifestContent, _, _, err := cacheImageStore.GetImageManifest(repo, tag) if err != nil { diff --git a/pkg/log/log.go b/pkg/log/log.go index efc34678..52ae9baf 100644 --- a/pkg/log/log.go +++ b/pkg/log/log.go @@ -68,8 +68,8 @@ func NewAuditLogger(level string, audit string) *Logger { return &Logger{Logger: auditLog.With().Timestamp().Logger()} } -// goroutineID adds goroutine-id to logs to help debug concurrency issues. -func goroutineID() int { +// GoroutineID adds goroutine-id to logs to help debug concurrency issues. +func GoroutineID() int { var buf [64]byte n := runtime.Stack(buf[:], false) idField := strings.Fields(strings.TrimPrefix(string(buf[:n]), "goroutine "))[0] @@ -86,6 +86,6 @@ type goroutineHook struct{} func (h goroutineHook) Run(e *zerolog.Event, level zerolog.Level, msg string) { if level != zerolog.NoLevel { - e.Int("goroutine", goroutineID()) + e.Int("goroutine", GoroutineID()) } } diff --git a/pkg/storage/s3/s3_test.go b/pkg/storage/s3/s3_test.go index 9fb7d061..5d88a11a 100644 --- a/pkg/storage/s3/s3_test.go +++ b/pkg/storage/s3/s3_test.go @@ -53,7 +53,7 @@ func skipIt(t *testing.T) { func createMockStorage(rootDir string, store driver.StorageDriver) storage.ImageStore { log := log.Logger{Logger: zerolog.New(os.Stdout)} metrics := monitoring.NewMetricsServer(false, log) - il := s3.NewImageStore(rootDir, false, false, log, metrics, store) + il := s3.NewImageStore(rootDir, false, false, false, log, metrics, store) return il } @@ -86,7 +86,7 @@ func createObjectsStore(rootDir string) (driver.StorageDriver, storage.ImageStor log := log.Logger{Logger: zerolog.New(os.Stdout)} metrics := monitoring.NewMetricsServer(false, log) - il := s3.NewImageStore(rootDir, false, false, log, metrics, store) + il := s3.NewImageStore(rootDir, false, false, false, log, metrics, store) return store, il, err } diff --git a/pkg/storage/s3/storage.go b/pkg/storage/s3/storage.go index af65e9a8..2f206728 100644 --- a/pkg/storage/s3/storage.go +++ b/pkg/storage/s3/storage.go @@ -63,7 +63,7 @@ func (is *ObjectStorage) DirExists(d string) bool { // NewObjectStorage returns a new image store backed by cloud storages. // see https://github.com/docker/docker.github.io/tree/master/registry/storage-drivers -func NewImageStore(rootDir string, gc bool, dedupe bool, log zlog.Logger, metrics monitoring.MetricServer, +func NewImageStore(rootDir string, gc, dedupe, commit bool, log zlog.Logger, metrics monitoring.MetricServer, store driver.StorageDriver) storage.ImageStore { imgStore := &ObjectStorage{ rootDir: rootDir, diff --git a/pkg/storage/scrub_test.go b/pkg/storage/scrub_test.go index f5a18ee2..01c21751 100644 --- a/pkg/storage/scrub_test.go +++ b/pkg/storage/scrub_test.go @@ -36,7 +36,7 @@ func TestCheckAllBlobsIntegrity(t *testing.T) { metrics := monitoring.NewMetricsServer(false, log) - imgStore := storage.NewImageStore(dir, true, true, log, metrics) + imgStore := storage.NewImageStore(dir, true, true, true, log, metrics) Convey("Scrub only one repo", t, func(c C) { // initialize repo diff --git a/pkg/storage/storage_fs.go b/pkg/storage/storage_fs.go index 38b0d921..4329a7b1 100644 --- a/pkg/storage/storage_fs.go +++ b/pkg/storage/storage_fs.go @@ -29,6 +29,7 @@ import ( zerr "zotregistry.io/zot/errors" "zotregistry.io/zot/pkg/extensions/monitoring" zlog "zotregistry.io/zot/pkg/log" + "zotregistry.io/zot/pkg/test" ) const ( @@ -61,6 +62,7 @@ type ImageStoreFS struct { cache *Cache gc bool dedupe bool + commit bool log zerolog.Logger metrics monitoring.MetricServer } @@ -103,7 +105,8 @@ func (sc StoreController) GetImageStore(name string) ImageStore { } // NewImageStore returns a new image store backed by a file storage. -func NewImageStore(rootDir string, gc bool, dedupe bool, log zlog.Logger, metrics monitoring.MetricServer) ImageStore { +func NewImageStore(rootDir string, gc, dedupe, commit bool, + log zlog.Logger, metrics monitoring.MetricServer) ImageStore { if _, err := os.Stat(rootDir); os.IsNotExist(err) { if err := os.MkdirAll(rootDir, DefaultDirPerms); err != nil { log.Error().Err(err).Str("rootDir", rootDir).Msg("unable to create root dir") @@ -118,6 +121,7 @@ func NewImageStore(rootDir string, gc bool, dedupe bool, log zlog.Logger, metric blobUploads: make(map[string]BlobUpload), gc: gc, dedupe: dedupe, + commit: commit, log: log.With().Caller().Logger(), metrics: metrics, } @@ -203,7 +207,7 @@ func (is *ImageStoreFS) initRepo(name string) error { is.log.Panic().Err(err).Msg("unable to marshal JSON") } - if err := ioutil.WriteFile(ilPath, buf, DefaultFilePerms); err != nil { + if err := is.writeFile(ilPath, buf); err != nil { is.log.Error().Err(err).Str("file", ilPath).Msg("unable to write file") return err @@ -221,7 +225,7 @@ func (is *ImageStoreFS) initRepo(name string) error { is.log.Panic().Err(err).Msg("unable to marshal JSON") } - if err := ioutil.WriteFile(indexPath, buf, DefaultFilePerms); err != nil { + if err := is.writeFile(indexPath, buf); err != nil { is.log.Error().Err(err).Str("file", indexPath).Msg("unable to write file") return err @@ -660,7 +664,7 @@ func (is *ImageStoreFS) PutImageManifest(repo string, reference string, mediaTyp _ = ensureDir(dir, is.log) file := path.Join(dir, mDigest.Encoded()) - if err := ioutil.WriteFile(file, body, DefaultFilePerms); err != nil { + if err := is.writeFile(file, body); err != nil { is.log.Error().Err(err).Str("file", file).Msg("unable to write") return "", err @@ -678,7 +682,7 @@ func (is *ImageStoreFS) PutImageManifest(repo string, reference string, mediaTyp return "", err } - if err := ioutil.WriteFile(file, buf, DefaultFilePerms); err != nil { + if err := is.writeFile(file, buf); err != nil { is.log.Error().Err(err).Str("file", file).Msg("unable to write") return "", err @@ -781,7 +785,7 @@ func (is *ImageStoreFS) DeleteImageManifest(repo string, reference string) error return err } - if err := ioutil.WriteFile(file, buf, DefaultFilePerms); err != nil { + if err := is.writeFile(file, buf); err != nil { return err } @@ -884,17 +888,20 @@ func (is *ImageStoreFS) PutBlobChunkStreamed(repo string, uuid string, body io.R return -1, zerr.ErrUploadNotFound } - file, err := os.OpenFile( - blobUploadPath, - os.O_WRONLY|os.O_CREATE, - DefaultFilePerms, - ) + file, err := os.OpenFile(blobUploadPath, os.O_WRONLY|os.O_CREATE, DefaultFilePerms) if err != nil { is.log.Error().Err(err).Msg("failed to open file") return -1, err } - defer file.Close() + + defer func() { + if is.commit { + _ = file.Sync() + } + + _ = file.Close() + }() if _, err := file.Seek(0, io.SeekEnd); err != nil { is.log.Error().Err(err).Msg("failed to seek file") @@ -929,17 +936,20 @@ func (is *ImageStoreFS) PutBlobChunk(repo string, uuid string, from int64, to in return -1, zerr.ErrBadUploadRange } - file, err := os.OpenFile( - blobUploadPath, - os.O_WRONLY|os.O_CREATE, - DefaultFilePerms, - ) + file, err := os.OpenFile(blobUploadPath, os.O_WRONLY|os.O_CREATE, DefaultFilePerms) if err != nil { is.log.Error().Err(err).Msg("failed to open file") return -1, err } - defer file.Close() + + defer func() { + if is.commit { + _ = file.Sync() + } + + _ = file.Close() + }() if _, err := file.Seek(from, io.SeekStart); err != nil { is.log.Error().Err(err).Msg("failed to seek file") @@ -1077,7 +1087,13 @@ func (is *ImageStoreFS) FullBlobUpload(repo string, body io.Reader, digest strin return "", -1, zerr.ErrUploadNotFound } - defer blobFile.Close() + defer func() { + if is.commit { + _ = blobFile.Sync() + } + + _ = blobFile.Close() + }() digester := sha256.New() mw := io.MultiWriter(blobFile, digester) @@ -1514,6 +1530,30 @@ func (is *ImageStoreFS) GetReferrers(repo, digest string, mediaType string) ([]a return result, nil } +func (is *ImageStoreFS) writeFile(filename string, data []byte) error { + if !is.commit { + return ioutil.WriteFile(filename, data, DefaultFilePerms) + } + + fhandle, err := os.OpenFile(filename, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, DefaultFilePerms) + if err != nil { + return err + } + + _, err = fhandle.Write(data) + + if err1 := test.Error(fhandle.Sync()); err1 != nil && err == nil { + err = err1 + is.log.Error().Err(err).Str("filename", filename).Msg("unable to sync file") + } + + if err1 := test.Error(fhandle.Close()); err1 != nil && err == nil { + err = err1 + } + + return err +} + func IsSupportedMediaType(mediaType string) bool { return mediaType == ispec.MediaTypeImageManifest || mediaType == artifactspec.MediaTypeArtifactManifest diff --git a/pkg/storage/storage_fs_test.go b/pkg/storage/storage_fs_test.go index deef505d..a51694c5 100644 --- a/pkg/storage/storage_fs_test.go +++ b/pkg/storage/storage_fs_test.go @@ -34,7 +34,7 @@ func TestStorageFSAPIs(t *testing.T) { log := log.Logger{Logger: zerolog.New(os.Stdout)} metrics := monitoring.NewMetricsServer(false, log) - imgStore := storage.NewImageStore(dir, true, true, log, metrics) + imgStore := storage.NewImageStore(dir, true, true, true, log, metrics) Convey("Repo layout", t, func(c C) { repoName := "test" @@ -171,7 +171,7 @@ func TestDedupeLinks(t *testing.T) { log := log.Logger{Logger: zerolog.New(os.Stdout)} metrics := monitoring.NewMetricsServer(false, log) - imgStore := storage.NewImageStore(dir, true, true, log, metrics) + imgStore := storage.NewImageStore(dir, true, true, true, log, metrics) Convey("Dedupe", t, func(c C) { // manifest1 @@ -311,7 +311,7 @@ func TestDedupe(t *testing.T) { log := log.Logger{Logger: zerolog.New(os.Stdout)} metrics := monitoring.NewMetricsServer(false, log) - il := storage.NewImageStore(dir, true, true, log, metrics) + il := storage.NewImageStore(dir, true, true, true, log, metrics) So(il.DedupeBlob("", "", ""), ShouldNotBeNil) }) @@ -330,9 +330,9 @@ func TestNegativeCases(t *testing.T) { log := log.Logger{Logger: zerolog.New(os.Stdout)} metrics := monitoring.NewMetricsServer(false, log) - So(storage.NewImageStore(dir, true, true, log, metrics), ShouldNotBeNil) + So(storage.NewImageStore(dir, true, true, true, log, metrics), ShouldNotBeNil) if os.Geteuid() != 0 { - So(storage.NewImageStore("/deadBEEF", true, true, log, metrics), ShouldBeNil) + So(storage.NewImageStore("/deadBEEF", true, true, true, log, metrics), ShouldBeNil) } }) @@ -345,7 +345,7 @@ func TestNegativeCases(t *testing.T) { log := log.Logger{Logger: zerolog.New(os.Stdout)} metrics := monitoring.NewMetricsServer(false, log) - imgStore := storage.NewImageStore(dir, true, true, log, metrics) + imgStore := storage.NewImageStore(dir, true, true, true, log, metrics) err = os.Chmod(dir, 0o000) // remove all perms if err != nil { @@ -384,7 +384,7 @@ func TestNegativeCases(t *testing.T) { log := log.Logger{Logger: zerolog.New(os.Stdout)} metrics := monitoring.NewMetricsServer(false, log) - imgStore := storage.NewImageStore(dir, true, true, log, metrics) + imgStore := storage.NewImageStore(dir, true, true, true, log, metrics) So(imgStore, ShouldNotBeNil) So(imgStore.InitRepo("test"), ShouldBeNil) @@ -502,7 +502,7 @@ func TestNegativeCases(t *testing.T) { log := log.Logger{Logger: zerolog.New(os.Stdout)} metrics := monitoring.NewMetricsServer(false, log) - imgStore := storage.NewImageStore(dir, true, true, log, metrics) + imgStore := storage.NewImageStore(dir, true, true, true, log, metrics) So(imgStore, ShouldNotBeNil) So(imgStore.InitRepo("test"), ShouldBeNil) @@ -529,7 +529,7 @@ func TestNegativeCases(t *testing.T) { log := log.Logger{Logger: zerolog.New(os.Stdout)} metrics := monitoring.NewMetricsServer(false, log) - imgStore := storage.NewImageStore(dir, true, true, log, metrics) + imgStore := storage.NewImageStore(dir, true, true, true, log, metrics) So(imgStore, ShouldNotBeNil) So(imgStore.InitRepo("test"), ShouldBeNil) @@ -574,7 +574,7 @@ func TestNegativeCases(t *testing.T) { log := log.Logger{Logger: zerolog.New(os.Stdout)} metrics := monitoring.NewMetricsServer(false, log) - imgStore := storage.NewImageStore(dir, true, true, log, metrics) + imgStore := storage.NewImageStore(dir, true, true, true, log, metrics) So(imgStore, ShouldNotBeNil) So(imgStore.InitRepo("test"), ShouldBeNil) @@ -636,7 +636,7 @@ func TestNegativeCases(t *testing.T) { log := log.Logger{Logger: zerolog.New(os.Stdout)} metrics := monitoring.NewMetricsServer(false, log) - imgStore := storage.NewImageStore(dir, true, true, log, metrics) + imgStore := storage.NewImageStore(dir, true, true, true, log, metrics) upload, err := imgStore.NewBlobUpload("dedupe1") So(err, ShouldBeNil) @@ -788,6 +788,55 @@ func TestHardLink(t *testing.T) { }) } +func TestWriteFile(t *testing.T) { + Convey("writeFile with commit", t, func() { + dir, err := ioutil.TempDir("", "oci-repo-test") + So(err, ShouldBeNil) + defer os.RemoveAll(dir) + + log := log.Logger{Logger: zerolog.New(os.Stdout)} + metrics := monitoring.NewMetricsServer(false, log) + imgStore := storage.NewImageStore(dir, true, true, true, log, metrics) + + Convey("Failure path1", func() { + injected := test.InjectFailure(0) + + err := imgStore.InitRepo("repo1") + if injected { + So(err, ShouldNotBeNil) + } else { + So(err, ShouldBeNil) + } + }) + + Convey("Failure path2", func() { + injected := test.InjectFailure(1) + + err := imgStore.InitRepo("repo2") + if injected { + So(err, ShouldNotBeNil) + } else { + So(err, ShouldBeNil) + } + }) + }) + + Convey("writeFile without commit", t, func() { + dir, err := ioutil.TempDir("", "oci-repo-test") + So(err, ShouldBeNil) + defer os.RemoveAll(dir) + + log := log.Logger{Logger: zerolog.New(os.Stdout)} + metrics := monitoring.NewMetricsServer(false, log) + imgStore := storage.NewImageStore(dir, true, true, false, log, metrics) + + Convey("Failure path not reached", func() { + err := imgStore.InitRepo("repo1") + So(err, ShouldBeNil) + }) + }) +} + func randSeq(n int) string { letters := []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") diff --git a/pkg/storage/storage_test.go b/pkg/storage/storage_test.go index d978a945..6cea6bc0 100644 --- a/pkg/storage/storage_test.go +++ b/pkg/storage/storage_test.go @@ -72,7 +72,7 @@ func createObjectsStore(rootDir string) (driver.StorageDriver, storage.ImageStor log := log.Logger{Logger: zerolog.New(os.Stdout)} metrics := monitoring.NewMetricsServer(false, log) - il := s3.NewImageStore(rootDir, false, false, log, metrics, store) + il := s3.NewImageStore(rootDir, false, false, false, log, metrics, store) return store, il, err } @@ -120,7 +120,7 @@ func TestStorageAPIs(t *testing.T) { log := log.Logger{Logger: zerolog.New(os.Stdout)} metrics := monitoring.NewMetricsServer(false, log) - imgStore = storage.NewImageStore(dir, true, true, log, metrics) + imgStore = storage.NewImageStore(dir, true, true, true, log, metrics) } Convey("Repo layout", t, func(c C) { @@ -711,11 +711,11 @@ func TestStorageHandler(t *testing.T) { metrics := monitoring.NewMetricsServer(false, log) // Create ImageStore - firstStore = storage.NewImageStore(firstRootDir, false, false, log, metrics) + firstStore = storage.NewImageStore(firstRootDir, false, false, false, log, metrics) - secondStore = storage.NewImageStore(secondRootDir, false, false, log, metrics) + secondStore = storage.NewImageStore(secondRootDir, false, false, false, log, metrics) - thirdStore = storage.NewImageStore(thirdRootDir, false, false, log, metrics) + thirdStore = storage.NewImageStore(thirdRootDir, false, false, false, log, metrics) } Convey("Test storage handler", t, func() { diff --git a/pkg/test/dev.go b/pkg/test/dev.go index 13fa4445..88693f05 100644 --- a/pkg/test/dev.go +++ b/pkg/test/dev.go @@ -9,6 +9,7 @@ import ( "sync" zerr "zotregistry.io/zot/errors" + "zotregistry.io/zot/pkg/log" ) func Ok(ok bool) bool { @@ -42,40 +43,49 @@ func Error(err error) error { **/ type inject struct { - skip int - enabled bool + skip int } //nolint:gochecknoglobals // only used by test code -var ( - injlock sync.Mutex - injst = inject{} -) +var injMap sync.Map func InjectFailure(skip int) bool { - injlock.Lock() - injst = inject{enabled: true, skip: skip} - injlock.Unlock() + gid := log.GoroutineID() + if gid < 0 { + panic("invalid goroutine id") + } + + if _, ok := injMap.Load(gid); ok { + panic("prior incomplete fault injection") + } + + injst := inject{skip: skip} + injMap.Store(gid, injst) return true } func injectedFailure() bool { - injlock.Lock() - defer injlock.Unlock() + gid := log.GoroutineID() - if !injst.enabled { + val, ok := injMap.Load(gid) + if !ok { return false } + injst, ok := val.(inject) + if !ok { + panic("invalid type") + } + if injst.skip == 0 { - // disable the injection point - injst.enabled = false + injMap.Delete(gid) return true } injst.skip-- + injMap.Store(gid, injst) return false } diff --git a/pkg/test/inject_test.go b/pkg/test/inject_test.go index 00547aa9..36ce3af2 100644 --- a/pkg/test/inject_test.go +++ b/pkg/test/inject_test.go @@ -117,4 +117,9 @@ func TestInject(t *testing.T) { ok := alwaysNotOk() So(test.Ok(ok), ShouldBeFalse) }) + + Convey("Incomplete injected failure", t, func(c C) { + test.InjectFailure(0) // inject a failure + So(func() { test.InjectFailure(0) }, ShouldPanic) + }) }