add ip_pref CNI options for primary pod ip

This fixes the TODO of this function and also expands on how the primary pod ip
is selected. This change allows the operator to prefer ipv4, ipv6, or retain the
ordering provided by the return results of the CNI plugins.

This makes it much more flexible for ops to configure containerd and how IPs are
set on the pod.

Signed-off-by: Michael Crosby <[email protected]>

docs/cri/config.md

@@ -238,6 +238,12 @@ version = 2
     # This will be deprecated when kubenet is deprecated.
     # See the "CNI Config Template" section for more details.
     conf_template = ""
+    # ip_pref specifies the strategy to use when selecting the main IP address for a pod.
+    # options include:
+    # * ipv4, "" - (default) select the first ipv4 address
+    # * ipv6 - select the first ipv6 address
+    # * cni - use the order returned by the CNI plugins, returning the first IP address from the results
+    ip_pref = "ipv4"
 
   # 'plugins."io.containerd.grpc.v1.cri".image_decryption' contains config related
   # to handling decryption of encrypted container images.

pkg/cri/config/config.go

@@ -111,6 +111,13 @@ type CniConfig struct {
 	// a temporary backward-compatible solution for them.
 	// TODO(random-liu): Deprecate this option when kubenet is deprecated.
 	NetworkPluginConfTemplate string `toml:"conf_template" json:"confTemplate"`
+	// IPPreference specifies the strategy to use when selecting the main IP address for a pod.
+	//
+	// Options include:
+	// * ipv4, "" - (default) select the first ipv4 address
+	// * ipv6 - select the first ipv6 address
+	// * cni - use the order returned by the CNI plugins, returning the first IP address from the results
+	IPPreference string `toml:"ip_pref" json:"ipPref"`
 }
 
 // Mirror contains the config related to the registry mirror

pkg/cri/server/sandbox_run.go

@@ -372,7 +372,7 @@ func (c *criService) setupPodNetwork(ctx context.Context, sandbox *sandboxstore.
 	logDebugCNIResult(ctx, id, result)
 	// Check if the default interface has IP config
 	if configs, ok := result.Interfaces[defaultIfName]; ok && len(configs.IPConfigs) > 0 {
-		sandbox.IP, sandbox.AdditionalIPs = selectPodIPs(configs.IPConfigs)
+		sandbox.IP, sandbox.AdditionalIPs = selectPodIPs(ctx, configs.IPConfigs, c.config.IPPreference)
 		sandbox.CNIResult = result
 		return nil
 	}
@@ -475,28 +475,46 @@ func toCNIDNS(dns *runtime.DNSConfig) *cni.DNS {
 	}
 }
 
-// selectPodIPs select an ip from the ip list. It prefers ipv4 more than ipv6
-// and returns the additional ips
-// TODO(random-liu): Revisit the ip order in the ipv6 beta stage. (cri#1278)
-func selectPodIPs(ipConfigs []*cni.IPConfig) (string, []string) {
-	var (
-		additionalIPs []string
-		ip            string
-	)
-	for _, c := range ipConfigs {
-		if c.IP.To4() != nil && ip == "" {
-			ip = c.IP.String()
-		} else {
-			additionalIPs = append(additionalIPs, c.IP.String())
-		}
+// selectPodIPs select an ip from the ip list.
+func selectPodIPs(ctx context.Context, configs []*cni.IPConfig, preference string) (string, []string) {
+	if len(configs) == 1 {
+		return ipString(configs[0]), nil
 	}
-	if ip != "" {
-		return ip, additionalIPs
+	toStrings := func(ips []*cni.IPConfig) (o []string) {
+		for _, i := range ips {
+			o = append(o, ipString(i))
+		}
+		return o
 	}
-	if len(ipConfigs) == 1 {
-		return additionalIPs[0], nil
+	var extra []string
+	switch preference {
+	default:
+		if preference != "ipv4" && preference != "" {
+			log.G(ctx).WithField("ip_pref", preference).Warn("invalid ip_pref, falling back to ipv4")
+		}
+		for i, ip := range configs {
+			if ip.IP.To4() != nil {
+				return ipString(ip), append(extra, toStrings(configs[i+1:])...)
+			}
+			extra = append(extra, ipString(ip))
+		}
+	case "ipv6":
+		for i, ip := range configs {
+			if ip.IP.To16() != nil {
+				return ipString(ip), append(extra, toStrings(configs[i+1:])...)
+			}
+			extra = append(extra, ipString(ip))
+		}
+	case "cni":
+		// use func default return
 	}
-	return additionalIPs[0], additionalIPs[1:]
+
+	all := toStrings(configs)
+	return all[0], all[1:]
+}
+
+func ipString(ip *cni.IPConfig) string {
+	return ip.IP.String()
 }
 
 // untrustedWorkload returns true if the sandbox contains untrusted workload.

pkg/cri/server/sandbox_run_test.go

@@ -26,6 +26,7 @@ import (
 	imagespec "github.com/opencontainers/image-spec/specs-go/v1"
 	runtimespec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/stretchr/testify/assert"
+	"golang.org/x/net/context"
 	runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
 
 	"github.com/containerd/containerd/pkg/cri/annotations"
@@ -260,12 +261,26 @@ func TestSelectPodIP(t *testing.T) {
 		ips                   []string
 		expectedIP            string
 		expectedAdditionalIPs []string
+		pref                  string
 	}{
 		"ipv4 should be picked even if ipv6 comes first": {
 			ips:                   []string{"2001:db8:85a3::8a2e:370:7334", "192.168.17.43"},
 			expectedIP:            "192.168.17.43",
 			expectedAdditionalIPs: []string{"2001:db8:85a3::8a2e:370:7334"},
 		},
+		"ipv6 should be picked even if ipv4 comes first": {
+			ips:                   []string{"2001:db8:85a3::8a2e:370:7334", "192.168.17.43"},
+			expectedIP:            "2001:db8:85a3::8a2e:370:7334",
+			expectedAdditionalIPs: []string{"192.168.17.43"},
+			pref:                  "ipv6",
+		},
+		"order should reflect ip selection": {
+			ips:                   []string{"2001:db8:85a3::8a2e:370:7334", "192.168.17.43"},
+			expectedIP:            "2001:db8:85a3::8a2e:370:7334",
+			expectedAdditionalIPs: []string{"192.168.17.43"},
+			pref:                  "cni",
+		},
+
 		"ipv4 should be picked when there is only ipv4": {
 			ips:                   []string{"192.168.17.43"},
 			expectedIP:            "192.168.17.43",
@@ -289,7 +304,7 @@ func TestSelectPodIP(t *testing.T) {
 				IP: net.ParseIP(ip),
 			})
 		}
-		ip, additionalIPs := selectPodIPs(ipConfigs)
+		ip, additionalIPs := selectPodIPs(context.Background(), ipConfigs, test.pref)
 		assert.Equal(t, test.expectedIP, ip)
 		assert.Equal(t, test.expectedAdditionalIPs, additionalIPs)
 	}