Repository: istio/istio
File: pkg/dns/server/name_table.go (Go)

Code before:
								continue
							}
							shortName := instance.HostName + "." + instance.SubDomain
							host := shortName + "." + parts[1] // Add cluster domain.
							hostMetadata[host] = struct {
								shortName string
								namespace string
							}{shortName, svc.Attributes.Namespace}
							if sameCluster {
								localAddresses[host] = append(localAddresses[host], instance.Addresses...)
							} else {
								remoteAddresses[host] = append(remoteAddresses[host], instance.Addresses...)
							}
						}
						skipForMulticluster := !cfg.MulticlusterHeadlessEnabled && !sameCluster
						if skipForMulticluster || !sameNetwork {
							// We take only cluster-local endpoints. While this seems contradictory to
							// our logic other parts of the code, where cross-cluster is the default.
							// However, this only impacts the DNS response. If we were to send all
							// endpoints, cross network routing would break, as we do passthrough LB and
							// don't go through the network gateway. While we could, hypothetically, send
							// "network-local" endpoints, this would still make enabling DNS give vastly
							// different load balancing than without, so its probably best to filter.
							// This ends up matching the behavior of Kubernetes DNS.
							continue
						}
						// TODO: should we skip the node's own IP like we do in listener?
						addressList = append(addressList, instance.Addresses...)
					}
					// Write local cluster entries first
					for host, ips := range localAddresses {
						metadata := hostMetadata[host]
						out.Table[host] = &dnsProto.NameTable_NameInfo{
							Ips:       ips,
							Registry:  string(svc.Attributes.ServiceRegistry),
							Namespace: metadata.namespace,
							Shortname: metadata.shortName,
						}
					}
					// Write remote cluster entries only if local doesn't exist
					for host, ips := range remoteAddresses {
						if _, exists := out.Table[host]; !exists {
							metadata := hostMetadata[host]
							out.Table[host] = &dnsProto.NameTable_NameInfo{
								Ips:       ips,
								Registry:  string(svc.Attributes.ServiceRegistry),
								Namespace: metadata.namespace,
								Shortname: metadata.shortName,
							}
						}
					}

Review comment (suggestion):
In theory we could let the entry list all addresses, with local ones first.

I am not sure if the old behavior of returning the local address first came from a limitation at the time, when the nametable did not support multiple IPs and preferring local was simply the obvious choice, or if there was some other reason behind it.
For now, I played it safe and made the code return local addresses over the remote ones when they are available.

Code after:
							}
							shortName := instance.HostName + "." + instance.SubDomain
							host := shortName + "." + parts[1] // Add cluster domain.
							hostMetadata[host] = types.NamespacedName{Name: shortName, Namespace: svc.Attributes.Namespace}
							if sameCluster {
								localAddresses[host] = append(localAddresses[host], instance.Addresses...)
							} else {
								remoteAddresses[host] = append(remoteAddresses[host], instance.Addresses...)
							}
						}
						skipForMulticluster := !cfg.MulticlusterHeadlessEnabled && !sameCluster
						if skipForMulticluster || !sameNetwork {
							// We take only cluster-local endpoints. While this seems contradictory to
							// our logic other parts of the code, where cross-cluster is the default.
							// However, this only impacts the DNS response. If we were to send all
							// endpoints, cross network routing would break, as we do passthrough LB and
							// don't go through the network gateway. While we could, hypothetically, send
							// "network-local" endpoints, this would still make enabling DNS give vastly
							// different load balancing than without, so its probably best to filter.
							// This ends up matching the behavior of Kubernetes DNS.
							continue
						}
						// TODO: should we skip the node's own IP like we do in listener?
						addressList = append(addressList, instance.Addresses...)
					}
					// Write local cluster entries first
					for host, ips := range localAddresses {
						meta := hostMetadata[host]
						out.Table[host] = &dnsProto.NameTable_NameInfo{
							Ips:       ips,
							Registry:  string(svc.Attributes.ServiceRegistry),
							Namespace: meta.Namespace,
							Shortname: meta.Name,
						}
					}
					// Write remote cluster entries only if local doesn't exist
					for host, ips := range remoteAddresses {
						if _, exists := localAddresses[host]; !exists {
							meta := hostMetadata[host]
							out.Table[host] = &dnsProto.NameTable_NameInfo{
								Ips:       ips,
								Registry:  string(svc.Attributes.ServiceRegistry),
								Namespace: meta.Namespace,
								Shortname: meta.Name,
							}
						}
					}
				}
			}
			if len(addressList) == 0 {
				// could not reliably determine the addresses of endpoints of headless service