diff --git a/pkg/cloud-controller-manager/instance.go b/pkg/cloud-controller-manager/instance.go index 6659364f..362f1ed0 100644 --- a/pkg/cloud-controller-manager/instance.go +++ b/pkg/cloud-controller-manager/instance.go @@ -4,8 +4,9 @@ import ( "context" "encoding/json" "fmt" - "net" + "net/netip" "slices" + "strings" "sync" ctlkubevirtv1 "github.com/harvester/harvester/pkg/generated/controllers/kubevirt.io/v1" @@ -18,6 +19,8 @@ import ( kubevirtv1 "kubevirt.io/api/core/v1" ) +var linkLocalIPv6Range = netip.MustParsePrefix("fe80::/10") + type instanceManager struct { vmClient ctlkubevirtv1.VirtualMachineClient vmiClient ctlkubevirtv1.VirtualMachineInstanceClient @@ -70,7 +73,10 @@ func (i *instanceManager) InstanceMetadata(ctx context.Context, node *v1.Node) ( meta.Zone = zone } - meta.NodeAddresses = getNodeAddresses(node, vmi) + meta.NodeAddresses, err = getNodeAddresses(node, vmi) + if err != nil { + return nil, err + } return meta, nil } @@ -84,46 +90,143 @@ func (i *instanceManager) getVM(node *v1.Node) (*kubevirtv1.VirtualMachine, erro } // getNodeAddresses return nodeAddresses only when the value of annotation `alpha.kubernetes.io/provided-node-ip` is not empty -func getNodeAddresses(node *v1.Node, vmi *kubevirtv1.VirtualMachineInstance) []v1.NodeAddress { - providedNodeIP, ok := node.Annotations[api.AnnotationAlphaProvidedIPAddr] - if !ok { - return nil - } - - aiIPs, err := getAdditionalInternalIPs(node) +func getNodeAddresses(node *v1.Node, vmi *kubevirtv1.VirtualMachineInstance) ([]v1.NodeAddress, error) { + internalIPRanges, err := getInternalIPRanges(node) if err != nil { - // if additional IPs are not correctly marked, only log an error, do not return this error - logrus.WithFields(logrus.Fields{ - "namespace": node.Namespace, - "name": node.Name, - }).Debugf("%s, skip it", err.Error()) + return nil, err } - nodeAddresses := make([]v1.NodeAddress, 0, len(vmi.Spec.Networks)+1) + // Optimistically assume that for every interface have one IP. Add one for the hostname address that we add later. + // Since the amount of IP addresses is probably very limited this should be fine. + nodeAddresses := make([]v1.NodeAddress, 0, len(vmi.Status.Interfaces)+1) + // Build a list of network names (names of NICs) on the VM. + networkNames := make([]string, 0, len(vmi.Spec.Networks)) for _, network := range vmi.Spec.Networks { - for _, networkInterface := range vmi.Status.Interfaces { - if network.Name == networkInterface.Name { - if ip := net.ParseIP(networkInterface.IP); ip != nil && ip.To4() != nil { - nodeAddr := v1.NodeAddress{ - Address: networkInterface.IP, - } - if networkInterface.IP == providedNodeIP || (aiIPs != nil && slices.Contains(aiIPs, networkInterface.IP)) { - nodeAddr.Type = v1.NodeInternalIP - } else { - nodeAddr.Type = v1.NodeExternalIP - } - nodeAddresses = append(nodeAddresses, nodeAddr) + networkNames = append(networkNames, network.Name) + } + + // Find all IP addresses of the VM + for _, networkInterface := range vmi.Status.Interfaces { + // The interface list might contain interfaces that do not belong to any NIC of the VM. Filter them out. + if !slices.Contains(networkNames, networkInterface.Name) { + // Ignore interface since it does not belong to one of the NICs. + continue + } + + for _, ipStr := range networkInterface.IPs { + ip, err := netip.ParseAddr(ipStr) + if err != nil { + // Failed to parse IP, skip it + logrus.WithFields(logrus.Fields{ + "namespace": node.Namespace, + "name": node.Name, + }).Warnf("Unable to parse IP %s, skip it: %s", ipStr, err.Error()) + continue + } + + // Skip addresses in link local range, other nodes don't seem to be able to reach this address during cluster bootstrapping. + if ip.Is6() && linkLocalIPv6Range.Contains(ip) { + continue + } + + // Determine if the IP should be listed as an internal or external IP. + ipType := v1.NodeExternalIP + for _, internalPrefix := range internalIPRanges { + if internalPrefix.Contains(ip) { + // IP is an internal IP, no need to check further. + ipType = v1.NodeInternalIP + break } } + + nodeAddresses = append(nodeAddresses, v1.NodeAddress{ + Type: ipType, + Address: ip.String(), + }) } } + nodeAddresses = append(nodeAddresses, v1.NodeAddress{ Type: v1.NodeHostName, Address: node.Name, }) - return nodeAddresses + return nodeAddresses, nil +} + +func getInternalIPRanges(node *v1.Node) ([]netip.Prefix, error) { + internalIPRanges := make([]netip.Prefix, 0, 1) // Most of the time we would only have 1 internal range defined, the provided node IP + + // Kubelet sets this node annotation if the --node-ip flag is set and an external cloud provider is used + providedNodeIP, ok := node.Annotations[api.AnnotationAlphaProvidedIPAddr] + if !ok { + // Annotation is not set, this could be because we are running in a dual stack setup. + // Assume all IPs are internal IPs. + internalIPRanges = append(internalIPRanges, netip.MustParsePrefix("0.0.0.0/0")) + internalIPRanges = append(internalIPRanges, netip.MustParsePrefix("::/0")) + return internalIPRanges, nil + } + + // We got an IP from kubelet, parse it and convert it to a prefix containing only this IP + nodeIPRange, err := ipStringToPrefix(providedNodeIP) + if err != nil { + return nil, fmt.Errorf("annotation \"%s\" is invalid: %w", api.AnnotationAlphaProvidedIPAddr, err) + } + internalIPRanges = append(internalIPRanges, nodeIPRange) + + // Support marking extra IPs as internal + extraInternalIPs, err := getAdditionalInternalIPs(node) + if err != nil { + // Unable to parse extra provided internal IP ranges, ignore them. + logrus.WithFields(logrus.Fields{ + "namespace": node.Namespace, + "name": node.Name, + }).Warnf("%s, skip it", err.Error()) + + // Return list without extra user defined IP ranges. + return internalIPRanges, nil + } + + for _, extraInternalIP := range extraInternalIPs { + extraRange, err := ipStringToPrefix(extraInternalIP) + if err != nil { + // IP (range) malformed, skip it. + logrus.WithFields(logrus.Fields{ + "namespace": node.Namespace, + "name": node.Name, + }).Warnf("Unable to parse IP %s, skip it: %s", extraInternalIP, err.Error()) + continue + } + internalIPRanges = append(internalIPRanges, extraRange) + } + + return internalIPRanges, nil +} + +// ipStringToPrefix converts an IP / CIDR range to a netip.Prefix. It supports IPv4 and IPv6 addresses. +// If a plain IP address is given, it returns a Prefix that only contains this IP. +// If a CIDR range is given, it returns a Prefix that contains the whole range. +func ipStringToPrefix(str string) (netip.Prefix, error) { + if strings.Contains(str, "/") { + // CIDR notation + return netip.ParsePrefix(str) + } + + // Plain IP address + addr, err := netip.ParseAddr(str) + if err != nil { + return netip.Prefix{}, fmt.Errorf("failed to parse IP address \"%s\": %w", str, err) + } + + // For a single IPv4 address, the prefix length is 32; for IPv6, it's 128. + prefixLen := 32 + if addr.Is6() { + prefixLen = 128 + } + + // Create a prefix with the single address in it. + return addr.Prefix(prefixLen) } // User may want to mark some IPs of the node also as internal diff --git a/pkg/cloud-controller-manager/instance_test.go b/pkg/cloud-controller-manager/instance_test.go index 3c8e1a82..7eb0df51 100644 --- a/pkg/cloud-controller-manager/instance_test.go +++ b/pkg/cloud-controller-manager/instance_test.go @@ -24,10 +24,11 @@ const ( func Test_getNodeAddresses(t *testing.T) { tests := []struct { - name string - node *v1.Node - vmi *kubevirtv1.VirtualMachineInstance - output []v1.NodeAddress + name string + node *v1.Node + vmi *kubevirtv1.VirtualMachineInstance + output []v1.NodeAddress + wantErr string }{ { name: "1 internal and 2 external IPs", @@ -61,15 +62,15 @@ func Test_getNodeAddresses(t *testing.T) { Interfaces: []kubevirtv1.VirtualMachineInstanceNetworkInterface{ { Name: networkDefault, - IP: networkDefaultIP, + IPs: []string{networkDefaultIP}, }, { Name: network120, - IP: network120IP, + IPs: []string{network120IP}, }, { Name: network130, - IP: network130IP, + IPs: []string{network130IP}, }, }, }, @@ -126,15 +127,15 @@ func Test_getNodeAddresses(t *testing.T) { Interfaces: []kubevirtv1.VirtualMachineInstanceNetworkInterface{ { Name: networkDefault, - IP: networkDefaultIP, + IPs: []string{networkDefaultIP}, }, { Name: network120, - IP: network120IP, + IPs: []string{network120IP}, }, { Name: network130, - IP: network130IP, + IPs: []string{network130IP}, }, }, }, @@ -191,15 +192,15 @@ func Test_getNodeAddresses(t *testing.T) { Interfaces: []kubevirtv1.VirtualMachineInstanceNetworkInterface{ { Name: networkDefault, - IP: networkDefaultIP, + IPs: []string{networkDefaultIP}, }, { Name: network120, - IP: network120IP, + IPs: []string{network120IP}, }, { Name: network130, - IP: network130IP, + IPs: []string{network130IP}, }, }, }, @@ -256,15 +257,15 @@ func Test_getNodeAddresses(t *testing.T) { Interfaces: []kubevirtv1.VirtualMachineInstanceNetworkInterface{ { Name: networkDefault, - IP: networkDefaultIP, + IPs: []string{networkDefaultIP}, }, { Name: network120, - IP: network120IP, + IPs: []string{network120IP}, }, { Name: network130, - IP: network130IP, + IPs: []string{network130IP}, }, }, }, @@ -321,15 +322,15 @@ func Test_getNodeAddresses(t *testing.T) { Interfaces: []kubevirtv1.VirtualMachineInstanceNetworkInterface{ { Name: networkDefault, - IP: networkDefaultIP, + IPs: []string{networkDefaultIP}, }, { Name: network120, - IP: network120IP, + IPs: []string{network120IP}, }, { Name: network130, - IP: network130IP, + IPs: []string{network130IP}, }, }, }, @@ -353,6 +354,261 @@ func Test_getNodeAddresses(t *testing.T) { }, }, }, + { + name: "Multiple IPs on one interface, all internal", + node: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: nodeName, + Annotations: map[string]string{ + api.AnnotationAlphaProvidedIPAddr: networkDefaultIP, + KeyAdditionalInternalIPs: "[\"192.168.120.10\", \"192.168.130.10\"]", + }, + }, + }, + vmi: &kubevirtv1.VirtualMachineInstance{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testNamespace, + Name: nodeName, + }, + Spec: kubevirtv1.VirtualMachineInstanceSpec{ + Networks: []kubevirtv1.Network{ + { + Name: networkDefault, + }, + }, + }, + Status: kubevirtv1.VirtualMachineInstanceStatus{ + Interfaces: []kubevirtv1.VirtualMachineInstanceNetworkInterface{ + { + Name: networkDefault, + IPs: []string{networkDefaultIP, network120IP, network130IP}, + }, + }, + }, + }, + output: []v1.NodeAddress{ + { + Type: v1.NodeInternalIP, + Address: networkDefaultIP, + }, + { + Type: v1.NodeInternalIP, + Address: network120IP, + }, + { + Type: v1.NodeInternalIP, + Address: network130IP, + }, + { + Type: v1.NodeHostName, + Address: nodeName, + }, + }, + }, + { + name: "Multiple IPs on one interface, mix internal and external", + node: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: nodeName, + Annotations: map[string]string{ + api.AnnotationAlphaProvidedIPAddr: networkDefaultIP, + KeyAdditionalInternalIPs: "[\"192.168.130.10\"]", + }, + }, + }, + vmi: &kubevirtv1.VirtualMachineInstance{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testNamespace, + Name: nodeName, + }, + Spec: kubevirtv1.VirtualMachineInstanceSpec{ + Networks: []kubevirtv1.Network{ + { + Name: networkDefault, + }, + }, + }, + Status: kubevirtv1.VirtualMachineInstanceStatus{ + Interfaces: []kubevirtv1.VirtualMachineInstanceNetworkInterface{ + { + Name: networkDefault, + IPs: []string{networkDefaultIP, network120IP, network130IP}, + }, + }, + }, + }, + output: []v1.NodeAddress{ + { + Type: v1.NodeInternalIP, + Address: networkDefaultIP, + }, + { + Type: v1.NodeExternalIP, + Address: network120IP, + }, + { + Type: v1.NodeInternalIP, + Address: network130IP, + }, + { + Type: v1.NodeHostName, + Address: nodeName, + }, + }, + }, + { + name: "Extra user defined internal IPs as range", + node: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: nodeName, + Annotations: map[string]string{ + api.AnnotationAlphaProvidedIPAddr: networkDefaultIP, + KeyAdditionalInternalIPs: "[\"172.20.0.0/24\", \"192.168.130.10\", \"2001:db8::1/64\"]", + }, + }, + }, + vmi: &kubevirtv1.VirtualMachineInstance{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testNamespace, + Name: nodeName, + }, + Spec: kubevirtv1.VirtualMachineInstanceSpec{ + Networks: []kubevirtv1.Network{ + { + Name: networkDefault, + }, + { + Name: network130, + }, + }, + }, + Status: kubevirtv1.VirtualMachineInstanceStatus{ + Interfaces: []kubevirtv1.VirtualMachineInstanceNetworkInterface{ + { + Name: networkDefault, + IPs: []string{networkDefaultIP, network120IP, "172.20.0.111", "2001:db8::1", "2001:f00::1"}, + }, + { + Name: network130, + IPs: []string{network130IP, "172.20.0.222", "2001:db8::2"}, + }, + }, + }, + }, + output: []v1.NodeAddress{ + { + Type: v1.NodeInternalIP, + Address: networkDefaultIP, + }, + { + Type: v1.NodeExternalIP, + Address: network120IP, + }, + { + Type: v1.NodeInternalIP, + Address: "172.20.0.111", + }, + { + Type: v1.NodeInternalIP, + Address: "2001:db8::1", + }, + { + Type: v1.NodeExternalIP, + Address: "2001:f00::1", + }, + { + Type: v1.NodeInternalIP, + Address: network130IP, + }, + { + Type: v1.NodeInternalIP, + Address: "172.20.0.222", + }, + { + Type: v1.NodeInternalIP, + Address: "2001:db8::2", + }, + { + Type: v1.NodeHostName, + Address: nodeName, + }, + }, + }, + { + name: "No provided node IP annotation", + node: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: nodeName, + Annotations: map[string]string{}, + }, + }, + vmi: &kubevirtv1.VirtualMachineInstance{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testNamespace, + Name: nodeName, + }, + Spec: kubevirtv1.VirtualMachineInstanceSpec{ + Networks: []kubevirtv1.Network{ + { + Name: networkDefault, + }, + { + Name: network130, + }, + }, + }, + Status: kubevirtv1.VirtualMachineInstanceStatus{ + Interfaces: []kubevirtv1.VirtualMachineInstanceNetworkInterface{ + { + Name: networkDefault, + IPs: []string{"172.20.0.111", "2001:db8::1", "2001:f00::1"}, + }, + { + Name: network130, + IPs: []string{"172.20.0.222", "2001:db8::2"}, + }, + }, + }, + }, + output: []v1.NodeAddress{ + { + Type: v1.NodeInternalIP, + Address: "172.20.0.111", + }, + { + Type: v1.NodeInternalIP, + Address: "2001:db8::1", + }, + { + Type: v1.NodeInternalIP, + Address: "2001:f00::1", + }, + { + Type: v1.NodeInternalIP, + Address: "172.20.0.222", + }, + { + Type: v1.NodeInternalIP, + Address: "2001:db8::2", + }, + { + Type: v1.NodeHostName, + Address: nodeName, + }, + }, + }, + { + name: "Malformed node IP annotation", + node: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: nodeName, + Annotations: map[string]string{ + api.AnnotationAlphaProvidedIPAddr: "broken", + }, + }, + }, + wantErr: "annotation \"alpha.kubernetes.io/provided-node-ip\" is invalid: failed to parse IP address \"broken\": ParseAddr(\"broken\"): unable to parse IP", + }, } checkOutputEqual := func(expected, output []v1.NodeAddress) bool { @@ -369,7 +625,16 @@ func Test_getNodeAddresses(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - ips := getNodeAddresses(tt.node, tt.vmi) + ips, err := getNodeAddresses(tt.node, tt.vmi) + + var errStr string + if err != nil { + errStr = err.Error() + } + + if errStr != tt.wantErr { + t.Errorf("getNodeAddresses() error = %v, wantErr %v", err, tt.wantErr) + } if !checkOutputEqual(tt.output, ips) { t.Errorf("case %v failed, expected output %+v, real output: %+v", tt.name, tt.output, ips) }