release: Allow a file:// release argument to release mirror #457
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ry-pick-182-to-release-4.3 [release-4.3] Bug 1776493: don't install quota CRDs
The RHEL7 support tools image is only built for AMD64, but there will soon be a need for an image that works on System Z (and eventually Power). Changing to the RHEL8 image gets us the additional architectures. Additionally, RHCOS has moved to RHEL8 content, so it's best to keep the support tools in sync.
…ry-pick-185-to-release-4.3 Bug 1777031: Update support tools container image
…place We've been injecting the release version into oc since 13215d7 (Bug 1715001: when extracting tools from payload, 'oc version' should report payload version, 2019-09-10, openshift#88). This commit refactors the split helpers from f55426f (Bug 1763728: fix 'oc adm release extract' version injection, 2019-10-17, openshift#131) with a single helper that takes a replacement slice. I've also pushed the unmatched warning down into the replacement helper itself so we don't have to return a slice of replacements that did or did not match. This sets the stage for injecting release version names into the installer as well, although I'm not doing that in this commit because Abhinav wants some time to look into the ecosystem around that [1]. Now that replacements are no longer baked in, I've made the unit test a bit more readable by dispensing with the fakeInput helpers and just hard-coding small input/output strings in the test-case definitions. There's also the "oc is overwriting its own ...RELEASE_VERSION_LOCATION..." constant issue, previously fixed by ded896d (Bug 1768516: fix extracted-oc adm extract oc, 2019-11-08, openshift#155). In this commit, I've avoided that by using ! as the leading character in the marker constants and replacing it with a null byte when constructing replacements in extractCommand. With a single replacement per marker, we need our marker constant to never match; I dunno what the previous doubled constant was about (but see the len(value) point next for why the previous doubled marker worked at all). I've also fixed some additional bugs: * The previous implementation only used len(value) of the marker when searching the incoming bytes. Especially for short strings like version replacement, this meant we were only looking for five bytes for 4.2.7, which is \x00_REL. That's not very specific. It does not distinguish between \x00_RELEASE_IMAGE_LOCATION_\x00XX... and \x00_RELEASE_VERSION_LOCATION_\x00XX.... It doesn't even distinguish between the defaultVersionPadded and defaultVersionPrefix constants in pkg/version. With this commit, we search for the full marker, regardless of len(value). * The previous implementation had: nextOffset := end - len(marker) ... _, wErr := w.Write(buf[:end-nextOffset]) ... copy(buf[:nextOffset], buf[end-nextOffset:end]) offset = nextOffset That was problematic for a few reasons: * It didn't write much data. Substituting for nextOffset, we were writing to: end - nextOffset = end - (end - len(marker)) = len(marker) this makes the buffer size largely meaningless, and means lots of inefficient, small reads/writes. With this commit, we have a writeTo variable that is everything except the *last* len(marker). * It ignored the amount of data written. You can *want* to write 1k bytes but only actually write 50 bytes with a given Write call. That didn't happen often before; because of the previous list entry we were only attempting to write a hundred or so bytes at a time. But now that we are trying to write the bulk of the buffer size, it happens more often. With this commit, we keep attempting Write until it errors out on us or we finish pushing all the bytes we no longer need. Because 13215d7, f55426f, and ded896d all touched the version marker, this commit will mean that oc build with this commit and later will not inject version names into oc built before ded896d. But that's ok, because 4.2 binaries have no version markers (just a RELEASE_IMAGE_LOCATION constant for the installer's marker). And we haven't cut 4.3 yet. So with this commit and no future changes to the oc marker, we'll be able to inject the version name into all supported oc which are prepared to receive injected version names. The Fprintf warning follows package precedent. Clayton [2]: > In these commands warnings are printed with fprintf. > > Klog is for debugging and servers, not end users. In action extracting from [3]: $ go build -o oc-mine ./cmd/oc $ ./oc-mine adm release extract --command=oc quay.io/openshift-release-dev/ocp-release-nightly@sha256:502d184ac8742073cb3007a0ad9abe8672b0a2db37331cfbfb4cb1b564c893fd $ ./oc version --client Client Version: 4.3.0-0.nightly-2019-11-13-233341 [1]: openshift/installer#2682 (comment) [2]: openshift#167 (comment) [3]: https://mirror.openshift.com/pub/openshift-v4/clients/ocp-dev-preview/4.3.0-0.nightly-2019-11-13-233341/release.txt
…ry-pick-167-to-release-4.3 Bug 1778882: pkg/cli/admin/release/extract_tools: Pass []replacements to copyAndReplace
When using `oc debug istag/foo:bar` it is not uncommon to want to create the debug pod in another namespace. Make that possible by adding `--to-namespace` which overrides the pod namespace.
The implication of --one-container is that we only want a single container.
We only printed the regular container names.
If a user specifies an init container, we have to preserve it even if `--keep-init-containers=false` is set. In that case, we strip all other init containers.
If there is a host port set, clear it. If this is a host network pod, clear all ports. This allows us to debug a pod on the same node for daemonsets that declare host network or host pods.
…ry-pick-169-to-release-4.3 [release-4.3] Bug 1779535: Wire in `oc rollout restart` from kubectl
…k-185-to-release-4.3 Revert "Bug 1777031: Update support tools container image"
…ry-pick-166-to-release-4.3 Bug 1779332: Fix a number of issues around debugging daemonset pods and init containers
…ry-pick-207-to-release-4.3 [release-4.3] Bug 1782817: make inspect more resilient and slightly faster
If job spec pointer dereference is nil, the result will be empty string instead of showing wrong result or runtime error. Reference: https://bugzilla.redhat.com/show_bug.cgi?id=1788016
…aller We've been injecting the release version into oc since 13215d7 (Bug 1715001: when extracting tools from payload, 'oc version' should report payload version, 2019-09-10, openshift#88), but we want to inject it into the installer to to avoid potential confusion from: $ oc adm release extract --command=openshift-install quay.io/openshift-release-dev/ocp-release:4.2.7 $ ./openshift-install version ./openshift-install v4.2.5 built from commit 425e4ff0037487e32571258640b39f56d5ee5572 release image quay.io/openshift-release-dev/ocp-release@sha256:bac62983757570b9b8f8bc84c740782984a255c16372b3e30cfc8b52c0a187b9
…ry-pick-262-to-release-4.3 [release-4.3] Bug 1792334: pods that are no longer running can still have logs
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.
…ry-pick-318-to-release-4.3 Bug 1807812: [release-4.3] override FileDir with FromFileDir if set in Complete
[release-4.3] Bug 1796675: bump(operator-framework/operator-registry)
…ry-pick-326-to-release-4.3 [release-4.3] Bug 1809360: fix(catalog): use registry image from openshift quay namespace
Bug 1802881: bump(github.com/mtrmac/gpgme): v0.1.2
Bug 1795302: initialize auth plugins
…ry-pick-245-to-release-4.3 Bug 1810959: `oc adm inspect namespace` doesn't collect pod logs
…ry-pick-341-to-release-4.3 [release-4.3] Bug 1810533: Don't default to always mirror manifestlists
Bug 1802262: Use polymorphic helpers from exec, when reading a pod
Bug 1790954: fix version ldflags
Kubernetes client-go requires only one authentication method to be present therefore this patch zeroes the content of BearerTokenFile as basic auth is used during prune. In a nutshell: or we use token based authentication or basic auth, never both at the same time. This is a cherrypick of 966f1bb
Bug 1821648: Zeroing BearerTokenFile for prune
When a file:// argument is provided as the source for oc adm release mirror, do a digest lookup within that repo for any image referenced by the payload, and if found use the local directory as the source. This allows releases to be mirrored to disk and then oc adm release mirror to be used to push it back with: oc adm release mirror file://openshift/release:4.3.1 --to REPO where before it would fail with "invalid image reference". Ensure that when file:// is used the correct instructions are printed for mirroring. Fix the mirror command provided to users to put the wildcard in quotes (to avoid bash shell expansion).
The command fails when specifying a non-default from directory because the from directory was not passed to either extract or mirror. Add that transformation. Also include the Parallel options as good practice even if they are not heavily used in these flows. Backporting 4882c8f (openshift#446) and resolving a context conflict because 4.3 does not include 3ddaf4d (cherry-pick 331c1a1, 2020-04-17, openshift#426).
|
@wking: PR needs rebase. 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. |
openshift-ci-robot
added
the
needs-rebase
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: wking 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 |
Member
Author
|
oops, wrong branch. |
Member
Author
|
Filed #458 against release-4.3. |
|
@wking: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
14 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This picks #327 and #446 back to release-4.3. #327 picks back cleanly (see #456), and I'm recycling the bot's 95de610 in this PR. #446 does not pick back cleanly, because release-4.3 lacks release-4.4's #426. I've resolved the minor context conflict manually to create dbe3f73.