-
Notifications
You must be signed in to change notification settings - Fork 427
Bug 1841885: Add remapping support for oc adm catalog mirror
#455
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@ecordell: This pull request references Bugzilla bug 1841885, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. 3 validation(s) were run on this bug
DetailsIn response to this:
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: This pull request references Bugzilla bug 1841885, which is valid. 3 validation(s) were run on this bug
DetailsIn response to this:
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. |
4617e01 to
1765714
Compare
|
/retest |
2 similar comments
|
/retest |
|
/retest |
soltysh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/hold
There's an ongoing discussion about making oc aware of ICSP, I'd prefer this effort is also aligned with what's being discussed there openshift/enhancements#334 (comment)
@sallyom fyi
pkg/cli/admin/catalog/mirror.go
Outdated
|
|
||
| # Perform an airgapped mirror in two steps: | ||
| # mirror the contents of a catalog to local files and save mapping file in ./to-local | ||
| %[1]s file://my-catalog:latest file://operators --to-manifests=./to-local |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the src argument here really have the file: prefix? I thought this example was meant to show pulling a remote image my-catalog:latest to file://operators -- am I mistaken?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nope you're right - I'll update these docs
pkg/cli/admin/catalog/mirror.go
Outdated
| %[1]s file://my-catalog:latest file://operators --to-manifests=./to-local | ||
|
|
||
| # mirror the contents of a catalog from local files into a target registry using previously calculated mapping file | ||
| %[1]s file://my-catalog:latest my-airgapped-registry:5000 --mapping=./rh-ops-manifests/mapping.txt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is the next step, I would expect the src argument to be file://operators. Am I misunderstanding this command?
Also:
- where is the
rh-ops-manifestsdirectory coming from? - is
--mappingsupposed to be--remap?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, the example is confusing.
where is the rh-ops-manifests directory coming from?
that's supposed to be the output of the previous command - I can make that clearer
is --mapping supposed to be --remap?
yep!
| DryRun bool | ||
| ManifestOnly bool | ||
| DatabasePath string | ||
| RemappingFiles []string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a need to support more than one mapping? If so, we should probably state the merge behavior in the command help.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just did this for parity with with oc image mirror, which takes a list of files: https://github.com/openshift/oc/blob/master/pkg/cli/image/mirror/mirror.go#L174
the second step in performing a "true" airgapped install required manual manipulation of the output mapping.txt and ICSP to properly mirror. The source of `oc adm catalog mirror` is always a catalog image that has a certain set of image references contained within. When mirroring these, an ImageContentSourcePolicy and mapping.txt file is generated. The ICSP always maps the image reference in the catalog to the final image defined by remapping into the destination registry, so that on cluster the correct translation can take place. The mapping.txt file (and the mirroring step performed by oc adm catalog mirror) always need to map from an existing image to the target image. When mirroring a catalog to an airgapped cluster, the images need to be mirrored twice: once to the local filesystem, and once from the local filesystem. This commit adds support for a `--remap` flag which can be passed to the mirror command. This "remaps" the source image found in the catalog manifests to another location prior to mirroring, so that multi-stage mirroring is easy. For example: mirror file://rh-ops:1 file://operators --to-manifests=local mirror file://rh-ops:1 localhost:50000 --remap=./local/mapping.txt This will mirror images from the external registry to the local fs, and then again from the local fs to the final registry. The ICSP generated is unmodified, since it will always need to map from the images defined in the manifests themselves.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: ecordell The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Generally mirror commands are expected to understand the context of the mapping. I.e. |
|
@ecordell: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
| `) | ||
|
|
||
| Mirroring into an airgapped environment requires two steps: mirroring from the catalog to local files, | ||
| and then again from local files into the airgapped registry. See examples ofr the "--remap" flag for more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ofr/of
also, I don't see any examples of using --remap flag, you will add those?
|
closing in favor of #611 |
/close |
|
@soltysh: Closed this PR. DetailsIn response to this:
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: This pull request references Bugzilla bug 1841885. The bug has been updated to no longer refer to the pull request using the external bug tracker. DetailsIn response to this:
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. |
The second step in performing a "true" airgapped install required
manual manipulation of the output
mapping.txtto properlymirror.
The source of
oc adm catalog mirroris always a catalog image thathas a certain set of image references contained within. When mirroring
these, an ImageContentSourcePolicy and mapping.txt file is generated.
The ICSP always maps the image reference in the catalog to the final
image defined by remapping into the destination registry, so that on
cluster the correct translation can take place.
The mapping.txt file (and the mirroring step performed by oc adm catalog
mirror) always need to map from an existing image to the target image.
When mirroring a catalog to an airgapped cluster, the images need to be
mirrored twice: once to the local filesystem, and once from the local
filesystem.
This commit adds support for a
--remapflag which can be passed tothe mirror command. This "remaps" the source image found in the catalog
manifests to another location prior to mirroring, so that multi-stage
mirroring is easy.
For example:
This will mirror images from the external registry to the local fs, and
then again from the local fs to the final registry.
The ICSP generated is unmodified, since it will always need to map from
the images defined in the manifests themselves.