Bug 1825565: pkg/cli/admin/release/mirror: Allow --apply-release-image-signature and --release-image-signature-to-dir #392
Conversation
|
@wking: This pull request references Bugzilla bug 1825565, which is invalid:
Comment In 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. |
openshift-ci-robot
added
the
bugzilla/invalid-bug
wking
force-pushed
the
release-image-signature-no-op-default
branch
from
32c23a4 to
1298072
Compare
|
/bugzilla refresh |
openshift-ci-robot
added
bugzilla/valid-bug
|
@wking: This pull request references Bugzilla bug 1825565, 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
In 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. |
wking
force-pushed
the
release-image-signature-no-op-default
branch
from
1298072 to
2bf554b
Compare
|
/assign @soltysh |
wking
force-pushed
the
release-image-signature-no-op-default
branch
3 times, most recently
from
588acff to
6bde5aa
Compare
…nd --release-image-signature-to-dir Applying directly to a cluster and writing to a local file are orthogonal actions, and we can do both or neither. This commit removes previous restrictions from 331c1a1 (Implement enhancements/oc/mirroring-release-signatures, 2020-04-17, openshift#343) to: * Allow users to set both flags for a single 'oc' invocation. To support this, some dry-run 'continue' were removed, because in the case where all of: --dry-run --apply-release-image-signature --release-image-signature-to-dir=whatever are set, we want to log both the fact that we'd be applying and writing-to-disk each signature, and not log the application but skip over the writing-to-disk log. * Allow for mirrors that set none of --apply-release-image-signature, --release-image-signature-to-dir, or --to-dir. This provides backwards compatibility with earlier 'oc', which did not interact with signatures at all [1]. Also fix a few nits by: * Using filepath.Join to create the ReleaseImageSignatureToDir fallback, so we are not sensitive to whether a given --to-dir did or did not end in whatever the local path separator happens to be. * Replacing tabs with spaces where they appeared within LongDesc lines. * Converting handleSignatures to return an error, so we don't end up exiting zero if signature handling is requested by the user but fails to happen. This also simplifies logging, because we only need to attach a little bit of context as we bubble the errors up, and final formatting for user display can happen at some higher-level, centralized location. * Only bothering with Signatures() and warning on their empty-ness if the user wants us do to something with the signatures. * Checking to ensure that the release digest appears in the signature cache. [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1825565
wking
force-pushed
the
release-image-signature-no-op-default
branch
from
6bde5aa to
2706f0f
Compare
|
@wking: This pull request references Bugzilla bug 1825565, which is valid. 3 validation(s) were run on this bug
In 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. |
1 similar comment
|
@wking: This pull request references Bugzilla bug 1825565, which is valid. 3 validation(s) were run on this bug
In 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. |
|
Thanks for quick fix. Could we run this through e2e-metal-ipi? It does a disconnected install. This PR optionally enables it for this repo: openshift/release#8415 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: soltysh, wking The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
2 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest |
|
@wking: All pull requests linked via external trackers have merged: openshift/oc#392. Bugzilla bug 1825565 has been moved to the MODIFIED state. In 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. |
Applying directly to a cluster and writing to a local file are orthogonal actions, and we can do both or neither. This commit removes previous restrictions from 331c1a1 (#343) to:
Allow users to set both flags for a single
ocinvocation. To support this, some dry-runcontinuewere removed, because in the case where all of:are set, we want to log both the fact that we'd be applying and writing-to-disk each signature, and not log the application but skip over the writing-to-disk log.
Allow for mirrors that set none of
--apply-release-image-signature,--release-image-signature-to-dir, or--to-dir. This provides backwards compatibility with earlieroc, which did not interact with signatures at all (rhbz#1825565).Also fix a few nits by:
filepath.Jointo create theReleaseImageSignatureToDirfallback, so we are not sensitive to whether a given--to-dirdid or did not end in whatever the local path separator happens to be.LongDesclines.handleSignaturesto return an error, so we don't end up exiting zero if signature handling is requested by the user but fails to happen. This also simplifies logging, because we only need to attach a little bit of context as we bubble the errors up, and final formatting for user display can happen at some higher-level, centralized location.Signatures()and warning on their empty-ness if the user wants us do to something with the signatures.