Skip to content

Commit

Permalink
fix tunnel interface name issue
Browse files Browse the repository at this point in the history 
the purpose of this commit is to fix a tunnel interface issue
founded during some IPSec PoC verification:

1. when the node name is like 'lan-k8s-0-0', the IPsec tunnel interface name will be
like '-k8s-0-2-e8dbe6', then it will failed to run command like
`ipsec up '-k8s-0-2-e8dbe6'` with error `/usr/lib/ipsec/stroke: invalid option -- 'k'`
due to the first char is '-', ipsec command interrupted it as an
option. so changed the `generateInterfaceName` method to use `strings.TrimLeft()` to remove '-' in left.

2. `createIPSecTunnelPort` method can't handle the case when tunnel
interface name changed when there is cache matched for the node. it
will reuse the existing tunnel name without creating a new one with new
name which means any change in `generateInterfaceName` won't take
affect.

and also add some unit test cases.

Signed-off-by: Lan Luo <luola@vmware.com>
  • Loading branch information
@luolanzone
luolanzone committed Jul 28, 2021
1 parent 42077cb commit e35417e
Show file tree
Hide file tree
Showing 3 changed files with 194 additions and 8 deletions.
28 changes: 22 additions & 6 deletions pkg/agent/controller/noderoute/node_route_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,9 @@ func (c *Controller) removeStaleTunnelPorts() error {
}

ifaceID := util.GenerateNodeTunnelInterfaceKey(node.Name)
validConfiguration := interfaceConfig.PSK == c.networkConfig.IPSecPSK &&
ifaceName := util.GenerateNodeTunnelInterfaceName(node.Name)
validConfiguration := interfaceConfig.InterfaceName == ifaceName &&
interfaceConfig.PSK == c.networkConfig.IPSecPSK &&
interfaceConfig.RemoteIP.Equal(peerNodeIP) &&
interfaceConfig.TunnelInterfaceConfig.Type == c.networkConfig.TunnelType
if validConfiguration {
Expand Down Expand Up @@ -529,16 +531,30 @@ func getPodCIDRsOnNode(node *corev1.Node) []string {
// createIPSecTunnelPort creates an IPSec tunnel port for the remote Node if the
// tunnel does not exist, and returns the ofport number.
func (c *Controller) createIPSecTunnelPort(nodeName string, nodeIP net.IP) (int32, error) {
portName := util.GenerateNodeTunnelInterfaceName(nodeName)
interfaceConfig, ok := c.interfaceStore.GetNodeTunnelInterface(nodeName)
if ok {
// TODO: check if Node IP, PSK, or tunnel type changes. This can
// happen if removeStaleTunnelPorts fails to remove a "stale"
// tunnel port for which the configuration has changed.
stalePortRemoved := false
// check if Node IP, PSK, or tunnel type changes. This can
// happen if removeStaleTunnelPorts fails to remove a "stale"
// tunnel port for which the configuration has changed.
if ok && (interfaceConfig.InterfaceName != portName ||
interfaceConfig.PSK != c.networkConfig.IPSecPSK ||
!interfaceConfig.RemoteIP.Equal(nodeIP) ||
interfaceConfig.TunnelInterfaceConfig.Type != c.networkConfig.TunnelType) {
klog.Infof("IPSec tunnel interface config mismatched with cached one, remove stale IPSec tunnel port %s", interfaceConfig.InterfaceName)
if err := c.ovsBridgeClient.DeletePort(interfaceConfig.PortUUID); err != nil {
klog.Errorf("fail to remove stale IPSec tunnel port %d for Node %s", interfaceConfig.OFPort, nodeName)
} else {
stalePortRemoved = true
}
}

if ok && !stalePortRemoved {
klog.V(2).Infof("find cached IPSec tunnel interface %s with port %d for Node %s", interfaceConfig.InterfaceName, interfaceConfig.OFPort, nodeName)
if interfaceConfig.OFPort != 0 {
return interfaceConfig.OFPort, nil
}
} else {
portName := util.GenerateNodeTunnelInterfaceName(nodeName)
ovsExternalIDs := map[string]interface{}{ovsExternalIDNodeName: nodeName}
portUUID, err := c.ovsBridgeClient.CreateTunnelPortExt(
portName,
Expand Down
169 changes: 169 additions & 0 deletions pkg/agent/controller/noderoute/node_route_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/containernetworking/plugins/pkg/ip"
"github.com/golang/mock/gomock"
"github.com/stretchr/testify/assert"

corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/informers"
Expand All @@ -32,6 +33,8 @@ import (
"antrea.io/antrea/pkg/agent/interfacestore"
oftest "antrea.io/antrea/pkg/agent/openflow/testing"
routetest "antrea.io/antrea/pkg/agent/route/testing"
"antrea.io/antrea/pkg/agent/util"
"antrea.io/antrea/pkg/ovs/ovsconfig"
ovsconfigtest "antrea.io/antrea/pkg/ovs/ovsconfig/testing"
)

Expand Down Expand Up @@ -228,3 +231,169 @@ func TestIPInPodSubnets(t *testing.T) {
assert.Equal(t, false, c.Controller.IPInPodSubnets(net.ParseIP("10.10.10.10")))
assert.Equal(t, false, c.Controller.IPInPodSubnets(net.ParseIP("8.8.8.8")))
}

func TestRemoveStaleTunnelPorts(t *testing.T) {
c, closeFn := newController(t)
defer closeFn()
defer c.queue.ShutDown()
stopCh := make(chan struct{})
defer close(stopCh)
c.informerFactory.Start(stopCh)
c.informerFactory.WaitForCacheSync(stopCh)

c.interfaceStore.AddInterface(&interfacestore.InterfaceConfig{
Type: 2,
InterfaceName: util.GenerateNodeTunnelInterfaceName("xyz-k8s-0-1"),
TunnelInterfaceConfig: &interfacestore.TunnelInterfaceConfig{
NodeName: "xyz-k8s-0-1",
Type: "vxlan",
PSK: "changeme",
RemoteIP: net.ParseIP("10.10.10.10"),
},
OVSPortConfig: &interfacestore.OVSPortConfig{
PortUUID: "123",
},
})
c.interfaceStore.AddInterface(&interfacestore.InterfaceConfig{
Type: 2,
InterfaceName: util.GenerateNodeTunnelInterfaceName("xyz-k8s-0-2"),
TunnelInterfaceConfig: &interfacestore.TunnelInterfaceConfig{
NodeName: "xyz-k8s-0-2",
Type: "vxlan",
},
OVSPortConfig: &interfacestore.OVSPortConfig{
PortUUID: "abc",
},
})

node1 := &corev1.Node{
ObjectMeta: metav1.ObjectMeta{
Name: "xyz-k8s-0-1",
},
Spec: corev1.NodeSpec{
PodCIDR: podCIDR.String(),
PodCIDRs: []string{podCIDR.String()},
},
Status: corev1.NodeStatus{
Addresses: []corev1.NodeAddress{
{
Type: corev1.NodeInternalIP,
Address: nodeIP1.String(),
},
},
},
}
c.clientset.CoreV1().Nodes().Create(context.TODO(), node1, metav1.CreateOptions{})
c.ovsClient.EXPECT().DeletePort("abc").Times(1)
tests := []struct {
name string
networkConfig *config.NetworkConfig
wantErr bool
}{
{
name: "good path with IPsec enable",
networkConfig: &config.NetworkConfig{
TrafficEncapMode: 0,
TunnelType: ovsconfig.TunnelType("vxlan"),
EnableIPSecTunnel: true,
IPSecPSK: "changeme",
},
wantErr: false,
},
}
for _, tt := range tests {
c.networkConfig = tt.networkConfig
t.Run(tt.name, func(t *testing.T) {
if err := c.removeStaleTunnelPorts(); (err != nil) != tt.wantErr {
t.Errorf("Controller.removeStaleTunnelPorts() error = %v, wantErr %v", err, tt.wantErr)
}
})
}
}

func TestCreateIPSecTunnelPort(t *testing.T) {
c, closeFn := newController(t)
defer closeFn()
defer c.queue.ShutDown()
c.interfaceStore.AddInterface(&interfacestore.InterfaceConfig{
Type: 2,
InterfaceName: "mismatchedname",
TunnelInterfaceConfig: &interfacestore.TunnelInterfaceConfig{
NodeName: "xyz-k8s-0-2",
Type: "vxlan",
PSK: "changeme",
RemoteIP: net.ParseIP("10.10.10.10"),
},
OVSPortConfig: &interfacestore.OVSPortConfig{
PortUUID: "123",
},
})

stopCh := make(chan struct{})
defer close(stopCh)
c.informerFactory.Start(stopCh)
c.informerFactory.WaitForCacheSync(stopCh)

node1PortName := util.GenerateNodeTunnelInterfaceName("xyz-k8s-0-1")
node2PortName := util.GenerateNodeTunnelInterfaceName("xyz-k8s-0-2")

c.ovsClient.EXPECT().CreateTunnelPortExt(
node1PortName, ovsconfig.TunnelType("vxlan"), int32(0),
false, "", "10.10.10.1", "changeme",
map[string]interface{}{ovsExternalIDNodeName: "xyz-k8s-0-1"}).Times(1)
c.ovsClient.EXPECT().CreateTunnelPortExt(
node2PortName, ovsconfig.TunnelType("vxlan"), int32(0),
false, "", "10.10.10.1", "changeme",
map[string]interface{}{ovsExternalIDNodeName: "xyz-k8s-0-2"}).Times(1)
c.ovsClient.EXPECT().GetOFPort(node1PortName).Return(int32(1), nil)
c.ovsClient.EXPECT().GetOFPort(node2PortName).Return(int32(2), nil)
c.ovsClient.EXPECT().DeletePort("123").Times(1)

tests := []struct {
name string
networkConfig *config.NetworkConfig
nodeName string
wantErr bool
want int32
}{
{
name: "good path",
networkConfig: &config.NetworkConfig{
TrafficEncapMode: 0,
TunnelType: ovsconfig.TunnelType("vxlan"),
EnableIPSecTunnel: true,
IPSecPSK: "changeme",
},
nodeName: "xyz-k8s-0-1",
wantErr: false,
want: 1,
},
{
name: "hit cache but interface name changed for the same node, need remove stale port",
networkConfig: &config.NetworkConfig{
TrafficEncapMode: 0,
TunnelType: "vxlan",
EnableIPSecTunnel: true,
IPSecPSK: "changeme",
},
nodeName: "xyz-k8s-0-2",
wantErr: false,
want: 2,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
c.networkConfig = tt.networkConfig
got, err := c.createIPSecTunnelPort(tt.nodeName, net.ParseIP("10.10.10.1"))
if (err != nil) != tt.wantErr {
t.Errorf("Controller.createIPSecTunnelPort() error = %v, wantErr %v", err, tt.wantErr)
return
}
if got != tt.want {
t.Errorf("Controller.createIPSecTunnelPort() got %v,want = %v", got, tt.want)
return
}
})
}
}
5 changes: 3 additions & 2 deletions pkg/agent/util/net.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"fmt"
"io"
"net"
"strings"
)

const (
Expand All @@ -39,9 +40,9 @@ func generateInterfaceName(key string, name string, useHead bool) string {
prefix := name
if len(name) > interfacePrefixLength {
if useHead {
prefix = name[:interfacePrefixLength]
prefix = strings.TrimLeft(name[:interfacePrefixLength], "-")
} else {
prefix = name[len(name)-interfacePrefixLength:]
prefix = strings.TrimLeft(name[len(name)-interfacePrefixLength:], "-")
}
}
return fmt.Sprintf("%s-%s", prefix, interfaceKey[:interfaceKeyLength])
Expand Down

0 comments on commit e35417e

Please sign in to comment.