search

kubeflow / spark-operator

Kubernetes operator for managing the lifecycle of Apache Spark applications on Kubernetes.
Apache License 2.0
2.8k stars 1.38k forks source link

Management of ingressUrlFormat with ingresscontrollers different than nginx #1762

Open gioppoluca opened 1 year ago

gioppoluca commented 1 year ago

It seems tha the way that: https://github.com/GoogleCloudPlatform/spark-on-k8s-operator/blob/master/pkg/controller/sparkapplication/sparkui.go manages ingressPath is peculiar to nginx ingress controller For others like traefik the grouping in case of (/|$)(.*) has to be dome differently (by adding a middleware that strips the path (for example). But the path that is created by adding the grouping and the annotation for nginx could break the proper path interpretation.

Could be made so that is possible to state we are not using nginx so these operations are not done and the ingresspath is not altered from the things written in ingressUrlFormat?

cmditch commented 11 months ago

I think I'm hitting the same issue with trying to access the Spark UI on a cluster running Traefik. @gioppoluca have you found any workarounds?

github-actions[bot] commented 3 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

gioppoluca commented 3 months ago

Still relevant

jacobsalway commented 3 months ago

It would be a fairly easy change to add if ingressClassName == "nginx" to https://github.com/kubeflow/spark-operator/blob/master/internal/controller/sparkapplication/driveringress.go#L139, however this would be a breaking change as users may be implicitly relying on this annotation even if they haven't specified the -ingress-class-name argument.

Would it help you if this was changed to if ingressClassName == "" || ingressClassName == "nginx" so that you can specify -ingress-class-name=traefik to disable this behaviour while still retaining the implicit behaviour?

gioppoluca commented 3 months ago

Could be a start, but I remember It was not only that, on vacation now but Will check It out asap. I think It should be a solution good also for other ingresso controllers

gioppoluca commented 3 months ago

I think that there Is something also on rows 169,170 to check. Seems linked to the row you pointed out.

github-actions[bot] commented 4 days ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.