Honor status.podIP over status.podIPs, node.spec.podCIDR over node.spec.podCIDRs

pkg/apis/core/v1/conversion.go

@@ -253,9 +253,13 @@ func Convert_v1_PodStatus_To_core_PodStatus(in *v1.PodStatus, out *core.PodStatu
 		return err
 	}
 
-	// If both fields (v1.PodIPs and v1.PodIP) are provided, then test v1.PodIP == v1.PodIPs[0]
+	// If both fields (v1.PodIPs and v1.PodIP) are provided and differ, then PodIP is authoritative for compatibility with older kubelets
 	if (len(in.PodIP) > 0 && len(in.PodIPs) > 0) && (in.PodIP != in.PodIPs[0].IP) {
-		return fmt.Errorf("conversion Error: v1.PodIP(%v) != v1.PodIPs[0](%v)", in.PodIP, in.PodIPs[0].IP)
+		out.PodIPs = []core.PodIP{
+			{
+				IP: in.PodIP,
+			},
+		}
 	}
 	// at the this point, autoConvert copied v1.PodIPs -> core.PodIPs
 	// if v1.PodIPs was empty but v1.PodIP is not, then set core.PodIPs[0] with v1.PodIP
@@ -321,9 +325,9 @@ func Convert_v1_NodeSpec_To_core_NodeSpec(in *v1.NodeSpec, out *core.NodeSpec, s
 	if err := autoConvert_v1_NodeSpec_To_core_NodeSpec(in, out, s); err != nil {
 		return err
 	}
-	// If both fields (v1.PodCIDRs and v1.PodCIDR) are provided, then test v1.PodCIDR == v1.PodCIDRs[0]
+	// If both fields (v1.PodCIDRs and v1.PodCIDR) are provided and differ, then PodCIDR is authoritative for compatibility with older clients
 	if (len(in.PodCIDR) > 0 && len(in.PodCIDRs) > 0) && (in.PodCIDR != in.PodCIDRs[0]) {
-		return fmt.Errorf("conversion Error: v1.PodCIDR(%v) != v1.CIDRs[0](%v)", in.PodCIDR, in.PodCIDRs[0])
+		out.PodCIDRs = []string{in.PodCIDR}
 	}
 
 	// at the this point, autoConvert copied v1.PodCIDRs -> core.PodCIDRs

pkg/apis/core/v1/conversion_test.go

@@ -398,16 +398,60 @@ func Test_core_PodStatus_to_v1_PodStatus(t *testing.T) {
 	}
 }
 func Test_v1_PodStatus_to_core_PodStatus(t *testing.T) {
-	// fail
+	asymmetricInputs := []struct {
-	v1FailTestInputs := []v1.PodStatus{
+		name string
+		in   v1.PodStatus
+		out  core.PodStatus
+	}{
 		{
-			PodIP: "1.1.2.1", // fail becaue PodIP != PodIPs[0]
+			name: "mismatched podIP",
-			PodIPs: []v1.PodIP{
+			in: v1.PodStatus{
-				{IP: "1.1.1.1"},
+				PodIP: "1.1.2.1", // Older field takes precedence for compatibility with patch by older clients
-				{IP: "2.2.2.2"},
+				PodIPs: []v1.PodIP{
+					{IP: "1.1.1.1"},
+					{IP: "2.2.2.2"},
+				},
+			},
+			out: core.PodStatus{
+				PodIPs: []core.PodIP{
+					{IP: "1.1.2.1"},
+				},
+			},
+		},
+		{
+			name: "matching podIP",
+			in: v1.PodStatus{
+				PodIP: "1.1.1.1",
+				PodIPs: []v1.PodIP{
+					{IP: "1.1.1.1"},
+					{IP: "2.2.2.2"},
+				},
+			},
+			out: core.PodStatus{
+				PodIPs: []core.PodIP{
+					{IP: "1.1.1.1"},
+					{IP: "2.2.2.2"},
+				},
+			},
+		},
+		{
+			name: "empty podIP",
+			in: v1.PodStatus{
+				PodIP: "",
+				PodIPs: []v1.PodIP{
+					{IP: "1.1.1.1"},
+					{IP: "2.2.2.2"},
+				},
+			},
+			out: core.PodStatus{
+				PodIPs: []core.PodIP{
+					{IP: "1.1.1.1"},
+					{IP: "2.2.2.2"},
+				},
 			},
 		},
 	}
+
 	// success
 	v1TestInputs := []v1.PodStatus{
 		// only Primary IP Provided
@@ -451,12 +495,18 @@ func Test_v1_PodStatus_to_core_PodStatus(t *testing.T) {
 			},
 		},
 	}
-	// run failed cases
+
-	for i, failedTest := range v1FailTestInputs {
+	// run asymmetric cases
+	for _, tc := range asymmetricInputs {
+		testInput := tc.in
+
 		corePodStatus := core.PodStatus{}
 		// convert..
-		if err := corev1.Convert_v1_PodStatus_To_core_PodStatus(&failedTest, &corePodStatus, nil); err == nil {
+		if err := corev1.Convert_v1_PodStatus_To_core_PodStatus(&testInput, &corePodStatus, nil); err != nil {
-			t.Errorf("%v: Convert v1.PodStatus to core.PodStatus should have failed for input %+v", i, failedTest)
+			t.Errorf("%s: Convert v1.PodStatus to core.PodStatus failed with error:%v for input %+v", tc.name, err.Error(), testInput)
+		}
+		if !reflect.DeepEqual(corePodStatus, tc.out) {
+			t.Errorf("%s: expected %#v, got %#v", tc.name, tc.out.PodIPs, corePodStatus.PodIPs)
 		}
 	}
 
@@ -541,10 +591,40 @@ func Test_core_NodeSpec_to_v1_NodeSpec(t *testing.T) {
 }
 
 func Test_v1_NodeSpec_to_core_NodeSpec(t *testing.T) {
-	failInputs := []v1.NodeSpec{
+	asymmetricInputs := []struct {
-		{ // fail PodCIDRs[0] != PodCIDR
+		name string
-			PodCIDR:  "10.0.0.0/24",
+		in   v1.NodeSpec
-			PodCIDRs: []string{"10.0.1.0/24", "ace:cab:deca::/8"},
+		out  core.NodeSpec
+	}{
+		{
+			name: "mismatched podCIDR",
+			in: v1.NodeSpec{
+				PodCIDR:  "10.0.0.0/24",
+				PodCIDRs: []string{"10.0.1.0/24", "ace:cab:deca::/8"},
+			},
+			out: core.NodeSpec{
+				PodCIDRs: []string{"10.0.0.0/24"},
+			},
+		},
+		{
+			name: "unset podCIDR",
+			in: v1.NodeSpec{
+				PodCIDR:  "",
+				PodCIDRs: []string{"10.0.1.0/24", "ace:cab:deca::/8"},
+			},
+			out: core.NodeSpec{
+				PodCIDRs: []string{"10.0.1.0/24", "ace:cab:deca::/8"},
+			},
+		},
+		{
+			name: "matching podCIDR",
+			in: v1.NodeSpec{
+				PodCIDR:  "10.0.1.0/24",
+				PodCIDRs: []string{"10.0.1.0/24", "ace:cab:deca::/8"},
+			},
+			out: core.NodeSpec{
+				PodCIDRs: []string{"10.0.1.0/24", "ace:cab:deca::/8"},
+			},
 		},
 	}
 
@@ -591,11 +671,17 @@ func Test_v1_NodeSpec_to_core_NodeSpec(t *testing.T) {
 		},
 	}
 
-	// fail cases
+	// run asymmetric cases
-	for i, failInput := range failInputs {
+	for _, tc := range asymmetricInputs {
+		testInput := tc.in
+
 		coreNodeSpec := core.NodeSpec{}
-		if err := corev1.Convert_v1_NodeSpec_To_core_NodeSpec(&failInput, &coreNodeSpec, nil); err == nil {
+		// convert..
-			t.Errorf("%v: Convert v1.NodeSpec to core.NodeSpec failed. Expected an error when coreNodeSpec.PodCIDR != coreNodeSpec.PodCIDRs[0]", i)
+		if err := corev1.Convert_v1_NodeSpec_To_core_NodeSpec(&testInput, &coreNodeSpec, nil); err != nil {
+			t.Errorf("%s: Convert v1.NodeSpec to core.NodeSpec failed with error:%v for input %+v", tc.name, err.Error(), testInput)
+		}
+		if !reflect.DeepEqual(coreNodeSpec, tc.out) {
+			t.Errorf("%s: expected %#v, got %#v", tc.name, tc.out.PodCIDRs, coreNodeSpec.PodCIDRs)
 		}
 	}