Skip to content

Commit cdf4fee

Browse files
oilbeateroilbeater
authored
fix: acquire specified subnet from annotation (#6180)
we should not fallback to all available subnets when annotation exists. Signed-off-by: Mengxin Liu <liumengxinfly@gmail.com>
1 parent 3979f55 commit cdf4fee

File tree

2 files changed

+210
-12
lines changed

2 files changed

+210
-12
lines changed

‎pkg/controller/pod.go‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2042,17 +2042,23 @@ func (c *Controller) acquireAddress(pod *v1.Pod, podNet *kubeovnNet) (string, st
20422042
var nsNets []*kubeovnNet
20432043
ippoolStr := pod.Annotations[fmt.Sprintf(util.IPPoolAnnotationTemplate, podNet.ProviderName)]
20442044
subnetStr := pod.Annotations[fmt.Sprintf(util.LogicalSwitchAnnotationTemplate, podNet.ProviderName)]
2045+
2046+
// Prepare nsNets based on subnet annotation
2047+
var err error
2048+
if subnetStr != "" {
2049+
nsNets = []*kubeovnNet{podNet}
2050+
} else if nsNets, err = c.getNsAvailableSubnets(pod, podNet); err != nil {
2051+
klog.Errorf("failed to get available subnets for pod %s/%s, %v", pod.Namespace, pod.Name, err)
2052+
return "", "", "", podNet.Subnet, err
2053+
}
2054+
20452055
if ippoolStr == "" && podNet.IsDefault {
20462056
// no ippool specified by pod annotation, use namespace ippools or ippools in the subnet specified by pod annotation
20472057
ns, err := c.namespacesLister.Get(pod.Namespace)
20482058
if err != nil {
20492059
klog.Errorf("failed to get namespace %s: %v", pod.Namespace, err)
20502060
return "", "", "", podNet.Subnet, err
20512061
}
2052-
if nsNets, err = c.getNsAvailableSubnets(pod, podNet); err != nil {
2053-
klog.Errorf("failed to get available subnets for pod %s/%s, %v", pod.Namespace, pod.Name, err)
2054-
return "", "", "", podNet.Subnet, err
2055-
}
20562062
subnetNames := make([]string, 0, len(nsNets))
20572063
for _, net := range nsNets {
20582064
if net.Subnet.Name == subnetStr {
@@ -2152,14 +2158,6 @@ func (c *Controller) acquireStaticAddressHelper(pod *v1.Pod, podNet *kubeovnNet,
21522158
var v4IP, v6IP, mac string
21532159
var err error
21542160

2155-
// The static ip can be assigned from any subnet after ns supports multi subnets
2156-
if nsNets == nil {
2157-
if nsNets, err = c.getNsAvailableSubnets(pod, podNet); err != nil {
2158-
klog.Errorf("failed to get available subnets for pod %s/%s, %v", pod.Namespace, pod.Name, err)
2159-
return "", "", "", podNet.Subnet, err
2160-
}
2161-
}
2162-
21632161
// Static allocate
21642162
if podNet.NadName != "" && podNet.NadNamespace != "" && podNet.InterfaceName != "" {
21652163
key := perInterfaceIPAnnotationKey(podNet.NadName, podNet.NadNamespace, podNet.InterfaceName)

‎pkg/controller/pod_test.go‎

Lines changed: 200 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1212

1313
kubeovnv1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1"
14+
"github.com/kubeovn/kube-ovn/pkg/ipam"
1415
"github.com/kubeovn/kube-ovn/pkg/util"
1516
)
1617

@@ -329,3 +330,202 @@ func TestGetPodKubeovnNetsNonPrimaryCNI(t *testing.T) {
329330
})
330331
}
331332
}
333+
334+
func TestAcquireAddressWithSpecifiedSubnet(t *testing.T) {
335+
tests := []struct {
336+
name string
337+
pod *corev1.Pod
338+
namespaces []*corev1.Namespace
339+
subnets []*kubeovnv1.Subnet
340+
setupIPAM func(*Controller)
341+
expectError bool
342+
expectedSubnet string
343+
description string
344+
}{
345+
{
346+
name: "User specifies subnet - should succeed",
347+
pod: &corev1.Pod{
348+
ObjectMeta: metav1.ObjectMeta{
349+
Name: "test-pod",
350+
Namespace: "default",
351+
Annotations: map[string]string{
352+
util.LogicalSwitchAnnotation: "subnet1",
353+
util.IPAddressAnnotation: "10.0.1.10",
354+
},
355+
},
356+
},
357+
namespaces: []*corev1.Namespace{
358+
{
359+
ObjectMeta: metav1.ObjectMeta{
360+
Name: "default",
361+
Annotations: map[string]string{
362+
util.LogicalSwitchAnnotation: "subnet1,subnet2",
363+
},
364+
},
365+
},
366+
},
367+
subnets: []*kubeovnv1.Subnet{
368+
{
369+
ObjectMeta: metav1.ObjectMeta{Name: "subnet1"},
370+
Spec: kubeovnv1.SubnetSpec{
371+
CIDRBlock: "10.0.1.0/24",
372+
Protocol: kubeovnv1.ProtocolIPv4,
373+
Provider: util.OvnProvider,
374+
},
375+
Status: kubeovnv1.SubnetStatus{V4AvailableIPs: 100},
376+
},
377+
{
378+
ObjectMeta: metav1.ObjectMeta{Name: "subnet2"},
379+
Spec: kubeovnv1.SubnetSpec{
380+
CIDRBlock: "10.0.1.0/24",
381+
Protocol: kubeovnv1.ProtocolIPv4,
382+
Provider: util.OvnProvider,
383+
},
384+
Status: kubeovnv1.SubnetStatus{V4AvailableIPs: 100},
385+
},
386+
},
387+
expectError: false,
388+
expectedSubnet: "subnet1",
389+
description: "Should allocate from specified subnet",
390+
},
391+
{
392+
name: "User specifies subnet but IP occupied - should NOT fallback",
393+
pod: &corev1.Pod{
394+
ObjectMeta: metav1.ObjectMeta{
395+
Name: "test-pod",
396+
Namespace: "default",
397+
Annotations: map[string]string{
398+
util.LogicalSwitchAnnotation: "subnet1",
399+
util.IPAddressAnnotation: "10.0.1.10",
400+
},
401+
},
402+
},
403+
namespaces: []*corev1.Namespace{
404+
{
405+
ObjectMeta: metav1.ObjectMeta{
406+
Name: "default",
407+
Annotations: map[string]string{
408+
util.LogicalSwitchAnnotation: "subnet1,subnet2",
409+
},
410+
},
411+
},
412+
},
413+
subnets: []*kubeovnv1.Subnet{
414+
{
415+
ObjectMeta: metav1.ObjectMeta{Name: "subnet1"},
416+
Spec: kubeovnv1.SubnetSpec{
417+
CIDRBlock: "10.0.1.0/24",
418+
Protocol: kubeovnv1.ProtocolIPv4,
419+
Provider: util.OvnProvider,
420+
},
421+
Status: kubeovnv1.SubnetStatus{V4AvailableIPs: 100},
422+
},
423+
{
424+
ObjectMeta: metav1.ObjectMeta{Name: "subnet2"},
425+
Spec: kubeovnv1.SubnetSpec{
426+
CIDRBlock: "10.0.1.0/24",
427+
Protocol: kubeovnv1.ProtocolIPv4,
428+
Provider: util.OvnProvider,
429+
},
430+
Status: kubeovnv1.SubnetStatus{V4AvailableIPs: 100},
431+
},
432+
},
433+
setupIPAM: func(c *Controller) {
434+
_, _, _, _ = c.ipam.GetStaticAddress("other-pod.default", "other-pod.default", "10.0.1.10", nil, "subnet1", true)
435+
},
436+
expectError: true,
437+
description: "Should NOT fallback to subnet2 when IP is occupied in specified subnet1",
438+
},
439+
{
440+
name: "No subnet specified - should try all namespace subnets",
441+
pod: &corev1.Pod{
442+
ObjectMeta: metav1.ObjectMeta{
443+
Name: "test-pod",
444+
Namespace: "default",
445+
Annotations: map[string]string{
446+
util.IPAddressAnnotation: "10.0.2.10",
447+
},
448+
},
449+
},
450+
namespaces: []*corev1.Namespace{
451+
{
452+
ObjectMeta: metav1.ObjectMeta{
453+
Name: "default",
454+
Annotations: map[string]string{
455+
util.LogicalSwitchAnnotation: "subnet1,subnet2",
456+
},
457+
},
458+
},
459+
},
460+
subnets: []*kubeovnv1.Subnet{
461+
{
462+
ObjectMeta: metav1.ObjectMeta{Name: "subnet1"},
463+
Spec: kubeovnv1.SubnetSpec{
464+
CIDRBlock: "10.0.1.0/24",
465+
Protocol: kubeovnv1.ProtocolIPv4,
466+
Provider: util.OvnProvider,
467+
},
468+
Status: kubeovnv1.SubnetStatus{V4AvailableIPs: 100},
469+
},
470+
{
471+
ObjectMeta: metav1.ObjectMeta{Name: "subnet2"},
472+
Spec: kubeovnv1.SubnetSpec{
473+
CIDRBlock: "10.0.2.0/24",
474+
Protocol: kubeovnv1.ProtocolIPv4,
475+
Provider: util.OvnProvider,
476+
},
477+
Status: kubeovnv1.SubnetStatus{V4AvailableIPs: 100},
478+
},
479+
},
480+
expectError: false,
481+
expectedSubnet: "subnet2",
482+
description: "Should try all subnets and find matching one when no subnet specified",
483+
},
484+
}
485+
486+
for _, tt := range tests {
487+
t.Run(tt.name, func(t *testing.T) {
488+
fakeController, err := newFakeControllerWithOptions(t, &FakeControllerOptions{
489+
Namespaces: tt.namespaces,
490+
Subnets: tt.subnets,
491+
Pods: []*corev1.Pod{tt.pod},
492+
})
493+
require.NoError(t, err)
494+
controller := fakeController.fakeController
495+
controller.ipam = newIPAMForTest(tt.subnets)
496+
497+
if tt.setupIPAM != nil {
498+
tt.setupIPAM(controller)
499+
}
500+
501+
podNets, err := controller.getPodKubeovnNets(tt.pod)
502+
require.NoError(t, err)
503+
require.Greater(t, len(podNets), 0)
504+
505+
_, _, _, subnet, err := controller.acquireAddress(tt.pod, podNets[0])
506+
507+
if tt.expectError {
508+
assert.Error(t, err, tt.description)
509+
} else {
510+
require.NoError(t, err, tt.description)
511+
assert.Equal(t, tt.expectedSubnet, subnet.Name, tt.description)
512+
}
513+
})
514+
}
515+
}
516+
517+
func newIPAMForTest(subnets []*kubeovnv1.Subnet) *ipam.IPAM {
518+
ipamInstance := ipam.NewIPAM()
519+
for _, subnet := range subnets {
520+
excludeIPs := subnet.Spec.ExcludeIps
521+
if len(excludeIPs) == 0 {
522+
excludeIPs = []string{}
523+
}
524+
s, err := ipam.NewSubnet(subnet.Name, subnet.Spec.CIDRBlock, excludeIPs)
525+
if err != nil {
526+
panic(err)
527+
}
528+
ipamInstance.Subnets[subnet.Name] = s
529+
}
530+
return ipamInstance
531+
}

0 commit comments

Comments
 (0)