-
Notifications
You must be signed in to change notification settings - Fork 4.8k
CNTRLPLANE-945: oidc: fix test issues #30040
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CNTRLPLANE-945: oidc: fix test issues #30040
Conversation
|
Skipping CI for Draft Pull Request. |
| Credentials: []credential{ | ||
| { | ||
| Temporary: true, | ||
| Temporary: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Testing locally, this looked like it was causing issues where the users we created were considered to be "incomplete" because they needed to set a real password.
| Error string `json:"error,omitempty"` | ||
| ErrorDescription string `json:"error_description,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will allow us to surface errors from Keycloak as we encounter them during authentication
| return false, err | ||
| // Only log the error so we continue to retry until the context has timed out | ||
| g.GinkgoLogr.Error(err, "updating authentication resource") | ||
| return false, nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had forgetten in the original PR that returning an error here immediately stops the polling so we weren't actually retrying this operation on a 409.
This should retry the request until the timeout of 5 minutes every 10 seconds and log the error
|
@everettraven: This pull request references CNTRLPLANE-945 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.20.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
@everettraven: This pull request references CNTRLPLANE-945 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.20.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
@everettraven: This pull request references CNTRLPLANE-945 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.20.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
@everettraven: This pull request references CNTRLPLANE-945 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.20.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
I tested these changes as best as I could against a clusterbot cluster, but I'd like to find a way to test these changes in a CI environment using the job configurations in openshift/release#66980 I'm hoping I can merge those, even when failing, to be able to use the Holding until I've got a better grasp on the direction I need to take. |
|
/test openshift/cluster-authentication-operator/master/e2e-aws-external-oidc |
|
@everettraven: The specified target(s) for The following commands are available to trigger optional jobs: Use 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-sigs/prow repository. |
|
Job Failure Risk Analysis for sha: 4c6ac62
|
|
@everettraven: This pull request references CNTRLPLANE-945 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.20.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/lgtm |
4179e46 to
247c09c
Compare
|
/hold cancel |
|
Job Failure Risk Analysis for sha: b1ee547
|
|
/test e2e-gcp-ovn-upgrade |
1 similar comment
|
/test e2e-gcp-ovn-upgrade |
|
/override e2e-gcp-ovn-upgrade |
|
@xueqzhan: /override requires failed status contexts, check run or a prowjob name to operate on.
Only the following failed contexts/checkruns were expected:
If you are trying to override a checkrun that has a space in it, you must put a double quote on the context. 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-sigs/prow repository. |
|
/override ci/prow/e2e-gcp-ovn-upgrade |
|
@xueqzhan: Overrode contexts on behalf of xueqzhan: ci/prow/e2e-gcp-ovn-upgrade 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-sigs/prow repository. |
|
Job Failure Risk Analysis for sha: b1ee547
|
|
Job Failure Risk Analysis for sha: b1ee547
|
|
/hold Revision b1ee547 was retested 3 times: holding |
|
/hold cancel |
|
Job Failure Risk Analysis for sha: b1ee547
|
|
Job Failure Risk Analysis for sha: b1ee547
|
|
/retest-required |
|
Job Failure Risk Analysis for sha: b1ee547
|
|
/override ci/prow/e2e-gcp-ovn-upgrade |
|
@sosiouxme: Overrode contexts on behalf of sosiouxme: ci/prow/e2e-gcp-ovn-upgrade 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-sigs/prow repository. |
|
Job Failure Risk Analysis for sha: b1ee547
|
1 similar comment
|
Job Failure Risk Analysis for sha: b1ee547
|
|
@everettraven: The following tests failed, say
Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
|
/retest-required |
9849104
into
openshift:main
|
[ART PR BUILD NOTIFIER] Distgit: openshift-enterprise-tests |
Fixes a handful of issues that happened to merge in #29917
Specifically: