Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support secondary network without IPAM #5762

Merged
merged 1 commit into from
Dec 1, 2023
+108 −38
Merged

Support secondary network without IPAM #5762

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions pkg/agent/cniserver/secondary.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,11 @@ func (pc *podConfigurator) ConfigureSriovSecondaryInterface(
containerIface := result.Interfaces[1]
klog.InfoS("Configured SR-IOV interface", "Pod", klog.KRef(podNamespace, podName), "interface", containerInterfaceName, "hostInterface", hostIface)

if err = pc.ifConfigurator.advertiseContainerAddr(containerNetNS, containerIface.Name, result); err != nil {
klog.ErrorS(err, "Failed to advertise IP address for SR-IOV interface", "container", containerID, "interface", containerInterfaceName)
if result.IPs != nil {
if err = pc.ifConfigurator.advertiseContainerAddr(containerNetNS, containerIface.Name, result); err != nil {
klog.ErrorS(err, "Failed to advertise IP address for SR-IOV interface",
"container", containerID, "interface", containerInterfaceName)
}
}
return nil
}
Expand Down Expand Up @@ -84,8 +87,11 @@ func (pc *podConfigurator) ConfigureVLANSecondaryInterface(
}
klog.InfoS("Configured VLAN interface", "Pod", klog.KRef(podNamespace, podName), "interface", containerInterfaceName, "hostInterface", hostIface)

if err := pc.ifConfigurator.advertiseContainerAddr(containerNetNS, containerIface.Name, result); err != nil {
klog.ErrorS(err, "Failed to advertise IP address for VLAN interface", "container", containerID, "interface", containerInterfaceName)
if result.IPs != nil {
if err := pc.ifConfigurator.advertiseContainerAddr(containerNetNS, containerIface.Name, result); err != nil {
klog.ErrorS(err, "Failed to advertise IP address for VLAN interface",
"container", containerID, "interface", containerInterfaceName)
}
}
success = true
return ovsPortUUID, nil
Expand Down
69 changes: 42 additions & 27 deletions pkg/agent/secondarynetwork/podwatch/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ func (pc *PodController) deletePodSecondaryNetwork(podCNIInfo *cnipodcache.CNICo
IFName: iface,
}
if err := pc.ipamAllocator.SecondaryNetworkRelease(podOwner); err != nil {
return fmt.Errorf("Failed to clean-up whereabouts IPAM %v", err)
return fmt.Errorf("failed to clean up IPAM: %v", err)
}

// Delete map entry for secNetInstIface, secNetInstConfig
Expand Down Expand Up @@ -231,10 +231,13 @@ func (pc *PodController) handleAddUpdatePod(obj interface{}) error {
}

err = pc.configurePodSecondaryNetwork(pod, networklist, podCNIInfo)
// We do not return error to retry, if at least one secondary network is configured.
if (err != nil) && (len(podCNIInfo.Interfaces) == 0) {
// Return error to requeue and retry.
return err
if err != nil {
klog.ErrorS(err, "Error when configuring secondary network", "pod", klog.KObj(pod))
if len(podCNIInfo.Interfaces) == 0 {
// Return error to requeue and retry.
return err
}
// We do not return error to retry, if at least one secondary network is configured.
Comment on lines +236 to +240
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you remember why we have this special case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think just that Arun wants to simplify the logic. We can fix in future (but I do feel there is complexity to fix partial failure or support network list update).

}
return nil
}
Expand Down Expand Up @@ -309,28 +312,42 @@ func (pc *PodController) configureSecondaryInterface(
}
}

podOwner := &crdv1a2.PodOwner{
Name: pod.Name,
Namespace: pod.Namespace,
ContainerID: podCNIInfo.ContainerID,
IFName: network.InterfaceRequest,
}
ipamResult, err := pc.ipamAllocator.SecondaryNetworkAllocate(podOwner, &networkConfig.NetworkConfig)
if err != nil {
return fmt.Errorf("secondary network IPAM failed: %v", err)
}
result := &ipamResult.Result
for _, ip := range result.IPs {
ip.Interface = current.Int(1)
var result *current.Result
var vlanID uint16
var ifConfigErr error
if networkConfig.IPAM != nil {
podOwner := &crdv1a2.PodOwner{
Name: pod.Name,
Namespace: pod.Namespace,
ContainerID: podCNIInfo.ContainerID,
IFName: network.InterfaceRequest,
}
ipamResult, err := pc.ipamAllocator.SecondaryNetworkAllocate(podOwner, &networkConfig.NetworkConfig)
if err != nil {
return fmt.Errorf("secondary network IPAM failed: %v", err)
}
defer func() {
if ifConfigErr != nil {
// Interface creation failed. Free allocated IP address
if err := pc.ipamAllocator.SecondaryNetworkRelease(podOwner); err != nil {
klog.ErrorS(err, "IPAM de-allocation failed: ", err)
}
}
}()
result = &ipamResult.Result
for _, ip := range result.IPs {
ip.Interface = current.Int(1)
}
vlanID = ipamResult.VLANID
} else {
result = &current.Result{}
}

var ovsPortUUID string
var ifConfigErr error
switch networkConfig.NetworkType {
case sriovNetworkType:
ifConfigErr = pc.configureSriovAsSecondaryInterface(pod, network, podCNIInfo, int(networkConfig.MTU), result)
case vlanNetworkType:
vlanID := ipamResult.VLANID
if networkConfig.VLAN > 0 {
// Let VLAN ID in the CNI network configuration override the IPPool subnet
// VLAN.
Expand All @@ -341,14 +358,10 @@ func (pc *PodController) configureSecondaryInterface(
podCNIInfo.ContainerID, podCNIInfo.ContainerNetNS, network.InterfaceRequest,
int(networkConfig.MTU), vlanID, result)
}

if ifConfigErr != nil {
// Interface creation failed. Free allocated IP address
if err := pc.ipamAllocator.SecondaryNetworkRelease(podOwner); err != nil {
klog.ErrorS(err, "IPAM de-allocation failed: ", err)
}
return ifConfigErr
}

// Update Pod CNI cache with the network config which was successfully configured.
if podCNIInfo.Interfaces == nil {
podCNIInfo.Interfaces = make(map[string]*cnipodcache.InterfaceInfo)
Expand Down Expand Up @@ -425,8 +438,10 @@ func validateNetworkConfig(cniConfig []byte) (*SecondaryNetworkConfig, error) {
if networkConfig.MTU < 0 {
return &networkConfig, fmt.Errorf("invalid MTU %d", networkConfig.MTU)
}
if networkConfig.IPAM.Type != ipam.AntreaIPAMType {
return &networkConfig, fmt.Errorf("unsupported IPAM type %s", networkConfig.IPAM.Type)
if networkConfig.IPAM != nil {
if networkConfig.IPAM.Type != ipam.AntreaIPAMType {
return &networkConfig, fmt.Errorf("unsupported IPAM type %s", networkConfig.IPAM.Type)
}
}

if networkConfig.MTU == 0 {
Expand Down
63 changes: 56 additions & 7 deletions pkg/agent/secondarynetwork/podwatch/controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,14 @@ const (
}
}`

netAttachNoIPAMTemplate = `{
"cniVersion": "{{.CNIVersion}}",
"type": "{{.CNIType}}",
"networkType": "{{.NetworkType}}",
"mtu": {{.MTU}},
"vlan": {{.VLAN}}
}`

defaultCNIVersion = "0.3.0"
defaultMTU = 1500
sriovDeviceID = "sriov-device-id"
Expand All @@ -82,10 +90,10 @@ const (
)

func testNetwork(name string, networkType cnipodcache.NetworkType) *netdefv1.NetworkAttachmentDefinition {
return testNetworkExt(name, "", "", string(networkType), "", 0, 0)
return testNetworkExt(name, "", "", string(networkType), "", 0, 0, false)
}

func testNetworkExt(name, cniVersion, cniType, networkType, ipamType string, mtu int, vlan int) *netdefv1.NetworkAttachmentDefinition {
func testNetworkExt(name, cniVersion, cniType, networkType, ipamType string, mtu, vlan int, noIPAM bool) *netdefv1.NetworkAttachmentDefinition {
if cniVersion == "" {
cniVersion = defaultCNIVersion
}
Expand All @@ -103,7 +111,13 @@ func testNetworkExt(name, cniVersion, cniType, networkType, ipamType string, mtu
MTU int
VLAN int
}{cniVersion, cniType, networkType, ipamType, mtu, vlan}
tmpl := template.Must(template.New("test").Parse(netAttachTemplate))

var tmpl *template.Template
if !noIPAM {
tmpl = template.Must(template.New("test").Parse(netAttachTemplate))
} else {
tmpl = template.Must(template.New("test").Parse(netAttachNoIPAMTemplate))
}
var b bytes.Buffer
tmpl.Execute(&b, &data)
return &netdefv1.NetworkAttachmentDefinition{
Expand Down Expand Up @@ -216,7 +230,6 @@ func TestPodControllerRun(t *testing.T) {
InterfaceRequest: interfaceName,
})
network := testNetwork(networkName, sriovNetworkType)

ipamResult := testIPAMResult("148.14.24.100/24")

var interfaceConfigured int32
Expand Down Expand Up @@ -288,6 +301,7 @@ func TestConfigurePodSecondaryNetwork(t *testing.T) {
ipamType string
mtu int
vlan int
noIPAM bool
doNotCreateNetwork bool
interfaceCreated bool
expectedErr string
Expand Down Expand Up @@ -316,7 +330,6 @@ func TestConfigurePodSecondaryNetwork(t *testing.T) {
{
name: "default MTU",
networkType: vlanNetworkType,
vlan: 0,
interfaceCreated: true,
expectedCalls: func(mockIPAM *podwatchtesting.MockIPAMAllocator, mockIC *podwatchtesting.MockInterfaceConfigurator) {
mockIPAM.EXPECT().SecondaryNetworkAllocate(podOwner, gomock.Any()).Return(testIPAMResult("148.14.24.100/24"), nil)
Expand All @@ -332,6 +345,24 @@ func TestConfigurePodSecondaryNetwork(t *testing.T) {
).Return(ovsPortUUID, nil)
},
},
{
name: "no IPAM",
networkType: vlanNetworkType,
noIPAM: true,
interfaceCreated: true,
expectedCalls: func(mockIPAM *podwatchtesting.MockIPAMAllocator, mockIC *podwatchtesting.MockInterfaceConfigurator) {
mockIC.EXPECT().ConfigureVLANSecondaryInterface(
podName,
testNamespace,
containerID,
containerNetNs(containerID),
interfaceName,
1500,
uint16(0),
gomock.Any(),
).Return(ovsPortUUID, nil)
},
},
{
name: "SRIOV network",
networkType: sriovNetworkType,
Expand Down Expand Up @@ -427,6 +458,24 @@ func TestConfigurePodSecondaryNetwork(t *testing.T) {
},
expectedErr: "interface creation failure",
},
{
name: "interface failure with no IPAM",
networkType: vlanNetworkType,
noIPAM: true,
expectedCalls: func(mockIPAM *podwatchtesting.MockIPAMAllocator, mockIC *podwatchtesting.MockInterfaceConfigurator) {
mockIC.EXPECT().ConfigureVLANSecondaryInterface(
podName,
testNamespace,
containerID,
containerNetNs(containerID),
interfaceName,
1500,
uint16(0),
gomock.Any(),
).Return("", errors.New("interface creation failure"))
},
expectedErr: "interface creation failure",
},
}

for _, tc := range tests {
Expand All @@ -435,7 +484,7 @@ func TestConfigurePodSecondaryNetwork(t *testing.T) {
pc, mockIPAM, interfaceConfigurator := testPodController(ctrl)
savedCNIConfig := *cniConfigInfo

network1 := testNetworkExt(networkName, tc.cniVersion, tc.cniType, string(tc.networkType), tc.ipamType, tc.mtu, tc.vlan)
network1 := testNetworkExt(networkName, tc.cniVersion, tc.cniType, string(tc.networkType), tc.ipamType, tc.mtu, tc.vlan, tc.noIPAM)
if !tc.doNotCreateNetwork {
pc.netAttachDefClient.NetworkAttachmentDefinitions(testNamespace).Create(context.Background(), network1, metav1.CreateOptions{})
}
Expand Down Expand Up @@ -499,7 +548,7 @@ func TestPodControllerAddPod(t *testing.T) {
savedCNIConfig := *cniConfig
network1 := testNetwork("net1", sriovNetworkType)
testVLAN := 100
network2 := testNetworkExt("net2", "", "", string(vlanNetworkType), "", defaultMTU, testVLAN)
network2 := testNetworkExt("net2", "", "", string(vlanNetworkType), "", defaultMTU, testVLAN, false)

podOwner1 := &crdv1a2.PodOwner{
Name: podName,
Expand Down
Loading