-
Notifications
You must be signed in to change notification settings - Fork 29
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 tls to kserve-router and add certs to ig service and always mount… #502
add tls to kserve-router and add certs to ig service and always mount… #502
Conversation
Skipping CI for Draft Pull Request. |
ce3cc46
to
ae1f199
Compare
ae1f199
to
4b63e4a
Compare
pkg/controller/v1beta1/inferenceservice/rawkube_controller_test.go
Outdated
Show resolved
Hide resolved
pkg/controller/v1beta1/inferenceservice/reconcilers/deployment/deployment_reconciler.go
Outdated
Show resolved
Hide resolved
pkg/controller/v1beta1/inferenceservice/reconcilers/service/service_reconciler_test.go
Outdated
Show resolved
Hide resolved
General question: Is the TLS for communicating Isvc pod to Isvc pod, or is the client to router endpoint for internal access? |
4b63e4a
to
bcd06ce
Compare
It is for pod to pod |
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.
nit: you missed some places to replace string.
I think it would be better to apply it equally everywhere.
pkg/controller/v1beta1/inferenceservice/rawkube_controller_test.go
Outdated
Show resolved
Hide resolved
pkg/controller/v1beta1/inferenceservice/rawkube_controller_test.go
Outdated
Show resolved
Hide resolved
pkg/controller/v1beta1/inferenceservice/rawkube_controller_test.go
Outdated
Show resolved
Hide resolved
pkg/controller/v1beta1/inferenceservice/rawkube_controller_test.go
Outdated
Show resolved
Hide resolved
I am not sure how this pr was verified but I have some questions about tls connections between router pod and isvc pod. If router pod is client in this scenario, I wonder how it can create SSL connection between them. Could you please elaborate this part? |
@VedantMahabaleshwarkar: 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. |
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.
May you rebase the PR?
pkg/controller/v1beta1/inferenceservice/reconcilers/deployment/deployment_reconciler.go
Outdated
Show resolved
Hide resolved
202334c
to
45420e5
Compare
@Jooho clarified this with Edgar, this JIRA is not for tls from router to isvc, it is only from client to the router. So this PR only includes the tls changes for listening in the router |
ok then it makes more sense. please fix th ci |
45420e5
to
3cb4a44
Compare
pkg/controller/v1beta1/inferenceservice/reconcilers/deployment/deployment_reconciler.go
Show resolved
Hide resolved
pkg/controller/v1beta1/inferenceservice/reconcilers/deployment/deployment_reconciler.go
Outdated
Show resolved
Hide resolved
… serving cert secret to raw deployment Signed-off-by: Vedant Mahabaleshwarkar <[email protected]> pr feedback Signed-off-by: Vedant Mahabaleshwarkar <[email protected]> modify callService to use TLS Signed-off-by: Vedant Mahabaleshwarkar <[email protected]> Revert "modify callService to use TLS" This reverts commit 1618bc8. edgar feedback Signed-off-by: Vedant Mahabaleshwarkar <[email protected]> fix lint errors Signed-off-by: Vedant Mahabaleshwarkar <[email protected]> bug fix Signed-off-by: Vedant Mahabaleshwarkar <[email protected]>
3cb4a44
to
59d60a9
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
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: israel-hdez, VedantMahabaleshwarkar 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 |
b6cf5bb
into
opendatahub-io:master
… serving cert secret to raw deployment
What this PR does / why we need it:
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes : RHOAIENG-18978
service.beta.openshift.io/serving-cert-secret-name=graph_name
to the graph servicetls.crt
andtls.key
filesType of changes
Please delete options that are not relevant.
Feature/Issue validation/testing:
Special notes for your reviewer:
Checklist:
Release note:
Re-running failed tests
/rerun-all
- rerun all failed workflows./rerun-workflow <workflow name>
- rerun a specific failed workflow. Only one workflow name can be specified. Multiple /rerun-workflow commands are allowed per comment.