What happened?

Related to: #1745 (comment), the current code looks like:

func GenerateTunnelName(nodeIP string) string {
	// remove dots from an IPv4 address
	strippedIP := strings.ReplaceAll(nodeIP, ".", "")
	// remove colons from an IPv6 address
	strippedIP = strings.ReplaceAll(strippedIP, ":", "")

	h := sha256.New()
	h.Write([]byte(strippedIP))
	sum := h.Sum(nil)

	return "tun-" + fmt.Sprintf("%x", sum)[0:11]
}

@twz123 points out: This would mean that "21.3.0.4" hashes to the same tunnel as "2.13.0.4". Is that something that kube-router should care about?

Wouldn't it be more straight forward to operate on the IP address bytes, instead of their string representation? E.g. like this:

func GenerateTunnelName(nodeIP net.IP) string {
	if v4 := nodeIP.To4(); v4 != nil {
		return fmt.Sprintf("tun-%x", []byte(v4))
	}

	hash := sha256.Sum256(nodeIP)
	return fmt.Sprintf("tun-%x", hash[:6])[:15]
}

The four bytes of an IPv4 address can be losslessly converted to an eight byte hex string. The 16 bytes of an IPv6 address need to be hashed, as before.

The tunnel names for IPv4 addresses will be shorter, and can be easily converted back to the original IP address. Not sure if that's an undesirable side effect. If yes, the four IPv4 bytes could be hashed, as before, although I suppose that this is not a problem at all?

What did you expect to happen?

Tunnel name generation should not have the potential to lead to a hash collision that could cause tunnels to not be created properly for IP addresses that are the same numerically in different parts of the octet.

Additional context

We currently rely on tunnel name generation being predictable in order to clean up old tunnels. This means that we would either need to implement fall-back logic for matching tunnels for a set of releases, or we would need to couple this change to a release where we advise cluster administrators to do a rolling reboot in conjunction with the kube-router rollout (advice that is usually either missed, or pushed back against by community members).