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

Change in domain templates #96

Closed
bartoszmajsak opened this issue Oct 3, 2023 · 10 comments
Closed

Change in domain templates #96

bartoszmajsak opened this issue Oct 3, 2023 · 10 comments
Assignees
Labels
kind/feature New feature

Comments

@bartoszmajsak
Copy link

/kind feature

Describe the solution you'd like
We would like to have an enhancement to the domain template used in KServe. Currently, it's defined as {{ .Name }}-{{ .Namespace }}.{{ .IngressDomain }}.

Changing it to {{ .Name }}.{{ .Namespace }}.{{ .IngressDomain }} will make the AuthConfigs more specific, allowing for the creation of a configuration for *.{{.Namespace}} directly instead of monitoring all models and creating new "hosts".

@bartoszmajsak
Copy link
Author

/cc @aslakknutsen

@Jooho
Copy link

Jooho commented Oct 3, 2023

@bartoszmajsak To be clear what we should do with this ticket, I have some questions.
is this request to change this configuraton:https://github.com/opendatahub-io/kserve/blob/master/config/configmap/inferenceservice.yaml#L181 to {{ .Name }}.{{ .Namespace }}.{{ .IngressDomain }} for AuthConfig?

@bartoszmajsak
Copy link
Author

bartoszmajsak commented Oct 3, 2023

@bartoszmajsak To be clear what we should do with this ticket, I have some questions. is this request to change this configuraton:master/config/configmap/inferenceservice.yaml#L181 to {{ .Name }}.{{ .Namespace }}.{{ .IngressDomain }} for AuthConfig?

Technically speaking more about this

"domainTemplate": "{{ .Name }}-{{ .Namespace }}.{{ .IngressDomain }}",

@aslakknutsen can you confirm?

@aslakknutsen
Copy link

aslakknutsen commented Oct 3, 2023

domainTemplateis part of the KServe configuration. On the Authorino side we canonly bind AuthConfigs to "hosts" with wildcard levels(*.x.y), not partly wildcards(*-x.y).

This change would allow us to handle 1 domain pattern between all models in a odh-project(namespace) without the need for additional listeners to update the host lists or filters to manipulate the incoming host name.

@aslakknutsen
Copy link

@aslakknutsen can you confirm?

Not 100% sure of the source of this configuration in the kserve world and how that is moved to osd-manifests etc, but that certainly would look like it.

@Jooho
Copy link

Jooho commented Oct 3, 2023

@vaibhavjainwiz @israel-hdez
If we change this, will there be any problems? I think this PR might need changes. What do you think?

@israel-hdez
Copy link

@aslakknutsen @bartoszmajsak Do you need to change only the InferenceService public domain? Or would you also need a similar change to the domain of the KNative services?

@aslakknutsen
Copy link

@israel-hdez Not sure what the KNative services are in this context? Would that be like the "internal" names?

@israel-hdez
Copy link

@aslakknutsen In KServe, when you create an InferenceService, it creates KNative services for you to deploy the models.

The ksvcs can also be publicly exposed with their own URL with a similar pattern that also seems to be {{ .Name }}-{{ .Namespace }}.{{ .IngressDomain }} by default.

So, several routes can be involved:

  1. The route for the ISVC itself.
  2. But we can also have several KNative routes.

My understanding is that you would want (1) changed, but not sure if routes in (2) would be relevant.

In any case, I was looking at doc comments in the inferenceservice-config ConfigMap, and it looks like the template is only applicable to RawDeployment which, at the moment, we are not using. See:

# domainTemplate specifies the template for generating domain/url for each inference service by combining variable from:
# Name of the inference service ( {{ .Name}} )
# Namespace of the inference service ( {{ .Namespace }} )
# Annotation of the inference service ( {{ .Annotations.key }} )
# Label of the inference service ( {{ .Labels.key }} )
# IngressDomain ( {{ .IngressDomain }} )
# If domain template is empty the default template {{ .Name }}-{{ .Namespace }}.{{ .IngressDomain }} is used.
# NOTE: This configuration only applicable for raw deployment.
"domainTemplate": "{{ .Name }}-{{ .Namespace }}.{{ .IngressDomain }}",
.

I haven't tested, but from those doc comments, my understanding is that changing that template won't have any impact on the generated URLs for KServe Serverless installation. It seems that KNative is the component that is controlling the URL template. However, I couldn't find any docs for configuring KNative's URL template.

So, my perception is that is not as easy as changing the template. It may be an enhancement that needs to be done to KServe.

aslakknutsen added a commit to aslakknutsen/kserve that referenced this issue Nov 21, 2023
aslakknutsen added a commit to aslakknutsen/kserve that referenced this issue Nov 21, 2023
aslakknutsen added a commit to aslakknutsen/opendatahub-operator that referenced this issue Nov 22, 2023
@israel-hdez
Copy link

Closing as no longer needed.

@israel-hdez israel-hdez closed this as not planned Won't fix, can't repro, duplicate, stale Dec 7, 2023
@github-project-automation github-project-automation bot moved this from New/Backlog to Done in ODH Model Serving Planning Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New feature
Projects
Status: Done
Status: No status
Status: Done
Development

No branches or pull requests

4 participants