Skip to content
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

Consolidate katib-cert-generator to katib-controller #2149

Closed
Tracked by #2156
tenzen-y opened this issue Apr 24, 2023 · 4 comments · Fixed by #2185
Closed
Tracked by #2156

Consolidate katib-cert-generator to katib-controller #2149

tenzen-y opened this issue Apr 24, 2023 · 4 comments · Fixed by #2185

Comments

@tenzen-y
Copy link
Member

/kind feature

Describe the solution you'd like
[A clear and concise description of what you want to happen.]
I would like to consolidate the katib-cert-generator to the katib-controller.

Currently, if we use the standalone cert-generator to generate self-signed certs for the webhooks, we can not use Fail as a failurePolicy for the mutator.pod.katib.kubeflow.org since we face the deadlock when we create the cert-generator pod via batch/job.

By generating the self-signed certs in katib-controller, we can avoid the above dead lock.

Ref: #2018

Anything else you would like to add:
[Miscellaneous information that will assist in solving the issue.]


Love this feature? Give it a 👍 We prioritize the features with the most 👍

@tenzen-y
Copy link
Member Author

We need to migrate the below args in the cert-generator to the katib-config or the args of the katib-controller.

f.StringVarP(&o.namespace, "namespace", "n", "kubeflow", "set namespace")
f.StringVarP(&o.jobName, "jobName", "j", consts.JobName, "set job name")
f.StringVarP(&o.serviceName, "serviceName", "s", consts.Service, "set service name")

I hesitate to add these args to the args of the controller since these args don't directly relate to operations for the controller. So I'd prefer to add these options to katib-config.

@andreyvelich @johnugeorge @gaocegege WDYT?

@andreyvelich
Copy link
Member

Thank you for starting this @tenzen-y.

Let's continue the discussion how we want to implement it after this discussion: #2150

@tenzen-y
Copy link
Member Author

Thank you for starting this @tenzen-y.

Let's continue the discussion how we want to implement it after this discussion: #2150

Sure!

@tenzen-y
Copy link
Member Author

/assign

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants