Skip to content

Conversation

@sallyom
Copy link
Contributor

@sallyom sallyom commented Dec 2, 2020

(copied from #439)
/cc @soltysh

This PR implements openshift/enhancements#334:

  • writes an ICSP file as part of oc adm release mirror to either:
    • ./config
    • --release-image-icsp-to-dir config subdirectory or --to-dir config subdirectory
  • adds logic to read cluster ICSP for image information - gathers a slice of imagesources and tries each until it succeeds (image is accessible). oc adm release will try in this order:
    • if --icsp-file is set, or --lookup-cluster-icsp then decode/use that - don't look further
    • image reference from user-passed release image (this is the original image from original registry/repo)
    • if ICSP found in cluster, add that to list of image source options, those will be:
      • icsp from cluster
      • user-passed release image (set registry/repo/name from user-given)
  • add ability to access using ICSP with oc image extract|mirror|append|info
    * if --icsp-file is set, or --lookup-cluster-icsp then decode/use that - don't look further
    * try image passed as/is
    * oc image command will not implicitly try alternative sources

set up a secure local registry with this script.
and see example test commands below.

@openshift-ci-robot openshift-ci-robot added bugzilla/severity-medium Referenced Bugzilla bug's severity is medium 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 Dec 2, 2020
@openshift-ci-robot
Copy link

@sallyom: This pull request references Bugzilla bug 1823143, 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)
Details

In response to this:

Bug 1823143: Add ImageContentSourcePolicy awareness to oc image, oc adm release

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
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: sallyom
To complete the pull request process, please assign tnozicka after the PR has been reviewed.
You can assign the PR to them by writing /assign @tnozicka in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sallyom
Copy link
Contributor Author

sallyom commented Dec 2, 2020

oc adm release test commands:
NOTE: oc adm release implicitly looks for icsp in cluster if other paths fail, you do not have to pass the --lookup-cluster-icsp flag.

$ oc adm release mirror registry.svc.ci.openshift.org/ocp/release:4.7.0-0.ci-2020-12-02-015504 --to localhost:5000/test/release
then
$ oc adm release -a ~/pull-secret-local-only extract --command openshift-install localhost:5000/test/release:4.7.0-0.ci-2020-12-02-015504 
or
$ oc adm release -a ~/pull-secret-local-only extract --command openshift-install  registry.svc.ci.openshift.org/ocp/release:4.7.0-0.ci-2020-12-02-015504 --icsp-file config/icsp..yaml
or
$ oc apply -f config/icsp...yaml
then
$ oc adm release -a ~/pull-secret-local-only extract --command openshift-install  registry.svc.ci.openshift.org/ocp/release:4.7.0-0.ci-2020-12-02-015504

oc image test commands:
NOTE: oc image will not implicitly look for icsp in cluster, you have to pass the --lookup-cluster-icsp flag.

$ oc adm release info registry.svc.ci.openshift.org/ocp/release:4.7.0-0.ci-2020-12-02-015504 --image-for cli-artifacts
registry.svc.ci.openshift.org/ocp/4.7-2020-12-02-015504@sha256:b2a9b9cbf7d36cea817739677c4123ebb3e3e0263ae7d48723097be0b6547096
$ oc image mirror registry.svc.ci.openshift.org/ocp/4.7-2020-12-02-015504@sha256:b2a9b9cbf7d36cea817739677c4123ebb3e3e0263ae7d48723097be0b6547096=localhost:5000/cli-artifacts:4.7.0-0.ci-2020-12-02-015504
$ oc image -a ~/ps-local info registry.svc.ci.openshift.org/ocp/4.7-2020-12-02-015504@sha256:b2a9b9cbf7d36cea817739677c4123ebb3e3e0263ae7d48723097be0b6547096
error: unable to read image registry.svc.ci.openshift.org/ocp/4.7-2020-12-02-015504@sha256:b2a9b9cbf7d36cea817739677c4123ebb3e3e0263ae7d48723097be0b6547096: Get "https://registry.svc.ci.openshift.org/v2/ocp/4.7-2020-12-02-015504/manifests/sha256:b2a9b9cbf7d36cea817739677c4123ebb3e3e0263ae7d48723097be0b6547096": unauthorized: authentication required

failed because not authorized - now run the same but pass the icsp file:
$ oc image -a ~/ps-local info registry.svc.ci.openshift.org/ocp/4.7-2020-12-02-015504@sha256:b2a9b9cbf7d36cea817739677c4123ebb3e3e0263ae7d48723097be0b6547096 --icsp-file config/icsp-sha256-2c0f33960669ce07.yaml
or, after 'oc apply -f config/icsp...yaml':
$ oc image -a ~/ps-local info registry.svc.ci.openshift.org/ocp/4.7-2020-12-02-015504@sha256:b2a9b9cbf7d36cea817739677c4123ebb3e3e0263ae7d48723097be0b6547096 --lookup-cluster-icsp=true

check image reference, icsp, then image instead of only using image references for 'oc adm release ...' commands

0. If user passes --icsp-file path, fail if no valid sources found from the file
1. Try the current flow of lookup image from any underlying image references.  If this fails, go to 2.
2. Try to gather image source info from ImageContentSourcePolicy, if this fails go to 3.
3. Set the registry/repo/name to be that of user-given release rather than its refs.  If image not found,
return the original error from 1.

When working with mirrored release payloads, a release from a mirrored registry,
mylocalregistry/ocp/release:4.5.0-0.nightly-2020-04-18-093630 mirrored from
registry.svc.ci.openshift.org/ocp/release:4.5.0-0.nightly-2020-04-18-093630 -
Both reference 'quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:2eb0a51...'.
In case of disconnected, oc will use 'mylocalregistry/ocp/release'
instead of 'quay.io/openshift-release-dev/ocp-v4.0-art-dev' _or_ will
get image source information from ICSP in cluster.

Also, `oc adm release mirror` will write ICSP file to local disk.
@sallyom sallyom force-pushed the new_icsp_bz1823143 branch from 9e562be to 1e7d78d Compare December 2, 2020 14:15
@openshift-ci-robot
Copy link

@sallyom: This pull request references Bugzilla bug 1823143, 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)
Details

In response to this:

Bug 1823143: Add ImageContentSourcePolicy awareness to oc image, oc adm release

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-merge-robot
Copy link
Contributor

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

Test name Commit Details Rerun command
ci/prow/e2e-aws-serial 1e7d78d link /test e2e-aws-serial
ci/prow/e2e-agnostic-cmd 1e7d78d link /test e2e-agnostic-cmd

Full PR test history. Your PR dashboard.

Details

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.

@smarterclayton
Copy link
Contributor

/hold

I am really not in favor of this approach, because it is not solving the root problem that generic registry access.

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 2, 2020
@sallyom
Copy link
Contributor Author

sallyom commented Dec 3, 2020

closing, in favor of original #439, with icsp lookup methods in library-go

@sallyom sallyom closed this Dec 3, 2020
@openshift-ci-robot
Copy link

@sallyom: This pull request references Bugzilla bug 1823143. The bug has been updated to no longer refer to the pull request using the external bug tracker.

Details

In response to this:

Bug 1823143: Add ImageContentSourcePolicy awareness to oc image, oc adm release

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugzilla/severity-medium Referenced Bugzilla bug's severity is medium 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. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants