multicluster doesn't work on EKS

[adleong]

When attempting to link a source cluster to a target EKS cluster using the linkerd multicluster link command, the service mirror controller in the source cluster logs this error:

(will retry): Inner errors:\n\tEndpoints \"linkerd-gateway-[xxx]\" is invalid: [subsets[0].addresses[0].ip: Invalid value: \"\": must be a valid IP address, (e.g. 10.9.8.7), 
subsets[0].addresses[0].hostname: Invalid value [...]

and fails to create the endpoints object for the gateway mirror.

This happens because the service mirror controller attempts to copy the hostname and IP fields from the gateway service's ingress struct to the EndpointAddress struct of the gateway mirror's endpoints object. In EKS, the ingress struct of the linkerd-gateway service is populated with a hostname and no IP. However, the IP field of EndpointAddress is mandatory.

[olix0r]

The problem is related to this code where we translate the remote service load balancer state to a local endpoint:

linkerd2/controller/cmd/service-mirror/cluster_watcher.go

Lines 1122 to 1128 in f01bcfe

	var gatewayEndpoints []corev1.EndpointAddress
	for _, ingress := range gateway.Status.LoadBalancer.Ingress {
	gatewayEndpoints = append(gatewayEndpoints, corev1.EndpointAddress{
	IP: ingress.IP,
	Hostname: ingress.Hostname,
	})
	}

There are at least two problems here:

1. The IP field is required on the EndpointAddress but is optional on the LoadBalancer.Ingress.
2. The Hostname field on the EndpointAddress must not be a FQDN, but it will almost always be a FQDN when set on the LoadBalancer.Ingress.

In order to handle this case, we'll need to resolve the remote Hostname via DNS (refreshing after the DNS record's TTL expires) to create an EndpointAddress based on the DNS results; or we can move to approach that avoids creating concrete EndpointAddress types in these cases (perhaps using ExternalService instead?).

[spaghettifunk]

In Istio they use the ExternalService (I guess is the externalName in the service deployment file) for tackling this issue.

[zaharidichev]

I did a bit of thinking around this one and here is one potential solution. I might be way off here so correct me if I am wrong. Not sure how feasible it is. I was reviewing #4563 and it occurred to me that EndpointsSlices have all the bells to express what we want. How about the following.. In a scenario where we do not have to have endpoints separately defined on each mirror service (as described in #4535 ), we can have one gateway mirror service that has all the traffic flowing through it. This service mirror can have one endpoints slice that might look like this in the case where we have an LB with a concrete IPv4 address:

apiVersion: discovery.k8s.io/v1beta1
kind: EndpointSlice
metadata:
  name: gateway-mirror-endpoints
  labels:
    kubernetes.io/service-name: gateway-mirror
addressType: IPv4
ports:
  - name: http
    protocol: TCP
    port: 80
endpoints:
  - addresses:
      - "10.1.2.3"
    conditions:
      ready: true

and like this in the case where we do not have an IP (EKS)

apiVersion: discovery.k8s.io/v1beta1
kind: EndpointSlice
metadata:
  name: gateway-mirror-endpoints
  labels:
    kubernetes.io/service-name: gateway-mirror
addressType: FQDN
ports:
  - name: http
    protocol: TCP
    port: 80
endpoints:
  - addresses:
      - "some.fqdn.com"
    conditions:
      ready: true

This way we have a perfectly valid way to express the either/or situation that is not valid with EndpointsSubsets. We need to change the destinations service for this purpose and its API. Namely we cna change WeightedAdd to incorporate the notion of returning fqdn instead of only IPv4/6 addresses and instead of:

type WeightedAddr struct {
	Addr                 *net.TcpAddress
	Weight               uint32           
	MetricLabels         map[string]string 
}

type WeightedAddr struct {
	Addr                 *Addr // <----- either *net.TcpAddress or string (fqdn)
	Weight               uint32           
	MetricLabels         map[string]string 
}

Now when the proxy receives a WeightedAddr that does not have a concrete net.TcpAddress but instead specifies a FQDN, it will do the DNS resolution much like it does DNS fallback when there are no endpoints coming from the destination service. This time however, it will have all the metrics labels, weight, auth overrides, etc, etc. Note that this should work with endpoints slices but if we instead want to do it with the current k8s primitives and not rely on such a new k8s feature, we can always have an EndpointsAddress that has the Hostname field set and the addresses populated with some dummy value. The latter feels like a hack.

@adleong @olix0r @grampelberg WDYT. Does that sound plausible?

[grampelberg]

@zaharidichev you can't run a k8s version that has EndpointSlices in it yet =( I think the long term solution is the simplification that you've recommended with exernal name services. AFAICT the most optimal short term solution is going to be polling the hostname and updating the endpoint with the right ip address.