Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow "kubelet --node-ip ::" to mean prefer IPv6 #85850

Merged
merged 1 commit into from Jan 14, 2020
+122 −12
Merged

Allow "kubelet --node-ip ::" to mean prefer IPv6 #85850

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/kubelet/app/options/options.go
View file
Open in desktop
Expand Up @@ -365,7 +365,7 @@ func (f *KubeletFlags) AddFlags(mainfs *pflag.FlagSet) {

fs.StringVar(&f.HostnameOverride, "hostname-override", f.HostnameOverride, "If non-empty, will use this string as identification instead of the actual hostname. If --cloud-provider is set, the cloud provider determines the name of the node (consult cloud provider documentation to determine if and how the hostname is used).")

fs.StringVar(&f.NodeIP, "node-ip", f.NodeIP, "IP address of the node. If set, kubelet will use this IP address for the node")
fs.StringVar(&f.NodeIP, "node-ip", f.NodeIP, "IP address of the node. If set, kubelet will use this IP address for the node. If unset, kubelet will use the node's default IPv4 address, if any, or its default IPv6 address if it has no IPv4 addresses. You can pass `::` to make it prefer the default IPv6 address rather than the default IPv4 address.")

fs.StringVar(&f.ProviderID, "provider-id", f.ProviderID, "Unique identifier for identifying the node in a machine database, i.e cloudprovider")

Expand Down
44 changes: 33 additions & 11 deletions pkg/kubelet/nodestatus/setters.go
View file
Open in desktop
Expand Up @@ -65,16 +65,20 @@ func NodeAddress(nodeIP net.IP, // typically Kubelet.nodeIP
cloud cloudprovider.Interface, // typically Kubelet.cloud
nodeAddressesFunc func() ([]v1.NodeAddress, error), // typically Kubelet.cloudResourceSyncManager.NodeAddresses
) Setter {
preferIPv4 := nodeIP == nil || nodeIP.To4() != nil
isPreferredIPFamily := func(ip net.IP) bool { return (ip.To4() != nil) == preferIPv4 }
nodeIPSpecified := nodeIP != nil && !nodeIP.IsUnspecified()

return func(node *v1.Node) error {
if nodeIP != nil {
if nodeIPSpecified {
if err := validateNodeIPFunc(nodeIP); err != nil {
return fmt.Errorf("failed to validate nodeIP: %v", err)
}
klog.V(2).Infof("Using node IP: %q", nodeIP.String())
}

if externalCloudProvider {
if nodeIP != nil {
if nodeIPSpecified {
if node.ObjectMeta.Annotations == nil {
node.ObjectMeta.Annotations = make(map[string]string)
}
Expand All @@ -101,7 +105,7 @@ func NodeAddress(nodeIP net.IP, // typically Kubelet.nodeIP
// that address Type (like InternalIP and ExternalIP), meaning other addresses of the same Type are discarded.
// See #61921 for more information: some cloud providers may supply secondary IPs, so nodeIP serves as a way to
// ensure that the correct IPs show up on a Node object.
if nodeIP != nil {
if nodeIPSpecified {
enforcedNodeAddresses := []v1.NodeAddress{}

nodeIPTypes := make(map[v1.NodeAddressType]bool)
Expand All @@ -125,6 +129,23 @@ func NodeAddress(nodeIP net.IP, // typically Kubelet.nodeIP
}

nodeAddresses = enforcedNodeAddresses
} else if nodeIP != nil {
// nodeIP is "0.0.0.0" or "::"; sort cloudNodeAddresses to
// prefer addresses of the matching family
sortedAddresses := make([]v1.NodeAddress, 0, len(cloudNodeAddresses))
for _, nodeAddress := range cloudNodeAddresses {
ip := net.ParseIP(nodeAddress.Address)
if ip == nil || isPreferredIPFamily(ip) {
sortedAddresses = append(sortedAddresses, nodeAddress)
}
}
for _, nodeAddress := range cloudNodeAddresses {
ip := net.ParseIP(nodeAddress.Address)
if ip != nil && !isPreferredIPFamily(ip) {
sortedAddresses = append(sortedAddresses, nodeAddress)
}
}
nodeAddresses = sortedAddresses
} else {
// If nodeIP is unset, just use the addresses provided by the cloud provider as-is
nodeAddresses = cloudNodeAddresses
Expand Down Expand Up @@ -168,12 +189,14 @@ func NodeAddress(nodeIP net.IP, // typically Kubelet.nodeIP
var ipAddr net.IP
var err error

// 1) Use nodeIP if set
// 1) Use nodeIP if set (and not "0.0.0.0"/"::")
// 2) If the user has specified an IP to HostnameOverride, use it
// 3) Lookup the IP from node name by DNS and use the first valid IPv4 address.
// If the node does not have a valid IPv4 address, use the first valid IPv6 address.
// 3) Lookup the IP from node name by DNS
// 4) Try to get the IP from the network interface used as default gateway
if nodeIP != nil {
//
// For steps 3 and 4, IPv4 addresses are preferred to IPv6 addresses
// unless nodeIP is "::", in which case it is reversed.

This comment was marked as resolved.

Sign in to view

if nodeIPSpecified {
ipAddr = nodeIP
} else if addr := net.ParseIP(hostname); addr != nil {
ipAddr = addr
Expand All @@ -182,18 +205,17 @@ func NodeAddress(nodeIP net.IP, // typically Kubelet.nodeIP
addrs, _ = net.LookupIP(node.Name)
for _, addr := range addrs {
if err = validateNodeIPFunc(addr); err == nil {
if addr.To4() != nil {
if isPreferredIPFamily(addr) {
ipAddr = addr
break
}
if addr.To16() != nil && ipAddr == nil {
} else if ipAddr == nil {
ipAddr = addr
}
}
}

if ipAddr == nil {
ipAddr, err = utilnet.ChooseHostInterface()
ipAddr, err = utilnet.ResolveBindAddress(nodeIP)
}
}

Expand Down
88 changes: 88 additions & 0 deletions pkg/kubelet/nodestatus/setters_test.go
View file
Open in desktop
Expand Up @@ -293,6 +293,94 @@ func TestNodeAddress(t *testing.T) {
hostnameOverride: true,
shouldError: false,
},
{
name: "Dual-stack cloud, IPv4 first, no nodeIP",
nodeAddresses: []v1.NodeAddress{
{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
{Type: v1.NodeInternalIP, Address: "fc01:1234::5678"},
{Type: v1.NodeHostName, Address: testKubeletHostname},
},
expectedAddresses: []v1.NodeAddress{
{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
{Type: v1.NodeInternalIP, Address: "fc01:1234::5678"},
{Type: v1.NodeHostName, Address: testKubeletHostname},
},
shouldError: false,
},
{
name: "Dual-stack cloud, IPv6 first, no nodeIP",
nodeAddresses: []v1.NodeAddress{
{Type: v1.NodeInternalIP, Address: "fc01:1234::5678"},
{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
{Type: v1.NodeHostName, Address: testKubeletHostname},
},
expectedAddresses: []v1.NodeAddress{
{Type: v1.NodeInternalIP, Address: "fc01:1234::5678"},
{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
{Type: v1.NodeHostName, Address: testKubeletHostname},
},
shouldError: false,
},
{
name: "Dual-stack cloud, IPv4 first, request IPv4",
nodeIP: net.ParseIP("0.0.0.0"),
nodeAddresses: []v1.NodeAddress{
{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
{Type: v1.NodeInternalIP, Address: "fc01:1234::5678"},
{Type: v1.NodeHostName, Address: testKubeletHostname},
},
expectedAddresses: []v1.NodeAddress{
{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
{Type: v1.NodeHostName, Address: testKubeletHostname},
{Type: v1.NodeInternalIP, Address: "fc01:1234::5678"},
},
shouldError: false,
},
{
name: "Dual-stack cloud, IPv6 first, request IPv4",
nodeIP: net.ParseIP("0.0.0.0"),
nodeAddresses: []v1.NodeAddress{
{Type: v1.NodeInternalIP, Address: "fc01:1234::5678"},
{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
{Type: v1.NodeHostName, Address: testKubeletHostname},
},
expectedAddresses: []v1.NodeAddress{
{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
{Type: v1.NodeHostName, Address: testKubeletHostname},
{Type: v1.NodeInternalIP, Address: "fc01:1234::5678"},
},
shouldError: false,
},
{
name: "Dual-stack cloud, IPv4 first, request IPv6",
nodeIP: net.ParseIP("::"),
nodeAddresses: []v1.NodeAddress{
{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
{Type: v1.NodeInternalIP, Address: "fc01:1234::5678"},
{Type: v1.NodeHostName, Address: testKubeletHostname},
},
expectedAddresses: []v1.NodeAddress{
{Type: v1.NodeInternalIP, Address: "fc01:1234::5678"},
{Type: v1.NodeHostName, Address: testKubeletHostname},
{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
},
shouldError: false,
},
{
name: "Dual-stack cloud, IPv6 first, request IPv6",
nodeIP: net.ParseIP("::"),
nodeAddresses: []v1.NodeAddress{
{Type: v1.NodeInternalIP, Address: "fc01:1234::5678"},
{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
{Type: v1.NodeHostName, Address: testKubeletHostname},
},
expectedAddresses: []v1.NodeAddress{
{Type: v1.NodeInternalIP, Address: "fc01:1234::5678"},
{Type: v1.NodeHostName, Address: testKubeletHostname},
{Type: v1.NodeInternalIP, Address: "10.1.1.1"},
},
shouldError: false,
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we add at least one IPv6 single stack test?

if you specify --node-ip 2600::1234 and the cloud provider returns "InternalIP 10.1.2.3, InternalIP 2600::1234, Hostname foo.example.com", then the code in pkg/kubelet/nodestatus/setters.go will filter out the non-matching InternalIP and set the node's addresses to just "InternalIP 2600::1234, Hostname foo.example.com", resulting in the provided --node-ip being primary

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code in question doesn't actually care about ipv4 vs ipv6; if you specify --node-ip and it matches one of the InternalIPs, then every other InternalIP is dropped. And that's already tested by one of the other cases.

}
for _, testCase := range cases {
t.Run(testCase.name, func(t *testing.T) {
Expand Down