Bug 1841885: Support airgapped mirroring with `oc adm catalog mirror` #611

ecordell · 2020-10-07T23:05:56Z

When mirroring to a file, oc adm catalog mirror will not rename any images. This means that image names can be inferred from the index contents when mirroring back out of a file store later.

Airgap mirroring then becomes a two step:

mirror from registry to file:///
mirror from file:// to the airgapped registry

This required a few changes:

the index image itself needed to be included in the mirror, so that it is available on the other side of the airgap
the catalog mirroring is updated to use typed image refs (i.e. file / registry discriminator)
bonus: parallelized the mirroring process

openshift-ci-robot · 2020-10-07T23:06:03Z

@ecordell: This pull request references Bugzilla bug 1841885, which is valid. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.7.0) matches configured target release for branch (4.7.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1841885: Support airgapped mirroring with oc adm catalog mirror

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

soltysh · 2020-10-09T10:56:53Z

/assign @sallyom

ecordell · 2020-10-12T18:16:27Z

/retest

ecordell · 2020-10-12T22:15:43Z

/retest

openshift-ci-robot · 2020-10-12T22:16:14Z

@ecordell: This pull request references Bugzilla bug 1841885, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.7.0) matches configured target release for branch (4.7.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1841885: Support airgapped mirroring with oc adm catalog mirror

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot · 2020-10-12T23:47:52Z

@ecordell: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/e2e-cmd	`10cc374`	link	`/test e2e-cmd`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

sallyom

This PR should add examples to the catalog mirror command help:
oc adm catalog mirror quay.io/my/image:latest file://local-catalog
and then
oc adm catalog mirror file://local-catalog some-registry:5000/local-catalog
I'm not seeing any examples of this in the documentation, either. (I'm not sure the above is the correct use but the correct way to mirror to disk then from disk to registry should be documented)

I'm wondering why oc adm catalog mirror varies from oc adm release mirror in ways that should be similar, for example, why is there no --to-dir or --to flag? To mirror to directory w/ oc adm release mirror you can either:
oc adm mirror 4.5.0 --to-dir=release and the output shows the user this:

To upload local images to a registry, run:
    oc image mirror --from-dir=release 'file://openshift/release:4.5.0*' REGISTRY/REPOSITORY

or oc adm mirror 4.5.0 --to file://release and output is shown:

To upload local images to a registry, run:
    oc image mirror 'file://release:4.5.0*' REGISTRY/REPOSITORY

Without the added examples or documentation on how to mirror to local disk and from local disk, it would be very difficult for users to use the oc adm mirror catalog command in this way.

sallyom · 2020-10-12T19:23:45Z

pkg/cli/admin/catalog/mirror.go

@@ -157,6 +157,16 @@ func (o *MirrorCatalogOptions) Complete(cmd *cobra.Command, args []string) error
 	}
 	o.DestRef = destRef

+	// do not modify image names which storing in file://


sallyom · 2020-10-12T19:37:50Z

pkg/cli/admin/catalog/mirror.go

 	for k, v := range mapping {
-		if _, err := w.WriteString(fmt.Sprintf("%s=%s\n", k, v.WithTag)); err != nil {
+		to := v
+		// render with a tag when mirroring so that the target registry doesn't GC the image


GC to garbage collect? maybe most don't know what GC is

sallyom · 2020-10-13T02:17:31Z

pkg/cli/admin/catalog/mirror.go

+	}
+	if err := ioutil.WriteFile(filepath.Join(dir, "catalogSource.yaml"), catalogSource, os.ModePerm); err != nil {
+		return fmt.Errorf("error writing CatalogSource")
+	}


Should these files be uniquely named - icsp.yaml, catalogSource.yaml, mapping.txt? (I see "imageContentSourcePolicy.yaml" is not part of this PR) See how the signature file is named here: https://github.com/openshift/oc/blob/master/pkg/cli/admin/release/mirror.go#L302-#L313 and a PR to add the writing of icsp file w/ oc adm release mirror will also use this pattern to name the icsp file. Won't these files be overwritten when mirror cmd is run more than once, and is this a potential problem?

the manifests all go into a directory, so I copied what must-gather does and added a timestamp to the containing folder

sg, and like you said below, we can align oc adm catalog mirror with oc adm release mirror in a follow-up PR if necessary.

ecordell · 2020-10-14T18:02:33Z

Thanks for the review @sallyom! I think I have addressed all of your feedback, with the exception of:

I'm wondering why oc adm catalog mirror varies from oc adm release mirror in ways that should be similar, for example, why is there no --to-dir or --to flag?

--to-dir is available here as --dir - I didn't add those flags in either place so I can't answer why they're different (see commit), but it's good to point out that they are.

For now I have added an output like you suggested indicating what the next step should be to mirror back to a registry from files. It's probably worth looking at differences between oc adm release mirror and oc adm catalog mirror and attempting to align them more in the future, but I'd like to keep this PR scoped to addressing the BZ.

sallyom · 2020-10-15T03:46:05Z

/lgtm

ecordell · 2020-10-15T16:27:25Z

/retest

ecordell · 2020-10-15T18:43:33Z

/retest

ecordell · 2020-10-16T12:37:05Z

/retest

ecordell · 2020-10-19T17:27:36Z

/retest

ecordell · 2020-10-19T20:26:43Z

/retest

ecordell · 2020-10-19T22:23:47Z

/retest

ecordell · 2020-10-20T18:29:24Z

/approve

ecordell · 2020-10-20T18:29:32Z

/retest

sallyom · 2020-10-21T00:46:30Z

/test e2e-cmd

openshift-bot · 2020-10-24T09:40:37Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-10-24T09:53:36Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-10-24T10:06:34Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-10-24T10:19:36Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-10-24T10:32:36Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-10-24T10:45:37Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-10-24T10:58:36Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-10-24T11:11:37Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-10-24T11:24:36Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-10-24T11:37:35Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-10-24T11:50:35Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-10-24T12:03:39Z

/retest

Please review the full test history for this PR and help us cut down flakes.

ecordell · 2020-10-24T12:07:10Z

/hold

Tests which seem unrelated are flaking consistently

soltysh · 2020-10-26T10:20:17Z

The failures are not related to this PR, from what I've manually checked:
/override ci/prow/e2e-cmd
/hold cancel

openshift-ci-robot · 2020-10-26T10:20:45Z

@soltysh: Overrode contexts on behalf of soltysh: ci/prow/e2e-cmd

In response to this:

The failures are not related to this PR, from what I've manually checked:
/override ci/prow/e2e-cmd
/hold cancel

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-bot · 2020-10-26T10:54:33Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-10-26T12:11:57Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-10-26T12:24:52Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-10-26T13:16:47Z

/retest

Please review the full test history for this PR and help us cut down flakes.

ecordell · 2020-10-26T13:38:36Z

/retest

ecordell · 2020-10-26T15:59:47Z

/retest

ecordell · 2020-10-26T16:31:23Z

/retest

openshift-ci-robot · 2020-10-26T17:37:49Z

@ecordell: All pull requests linked via external trackers have merged:

openshift/oc#611

Bugzilla bug 1841885 has been moved to the MODIFIED state.

In response to this:

Bug 1841885: Support airgapped mirroring with oc adm catalog mirror

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

ecordell · 2021-01-15T14:21:56Z

/cherry-pick release-4.6

openshift-cherrypick-robot · 2021-01-15T14:22:10Z

@ecordell: #611 failed to apply on top of branch "release-4.6":

Applying: oc adm catalog mirror: mirror index image and generate catalogsource
Applying: bump library-go
Using index info to reconstruct a base tree...
M	go.mod
M	go.sum
M	vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.go
M	vendor/modules.txt
Falling back to patching base and 3-way merge...
Auto-merging vendor/modules.txt
CONFLICT (content): Merge conflict in vendor/modules.txt
Auto-merging vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.go
Auto-merging go.sum
CONFLICT (content): Merge conflict in go.sum
Auto-merging go.mod
CONFLICT (content): Merge conflict in go.mod
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0002 bump library-go
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release-4.6

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot added bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Oct 7, 2020

openshift-ci-robot requested review from deads2k and smarterclayton October 7, 2020 23:06

ecordell mentioned this pull request Oct 7, 2020

Bug 1841885: Add remapping support for oc adm catalog mirror #455

Closed

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 8, 2020

ecordell mentioned this pull request Oct 8, 2020

4.6 OLM disconnected mirroring openshift/openshift-docs#26067

Merged

openshift-ci-robot assigned sallyom Oct 9, 2020

ecordell force-pushed the airgap-catalog branch 2 times, most recently from 56646a8 to 10cc374 Compare October 12, 2020 15:29

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 12, 2020

sallyom suggested changes Oct 13, 2020

View reviewed changes

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 15, 2020

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 24, 2020

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 26, 2020

openshift-merge-robot merged commit a856ab0 into openshift:master Oct 26, 2020

ecordell mentioned this pull request Jan 18, 2021

Bug 1917548: 4.6 backports for oc adm catalog mirror #711

Merged

adellape mentioned this pull request Mar 12, 2021

Add OLM mirroring using offline media openshift/openshift-docs#30230

Merged

Bug 1841885: Support airgapped mirroring with oc adm catalog mirror #611

Bug 1841885: Support airgapped mirroring with oc adm catalog mirror #611

Conversation

ecordell commented Oct 7, 2020 • edited

openshift-ci-robot commented Oct 7, 2020

soltysh commented Oct 9, 2020

ecordell commented Oct 12, 2020

ecordell commented Oct 12, 2020

openshift-ci-robot commented Oct 12, 2020

openshift-ci-robot commented Oct 12, 2020

sallyom left a comment

Choose a reason for hiding this comment

sallyom Oct 12, 2020

Choose a reason for hiding this comment

sallyom Oct 12, 2020

Choose a reason for hiding this comment

sallyom Oct 13, 2020

Choose a reason for hiding this comment

ecordell Oct 14, 2020

Choose a reason for hiding this comment

sallyom Oct 15, 2020

Choose a reason for hiding this comment

ecordell commented Oct 14, 2020 • edited

sallyom commented Oct 15, 2020

ecordell commented Oct 15, 2020

ecordell commented Oct 15, 2020

ecordell commented Oct 16, 2020

ecordell commented Oct 19, 2020

ecordell commented Oct 19, 2020

ecordell commented Oct 19, 2020

ecordell commented Oct 20, 2020

ecordell commented Oct 20, 2020

sallyom commented Oct 21, 2020

openshift-bot commented Oct 24, 2020

openshift-bot commented Oct 24, 2020

openshift-bot commented Oct 24, 2020

openshift-bot commented Oct 24, 2020

openshift-bot commented Oct 24, 2020

openshift-bot commented Oct 24, 2020

openshift-bot commented Oct 24, 2020

openshift-bot commented Oct 24, 2020

openshift-bot commented Oct 24, 2020

openshift-bot commented Oct 24, 2020

openshift-bot commented Oct 24, 2020

openshift-bot commented Oct 24, 2020

ecordell commented Oct 24, 2020

soltysh commented Oct 26, 2020

openshift-ci-robot commented Oct 26, 2020

openshift-bot commented Oct 26, 2020

openshift-bot commented Oct 26, 2020

openshift-bot commented Oct 26, 2020

openshift-bot commented Oct 26, 2020

ecordell commented Oct 26, 2020

ecordell commented Oct 26, 2020

ecordell commented Oct 26, 2020

openshift-ci-robot commented Oct 26, 2020

ecordell commented Jan 15, 2021

openshift-cherrypick-robot commented Jan 15, 2021

Bug 1841885: Support airgapped mirroring with `oc adm catalog mirror` #611

Bug 1841885: Support airgapped mirroring with `oc adm catalog mirror` #611

ecordell commented Oct 7, 2020 •

edited

ecordell commented Oct 14, 2020 •

edited