Enabling Service Mesh

[bartoszmajsak]

I see there's a code for creating Maistra specific resource (ServiceMeshMember) for making a namespace part of the mesh but I don't see it being used anywhere yet. There is a similar need for notebook namespaces in ODH, therefore I would like to suggest the following:

• create a separate controller taking care of registering the namespace and creating ns-wide resources (eg. authorization config)
• keep component-specific resource creation in their respective controllers, they should take care of creating needed such as VirtualServices as they have a complete information of what is needed

I started odh-project-controller to address the first point and also adjusted odh-notebook-controller according to the second one. I'm happy to help with changes in this controller if we agree on the direction.

WDYT?

[israel-hdez]

Hi @bartoszmajsak !

I see there's a code for creating Maistra specific resource (ServiceMeshMember) for making a namespace part of the mesh but I don't see it being used anywhere yet.

I think this was a previous attempt to integrate Service Mesh into ModelMesh-serving. However, when thinking on the overall ODH project, most likely managing the ServiceMeshMembers here may not make sense. So, if you already addressed this in odh-project-controller, I think it is safe to remove this code.

I'm happy to help with changes in this controller if we agree on the direction.

@aslakknutsen and I already did some changes that are in the service-mesh-integration branch. In short, the changes are for creating the needed VirtualServices so that Maistra can act as the entry point for traffic. Also, VirtualServices are created to support traffic splitting.

There are a few things left to fix in that branch:

• The Gateway for VSs is, at the moment, hard-coded to be opendatahub/odh-gateway. I'd like to parametrize this, minimally, moving it to be a value from an ENVVAR, so if it is going to be always opendatahub/odh-gateway, this string would be moved to the manifests. Or, maybe, is it better to read this value from an annotation in the namespace? Probably, you know better how to answer this.
  • Perhaps, the first question to address is: what is going to create the Gateway resource? is it odh-project-controller? is it this controller? something else?
• The internal hostname of the ingressgateway that is associated to the Gateway is hard-coded as istio-ingressgateway.istio-system.svc.cluster.local, but it most likely shouldn't be hard-coded. This is specific to support traffic splitting.
  • Right now, I'm thinking changing implementation a bit to not need to specify the host of the ingress gateway (read the next point).
• At the moment, exposing models (through the ingress gateway) is forced, because the VirtualServices are using * for the host attribute. So, we cannot have in-cluster only models.
  • I'm thinking to fix this by deploying a dummy workload in the namespace (perhaps using the sleep or pause container) with a sidecar injected. This would become the entrypoint of traffic and, at the same time, would be the sidecar supporting traffic-splitting.

That seems to be the remaining work in this repository, unless you have something additional in mind. Hopefully, we can agree on something for the listed points. And we can discuss anything else you have.

[bartoszmajsak]

I think it is safe to remove this code.

This is already in the other controller so I would say by all means throw this code away :)

[bartoszmajsak]

Perhaps, the first question to address is: what is going to create the Gateway resource? is it odh-project-controller? is it this controller? something else?

At this moment we have only one gateway, but based on my discussions with @aslakknutsen we want to make gw per ns - so I think we should make this a responsibility of the odh-project-controller. Would you agree on this approach?

[bartoszmajsak]

I'm thinking to fix this by deploying a dummy workload in the namespace (perhaps using the sleep or pause container) with a sidecar injected. This would become the entrypoint of traffic and, at the same time, would be the sidecar supporting traffic-splitting.

@israel-hdez can you elaborate a bit more on this? What sidecar are we talking about? Istio proxy? What about in-cluster only models? Are they going to be part of the mesh too?

[israel-hdez]

based on my discussions with @aslakknutsen we want to make gw per ns - so I think we should make this a responsibility of the odh-project-controller. Would you agree on this approach?

Do you mean the Gateway CRD? or a gateway workload together with its Gateways CRDs?

[israel-hdez]

I'm thinking to fix this by deploying a dummy workload in the namespace (perhaps using the sleep or pause container) with a sidecar injected. This would become the entrypoint of traffic and, at the same time, would be the sidecar supporting traffic-splitting.

@israel-hdez can you elaborate a bit more on this? What sidecar are we talking about? Istio proxy? What about in-cluster only models? Are they going to be part of the mesh too?

I'm talking about the sidecar of the dummy workload, with a sidecar.istio.io/inject: true label.

I'm assuming that models are not going to be part of the Service Mesh. Adding models to the mesh may not be desirable, because the Istio sidecar logic may enter in conflict with ModelMesh's own routing. Models are the leafs of traffic, so I think we really don't need to add Models to the mesh, because Istio's traffic rules are applied at client side.

Having mentioned the "client side", I have to say that is unknown to me if clients/apps are going to be part of the mesh. So, to be on the "safe" side, I'm assuming clients/apps won't be part of the mesh. For out-of-cluster clients, this is always true and we have the advantage that traffic goes through the Istio's ingress-gateway and we have a natural entry point of traffic.

But then, we can have in-cluster clients/apps which, most likely, are what are going to be most common. If these are not going to be part of the mesh, then traffic never enters the mesh and we have no way to apply traffic splitting via VirtualServices. At the moment, the workaround is to use istio's ingress gateway as the entry point also for in-cluster apps/clients. You can see a VirtualService configured for such assumption here. What is problematic is that models are exposed as a must, which is not desirable for all models. If we put aside the ingress-gateway, we still need something in the middle as the Mesh entry point for traffic, and this is the reason I was thinking on a dummy workload.

[aslakknutsen]

At the moment, exposing models (through the ingress gateway) is forced, because the VirtualServices are using * for the host attribute. So, we cannot have in-cluster only models.

This is correct. * will go away. We're looking at the option of using "an ingress gw" for this type of work connected to a local host name and not exposed externally. The Ingress GW would act as the "thing that gets you into the mesh". As it currently stands, you'll get Auth rules applied as they are Target Side validated, but not Routing rules as they are Caller side validated.