AGENT-1193: Add --mirror-path flag to support pre-mirrored images#634
AGENT-1193: Add --mirror-path flag to support pre-mirrored images#634rwsu wants to merge 4 commits intoopenshift:masterfrom
Conversation
be9a81e to
c10ba39
Compare
|
@rwsu: This pull request references AGENT-1193 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.22.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
@rwsu: This pull request references AGENT-1193 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.22.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
@rwsu: This pull request references AGENT-1193 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.22.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
@rwsu: This pull request references AGENT-1193 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.22.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/cc @danielerez |
| } | ||
| releaseDigest = strings.Trim(releaseDigest, "'") | ||
| releaseImage = fmt.Sprintf("%s@%s", strings.Split(releaseImage, ":")[0], releaseDigest) | ||
| releaseImage = fmt.Sprintf("%s@%s", releaseImage, releaseDigest) |
There was a problem hiding this comment.
Yes, it was to handle this error:
FATAL failed to fetch Appliance live ISO: failed to fetch dependency of "Appliance live ISO": failed to generate asset "Base ISO (CoreOS)": Failed to execute cmd (/usr/local/bin/oc adm release info --image-for=machine-os-images --insecure=true virthost.ostest.test.metalkube.org@sha256:dc120ba1d210d398da3bcd712c9a7bccf0086c1b7fde42fca8b186d7d16f3fa8): error: unable to connect to image repository : invalid reference format
The problem is the custom release image has a url of virthost.ostest.test.metalkube.org:5000/localimages/local-release-image:latest which contains two colons. The original code takes the first part losing the port number.
There was a problem hiding this comment.
Hmm, so maybe do this change only when using a custom release image?
I mean to avoid breaking the flow for other images without port...
There was a problem hiding this comment.
I've create a unit test to verify it doesn't break images without port.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rwsu The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
9e775e7 to
17dc5e3
Compare
| } | ||
| releaseDigest = strings.Trim(releaseDigest, "'") | ||
| releaseImage = fmt.Sprintf("%s@%s", strings.Split(releaseImage, ":")[0], releaseDigest) | ||
| releaseImage = fmt.Sprintf("%s@%s", releaseImage, releaseDigest) |
There was a problem hiding this comment.
Hmm, so maybe do this change only when using a custom release image?
I mean to avoid breaking the flow for other images without port...
| logrus.Infof("Using pre-mirrored images from: %s", mirrorPath) | ||
| tempDir = mirrorPath | ||
| } else { | ||
| // Normal mirroring flow - run oc-mirror |
There was a problem hiding this comment.
can you swap the condition? just for keeping convention.
i.e. check mirrorPath == "" (normal flow first)
| imageGen := genisoimage.NewGenIsoImage(nil) | ||
| if err = imageGen.GenerateImage(envConfig.CacheDir, dataIsoName, filepath.Join(envConfig.TempDir, dataDir), dataVolumeName); err != nil { | ||
|
|
||
| // When mirror-path is provided, copy the Docker registry data from mirror-path/data |
There was a problem hiding this comment.
this Generate func is getting a bit too long:)
can you extract the new logic to another func?
There was a problem hiding this comment.
Yes, the new logic is now in the copyMirrorRegistryData func.
Add support for using pre-mirrored images from oc-mirror workspace by
specifying the mirror path in appliance-config.yaml. When mirrorPath is
configured, the appliance skips running oc-mirror and uses the pre-mirrored
registry data directly.
Also fixes issues when using custom registries (non-quay.io) where oc adm
release info may return incomplete image references missing port and
repository path. Adds IDMS entries to ensure the cluster redirects pulls
from custom registries to the appliance's internal registry.
Changes:
- Add MirrorPath field to top level of ApplianceConfig in types
- Update appliance-config.yaml template to document the new field
- Update code to read mirrorPath from ApplianceConfig.Config.MirrorPath
- Add validateMirrorPath() to ApplianceConfig with comprehensive validation
- Use pre-mirrored images when mirrorPath is provided
- Add fixImageReference() to repair incomplete image refs from oc commands
Example: registry.example.com@sha256:... becomes
registry.example.com:5000/repo/image@sha256:...
- Add addLocalRegistryIDMS() to generate IDMS for custom registry mirrors
- Fix release image tag parsing to preserve port in registry URLs
The mirror-path can be specified in appliance-config.yaml as:
mirrorPath: /path/to/oc-mirror/workspace
Assisted-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Add debug logging for IsLiveISO tracking
Pass --registry-config ~/.docker/config.json to all oc commands (release info, release extract, oc mirror) and --authfile to skopeo. This allows the appliance builder to authenticate against private registries such as the one used in dev-scripts (virthost.ostest.test.metalkube.org:5000). The pull secret from appliance-config is written to ~/.docker/config.json (existing behavior) so no separate credential management is needed. Assisted-by: Claude Sonnet 4.6 <noreply@anthropic.com>
strings.Split(image, ":")[0] incorrectly strips the registry port when the release image URL contains one. For example, with the dev-scripts local registry virthost.ostest.test.metalkube.org:5000/openshift/release-images:tag, Split on ":" takes only "virthost.ostest.test.metalkube.org", losing the port and path entirely. Use LastIndex to locate the tag colon after the last "/" so the port is preserved, producing the correct digest reference: virthost.ostest.test.metalkube.org:5000/openshift/release-images@sha256:... Extract the logic into appendDigest() with a unit test. Assisted-by: Claude Sonnet 4.6 <noreply@anthropic.com>
- Swap mirrorPath condition in mirrorImages() so normal flow comes first - Extract mirror-path registry data copy logic into copyMirrorRegistryData() Assisted-by: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@rwsu: all tests passed! 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-sigs/prow repository. I understand the commands that are listed here. |
Adds support for using pre-mirrored OCP release images instead of running
oc-mirror during the build process. This is useful when images have already
been mirrored in a separate step, avoiding redundant mirroring operations.
Changes:
When using custom registries instead of official OpenShift release images,
oc commands need authentication credentials to pull images. This adds the
--registry-config flag to all oc commands and --authfile to skopeo.
store instance and reusing it throughout the command execution, we ensure
EnvConfig is only generated once with the correct values and not overwritten.
Assisted-by: Claude Sonnet 4.5 noreply@anthropic.com