hubble: Add "hubble-prefer-ipv6" option

[mKeRix]

Please ensure your pull request adheres to the following guidelines:

• For first time contributors, read Submitting a pull request
• All code is covered by unit and/or runtime tests where feasible.
• All commits contain a well written commit description including a title,
  description and a Fixes: #XXX line if the commit addresses a particular
  GitHub issue.
• If your commit description contains a Fixes: <commit-id> tag, then
  please add the commit author[s] as reviewer[s] to this issue.
• All commits are signed off. See the section Developer’s Certificate of Origin
• Provide a title or release-note blurb suitable for the release notes.
• Thanks for contributing!

This change adds an option to the Hubble Relay communication that allows to configure the previously hardcoded IPv4/IPv6 preference. When a node has mixed IPv4 and IPv6 addresses, but internal cluster communication is only IPv6-based, Hubble would announce wrong node addresses which would break communication to the agents. The previous IPv4 preference is kept as default.

Our specific use case for this is EKS with IPv6. When an instance is put into a public subnet and receives a public IP, this IP will be mapped as ExternalIP onto the node. Hubble will then announce that IP, even though no communication can happen on it. Instead, it should announce the IPv6 address the EC2 instance (like it does for nodes without public IP).

We've tested this changeset successfully in real IPv6 EKS clusters using a custom build of v1.12.2.

hubble: Add "hubble-prefer-ipv6" option

[kaworu]

Thank you @mKeRix for the PR!

The --hubble-prefer-ipv6 flag is fine to me, but in code I would slightly prefer (no pun intended) to express the addr family preference like k8s apimachinery rather than bubbling down a bool (in other words, "parsing" the flag as a AddressFamilyPreference). That way nodeAddress would be easier to read IMHO, e.g.

func nodeAddress(n types.Node, pref AddressFamilyPreference) string {
    for _, family := range pref {
        switch family {
        case familyIPv4:
            if addr := n.GetNodeIP(false); addr.To4() != nil {
                return addr.String()
            }
        case familyIPv6:
            if addr := n.GetNodeIP(true); addr.To4() == nil {
                return addr.String()
            }
        }
    }
    return ""
}

[mKeRix]

Hey @kaworu,

thanks for your remarks! I tried to integrate your proposals and pushed a new version. I was a bit insecure about where to put the AddressFamily types. I've now thrown them into the serviceoption package, but if you think there is a better place for them I'd appreciate a pointer.

[kaworu]

Awesome @mKeRix thanks for the update, small nit but other than that the patch LGTM.

[rolinh]

Looks solid, thanks!

[jibi]

Thanks for the PR @mKeRix! I think we are missing the Helm chart changes to export this new option. It should be just a matter of adding a new value and then using it in the configmap template.

As an example you can have a look at how it's done for the Hubble listening address:

• cilium/install/kubernetes/cilium/values.yaml

  Line 755 in 3c9a5c1

  listenAddress: ":4244"

• cilium/install/kubernetes/cilium/templates/cilium-configmap.yaml

  Lines 749 to 751 in 3c9a5c1

  	{{- if hasKey .Values.hubble "listenAddress" }}
  	# An additional address for Hubble server to listen to (e.g. ":4244").
  	hubble-listen-address: {{ .Values.hubble.listenAddress | quote }}

(and then remember to run make -C Documentation update-helm-values to update the docs)

[mKeRix]

Thank you for the hint @jibi! I've adapted the PR accordingly.

[jibi]

/test

[qmonnet]

Looks good, thank you

[jibi]

/test-runtime

[jibi]

test-1.23-5.4 is failing due to #21735, ci-external-workloads is failing to provision the cluster (#21885 should fix it), all other required tests are green, marking as ready