search

opendatahub-io / model-registry-operator

Apache License 2.0
3 stars 19 forks source link

Consider adding `status.address.url` in CR #28

Closed tarilabs closed 11 months ago

tarilabs commented 1 year ago

Is your feature request related to a problem? Please describe. Suggestion while discussed with @danielezonca

Complementary to: https://github.com/opendatahub-io/model-registry-operator/issues/19 in the MR custom resource as part of the reconcile loop, consider adding status.address.url similar to Knative Addressable: https://knative.dev/docs/eventing/sinks/#:~:text=Addressable%20objects%20receive%20and%20acknowledge%20an%20event%20delivered%20over%20HTTP%20to%20an%20address%20defined%20in%20their%20status.address.url%20field

Describe the solution you'd like As above

Describe alternatives you've considered Currently UI and other service can only "match" on Service based on naming convention

Additional context This solution avoid assuming "naming conventions" to where the MR is reachable and exposed

dhirajsb commented 11 months ago

Since K8s itself supports using . as service address, there is no need to add a separate url in status. @lampajr has validated that it works.

rareddy commented 11 months ago

close this?

lampajr commented 11 months ago

@lampajr has validated that it works.

I can confirm that building the address as <service-name>.<namespace>.svc.cluster.local:<port> works for intra-cluster communications.

On the other hand IMO exposing the Route hostname/address in the CR status could be helpful as in that case whoever needs it can easily retrieve it from the CR rather than building it.

danielezonca commented 10 months ago

The suggestion behind this ticket is to expose the URL of the MR in the ModelRegistry CR object. The reason for this was to decouple where the registry is deployed and the CR. The naming convention that @lampajr tested makes strong assumption on the CR and the actual topology of the deployment: aka if the ModelRegistry object has a name, this name must be used for the service/route and the registry must be deployed in the same namespace.

This suggestion was also to be "more" future proof about making Model Registry a single instance: nobody should make assumption on where the instance is.

I hope this better clarifies the idea behind :)