-
Notifications
You must be signed in to change notification settings - Fork 61
add: raycluster oauth mutating webhook #522
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
add: raycluster oauth mutating webhook #522
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
9693f54
to
2be384d
Compare
@@ -15,5 +15,9 @@ resources: | |||
domain: codeflare.dev |
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.
Probably the domain needs to be fixed.
2e12a24
to
2f0e817
Compare
2f0e817
to
c109d99
Compare
c109d99
to
1e3bedc
Compare
89e9b2a
to
cd5396f
Compare
if !alreadyExists { | ||
rayclusterlog.Info("Adding OAuth sidecar container") | ||
// definition of the new container | ||
newOAuthSidecar := corev1.Container{ |
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.
Would it have sense to limit Resources for oauth container?
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.
Thanks @sutaakar
I feel this is a good idea, I'd be unsure of the limits to set, so I'll pull in @astefanutti :)
kind: Service | ||
metadata: | ||
name: webhook-service | ||
namespace: openshift-operators |
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.
Shouldn't this be namespace: system
?
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.
Yea, I'll switch this :)
Closing, included via #530 |
PR needs rebase. 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. |
Issue link
https://issues.redhat.com/browse/RHOAIENG-1991
What changes have been made
Add Mutating Webhook to add OAuth SideCar to RayCluster
Verification steps
Setup environment.
Build and Deploy the codeflare-operator
Ensure all configurations have been applied
From the sdk, create a raycluster
Checks