CNTRLPLANE-1314: Update openshift/oauth-server to latest Kubernetes 1.33 version #197
Conversation
|
/hold for openshift/kubernetes-apiserver#67 |
openshift-ci
bot
added
the
do-not-merge/hold
sanchezl
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@sanchezl: This pull request references CNTRLPLANE-1314 which is a valid jira issue. 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 openshift-eng/jira-lifecycle-plugin repository. |
pkg/server/grant/grant.go
Outdated
| if err != nil || client == nil { | ||
| if err != nil || client == nil || reflect.ValueOf(client).Elem().IsZero() { |
There was a problem hiding this comment.
- The new generic client-go changes the behavior of
Getto return a zero value instead of a nil. - The
OAuthClientstruct is non-comparable, so I had to resort to using thereflectpackage here. - This change is needed for tests that use the badClientRegistry, which IMO is an especially bad client registry.
- It returns
true, nil, nilwhich I do not think it is a valid result. - As I see it, it should always return a valid client when err == nil, and we should be expecting a NotFound error in the case where it is currently returning
nil, nil.
- It returns
In the end, I opted to perform the minimum needed to pass the test, more testing would be needed to fully refactor.
There was a problem hiding this comment.
Further looking into it, this new behavior from the generic fake Get actually matches the actual Get behavior..
There was a problem hiding this comment.
I agree this isn't something that is supposed to happen. Can we change the two test cases to call badClientRegistry with non-nil errors instead? I'd prefer to see the change in the fake client behavior addressed with test-only changes.
There was a problem hiding this comment.
I've changed the test case to match the expected behavior.
|
/hold cancel |
openshift-ci
bot
removed
the
do-not-merge/hold
|
@sanchezl: This pull request references CNTRLPLANE-1314 which is a valid jira issue. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
@sanchezl: This pull request references CNTRLPLANE-1314 which is a valid jira issue. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/lgtm |
|
/payload-with-prs 4.20 nightly informing #197 openshift/oauth-apiserver#139 |
|
@sanchezl: trigger 67 job(s) of type informing for the nightly release of OCP 4.20
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/207a7460-8433-11f0-9625-32a948b50521-0 |
|
@sanchezl: This PR was included in a payload test run from #197
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/207a7460-8433-11f0-9625-32a948b50521-0 |
|
/retest required |
|
@davegord: The The following commands are available to trigger optional jobs: Use 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-sigs/prow repository. |
|
/retest-required |
|
/retest |
|
/label acknowledge-critical-fixes-only |
openshift-ci
bot
added
the
acknowledge-critical-fixes-only
|
/retest |
|
/lgtm |
openshift-ci-robot
added
the
verified
|
@benluddy: This PR has been marked as verified by 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/assign @ibihim |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: benluddy, ibihim, sanchezl 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
bot
added
the
approved
|
/retest-required Automated retry triggered by CI triage analysis. Failure Analysis:
This is a transient timing issue, not a code problem. |
|
@sanchezl: all tests passed! Full PR test history. Your PR dashboard. 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-sigs/prow repository. I understand the commands that are listed here. |
5 participants
Bump kube and openshift dependencies.