Workaround for container registry push/pull errors (#21862)

This PR addresses #19586

I added a mutex to the upload version creation which will prevent the
push errors when two requests try to create these database entries. I'm
not sure if this should be the final solution for this problem.

I added a workaround to allow a reupload of missing blobs. Normally a
reupload is skipped because the database knows the blob is already
present. The workaround checks if the blob exists on the file system.
This should not be needed anymore with the above fix so I marked this
code to be removed with Gitea v1.20.

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
This commit is contained in:
KN4CK3R 2022-11-25 06:47:46 +01:00 committed by GitHub
parent 9ce5e092f3
commit a1ae83f36e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 103 additions and 4 deletions

View file

@ -32,6 +32,13 @@ func (s *ContentStore) Get(key BlobHash256Key) (storage.Object, error) {
return s.store.Open(KeyToRelativePath(key)) return s.store.Open(KeyToRelativePath(key))
} }
// FIXME: Workaround to be removed in v1.20
// https://github.com/go-gitea/gitea/issues/19586
func (s *ContentStore) Has(key BlobHash256Key) error {
_, err := s.store.Stat(KeyToRelativePath(key))
return err
}
// Save stores a package blob // Save stores a package blob
func (s *ContentStore) Save(key BlobHash256Key, r io.Reader, size int64) error { func (s *ContentStore) Save(key BlobHash256Key, r io.Reader, size int64) error {
_, err := s.store.Save(KeyToRelativePath(key), r, size) _, err := s.store.Save(KeyToRelativePath(key), r, size)

View file

@ -7,8 +7,11 @@ package container
import ( import (
"context" "context"
"encoding/hex" "encoding/hex"
"errors"
"fmt" "fmt"
"os"
"strings" "strings"
"sync"
"code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/db"
packages_model "code.gitea.io/gitea/models/packages" packages_model "code.gitea.io/gitea/models/packages"
@ -16,9 +19,12 @@ import (
"code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/log"
packages_module "code.gitea.io/gitea/modules/packages" packages_module "code.gitea.io/gitea/modules/packages"
container_module "code.gitea.io/gitea/modules/packages/container" container_module "code.gitea.io/gitea/modules/packages/container"
"code.gitea.io/gitea/modules/util"
packages_service "code.gitea.io/gitea/services/packages" packages_service "code.gitea.io/gitea/services/packages"
) )
var uploadVersionMutex sync.Mutex
// saveAsPackageBlob creates a package blob from an upload // saveAsPackageBlob creates a package blob from an upload
// The uploaded blob gets stored in a special upload version to link them to the package/image // The uploaded blob gets stored in a special upload version to link them to the package/image
func saveAsPackageBlob(hsr packages_module.HashedSizeReader, pi *packages_service.PackageInfo) (*packages_model.PackageBlob, error) { func saveAsPackageBlob(hsr packages_module.HashedSizeReader, pi *packages_service.PackageInfo) (*packages_model.PackageBlob, error) {
@ -28,6 +34,11 @@ func saveAsPackageBlob(hsr packages_module.HashedSizeReader, pi *packages_servic
contentStore := packages_module.NewContentStore() contentStore := packages_module.NewContentStore()
var uploadVersion *packages_model.PackageVersion
// FIXME: Replace usage of mutex with database transaction
// https://github.com/go-gitea/gitea/pull/21862
uploadVersionMutex.Lock()
err := db.WithTx(db.DefaultContext, func(ctx context.Context) error { err := db.WithTx(db.DefaultContext, func(ctx context.Context) error {
created := true created := true
p := &packages_model.Package{ p := &packages_model.Package{
@ -68,11 +79,30 @@ func saveAsPackageBlob(hsr packages_module.HashedSizeReader, pi *packages_servic
} }
} }
uploadVersion = pv
return nil
})
uploadVersionMutex.Unlock()
if err != nil {
return nil, err
}
err = db.WithTx(db.DefaultContext, func(ctx context.Context) error {
pb, exists, err = packages_model.GetOrInsertBlob(ctx, pb) pb, exists, err = packages_model.GetOrInsertBlob(ctx, pb)
if err != nil { if err != nil {
log.Error("Error inserting package blob: %v", err) log.Error("Error inserting package blob: %v", err)
return err return err
} }
// FIXME: Workaround to be removed in v1.20
// https://github.com/go-gitea/gitea/issues/19586
if exists {
err = contentStore.Has(packages_module.BlobHash256Key(pb.HashSHA256))
if err != nil && (errors.Is(err, util.ErrNotExist) || errors.Is(err, os.ErrNotExist)) {
log.Debug("Package registry inconsistent: blob %s does not exist on file system", pb.HashSHA256)
exists = false
}
}
if !exists { if !exists {
if err := contentStore.Save(packages_module.BlobHash256Key(pb.HashSHA256), hsr, hsr.Size()); err != nil { if err := contentStore.Save(packages_module.BlobHash256Key(pb.HashSHA256), hsr, hsr.Size()); err != nil {
log.Error("Error saving package blob in content store: %v", err) log.Error("Error saving package blob in content store: %v", err)
@ -83,7 +113,7 @@ func saveAsPackageBlob(hsr packages_module.HashedSizeReader, pi *packages_servic
filename := strings.ToLower(fmt.Sprintf("sha256_%s", pb.HashSHA256)) filename := strings.ToLower(fmt.Sprintf("sha256_%s", pb.HashSHA256))
pf := &packages_model.PackageFile{ pf := &packages_model.PackageFile{
VersionID: pv.ID, VersionID: uploadVersion.ID,
BlobID: pb.ID, BlobID: pb.ID,
Name: filename, Name: filename,
LowerName: filename, LowerName: filename,

View file

@ -10,6 +10,7 @@ import (
"io" "io"
"net/http" "net/http"
"net/url" "net/url"
"os"
"regexp" "regexp"
"strconv" "strconv"
"strings" "strings"
@ -24,6 +25,7 @@ import (
container_module "code.gitea.io/gitea/modules/packages/container" container_module "code.gitea.io/gitea/modules/packages/container"
"code.gitea.io/gitea/modules/packages/container/oci" "code.gitea.io/gitea/modules/packages/container/oci"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/util"
"code.gitea.io/gitea/routers/api/packages/helper" "code.gitea.io/gitea/routers/api/packages/helper"
packages_service "code.gitea.io/gitea/services/packages" packages_service "code.gitea.io/gitea/services/packages"
container_service "code.gitea.io/gitea/services/packages/container" container_service "code.gitea.io/gitea/services/packages/container"
@ -193,7 +195,7 @@ func InitiateUploadBlob(ctx *context.Context) {
mount := ctx.FormTrim("mount") mount := ctx.FormTrim("mount")
from := ctx.FormTrim("from") from := ctx.FormTrim("from")
if mount != "" { if mount != "" {
blob, _ := container_model.GetContainerBlob(ctx, &container_model.BlobSearchOptions{ blob, _ := workaroundGetContainerBlob(ctx, &container_model.BlobSearchOptions{
Image: from, Image: from,
Digest: mount, Digest: mount,
}) })
@ -406,7 +408,7 @@ func getBlobFromContext(ctx *context.Context) (*packages_model.PackageFileDescri
return nil, container_model.ErrContainerBlobNotExist return nil, container_model.ErrContainerBlobNotExist
} }
return container_model.GetContainerBlob(ctx, &container_model.BlobSearchOptions{ return workaroundGetContainerBlob(ctx, &container_model.BlobSearchOptions{
OwnerID: ctx.Package.Owner.ID, OwnerID: ctx.Package.Owner.ID,
Image: ctx.Params("image"), Image: ctx.Params("image"),
Digest: digest, Digest: digest,
@ -548,7 +550,7 @@ func getManifestFromContext(ctx *context.Context) (*packages_model.PackageFileDe
return nil, container_model.ErrContainerBlobNotExist return nil, container_model.ErrContainerBlobNotExist
} }
return container_model.GetContainerBlob(ctx, opts) return workaroundGetContainerBlob(ctx, opts)
} }
// https://github.com/opencontainers/distribution-spec/blob/main/spec.md#checking-if-content-exists-in-the-registry // https://github.com/opencontainers/distribution-spec/blob/main/spec.md#checking-if-content-exists-in-the-registry
@ -688,3 +690,23 @@ func GetTagList(ctx *context.Context) {
Tags: tags, Tags: tags,
}) })
} }
// FIXME: Workaround to be removed in v1.20
// https://github.com/go-gitea/gitea/issues/19586
func workaroundGetContainerBlob(ctx *context.Context, opts *container_model.BlobSearchOptions) (*packages_model.PackageFileDescriptor, error) {
blob, err := container_model.GetContainerBlob(ctx, opts)
if err != nil {
return nil, err
}
err = packages_module.NewContentStore().Has(packages_module.BlobHash256Key(blob.Blob.HashSHA256))
if err != nil {
if errors.Is(err, util.ErrNotExist) || errors.Is(err, os.ErrNotExist) {
log.Debug("Package registry inconsistent: blob %s does not exist on file system", blob.Blob.HashSHA256)
return nil, container_model.ErrContainerBlobNotExist
}
return nil, err
}
return blob, nil
}

View file

@ -6,8 +6,10 @@ package container
import ( import (
"context" "context"
"errors"
"fmt" "fmt"
"io" "io"
"os"
"strings" "strings"
"code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/db"
@ -19,6 +21,7 @@ import (
packages_module "code.gitea.io/gitea/modules/packages" packages_module "code.gitea.io/gitea/modules/packages"
container_module "code.gitea.io/gitea/modules/packages/container" container_module "code.gitea.io/gitea/modules/packages/container"
"code.gitea.io/gitea/modules/packages/container/oci" "code.gitea.io/gitea/modules/packages/container/oci"
"code.gitea.io/gitea/modules/util"
packages_service "code.gitea.io/gitea/services/packages" packages_service "code.gitea.io/gitea/services/packages"
) )
@ -403,6 +406,15 @@ func createManifestBlob(ctx context.Context, mci *manifestCreationInfo, pv *pack
log.Error("Error inserting package blob: %v", err) log.Error("Error inserting package blob: %v", err)
return nil, false, "", err return nil, false, "", err
} }
// FIXME: Workaround to be removed in v1.20
// https://github.com/go-gitea/gitea/issues/19586
if exists {
err = packages_module.NewContentStore().Has(packages_module.BlobHash256Key(pb.HashSHA256))
if err != nil && (errors.Is(err, util.ErrNotExist) || errors.Is(err, os.ErrNotExist)) {
log.Debug("Package registry inconsistent: blob %s does not exist on file system", pb.HashSHA256)
exists = false
}
}
if !exists { if !exists {
contentStore := packages_module.NewContentStore() contentStore := packages_module.NewContentStore()
if err := contentStore.Save(packages_module.BlobHash256Key(pb.HashSHA256), buf, buf.Size()); err != nil { if err := contentStore.Save(packages_module.BlobHash256Key(pb.HashSHA256), buf, buf.Size()); err != nil {

View file

@ -6,10 +6,12 @@ package integration
import ( import (
"bytes" "bytes"
"crypto/sha256"
"encoding/base64" "encoding/base64"
"fmt" "fmt"
"net/http" "net/http"
"strings" "strings"
"sync"
"testing" "testing"
"code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/db"
@ -594,6 +596,32 @@ func TestPackageContainer(t *testing.T) {
}) })
} }
// https://github.com/go-gitea/gitea/issues/19586
t.Run("ParallelUpload", func(t *testing.T) {
defer tests.PrintCurrentTest(t)()
url := fmt.Sprintf("%sv2/%s/parallel", setting.AppURL, user.Name)
var wg sync.WaitGroup
for i := 0; i < 10; i++ {
wg.Add(1)
content := []byte{byte(i)}
digest := fmt.Sprintf("sha256:%x", sha256.Sum256(content))
go func() {
defer wg.Done()
req := NewRequestWithBody(t, "POST", fmt.Sprintf("%s/blobs/uploads?digest=%s", url, digest), bytes.NewReader(content))
addTokenAuthHeader(req, userToken)
resp := MakeRequest(t, req, http.StatusCreated)
assert.Equal(t, digest, resp.Header().Get("Docker-Content-Digest"))
}()
}
wg.Wait()
})
t.Run("OwnerNameChange", func(t *testing.T) { t.Run("OwnerNameChange", func(t *testing.T) {
defer tests.PrintCurrentTest(t)() defer tests.PrintCurrentTest(t)()