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

DomainMapping does not work with local gateway "mesh" mode #562

Closed
ZhiminXiang opened this issue Mar 18, 2021 · 8 comments
Closed

DomainMapping does not work with local gateway "mesh" mode #562

ZhiminXiang opened this issue Mar 18, 2021 · 8 comments
Labels
lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.

Comments

@ZhiminXiang
Copy link

When I add the entry "local-gateway.mesh: mesh" into config-istio ConfigMap to indicate using mesh mode, DomainMapping does not work.

The request sent to the custom domain will get the response: no healthy upstream.

I think this is because the destination of the VS of the DomainMapping is the k8s service of the target ksvc. When setting "local-gateway.mesh: mesh", the k8s service of the target ksvc will not be ExternalName service. Instead, it is just a headless service. So the destination of the VirtualService of the DomainMapping will become invalid.

/cc @julz

@julz
Copy link
Contributor

julz commented Mar 22, 2021

@ZhiminXiang is this the case before/after/both-before-and-after #512?

@ZhiminXiang
Copy link
Author

I can confirm that it happens after that change. I haven't tried to test it before the change. but I will test it.

@julz
Copy link
Contributor

julz commented Mar 23, 2021

Thanks @ZhiminXiang thatd be helpful. What's the correct way to reference a service from outside the mesh in mesh mode?

@ZhiminXiang
Copy link
Author

I think essentially the issue is in the mesh mode, the cluster local gateway is not needed and can be removed, while the DomainMapping feature depends on the cluster local gateway for routing requests.

I think the fix could be:

  1. we need to explicitly ask users to not remove the cluster local gateway if they want to use mesh mode.
  2. we may want to provide an extra field in the config-istio CM to set the cluster local gateway service used by DM when users want to use mesh mode.
  • If the route service is ExternalName service, then we just use the ExternalName service as the destination service.
  • If the route service is headless service which means users use mesh mode, then we fall back to the cluster local gateway service defined in the config-istio CM.

@ZhiminXiang
Copy link
Author

ZhiminXiang commented Mar 23, 2021

Another possible option is we may be able to always keep cluster local gateway service and set the route service as ExternalName service for both mesh and non-mesh mode, and deprecate "local-gateway.mesh: mesh".
But for the mesh mode, I am not sure for the internal service to service scenario, if the requests will be sent to the cluster local gateway (i.e. client envoy -> cluster-local-gateway -> destination) or directly to the destination (client envoy -> destination).
If it will have the extra hop (cluster-local-gateway), then this is not an option.

/cc @JRBANCEL this is related to #549

@ZhiminXiang
Copy link
Author

ZhiminXiang commented Mar 30, 2021

I have tested the case with the following combined setup

  1. Istio sidecar injection enabled AND
  2. no entry "local-gateway.mesh: mesh" in the config-istio CM AND
  3. using a dummy knative-local-gateway service (i.e. the service label selector selects dummy labels that map to no pods).

And the service to service communication still works.

So I think the entry "local-gateway.mesh: mesh" is not necessary. The service to service requests won't go to cluster local gateway when sidecar is injected in the client side as long as the service knative-local-gateway service exists.

@ZhiminXiang
Copy link
Author

So I think the solution to handle the "mesh" mode could be:

  1. We explicitly require users to install local gateway service and keep the cluster local gateway entry in the config-istio CM if users want to use DomainMapping feature no matter if users want to use "mesh" or "no mesh".
  2. We should not recommend users to use entry "local-gateway.mesh: mesh" in the config-istio CM and to remove the cluster local gateways service because 1) there is no difference in terms of the mesh behavior when using or not using entry "local-gateway.mesh: mesh", 2) the overhead of keeping cluster local gateway service is small (just an extra k8s service and Istio Gateway)
  3. We will still support "local-gateway.mesh: mesh" at least for the short term. For the long-term, we may want to deprecate it.

@github-actions
Copy link

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.
Projects
None yet
Development

No branches or pull requests

2 participants