Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 70 additions & 0 deletions tests/integration/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -805,3 +805,73 @@ func isOCMirrorVersionBefore(major int, minor int) bool {
}
return verMajor < major
}

// logOcMirrorResult writes the oc-mirror command result to GinkgoWriter for diagnostics.
func logOcMirrorResult(label string, result *ocmirror.Result) {
if result == nil {
GinkgoWriter.Printf("[%s] result is nil\n", label)
return
}
GinkgoWriter.Printf("[%s] exit_code=%d duration=%s\n", label, result.ExitCode, result.Duration)
if result.Stdout != "" {
GinkgoWriter.Printf("[%s] stdout:\n%s\n", label, result.Stdout)
}
if result.Stderr != "" {
GinkgoWriter.Printf("[%s] stderr:\n%s\n", label, result.Stderr)
}
}

// logTarSummary logs the size, total entry count, and blob count of a tar archive.
func logTarSummary(label, tarPath string) {
info, err := os.Stat(tarPath)
Expect(err).NotTo(HaveOccurred())

entries := listTarEntries(tarPath)

blobCount := 0
repoSet := map[string]bool{}
for _, e := range entries {
if strings.Contains(e, "blobs/sha256/") {
blobCount++
}
if strings.HasPrefix(e, tarRepositoriesPath) {
parts := strings.SplitN(strings.TrimPrefix(e, tarRepositoriesPath), "/", 3)
if len(parts) >= 2 {
repoSet[parts[0]+"/"+parts[1]] = true
}
}
}

GinkgoWriter.Printf("[%s tar] path=%s size=%d bytes entries=%d blobs=%d repositories=%d\n",
label, tarPath, info.Size(), len(entries), blobCount, len(repoSet))
for repo := range repoSet {
GinkgoWriter.Printf("[%s tar] repo: %s\n", label, repo)
}
}

// collectTarBlobPaths returns the sorted list of tar entry names that fall under
// the given prefix. OCI blobs are content-addressed (the SHA-256 digest is part of
// the path), so comparing blob paths is equivalent to comparing blob content.
func collectTarBlobPaths(tarPath, prefix string) []string {
prefix = filepath.ToSlash(prefix)
if !strings.HasSuffix(prefix, "/") {
prefix += "/"
}

var paths []string
for _, entry := range listTarEntries(tarPath) {
e := filepath.ToSlash(entry)
if strings.HasPrefix(e, prefix) {
paths = append(paths, e)
}
}
return paths
}

// expectTarDoesNotContainPath verifies that no tar entry contains the given substring.
func expectTarDoesNotContainPath(entries []string, substring string) {
for _, entry := range entries {
Expect(entry).NotTo(ContainSubstring(substring),
"tar archive should not contain path matching %q, but found: %s", substring, entry)
}
}
93 changes: 86 additions & 7 deletions tests/integration/incremental_mirroring_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// incremental_mirroring_test.go validates the incremental mirroring workflow using the --since flag.
// It verifies that oc-mirror produces a smaller archive (no image layer blobs) when the
// --since date filters out previously mirrored content.
// incremental_mirroring_test.go validates incremental mirroring workflows.
// It verifies that oc-mirror produces archives containing only new content on
// subsequent runs, whether filtered by --since date or by config changes.
package integration_test

import (
Expand Down Expand Up @@ -54,7 +54,6 @@ func expectNoImageBlobsInArchives(tarFiles []string) {
len(blobEntries), strings.Join(blobEntries, "\n"))
}

// CLID-655: Validate incremental mirroring using archives filtered by date.
var _ = Describe("incremental mirroring", func() {
var workDir string

Expand All @@ -66,9 +65,10 @@ var _ = Describe("incremental mirroring", func() {
cleanupWorkDir(workDir)
})

// Verifies that --since flag produces an incremental archive with only new content.
// When --since is set to a future date (nothing cached after that date), the archive
// should contain no image layer blobs, only repository metadata links.
// CLID-655: Validate date-based incremental mirroring using --since.
// Runs mirrorToDisk twice with the same ISC: the first run produces a full
// archive, and the second run with --since set to a future date produces an
// archive with no image blobs (nothing is newer than the cutoff).
Describe("mirrorToDisk incremental with --since flag", func() {
iscFile := filepath.Join("happy_path", "isc-happy-path.yaml")

Expand Down Expand Up @@ -108,4 +108,83 @@ var _ = Describe("incremental mirroring", func() {
expectNoImageBlobsInArchives(incrementalArchives)
})
})

// CLID-614: Validate config-based incremental mirroring for operators.
// Runs mirrorToDisk twice with different ISCs against the same workspace:
// Step 1: mirror foo pinned at 0.2.0
// Step 2: expand foo to 0.2.0–0.3.1 and add bar (stable)
// The second archive should contain only the new blobs (expanded foo versions
// and bar) with no overlap from the first archive.
Describe("operator incremental mirrorToDisk", func() {
iscInitial := filepath.Join("operators", "isc-operator-incremental-initial.yaml")
iscUpdate := filepath.Join("operators", "isc-operator-incremental-update.yaml")

It("should produce a second tar with only incremental blob content", func() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify this spec currently lacks SpecTimeout and uses non-spec context in MirrorToDisk calls.
rg -nP --type=go -C2 'It\("should produce a second tar with only incremental blob content",\s*func\(' tests/integration/incremental_mirroring_test.go
rg -nP --type=go -C2 'runner\.MirrorToDisk\(' tests/integration/incremental_mirroring_test.go

Repository: openshift/oc-mirror

Length of output: 1647


Add timeout to long-running incremental mirroring test and pass SpecContext to mirroring calls.

Line 122 defines a long-running integration test with two MirrorToDisk operations but no SpecTimeout, creating risk of suite hangs. Additionally, the test should pass Ginkgo's SpecContext to the mirroring operations instead of the outer ctx.

Suggested fix
-		It("should produce a second tar with only incremental blob content", func() {
+		It("should produce a second tar with only incremental blob content", SpecTimeout(10*time.Minute), func(specCtx SpecContext) {
@@
-			result, err := runner.MirrorToDisk(ctx, iscInitialPath, workDir, "--remove-signatures=true")
+			result, err := runner.MirrorToDisk(specCtx, iscInitialPath, workDir, "--remove-signatures=true")
@@
-			result, err = runner.MirrorToDisk(ctx, iscUpdatePath, workDir, "--remove-signatures=true")
+			result, err = runner.MirrorToDisk(specCtx, iscUpdatePath, workDir, "--remove-signatures=true")

Also applies to lines 134–135, 155–156.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/integration/incremental_mirroring_test.go` at line 122, The test
"should produce a second tar with only incremental blob content" is a
long-running integration test that lacks a timeout and needs to pass SpecContext
to its mirroring operations. Add a SpecTimeout parameter to the It() function
call to prevent test suite hangs, and update all MirrorToDisk operation calls
within this test (and the related calls at lines 134-135 and 155-156) to accept
and use the SpecContext parameter instead of the outer ctx variable to ensure
proper test context handling within Ginkgo.

Source: Coding guidelines

findSingleMirrorTar := func(dir, phase string) string {
matches, err := filepath.Glob(filepath.Join(dir, "mirror_*.tar"))
Expect(err).NotTo(HaveOccurred())
Expect(matches).To(HaveLen(1), "%s expected exactly one tar archive, got: %v", phase, matches)
return matches[0]
}

iscInitialPath := filepath.Join(iscDir, iscInitial)
iscUpdatePath := filepath.Join(iscDir, iscUpdate)

By("running mirrorToDisk with the initial ISC (foo pinned at 0.2.0)")
result, err := runner.MirrorToDisk(ctx, iscInitialPath, workDir, "--remove-signatures=true")
logOcMirrorResult("incremental step-1 mirrorToDisk", result)
expectOcMirrorCommandSuccess(result, err)

By("verifying the initial tar archive was created")
initialTar := findSingleMirrorTar(workDir, "initial")
Expect(initialTar).To(BeAnExistingFile(), "initial tar archive not found")
logTarSummary("initial", initialTar)

By("verifying the initial tar contains expected content")
expectCorrectTarArchiveContents(iscInitialPath, workDir)

By("moving the initial tar outside the working directory to preserve it")
preserveDir, err := os.MkdirTemp("", "oc-mirror-preserved-tar-*")
Expect(err).NotTo(HaveOccurred())
defer os.RemoveAll(preserveDir)
preservedTar := filepath.Join(preserveDir, "mirror_initial.tar")
Comment thread
coderabbitai[bot] marked this conversation as resolved.
err = os.Rename(initialTar, preservedTar)
Expect(err).NotTo(HaveOccurred(), "failed to move initial tar")

By("running mirrorToDisk with the updated ISC (foo 0.2.0-0.3.1 + bar stable)")
result, err = runner.MirrorToDisk(ctx, iscUpdatePath, workDir, "--remove-signatures=true")
logOcMirrorResult("incremental step-2 mirrorToDisk", result)
expectOcMirrorCommandSuccess(result, err)

By("verifying the incremental tar archive was created")
incrementalTar := findSingleMirrorTar(workDir, "incremental")
Expect(incrementalTar).To(BeAnExistingFile(), "incremental tar archive not found")
logTarSummary("incremental", incrementalTar)

By("comparing blob paths between the two tars")
const blobPrefix = "docker/registry/v2/blobs/sha256"
initialBlobs := collectTarBlobPaths(preservedTar, blobPrefix)
incrementalBlobs := collectTarBlobPaths(incrementalTar, blobPrefix)
GinkgoWriter.Printf("initial tar blob count: %d\n", len(initialBlobs))
GinkgoWriter.Printf("incremental tar blob count: %d\n", len(incrementalBlobs))

Expect(incrementalBlobs).NotTo(BeEmpty(), "incremental tar should contain at least one blob")
initialBlobSet := make(map[string]struct{}, len(initialBlobs))
for _, b := range initialBlobs {
initialBlobSet[b] = struct{}{}
}
for _, b := range incrementalBlobs {
_, alreadyMirrored := initialBlobSet[b]
Expect(alreadyMirrored).To(BeFalse(),
"incremental tar re-included previously mirrored blob: %s", b)
}

By("verifying the initial tar does not contain the new operator package")
initialEntries := listTarEntries(preservedTar)
expectTarDoesNotContainPath(initialEntries, "/bar")

By("verifying the incremental tar contains the expected repositories")
expectCorrectTarArchiveContents(iscUpdatePath, workDir)
})
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# ImageSetConfig for incremental mirror test - Step 1: mirror a single pinned operator version
kind: ImageSetConfiguration
apiVersion: mirror.openshift.io/v2alpha1
mirror:
operators:
- catalog: quay.io/oc-mirror/oc-mirror-dev:test-catalog-latest
packages:
- name: foo
channels:
- name: beta
minVersion: '0.2.0'
maxVersion: '0.2.0'
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# ImageSetConfig for incremental mirror test - Step 2: expand version range and add a new package
kind: ImageSetConfiguration
apiVersion: mirror.openshift.io/v2alpha1
mirror:
operators:
- catalog: quay.io/oc-mirror/oc-mirror-dev:test-catalog-latest
packages:
- name: foo
channels:
- name: beta
minVersion: '0.2.0'
maxVersion: '0.3.1'
- name: bar
channels:
- name: stable