Skip to content

Commit f221dbb

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#88505 from liggitt/pod-ip-patch
Honor status.podIP over status.podIPs and node.spec.podCIDR over node.spec.PodCIDRs when mismatched
2 parents 6c55e4b + 60da52a commit f221dbb

File tree

2 files changed

+112
-22
lines changed

2 files changed

+112
-22
lines changed

‎pkg/apis/core/v1/conversion.go‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -253,9 +253,13 @@ func Convert_v1_PodStatus_To_core_PodStatus(in *v1.PodStatus, out *core.PodStatu
253253
return err
254254
}
255255

256-
// If both fields (v1.PodIPs and v1.PodIP) are provided, then test v1.PodIP == v1.PodIPs[0]
256+
// If both fields (v1.PodIPs and v1.PodIP) are provided and differ, then PodIP is authoritative for compatibility with older kubelets
257257
if (len(in.PodIP) > 0 && len(in.PodIPs) > 0) && (in.PodIP != in.PodIPs[0].IP) {
258-
return fmt.Errorf("conversion Error: v1.PodIP(%v) != v1.PodIPs[0](%v)", in.PodIP, in.PodIPs[0].IP)
258+
out.PodIPs = []core.PodIP{
259+
{
260+
IP: in.PodIP,
261+
},
262+
}
259263
}
260264
// at the this point, autoConvert copied v1.PodIPs -> core.PodIPs
261265
// 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
321325
if err := autoConvert_v1_NodeSpec_To_core_NodeSpec(in, out, s); err != nil {
322326
return err
323327
}
324-
// If both fields (v1.PodCIDRs and v1.PodCIDR) are provided, then test v1.PodCIDR == v1.PodCIDRs[0]
328+
// If both fields (v1.PodCIDRs and v1.PodCIDR) are provided and differ, then PodCIDR is authoritative for compatibility with older clients
325329
if (len(in.PodCIDR) > 0 && len(in.PodCIDRs) > 0) && (in.PodCIDR != in.PodCIDRs[0]) {
326-
return fmt.Errorf("conversion Error: v1.PodCIDR(%v) != v1.CIDRs[0](%v)", in.PodCIDR, in.PodCIDRs[0])
330+
out.PodCIDRs = []string{in.PodCIDR}
327331
}
328332

329333
// at the this point, autoConvert copied v1.PodCIDRs -> core.PodCIDRs

‎pkg/apis/core/v1/conversion_test.go‎

Lines changed: 104 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -398,16 +398,60 @@ func Test_core_PodStatus_to_v1_PodStatus(t *testing.T) {
398398
}
399399
}
400400
func Test_v1_PodStatus_to_core_PodStatus(t *testing.T) {
401-
// fail
402-
v1FailTestInputs := []v1.PodStatus{
401+
asymmetricInputs := []struct {
402+
name string
403+
in v1.PodStatus
404+
out core.PodStatus
405+
}{
403406
{
404-
PodIP: "1.1.2.1", // fail becaue PodIP != PodIPs[0]
405-
PodIPs: []v1.PodIP{
406-
{IP: "1.1.1.1"},
407-
{IP: "2.2.2.2"},
407+
name: "mismatched podIP",
408+
in: v1.PodStatus{
409+
PodIP: "1.1.2.1", // Older field takes precedence for compatibility with patch by older clients
410+
PodIPs: []v1.PodIP{
411+
{IP: "1.1.1.1"},
412+
{IP: "2.2.2.2"},
413+
},
414+
},
415+
out: core.PodStatus{
416+
PodIPs: []core.PodIP{
417+
{IP: "1.1.2.1"},
418+
},
419+
},
420+
},
421+
{
422+
name: "matching podIP",
423+
in: v1.PodStatus{
424+
PodIP: "1.1.1.1",
425+
PodIPs: []v1.PodIP{
426+
{IP: "1.1.1.1"},
427+
{IP: "2.2.2.2"},
428+
},
429+
},
430+
out: core.PodStatus{
431+
PodIPs: []core.PodIP{
432+
{IP: "1.1.1.1"},
433+
{IP: "2.2.2.2"},
434+
},
435+
},
436+
},
437+
{
438+
name: "empty podIP",
439+
in: v1.PodStatus{
440+
PodIP: "",
441+
PodIPs: []v1.PodIP{
442+
{IP: "1.1.1.1"},
443+
{IP: "2.2.2.2"},
444+
},
445+
},
446+
out: core.PodStatus{
447+
PodIPs: []core.PodIP{
448+
{IP: "1.1.1.1"},
449+
{IP: "2.2.2.2"},
450+
},
408451
},
409452
},
410453
}
454+
411455
// success
412456
v1TestInputs := []v1.PodStatus{
413457
// only Primary IP Provided
@@ -451,12 +495,18 @@ func Test_v1_PodStatus_to_core_PodStatus(t *testing.T) {
451495
},
452496
},
453497
}
454-
// run failed cases
455-
for i, failedTest := range v1FailTestInputs {
498+
499+
// run asymmetric cases
500+
for _, tc := range asymmetricInputs {
501+
testInput := tc.in
502+
456503
corePodStatus := core.PodStatus{}
457504
// convert..
458-
if err := corev1.Convert_v1_PodStatus_To_core_PodStatus(&failedTest, &corePodStatus, nil); err == nil {
459-
t.Errorf("%v: Convert v1.PodStatus to core.PodStatus should have failed for input %+v", i, failedTest)
505+
if err := corev1.Convert_v1_PodStatus_To_core_PodStatus(&testInput, &corePodStatus, nil); err != nil {
506+
t.Errorf("%s: Convert v1.PodStatus to core.PodStatus failed with error:%v for input %+v", tc.name, err.Error(), testInput)
507+
}
508+
if !reflect.DeepEqual(corePodStatus, tc.out) {
509+
t.Errorf("%s: expected %#v, got %#v", tc.name, tc.out.PodIPs, corePodStatus.PodIPs)
460510
}
461511
}
462512

@@ -541,10 +591,40 @@ func Test_core_NodeSpec_to_v1_NodeSpec(t *testing.T) {
541591
}
542592

543593
func Test_v1_NodeSpec_to_core_NodeSpec(t *testing.T) {
544-
failInputs := []v1.NodeSpec{
545-
{ // fail PodCIDRs[0] != PodCIDR
546-
PodCIDR: "10.0.0.0/24",
547-
PodCIDRs: []string{"10.0.1.0/24", "ace:cab:deca::/8"},
594+
asymmetricInputs := []struct {
595+
name string
596+
in v1.NodeSpec
597+
out core.NodeSpec
598+
}{
599+
{
600+
name: "mismatched podCIDR",
601+
in: v1.NodeSpec{
602+
PodCIDR: "10.0.0.0/24",
603+
PodCIDRs: []string{"10.0.1.0/24", "ace:cab:deca::/8"},
604+
},
605+
out: core.NodeSpec{
606+
PodCIDRs: []string{"10.0.0.0/24"},
607+
},
608+
},
609+
{
610+
name: "unset podCIDR",
611+
in: v1.NodeSpec{
612+
PodCIDR: "",
613+
PodCIDRs: []string{"10.0.1.0/24", "ace:cab:deca::/8"},
614+
},
615+
out: core.NodeSpec{
616+
PodCIDRs: []string{"10.0.1.0/24", "ace:cab:deca::/8"},
617+
},
618+
},
619+
{
620+
name: "matching podCIDR",
621+
in: v1.NodeSpec{
622+
PodCIDR: "10.0.1.0/24",
623+
PodCIDRs: []string{"10.0.1.0/24", "ace:cab:deca::/8"},
624+
},
625+
out: core.NodeSpec{
626+
PodCIDRs: []string{"10.0.1.0/24", "ace:cab:deca::/8"},
627+
},
548628
},
549629
}
550630

@@ -591,11 +671,17 @@ func Test_v1_NodeSpec_to_core_NodeSpec(t *testing.T) {
591671
},
592672
}
593673

594-
// fail cases
595-
for i, failInput := range failInputs {
674+
// run asymmetric cases
675+
for _, tc := range asymmetricInputs {
676+
testInput := tc.in
677+
596678
coreNodeSpec := core.NodeSpec{}
597-
if err := corev1.Convert_v1_NodeSpec_To_core_NodeSpec(&failInput, &coreNodeSpec, nil); err == nil {
598-
t.Errorf("%v: Convert v1.NodeSpec to core.NodeSpec failed. Expected an error when coreNodeSpec.PodCIDR != coreNodeSpec.PodCIDRs[0]", i)
679+
// convert..
680+
if err := corev1.Convert_v1_NodeSpec_To_core_NodeSpec(&testInput, &coreNodeSpec, nil); err != nil {
681+
t.Errorf("%s: Convert v1.NodeSpec to core.NodeSpec failed with error:%v for input %+v", tc.name, err.Error(), testInput)
682+
}
683+
if !reflect.DeepEqual(coreNodeSpec, tc.out) {
684+
t.Errorf("%s: expected %#v, got %#v", tc.name, tc.out.PodCIDRs, coreNodeSpec.PodCIDRs)
599685
}
600686
}
601687

0 commit comments

Comments
 (0)