Skip to content

Commit

Permalink
Add more unit tests for secondarynetwork feature (#4180)
Browse files Browse the repository at this point in the history 
Fix one of the tests (`TestPodControllerAddPod/missing_Status.PodIPs`)
and add a few more unit tests.

For #4142

Signed-off-by: Antonin Bas <abas@vmware.com>
  • Loading branch information
@antoninbas
antoninbas authored Sep 1, 2022
1 parent 5dfdf24 commit ff15b16
Show file tree
Hide file tree
Showing 2 changed files with 119 additions and 18 deletions.
26 changes: 13 additions & 13 deletions pkg/agent/secondarynetwork/podwatch/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,11 @@ import (
"context"
"encoding/json"
"fmt"
"strconv"
"strings"
"time"

corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/rand"
"k8s.io/apimachinery/pkg/util/wait"
clientset "k8s.io/client-go/kubernetes"
"k8s.io/client-go/tools/cache"
Expand Down Expand Up @@ -54,8 +52,7 @@ const (
cniPath = "/opt/cni/bin/"
defaultSecondaryInterfaceName = "eth1"
startIfaceIndex = 1
endIfaceIndex = 11
maxRandIndex = 25
endIfaceIndex = 101
)

// Set resyncPeriod to 0 to disable resyncing.
Expand Down Expand Up @@ -117,23 +114,21 @@ func podKeyGet(pod *corev1.Pod) string {
return pod.Namespace + "/" + pod.Name
}

func generatePodSecondaryIfaceName(podCNIInfo *cnipodcache.CNIConfigInfo) string {
func generatePodSecondaryIfaceName(podCNIInfo *cnipodcache.CNIConfigInfo) (string, error) {
// Assign default interface name, if podCNIInfo.NetworkConfig is empty.
if count := len(podCNIInfo.NetworkConfig); count == 0 {
return defaultSecondaryInterfaceName
return defaultSecondaryInterfaceName, nil
} else {
// Generate new interface name (eth1,eth2..eth10) and return to caller.
// Generate new interface name (eth1,eth2..eth100) and return to caller.
for ifaceIndex := startIfaceIndex; ifaceIndex < endIfaceIndex; ifaceIndex++ {
ifName := fmt.Sprintf("%s%d", "eth", ifaceIndex)
_, exist := podCNIInfo.NetworkConfig[ifName]
if !exist {
return ifName
return ifName, nil
}
}
}
// Generates random interface name and return. Above execution will try to ensure allocating interface names in order.
// If none available between eth1 to eth10 (already used per Pod), generate random name with the integer range.
return string("eth") + strconv.Itoa(rand.IntnRange(endIfaceIndex, maxRandIndex))
return "", fmt.Errorf("no more interface names")
}

func whereaboutsArgsBuilder(cmd string, interfaceName string, podCNIInfo *cnipodcache.CNIConfigInfo) *invoke.Args {
Expand Down Expand Up @@ -309,7 +304,12 @@ func (pc *PodController) configureSriovAsSecondaryInterface(pod *corev1.Pod, net
func (pc *PodController) configureSecondaryInterface(pod *corev1.Pod, network *netdefv1.NetworkSelectionElement, podCNIInfo *cnipodcache.CNIConfigInfo, cniConfig []byte) error {
// Generate and assign new interface name, If secondary interface name was not provided in Pod annotation.
if len(network.InterfaceRequest) == 0 {
network.InterfaceRequest = generatePodSecondaryIfaceName(podCNIInfo)
var err error
if network.InterfaceRequest, err = generatePodSecondaryIfaceName(podCNIInfo); err != nil {
klog.ErrorS(err, "Cannot generate interface name", "Pod", klog.KObj(pod))
// do not return error: no need to requeue
return nil
}
}
// PluginArgs added to provide additional arguments required for whereabouts v0.5.1 and above.
cmdArgs := whereaboutsArgsBuilder("ADD", network.InterfaceRequest, podCNIInfo)
Expand Down Expand Up @@ -366,7 +366,7 @@ func (pc *PodController) configureSecondaryNetwork(pod *corev1.Pod, networklist
if networkConfig.NetworkType != sriovNetworkType {
// same as above, if updated, we will not process the request again.
klog.ErrorS(err, "NetworkType not supported for Pod", "NetworkAttachmentDefinition", klog.KObj(netDefCRD), "Pod", klog.KObj(pod))
return nil
continue
}
// secondary network information retrieved from API server. Proceed to configure secondary interface now.
if err = pc.configureSecondaryInterface(pod, network, podCNIInfo, cniConfig); err != nil {
Expand Down
111 changes: 106 additions & 5 deletions pkg/agent/secondarynetwork/podwatch/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -275,11 +275,11 @@ func TestPodControllerAddPod(t *testing.T) {
podIP,
netdefv1.NetworkSelectionElement{
Name: "net1",
InterfaceRequest: "eth1",
InterfaceRequest: "eth10",
},
netdefv1.NetworkSelectionElement{
Name: "net2",
InterfaceRequest: "eth2",
InterfaceRequest: "eth11",
},
)
network1 := testNetwork("net1")
Expand All @@ -290,7 +290,7 @@ func TestPodControllerAddPod(t *testing.T) {
testNamespace,
containerID,
containerNetNs(containerID),
"eth1",
"eth10",
defaultMTU,
gomock.Any(),
gomock.Any(),
Expand All @@ -300,7 +300,7 @@ func TestPodControllerAddPod(t *testing.T) {
testNamespace,
containerID,
containerNetNs(containerID),
"eth2",
"eth11",
defaultMTU,
gomock.Any(),
gomock.Any(),
Expand Down Expand Up @@ -351,7 +351,7 @@ func TestPodControllerAddPod(t *testing.T) {
defer ctrl.Finish()
podController, _, _ := newPodController(ctrl)

pod, cniConfig := testPod(podName, containerID, podIP)
pod, cniConfig := testPod(podName, containerID, "")
network := testNetwork(networkName)

podController.podCache.AddCNIConfigInfo(cniConfig)
Expand Down Expand Up @@ -403,4 +403,105 @@ func TestPodControllerAddPod(t *testing.T) {
require.NoError(t, err, "error when creating test NetworkAttachmentDefinition")
assert.NoError(t, podController.handleAddUpdatePod(pod))
})

t.Run("no interface name", func(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
podController, mockIPAM, interfaceConfigurator := newPodController(ctrl)

pod, cniConfig := testPod(
podName,
containerID,
podIP,
netdefv1.NetworkSelectionElement{
Name: networkName,
InterfaceRequest: "",
},
netdefv1.NetworkSelectionElement{
Name: networkName,
InterfaceRequest: "",
},
)
network := testNetwork(networkName)

interfaceConfigurator.EXPECT().ConfigureSriovSecondaryInterface(
podName,
testNamespace,
containerID,
containerNetNs(containerID),
"eth1",
defaultMTU,
gomock.Any(),
gomock.Any(),
)
interfaceConfigurator.EXPECT().ConfigureSriovSecondaryInterface(
podName,
testNamespace,
containerID,
containerNetNs(containerID),
"eth2",
defaultMTU,
gomock.Any(),
gomock.Any(),
)

mockIPAM.EXPECT().GetIPAMSubnetAddress(gomock.Any(), gomock.Any()).Return(testIPAMResult("148.14.24.100/24"), nil)
mockIPAM.EXPECT().GetIPAMSubnetAddress(gomock.Any(), gomock.Any()).Return(testIPAMResult("148.14.24.101/24"), nil)

podController.podCache.AddCNIConfigInfo(cniConfig)
_, err := podController.kubeClient.CoreV1().Pods(testNamespace).Create(context.Background(), pod, metav1.CreateOptions{})
require.NoError(t, err, "error when creating test Pod")
_, err = podController.netAttachDefClient.NetworkAttachmentDefinitions(testNamespace).Create(context.Background(), network, metav1.CreateOptions{})
require.NoError(t, err, "error when creating test NetworkAttachmentDefinition")
assert.NoError(t, podController.handleAddUpdatePod(pod))
})

t.Run("error when creating interface", func(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
podController, mockIPAM, interfaceConfigurator := newPodController(ctrl)

network := testNetwork(networkName)

interfaceConfigurator.EXPECT().ConfigureSriovSecondaryInterface(
podName,
testNamespace,
containerID,
containerNetNs(containerID),
interfaceName,
defaultMTU,
gomock.Any(),
gomock.Any(),
).Return(fmt.Errorf("error when creating interface"))

mockIPAM.EXPECT().GetIPAMSubnetAddress(gomock.Any(), gomock.Any()).Return(testIPAMResult("148.14.24.100/24"), nil)
mockIPAM.EXPECT().DelIPAMSubnetAddress(gomock.Any(), gomock.Any())

podController.podCache.AddCNIConfigInfo(cniConfig)
_, err := podController.kubeClient.CoreV1().Pods(testNamespace).Create(context.Background(), pod, metav1.CreateOptions{})
require.NoError(t, err, "error when creating test Pod")
_, err = podController.netAttachDefClient.NetworkAttachmentDefinitions(testNamespace).Create(context.Background(), network, metav1.CreateOptions{})
require.NoError(t, err, "error when creating test NetworkAttachmentDefinition")
assert.Error(t, podController.handleAddUpdatePod(pod))
})

t.Run("invalid networks annotation", func(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
podController, _, _ := newPodController(ctrl)

pod, cniConfig := testPod(podName, containerID, podIP)
pod.Annotations = map[string]string{
networkAttachDefAnnotationKey: "<invalid>",
}
network := testNetwork(networkName)

podController.podCache.AddCNIConfigInfo(cniConfig)
_, err := podController.kubeClient.CoreV1().Pods(testNamespace).Create(context.Background(), pod, metav1.CreateOptions{})
require.NoError(t, err, "error when creating test Pod")
_, err = podController.netAttachDefClient.NetworkAttachmentDefinitions(testNamespace).Create(context.Background(), network, metav1.CreateOptions{})
require.NoError(t, err, "error when creating test NetworkAttachmentDefinition")
// we don't expect an error here, no requeueing
assert.NoError(t, podController.handleAddUpdatePod(pod))
})
}

0 comments on commit ff15b16

Please sign in to comment.