Bug 1810256: Don't default to always mirror manifestlists #339
Conversation
openshift-ci-robot
added
the
size/S
openshift-ci-robot
requested review from
ecordell,
eparis,
shawn-hurley,
smarterclayton and
soltysh
kevinrizza
changed the title
|
@kevinrizza: This pull request references Bugzilla bug 1810256, 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. |
openshift-ci-robot
added
the
bugzilla/valid-bug
|
/lgtm |
|
/assign @mfojtik |
|
/lgtm |
openshift-ci-robot
added
do-not-merge/hold
|
/retest |
1 similar comment
|
/retest |
pkg/cli/image/mirror/mirror.go
Outdated
| flag.BoolVar(&o.SkipMount, "skip-mount", o.SkipMount, "Always push layers instead of cross-mounting them") | ||
| flag.BoolVar(&o.SkipMultipleScopes, "skip-multiple-scopes", o.SkipMultipleScopes, "Some registries do not support multiple scopes passed to the registry login.") | ||
| flag.BoolVar(&o.Force, "force", o.Force, "Attempt to write all layers and manifests even if they exist in the remote repository.") | ||
| flag.BoolVar(&o.ForceManifestList, "force-manifest-list", o.ForceManifestList, "Force mirroring the manifest list if it exists.") |
There was a problem hiding this comment.
Would probably prefer the flag be keep-manifest-list and the description be If an image is part of a manifest list, always mirror the list even if only one image is found. The default is to mirror the specific image unless unless --filter-by-os is '.*'..
There was a problem hiding this comment.
The reason why I don't want to use force is because in our CLI lexicon force means "override the safe behavior with an unsafe behavior that may still be what you want", where as this is more "avoid an optimization".
Actually, we should probably use --keep-manifest-list instead of --include-manifest-list (keep and preserve are our "softer" verbs)
There was a problem hiding this comment.
@smarterclayton Makes sense. I just updated the flag to --keep-manifest-list and updated the description. ptal
A previous commit introduced new behavior to always mirror manifest list digests when running `oc image mirror`. This commit reverts that behavior, and instead adds a flag `--force-manifest-list` to allow that option. Additionally, the force behavior occurs when `--filter-by-os` is set to a wildcard.
openshift-ci-robot
added
size/M
|
/hold cancel thanks |
openshift-ci-robot
removed
the
do-not-merge/hold
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Bowenislandsong, kevinrizza, mfojtik, smarterclayton 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 |
|
@kevinrizza: All pull requests linked via external trackers have merged. Bugzilla bug 1810256 has been moved to the MODIFIED state. 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. |
|
/cherry-pick release-4.4 |
|
@kevinrizza: new pull request created: #341 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. |
8 participants
A previous commit introduced new behavior to always mirror manifest list
digests when running
oc image mirror. This commit reverts thatbehavior, and instead adds a flag
--force-manifest-listto allow thatoption. Additionally, the force behavior occurs when
--filter-by-osisset to a wildcard.