-
Notifications
You must be signed in to change notification settings - Fork 61
Allow for custom CertGeneratorImage #547
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
Allow for custom CertGeneratorImage #547
Conversation
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.
Looks good, just a couple comments 👍🏻
@Fiona-Waters |
The ray image specified in the cluster config in demo-notebooks will be the main ray image used. The one set in the config map will be used just for tls cert creation in the init containers. It can be the same image for both but it doesn't have to be. |
34be621
to
d6a6600
Compare
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.
/lgtm
Working as expected nice work Fiona
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.
One small comment
config/e2e/config.yaml
Outdated
@@ -7,3 +7,4 @@ data: | |||
kuberay: | |||
rayDashboardOAuthEnabled: false | |||
ingressDomain: "kind" | |||
mtLSEnabled: 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.
mtLSEnabled: false | |
mTLSEnabled: false |
Why is this needed here?
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.
Updated. It is needed as the e2e test is out of date. Creating a follow on jira to cover this.
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.
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.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: KPostOffice The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
2c2b0a2
into
project-codeflare:main
Issue link
RHOAIENG-6505
What changes have been made
I updated the CFO config to include an image to be used for cert creation in the init containers added by the CFO webhook. This is added so that in the case of a disconnected cluster there is not an extra image required.
Verification steps
codeflare-operator-config
configMap adding the image to the kuberay configuration like this:Checks