diff --git a/pkg/agent/agent.go b/pkg/agent/agent.go index 0c81b5b70a7..62b51ec7152 100644 --- a/pkg/agent/agent.go +++ b/pkg/agent/agent.go @@ -27,6 +27,7 @@ import ( "time" "github.com/containernetworking/plugins/pkg/ip" + "github.com/spf13/afero" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" apitypes "k8s.io/apimachinery/pkg/types" @@ -35,6 +36,7 @@ import ( clientset "k8s.io/client-go/kubernetes" "k8s.io/client-go/util/retry" "k8s.io/klog/v2" + clockutils "k8s.io/utils/clock" "antrea.io/antrea/pkg/agent/cniserver" "antrea.io/antrea/pkg/agent/config" @@ -90,6 +92,12 @@ var ( // need to be deleted when changing to "psk". var otherConfigKeysForIPsecCertificates = []string{"certificate", "private_key", "ca_cert", "remote_cert", "remote_name"} +var ( + // Declared as variables for testing. + defaultFs = afero.NewOsFs() + clock clockutils.WithTicker = &clockutils.RealClock{} +) + // Initializer knows how to setup host networking, OpenVSwitch, and Openflow. type Initializer struct { client clientset.Interface @@ -211,7 +219,7 @@ func (i *Initializer) setupOVSBridge() error { } func (i *Initializer) validateSupportedDPFeatures() error { - gotFeatures, err := ovsctl.NewClient(i.ovsBridge).GetDPFeatures() + gotFeatures, err := i.ovsCtlClient.GetDPFeatures() if err != nil { return err } @@ -1064,19 +1072,19 @@ func (i *Initializer) waitForIPsecMonitorDaemon() error { // PID files before starting the OVS daemons, it is safe to assume that // if this file exists, the IPsec monitor is indeed running. const ovsMonitorIPSecPID = "/var/run/openvswitch/ovs-monitor-ipsec.pid" - timer := time.NewTimer(10 * time.Second) + timer := clock.NewTimer(10 * time.Second) defer timer.Stop() - ticker := time.NewTicker(1 * time.Second) + ticker := clock.NewTicker(1 * time.Second) defer ticker.Stop() for { - if _, err := os.Stat(ovsMonitorIPSecPID); err == nil { + if _, err := defaultFs.Stat(ovsMonitorIPSecPID); err == nil { klog.V(2).Infof("OVS IPsec monitor seems to be present") break } select { - case <-ticker.C: + case <-ticker.C(): continue - case <-timer.C: + case <-timer.C(): return fmt.Errorf("IPsec was requested, but the OVS IPsec monitor does not seem to be running") } } diff --git a/pkg/agent/agent_linux.go b/pkg/agent/agent_linux.go index cea934785d4..eebc2851f3a 100644 --- a/pkg/agent/agent_linux.go +++ b/pkg/agent/agent_linux.go @@ -34,6 +34,11 @@ import ( utilip "antrea.io/antrea/pkg/util/ip" ) +var ( + // getInterfaceByName is meant to be overridden for testing. + getInterfaceByName = net.InterfaceByName +) + // prepareHostNetwork returns immediately on Linux. func (i *Initializer) prepareHostNetwork() error { return nil @@ -89,7 +94,7 @@ func (i *Initializer) prepareOVSBridgeForK8sNode() error { } else { uplinkNetConfig.OFPort = uint32(uplinkOFPort) } - if adapter, err := net.InterfaceByName(bridgedUplinkName); err != nil { + if adapter, err := getInterfaceByName(bridgedUplinkName); err != nil { return fmt.Errorf("cannot find uplink interface %s: err=%w", bridgedUplinkName, err) } else { uplinkNetConfig.Index = adapter.Index diff --git a/pkg/agent/agent_linux_test.go b/pkg/agent/agent_linux_test.go index 5abb5cc4e90..02c873399a7 100644 --- a/pkg/agent/agent_linux_test.go +++ b/pkg/agent/agent_linux_test.go @@ -14,6 +14,117 @@ package agent +import ( + "fmt" + "net" + "strings" + "testing" + + mock "github.com/golang/mock/gomock" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "antrea.io/antrea/pkg/agent/config" + "antrea.io/antrea/pkg/agent/interfacestore" + "antrea.io/antrea/pkg/ovs/ovsconfig" + ovsconfigtest "antrea.io/antrea/pkg/ovs/ovsconfig/testing" +) + func mockSetInterfaceMTU(returnErr error) func() { return func() {} } + +func mockGetInterfaceByName(ipDevice *net.Interface) func() { + prevGetInterfaceByName := getInterfaceByName + getInterfaceByName = func(name string) (*net.Interface, error) { + return ipDevice, nil + } + return func() { getInterfaceByName = prevGetInterfaceByName } +} + +func TestPrepareOVSBridgeForK8sNode(t *testing.T) { + macAddr, _ := net.ParseMAC("00:00:5e:00:53:01") + _, nodeIPNet, _ := net.ParseCIDR("192.168.10.10/24") + ipDevice := &net.Interface{ + Index: 10, + MTU: 1500, + Name: "ens160", + HardwareAddr: macAddr, + } + datapathID := "0000" + strings.Replace(macAddr.String(), ":", "", -1) + nodeConfig := &config.NodeConfig{ + UplinkNetConfig: new(config.AdapterNetConfig), + NodeIPv4Addr: nodeIPNet, + } + + tests := []struct { + name string + connectUplinkToBridge bool + expectedCalls func(m *ovsconfigtest.MockOVSBridgeClient) + expectedHostInterfaceOFPort uint32 + expectedUplinkOFPort uint32 + expectedErr string + }{ + { + name: "connectUplinkToBridge is false, do nothing", + }, + { + name: "failed to set datapath_id", + connectUplinkToBridge: true, + expectedCalls: func(m *ovsconfigtest.MockOVSBridgeClient) { + m.EXPECT().SetDatapathID(datapathID).Return(ovsconfig.InvalidArgumentsError("unable to set datapath_id")) + }, + expectedErr: fmt.Sprintf("failed to set datapath_id %s: err=unable to set datapath_id", datapathID), + }, + { + name: "local port does not exist, allocate it", + connectUplinkToBridge: true, + expectedCalls: func(m *ovsconfigtest.MockOVSBridgeClient) { + m.EXPECT().SetDatapathID(datapathID).Return(nil) + m.EXPECT().GetOFPort(ipDevice.Name, false).Return(int32(0), ovsconfig.InvalidArgumentsError("interface not found")) + m.EXPECT().AllocateOFPort(config.UplinkOFPort).Return(int32(2), nil) + m.EXPECT().AllocateOFPort(config.UplinkOFPort).Return(int32(3), nil) + }, + expectedUplinkOFPort: 2, + expectedHostInterfaceOFPort: 3, + }, + { + name: "uplink interface found", + connectUplinkToBridge: true, + expectedCalls: func(m *ovsconfigtest.MockOVSBridgeClient) { + m.EXPECT().SetDatapathID(datapathID).Return(nil) + m.EXPECT().GetOFPort(ipDevice.Name, false).Return(int32(2), nil) + m.EXPECT().GetOFPort(ipDevice.Name+"~", false).Return(int32(3), nil) + }, + expectedHostInterfaceOFPort: 2, + expectedUplinkOFPort: 3, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + controller := mock.NewController(t) + mockOVSBridgeClient := ovsconfigtest.NewMockOVSBridgeClient(controller) + store := interfacestore.NewInterfaceStore() + initializer := newAgentInitializer(mockOVSBridgeClient, store) + initializer.nodeType = config.K8sNode + initializer.connectUplinkToBridge = tt.connectUplinkToBridge + initializer.nodeConfig = nodeConfig + defer mockGetIPNetDeviceFromIP(nodeIPNet, ipDevice)() + defer mockGetInterfaceByName(ipDevice)() + if tt.expectedCalls != nil { + tt.expectedCalls(mockOVSBridgeClient) + } + err := initializer.prepareOVSBridgeForK8sNode() + if tt.expectedErr != "" { + assert.ErrorContains(t, err, tt.expectedErr) + } else { + require.NoError(t, err) + if tt.connectUplinkToBridge { + assert.Equal(t, tt.expectedUplinkOFPort, initializer.nodeConfig.UplinkNetConfig.OFPort) + assert.Equal(t, tt.expectedHostInterfaceOFPort, initializer.nodeConfig.HostInterfaceOFPort) + } + } + }) + } +} diff --git a/pkg/agent/agent_test.go b/pkg/agent/agent_test.go index f18e820639f..625731553a2 100644 --- a/pkg/agent/agent_test.go +++ b/pkg/agent/agent_test.go @@ -21,22 +21,29 @@ import ( "os" "strings" "testing" + "time" mock "github.com/golang/mock/gomock" "github.com/google/uuid" + "github.com/spf13/afero" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/kubernetes/fake" + clockutils "k8s.io/utils/clock" + clocktesting "k8s.io/utils/clock/testing" "antrea.io/antrea/pkg/agent/cniserver" "antrea.io/antrea/pkg/agent/config" "antrea.io/antrea/pkg/agent/interfacestore" "antrea.io/antrea/pkg/agent/types" + crdv1alpha1 "antrea.io/antrea/pkg/apis/crd/v1alpha1" + fakeversioned "antrea.io/antrea/pkg/client/clientset/versioned/fake" "antrea.io/antrea/pkg/ovs/ovsconfig" ovsconfigtest "antrea.io/antrea/pkg/ovs/ovsconfig/testing" + "antrea.io/antrea/pkg/ovs/ovsctl" ovsctltest "antrea.io/antrea/pkg/ovs/ovsctl/testing" "antrea.io/antrea/pkg/util/env" "antrea.io/antrea/pkg/util/ip" @@ -162,7 +169,7 @@ func TestGetRoundInfo(t *testing.T) { assert.Equal(t, uint64(initialRoundNum), roundInfo.RoundNum, "Unexpected round number") } -func TestInitNodeLocalConfig(t *testing.T) { +func TestInitK8sNodeLocalConfig(t *testing.T) { nodeName := "node1" ovsBridge := "br-int" nodeIPStr := "192.168.10.10" @@ -708,3 +715,271 @@ func TestRestorePortConfigs(t *testing.T) { }) } } + +func TestSetOVSDatapath(t *testing.T) { + tests := []struct { + name string + expectedCalls func(m *ovsconfigtest.MockOVSBridgeClient) + expectedErr string + }{ + { + name: "fail to read OVS bridge other_config", + expectedCalls: func(m *ovsconfigtest.MockOVSBridgeClient) { + m.EXPECT().GetOVSOtherConfig().Return(nil, ovsconfig.NewTransactionError(fmt.Errorf("failed to read OVS bridge other_config"), true)) + }, + expectedErr: "failed to read OVS bridge other_config", + }, + { + name: "fail to set OVS bridge datapath_id", + expectedCalls: func(m *ovsconfigtest.MockOVSBridgeClient) { + m.EXPECT().GetOVSOtherConfig().Return(map[string]string{}, nil) + m.EXPECT().SetDatapathID(mock.Any()).Return(ovsconfig.NewTransactionError(fmt.Errorf("failed to set OVS bridge datapath_id"), true)) + }, + expectedErr: "failed to set OVS bridge datapath_id", + }, + { + name: "datapath-id exists in other_config on OVS bridge", + expectedCalls: func(m *ovsconfigtest.MockOVSBridgeClient) { + m.EXPECT().GetOVSOtherConfig().Return(map[string]string{ + ovsconfig.OVSOtherConfigDatapathIDKey: "datapathId", + }, nil) + }, + }, + { + name: "generate and set datapath ID for OVS bridge", + expectedCalls: func(m *ovsconfigtest.MockOVSBridgeClient) { + m.EXPECT().GetOVSOtherConfig().Return(map[string]string{}, nil) + m.EXPECT().SetDatapathID(mock.Any()) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + controller := mock.NewController(t) + mockOVSBridgeClient := ovsconfigtest.NewMockOVSBridgeClient(controller) + initializer := newAgentInitializer(mockOVSBridgeClient, nil) + tt.expectedCalls(mockOVSBridgeClient) + + err := initializer.setOVSDatapath() + if tt.expectedErr != "" { + assert.ErrorContains(t, err, tt.expectedErr) + } else { + require.NoError(t, err) + } + }) + } +} + +func mockIPsecPSKEnv(name string) func() { + os.Setenv(ipsecPSKEnvKey, name) + return func() { os.Unsetenv(ipsecPSKEnvKey) } +} + +func TestReadIPSecPSK(t *testing.T) { + tests := []struct { + name string + isIPsecPSK bool + expectedErr string + }{ + { + name: "IPsec PSK env variable not set", + expectedErr: "IPsec PSK environment variable 'ANTREA_IPSEC_PSK' is not set or is empty", + }, + { + name: "IPsec PSK env variable set", + isIPsecPSK: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + initializer := &Initializer{ + networkConfig: &config.NetworkConfig{ + IPsecConfig: config.IPsecConfig{}, + }, + } + if tt.isIPsecPSK { + defer mockIPsecPSKEnv("key")() + } + + err := initializer.readIPSecPSK() + if tt.expectedErr != "" { + assert.ErrorContains(t, err, tt.expectedErr) + } else { + require.NoError(t, err) + } + }) + } +} + +func TestWaitForIPSecMonitorDaemon(t *testing.T) { + tests := []struct { + name string + isIPsecMonitorRunning bool + expectedErr string + }{ + { + name: "IPsec monitor is not running", + expectedErr: "IPsec was requested, but the OVS IPsec monitor does not seem to be running", + }, + { + name: "IPsec monitor running", + isIPsecMonitorRunning: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + initializer := &Initializer{} + if tt.isIPsecMonitorRunning { + appFS := afero.NewMemMapFs() + err := appFS.MkdirAll("/var/run/openvswitch", 0777) + require.NoError(t, err) + _, err = appFS.Create("/var/run/openvswitch/ovs-monitor-ipsec.pid") + require.NoError(t, err) + defaultFs = appFS + defer func() { + defaultFs = afero.NewOsFs() + }() + } else { + fakeClock := clocktesting.NewFakeClock(time.Now()) + clock = fakeClock + defer func() { + clock = clockutils.RealClock{} + }() + go func() { + require.Eventually(t, func() bool { + return fakeClock.HasWaiters() + }, 1*time.Second, 10*time.Millisecond) + fakeClock.Step(10 * time.Second) + }() + } + + err := initializer.waitForIPsecMonitorDaemon() + if tt.expectedErr != "" { + assert.ErrorContains(t, err, tt.expectedErr) + } else { + require.NoError(t, err) + } + }) + } +} + +func TestInitVMLocalConfig(t *testing.T) { + ipDevice := &net.Interface{ + Name: "fakeUplinkInterface", + } + testNode := &crdv1alpha1.ExternalNode{ + ObjectMeta: metav1.ObjectMeta{Name: "testNode", Namespace: "external"}, + Spec: crdv1alpha1.ExternalNodeSpec{ + Interfaces: []crdv1alpha1.NetworkInterface{ + { + IPs: []string{"192.168.1.2"}, + }, + }, + }, + } + + tests := []struct { + name string + nodeName string + crdClient *fakeversioned.Clientset + expectedErr string + }{ + { + name: "Finished VM config initialization", + nodeName: "testNode", + crdClient: fakeversioned.NewSimpleClientset(testNode), + }, + { + name: "provided external Node unavailable", + nodeName: "testNode", + crdClient: fakeversioned.NewSimpleClientset(), + expectedErr: "timed out waiting for the condition", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + stopCh := make(chan struct{}) + initializer := &Initializer{ + crdClient: tt.crdClient, + ovsBridge: "br-int", + stopCh: stopCh, + networkConfig: &config.NetworkConfig{}, + nodeType: config.ExternalNode, + externalNodeNamespace: "external", + } + close(stopCh) + defer mockGetIPNetDeviceFromIP(nil, ipDevice)() + err := initializer.initVMLocalConfig(tt.nodeName) + if tt.expectedErr != "" { + assert.ErrorContains(t, err, tt.expectedErr) + } else { + require.NoError(t, err) + } + }) + } +} + +func TestValidateSupportedDPFeatures(t *testing.T) { + tests := []struct { + name string + expectedCalls func(m *ovsctltest.MockOVSCtlClient) + expectedErr string + }{ + { + name: "error listing DP features", + expectedCalls: func(m *ovsctltest.MockOVSCtlClient) { + m.EXPECT().GetDPFeatures().Return(map[ovsctl.DPFeature]bool{}, fmt.Errorf("error listing DP features")) + }, + expectedErr: "error listing DP features", + }, + { + name: "required feature is unknown", + expectedCalls: func(m *ovsctltest.MockOVSCtlClient) { + m.EXPECT().GetDPFeatures().Return(map[ovsctl.DPFeature]bool{}, nil) + }, + expectedErr: "the required OVS DP feature 'CT state' support is unknown", + }, + { + name: "required feature is not supported", + expectedCalls: func(m *ovsctltest.MockOVSCtlClient) { + m.EXPECT().GetDPFeatures().Return(map[ovsctl.DPFeature]bool{ + ovsctl.CTStateFeature: false, + }, nil) + }, + expectedErr: "the required OVS DP feature 'CT state' is not supported", + }, + { + name: "required features supported", + expectedCalls: func(m *ovsctltest.MockOVSCtlClient) { + m.EXPECT().GetDPFeatures().Return(map[ovsctl.DPFeature]bool{ + ovsctl.CTStateFeature: true, + ovsctl.CTZoneFeature: true, + ovsctl.CTMarkFeature: true, + ovsctl.CTLabelFeature: true, + ovsctl.CTStateNATFeature: true, + }, nil) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + controller := mock.NewController(t) + mockOVSCtlClient := ovsctltest.NewMockOVSCtlClient(controller) + tt.expectedCalls(mockOVSCtlClient) + initializer := &Initializer{ + ovsCtlClient: mockOVSCtlClient, + } + err := initializer.validateSupportedDPFeatures() + if tt.expectedErr != "" { + assert.ErrorContains(t, err, tt.expectedErr) + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/pkg/agent/agent_windows.go b/pkg/agent/agent_windows.go index abfc5e2d46e..beb50c9ef1f 100644 --- a/pkg/agent/agent_windows.go +++ b/pkg/agent/agent_windows.go @@ -440,7 +440,7 @@ func (i *Initializer) setVMNodeConfig(en *v1alpha1.ExternalNode, nodeName string } else { ipFilter = &utilip.DualStackIPs{IPv6: epIP} } - _, _, uplinkInterface, err = util.GetIPNetDeviceFromIP(ipFilter, nil) + _, _, uplinkInterface, err = getIPNetDeviceFromIP(ipFilter, nil) if err != nil { klog.InfoS("Unable to get net device by IP", "IP", addr) } else {