canonical / bundle-kubeflow

Charmed Kubeflow
Apache License 2.0
103 stars 50 forks source link

Cannot expose Kubeflow on other path then "/" via ingress #524

Open Barteus opened 1 year ago

Barteus commented 1 year ago

Istio VirtalServers set the prefix and rewrite it in a way that does not support suffixes (it's based on the relation that does not have this information)

Works: http://10.64.140.43/dex/.well-known/openid-configuration Does not work: http://10.64.140.43/kubeflow/dex/.well-known/openid-configuration

camille-rodriguez commented 1 year ago

This affects additional customers wanting to use istio for other applications than kubeflow. Since istio can only be deployed once in a kubernetes cluster, it doesn't make sense to prevent other applications from using it as well. @andreeamun fyi this is a feature requested by my customer

ca-scribner commented 1 year ago

Responding to @camille-rodriguez, I think this is because Charmed Kubeflow's VirtualServices for the kubeflow dashboard redirect all traffic entering through the kubeflow gateway with prefix /. So I'm guessing (but haven't tested) that other users on the cluster can still get to anything they want so long as they add their own Gateway.

That being said, we might be doing other things that really inconvenience other Istio users too. The main one being that our istio integration as implemented right now has to own istio - we don't have a way to integrate into an existing Istio.

ca-scribner commented 1 year ago

We should be able to add flexibility in the VirtualServices that we create to at least allow for suffixes. I don't think there's anything in kubeflow that would dislike that, although we'd need to test first manually.

Where we want to implement this new control is more complicated. At present, the charms that want a VirtualService ask for one from istio-pilot via the ingress relation, and in doing that they specify their prefix and rewrite. So a naive implementation would mean that every charm using an ingress relation would need to have the additional config to control the prefix, and then in a bundle all these charms would need something like:

bundle: kubernetes
name: kubeflow
applications:
  dex-auth:
    charm: dex-auth
    channel: 2.31/stable
    scale: 1
    trust: true
    prefix: myKubeflowPrefix/dex  # <---------- new
  jupyter-ui:
    charm: jupyter-ui
    channel: 1.6/stable
    scale: 1
    prefix: myKubeflowPrefix/jupyter  # <---------- new
  katib-ui:
    charm: katib-ui
    channel: 0.14/stable
    scale: 1
    prefix: myKubeflowPrefix/katib  # <---------- new
  kubeflow-dashboard:
    charm: kubeflow-dashboard
    channel: 1.6/stable
    scale: 1
    prefix: myKubeflowPrefix/  # <---------- new
  ...
relations:
  - [istio-pilot:ingress, dex-auth:ingress]
  - [istio-pilot:ingress, jupyter-ui:ingress]
  - [istio-pilot:ingress, katib-ui:ingress]
  - [istio-pilot:ingress, kubeflow-dashboard:ingress]

which feels like a very annoying design. Maybe we'd want to implement some sort of global prefix in the istio-pilot charm, so we can say "all ingresses you provide, add this suffix to their prefix"? That would feel better integrated.