Bug 1795302: initialize auth plugins #267
Closed
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
|
@soltysh: This pull request references Bugzilla bug 1795302, which is invalid:
Comment 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/invalid-bug
|
@soltysh: 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
needs-rebase
Contributor
Author
|
Wrong branch. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: soltysh 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 |
openshift-ci-robot
added
the
approved
|
@soltysh: 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
8 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.
Manual cherry-pick of #266 b/c this is touching deps.
/assign @deads2k