Fix coredns NodeHosts on dual-stack clusters

pkg/agent/run.go

@@ -426,7 +426,7 @@ func updateLegacyAddressLabels(agentConfig *daemonconfig.Agent, nodeLabels map[s
 	if ls.Has(cp.InternalIPKey) || ls.Has(cp.HostnameKey) {
 		result := map[string]string{
 			cp.InternalIPKey: agentConfig.NodeIP,
-			cp.HostnameKey:   agentConfig.NodeName,
+			cp.HostnameKey:   getHostname(agentConfig),
 		}
 
 		if agentConfig.NodeExternalIP != "" {
@@ -444,7 +444,7 @@ func updateAddressAnnotations(nodeConfig *daemonconfig.Node, nodeAnnotations map
 	agentConfig := &nodeConfig.AgentConfig
 	result := map[string]string{
 		cp.InternalIPKey: util.JoinIPs(agentConfig.NodeIPs),
-		cp.HostnameKey:   agentConfig.NodeName,
+		cp.HostnameKey:   getHostname(agentConfig),
 	}
 
 	if agentConfig.NodeExternalIP != "" {
@@ -539,3 +539,13 @@ func tunnelSetup(ctx context.Context, nodeConfig *daemonconfig.Node, cfg cmds.Ag
 	}
 	return tunnel.Setup(ctx, nodeConfig, proxy)
 }
+
+// getHostname returns the actual system hostname.
+// If the hostname cannot be determined, or is invalid, the node name is used.
+func getHostname(agentConfig *daemonconfig.Agent) string {
+	hostname, err := os.Hostname()
+	if err != nil || hostname == "" || strings.Contains(hostname, "localhost") {
+		return agentConfig.NodeName
+	}
+	return hostname
+}

pkg/node/controller.go

@@ -1,14 +1,18 @@
 package node
 
 import (
+	"bytes"
 	"context"
+	"net"
+	"sort"
 	"strings"
 
 	"github.com/k3s-io/k3s/pkg/nodepassword"
 	"github.com/pkg/errors"
 	coreclient "github.com/rancher/wrangler/pkg/generated/controllers/core/v1"
 	"github.com/sirupsen/logrus"
 	core "k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -48,14 +52,22 @@ func (h *handler) onRemove(key string, node *core.Node) (*core.Node, error) {
 
 func (h *handler) updateHosts(node *core.Node, removed bool) (*core.Node, error) {
 	var (
-		nodeName    string
-		nodeAddress string
+		nodeName string
+		hostName string
+		nodeIPv4 string
+		nodeIPv6 string
 	)
 	nodeName = node.Name
 	for _, address := range node.Status.Addresses {
-		if address.Type == "InternalIP" {
-			nodeAddress = address.Address
-			break
+		switch address.Type {
+		case v1.NodeInternalIP:
+			if strings.Contains(address.Address, ":") {
+				nodeIPv6 = address.Address
+			} else {
+				nodeIPv4 = address.Address
+			}
+		case v1.NodeHostName:
+			hostName = address.Address
 		}
 	}
 	if removed {
@@ -64,57 +76,97 @@ func (h *handler) updateHosts(node *core.Node, removed bool) (*core.Node, error)
 		}
 	}
 	if h.modCoreDNS {
-		if err := h.updateCoreDNSConfigMap(nodeName, nodeAddress, removed); err != nil {
+		if err := h.updateCoreDNSConfigMap(nodeName, hostName, nodeIPv4, nodeIPv6, removed); err != nil {
 			return nil, err
 		}
 	}
 	return nil, nil
 }
 
-func (h *handler) updateCoreDNSConfigMap(nodeName, nodeAddress string, removed bool) error {
-	if nodeAddress == "" && !removed {
-		logrus.Errorf("No InternalIP found for node " + nodeName)
+func (h *handler) updateCoreDNSConfigMap(nodeName, hostName, nodeIPv4, nodeIPv6 string, removed bool) error {
+	if removed {
+		nodeIPv4 = ""
+		nodeIPv6 = ""
+	} else if nodeIPv4 == "" && nodeIPv6 == "" {
+		logrus.Errorf("No InternalIP addresses found for node " + nodeName)
 		return nil
 	}
 
+	nodeNames := nodeName
+	if hostName != nodeName {
+		nodeNames += " " + hostName
+	}
+
 	configMap, err := h.configMaps.Get("kube-system", "coredns", metav1.GetOptions{})
 	if err != nil || configMap == nil {
 		logrus.Warn(errors.Wrap(err, "Unable to fetch coredns config map"))
 		return nil
 	}
 
-	hosts := configMap.Data["NodeHosts"]
-	hostsMap := map[string]string{}
+	addressMap := map[string]string{}
 
-	for _, line := range strings.Split(hosts, "\n") {
+	// extract current entries from hosts file, skipping any entries that are
+	// empty, unparsable, or hold an incorrect address for the current node.
+	for _, line := range strings.Split(configMap.Data["NodeHosts"], "\n") {
+		line, _, _ = strings.Cut(line, "#")
 		if line == "" {
 			continue
 		}
 		fields := strings.Fields(line)
-		if len(fields) != 2 {
+		if len(fields) < 2 {
 			logrus.Warnf("Unknown format for hosts line [%s]", line)
 			continue
 		}
 		ip := fields[0]
-		host := fields[1]
-		if host == nodeName {
-			if removed {
-				continue
-			}
-			if ip == nodeAddress {
-				return nil
+		if fields[1] == nodeName {
+			if strings.Contains(ip, ":") {
+				if ip != nodeIPv6 {
+					continue
+				}
+			} else {
+				if ip != nodeIPv4 {
+					continue
+				}
 			}
 		}
-		hostsMap[host] = ip
+		names := strings.Join(fields[1:], " ")
+		addressMap[ip] = names
+	}
+
+	// determine what names we should have for each address family
+	var namesv6, namesv4 string
+	if nodeIPv4 != "" {
+		namesv4 = nodeNames
+	}
+	if nodeIPv6 != "" {
+		namesv6 = nodeNames
+	}
+
+	// don't need to do anything if the addresses are in sync
+	if !removed && addressMap[nodeIPv4] == namesv4 && addressMap[nodeIPv6] == namesv6 {
+		return nil
+	}
+
+	// Something's out of sync, set the desired entries
+	if nodeIPv4 != "" {
+		addressMap[nodeIPv4] = namesv4
+	}
+	if nodeIPv6 != "" {
+		addressMap[nodeIPv6] = namesv6
 	}
 
-	if !removed {
-		hostsMap[nodeName] = nodeAddress
+	// sort addresses by IP
+	addresses := make([]string, 0, len(addressMap))
+	for ip := range addressMap {
+		addresses = append(addresses, ip)
 	}
+	sort.Slice(addresses, func(i, j int) bool {
+		return bytes.Compare(net.ParseIP(addresses[i]), net.ParseIP(addresses[j])) < 0
+	})
 
 	var newHosts string
-	for host, ip := range hostsMap {
-		newHosts += ip + " " + host + "\n"
+	for _, ip := range addresses {
+		newHosts += ip + " " + addressMap[ip] + "\n"
 	}
 
 	if configMap.Data == nil {
@@ -132,7 +184,7 @@ func (h *handler) updateCoreDNSConfigMap(nodeName, nodeAddress string, removed b
 	} else {
 		actionType = "Updated"
 	}
-	logrus.Infof("%s coredns node hosts entry [%s]", actionType, nodeAddress+" "+nodeName)
+	logrus.Infof("%s coredns NodeHosts entry for %s", actionType, nodeName)
 	return nil
 }