diff --git a/go.mod b/go.mod index e068172d373..db9195d114f 100644 --- a/go.mod +++ b/go.mod @@ -195,3 +195,5 @@ require ( gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 // indirect tags.cncf.io/container-device-interface/specs-go v1.0.0 // indirect ) + +replace go.podman.io/common => github.com/rhatdan/container-libs b801c9d727f587ca2c9d50ca3a3278fd6c4c979d diff --git a/test/e2e/artifact_test.go b/test/e2e/artifact_test.go index 4e82cc05e69..7b385035c47 100644 --- a/test/e2e/artifact_test.go +++ b/test/e2e/artifact_test.go @@ -741,6 +741,62 @@ var _ = Describe("Podman artifact", func() { session = podmanTest.PodmanExitCleanly("artifact", "inspect", artifactDigest[:12], "-f", "{{.Name}}") Expect(session.OutputToString()).To(Equal(artifact1Name)) }) + + // Regression test for https://github.com/containers/podman/issues/27569 + It("podman artifact add concurrent - no race condition", func() { + const numArtifacts = 5 + + // Create temporary files for artifacts + artifactFiles := make([]string, numArtifacts) + artifactNames := make([]string, numArtifacts) + for i := 0; i < numArtifacts; i++ { + file, err := createArtifactFile(int64(1024 * (i + 1))) // Different sizes + Expect(err).ToNot(HaveOccurred()) + artifactFiles[i] = file + artifactNames[i] = fmt.Sprintf("localhost/concurrent/artifact%d", i) + } + + // Run all artifact add commands concurrently + sessions := make([]*PodmanSessionIntegration, numArtifacts) + for i := 0; i < numArtifacts; i++ { + // Start all commands without waiting + sessions[i] = podmanTest.Podman([]string{"artifact", "add", artifactNames[i], artifactFiles[i]}) + } + + // Wait for all to complete and collect results + digests := make([]string, numArtifacts) + for i := 0; i < numArtifacts; i++ { + sessions[i].WaitWithDefaultTimeout() + Expect(sessions[i]).Should(ExitCleanly()) + digests[i] = sessions[i].OutputToString() + Expect(digests[i]).To(HaveLen(64)) // SHA256 digest length + } + + // Verify all artifacts were created successfully + listSession := podmanTest.PodmanExitCleanly("artifact", "ls", "--format", "{{.Repository}}") + output := listSession.OutputToStringArray() + + // Check that all artifact names are in the list + for i := 0; i < numArtifacts; i++ { + Expect(output).To(ContainElement(artifactNames[i]), + fmt.Sprintf("Artifact %s should be in the list", artifactNames[i])) + } + + // Verify each artifact can be inspected + for i := 0; i < numArtifacts; i++ { + inspectSession := podmanTest.PodmanExitCleanly("artifact", "inspect", artifactNames[i]) + Expect(inspectSession.OutputToString()).To(ContainSubstring(artifactNames[i])) + } + + // Verify each artifact has the correct size + for i := 0; i < numArtifacts; i++ { + expectedSize := 1024 * (i + 1) + sizeSession := podmanTest.PodmanExitCleanly("artifact", "ls", "--format", "{{.VirtualSize}}", "--noheading") + sizes := sizeSession.OutputToStringArray() + Expect(sizes).To(ContainElement(fmt.Sprintf("%d", expectedSize)), + fmt.Sprintf("Artifact %d should have size %d", i, expectedSize)) + } + }) }) func digestToFilename(digest string) string { diff --git a/vendor/go.podman.io/common/pkg/libartifact/store/store.go b/vendor/go.podman.io/common/pkg/libartifact/store/store.go index 0665cbe73cd..7730222fc64 100644 --- a/vendor/go.podman.io/common/pkg/libartifact/store/store.go +++ b/vendor/go.podman.io/common/pkg/libartifact/store/store.go @@ -218,13 +218,8 @@ func (as ArtifactStore) Add(ctx context.Context, dest string, artifactBlobs []li return nil, errors.New("append option is not compatible with type option") } - locked := true as.lock.Lock() - defer func() { - if locked { - as.lock.Unlock() - } - }() + defer as.lock.Unlock() // Check if artifact already exists artifacts, err := as.getArtifacts(ctx, nil) @@ -297,10 +292,11 @@ func (as ArtifactStore) Add(ctx context.Context, dest string, artifactBlobs []li } defer imageDest.Close() - // Unlock around the actual pull of the blobs. - // This is ugly as hell, but should be safe. - locked = false - as.lock.Unlock() + // Keep the lock held during blob copying to prevent concurrent artifact adds + // from racing on the OCI layout index file. The lock was previously released + // here as an optimization, but this created a race condition where concurrent + // artifact additions could overwrite each other's index entries. + // See: https://github.com/containers/podman/issues/27569 // ImageDestination, in general, requires the caller to write a full image; here we may write only the added layers. // This works for the oci/layout transport we hard-code. @@ -356,8 +352,7 @@ func (as ArtifactStore) Add(ctx context.Context, dest string, artifactBlobs []li artifactManifest.Layers = append(artifactManifest.Layers, newLayer) } - as.lock.Lock() - locked = true + // Lock is still held from the beginning of the function rawData, err := json.Marshal(artifactManifest) if err != nil {