From 3fed840828a545997f73d4b2ed9aabb640763d33 Mon Sep 17 00:00:00 2001 From: Wenying Dong Date: Tue, 7 Dec 2021 03:35:49 -0800 Subject: [PATCH] Use OVS port external_ids to save Antrea interface type (#3027) Introduce Antrea interface type to map the OVS port to Antrea created interfaces, and use OVS port external_ids field to save the configuration. This is helpful for Antrea to define more interface types. Signed-off-by: wenyingd --- build/images/scripts/start_ovs | 2 +- pkg/agent/agent.go | 131 ++++++++++++++------ pkg/agent/agent_linux.go | 7 +- pkg/agent/agent_test.go | 17 +++ pkg/agent/agent_windows.go | 5 +- pkg/agent/cniserver/pod_configuration.go | 1 + pkg/agent/interfacestore/types.go | 8 ++ pkg/ovs/ovsconfig/interfaces.go | 1 + pkg/ovs/ovsconfig/ovs_client.go | 17 +++ pkg/ovs/ovsconfig/testing/mock_ovsconfig.go | 14 +++ 10 files changed, 163 insertions(+), 40 deletions(-) diff --git a/build/images/scripts/start_ovs b/build/images/scripts/start_ovs index ab1fee9251d..e4efd603181 100755 --- a/build/images/scripts/start_ovs +++ b/build/images/scripts/start_ovs @@ -111,7 +111,7 @@ function quit { log_info $CONTAINER_NAME "Stopping OVS before quit" # sleep until uplink is removed from OVS during antrea-agent shutdown, and initial host network configuration has # been restored. The uplink is moved to the OVS bridge to support AntreaFlexibleIPAM mode. - while [ "`ovsdb-client dump Port|grep \"{antrea-uplink=\\\"true\\\"}\"`" != "" ]; do + while [ "`ovsdb-client dump Port|grep antrea-type=uplink`" != "" ]; do log_info $CONTAINER_NAME "Uplink found on OVS, wait 1s and retry..." sleep 1 & SLEEP_PID=$! diff --git a/pkg/agent/agent.go b/pkg/agent/agent.go index 340639b0500..9ec70abf7cd 100644 --- a/pkg/agent/agent.go +++ b/pkg/agent/agent.go @@ -226,50 +226,103 @@ func (i *Initializer) initInterfaceStore() error { return err } + parseGatewayInterfaceFunc := func(port *ovsconfig.OVSPortData, ovsPort *interfacestore.OVSPortConfig) *interfacestore.InterfaceConfig { + intf := &interfacestore.InterfaceConfig{ + Type: interfacestore.GatewayInterface, + InterfaceName: port.Name, + OVSPortConfig: ovsPort} + if intf.InterfaceName != i.hostGateway { + klog.Warningf("The discovered gateway interface name %s is different from the configured value: %s", + intf.InterfaceName, i.hostGateway) + // Set the gateway interface name to the discovered name. + i.hostGateway = intf.InterfaceName + } + return intf + } + parseUplinkInterfaceFunc := func(port *ovsconfig.OVSPortData, ovsPort *interfacestore.OVSPortConfig) *interfacestore.InterfaceConfig { + return &interfacestore.InterfaceConfig{ + Type: interfacestore.UplinkInterface, + InterfaceName: port.Name, + OVSPortConfig: ovsPort, + } + } + parseTunnelInterfaceFunc := func(port *ovsconfig.OVSPortData, ovsPort *interfacestore.OVSPortConfig) *interfacestore.InterfaceConfig { + intf := noderoute.ParseTunnelInterfaceConfig(port, ovsPort) + if intf != nil && port.OFPort == config.DefaultTunOFPort && + intf.InterfaceName != i.nodeConfig.DefaultTunName { + klog.Infof("The discovered default tunnel interface name %s is different from the default value: %s", + intf.InterfaceName, i.nodeConfig.DefaultTunName) + // Set the default tunnel interface name to the discovered name. + i.nodeConfig.DefaultTunName = intf.InterfaceName + } + return intf + } ifaceList := make([]*interfacestore.InterfaceConfig, 0, len(ovsPorts)) - uplinkIfName := i.nodeConfig.UplinkNetConfig.Name for index := range ovsPorts { port := &ovsPorts[index] ovsPort := &interfacestore.OVSPortConfig{ PortUUID: port.UUID, OFPort: port.OFPort} var intf *interfacestore.InterfaceConfig - switch { - case port.OFPort == config.HostGatewayOFPort: - intf = &interfacestore.InterfaceConfig{ - Type: interfacestore.GatewayInterface, - InterfaceName: port.Name, - OVSPortConfig: ovsPort} - if intf.InterfaceName != i.hostGateway { - klog.Warningf("The discovered gateway interface name %s is different from the configured value: %s", - intf.InterfaceName, i.hostGateway) - // Set the gateway interface name to the discovered name. - i.hostGateway = intf.InterfaceName + interfaceType, ok := port.ExternalIDs[interfacestore.AntreaInterfaceTypeKey] + if !ok { + interfaceType = interfacestore.AntreaUnset + } + if interfaceType != interfacestore.AntreaUnset { + switch interfaceType { + case interfacestore.AntreaGateway: + intf = parseGatewayInterfaceFunc(port, ovsPort) + case interfacestore.AntreaUplink: + intf = parseUplinkInterfaceFunc(port, ovsPort) + case interfacestore.AntreaTunnel: + intf = parseTunnelInterfaceFunc(port, ovsPort) + case interfacestore.AntreaHost: + // Not load the host interface, because it is configured on the OVS bridge port, and we don't need a + // specific interface in the interfaceStore. + intf = nil + case interfacestore.AntreaContainer: + // The port should be for a container interface. + intf = cniserver.ParseOVSPortInterfaceConfig(port, ovsPort, true) + default: + klog.InfoS("Unknown Antrea interface type", "type", interfaceType) + } + } else { + // Antrea Interface type is not saved in OVS port external_ids in earlier Antrea versions, so we use + // the old way to decide the interface type for the upgrade case. + uplinkIfName := i.nodeConfig.UplinkNetConfig.Name + var antreaIFType string + switch { + case port.OFPort == config.HostGatewayOFPort: + intf = parseGatewayInterfaceFunc(port, ovsPort) + antreaIFType = interfacestore.AntreaGateway + case port.Name == uplinkIfName: + intf = parseUplinkInterfaceFunc(port, ovsPort) + antreaIFType = interfacestore.AntreaUplink + case port.IFType == ovsconfig.GeneveTunnel: + fallthrough + case port.IFType == ovsconfig.VXLANTunnel: + fallthrough + case port.IFType == ovsconfig.GRETunnel: + fallthrough + case port.IFType == ovsconfig.STTTunnel: + intf = parseTunnelInterfaceFunc(port, ovsPort) + antreaIFType = interfacestore.AntreaTunnel + case port.Name == i.ovsBridge: + intf = nil + antreaIFType = interfacestore.AntreaHost + default: + // The port should be for a container interface. + intf = cniserver.ParseOVSPortInterfaceConfig(port, ovsPort, true) + antreaIFType = interfacestore.AntreaContainer } - case port.Name == uplinkIfName: - intf = &interfacestore.InterfaceConfig{ - Type: interfacestore.UplinkInterface, - InterfaceName: port.Name, - OVSPortConfig: ovsPort, + updatedExtIDs := make(map[string]interface{}) + for k, v := range port.ExternalIDs { + updatedExtIDs[k] = v } - case port.IFType == ovsconfig.GeneveTunnel: - fallthrough - case port.IFType == ovsconfig.VXLANTunnel: - fallthrough - case port.IFType == ovsconfig.GRETunnel: - fallthrough - case port.IFType == ovsconfig.STTTunnel: - intf = noderoute.ParseTunnelInterfaceConfig(port, ovsPort) - if intf != nil && port.OFPort == config.DefaultTunOFPort && - intf.InterfaceName != i.nodeConfig.DefaultTunName { - klog.Infof("The discovered default tunnel interface name %s is different from the default value: %s", - intf.InterfaceName, i.nodeConfig.DefaultTunName) - // Set the default tunnel interface name to the discovered name. - i.nodeConfig.DefaultTunName = intf.InterfaceName + updatedExtIDs[interfacestore.AntreaInterfaceTypeKey] = antreaIFType + if err := i.ovsBridgeClient.SetPortExternalIDs(port.Name, updatedExtIDs); err != nil { + klog.ErrorS(err, "Failed to set Antrea interface type on OVS port", "port", port.Name) } - default: - // The port should be for a container interface. - intf = cniserver.ParseOVSPortInterfaceConfig(port, ovsPort, true) } if intf != nil { ifaceList = append(ifaceList, intf) @@ -556,7 +609,10 @@ func (i *Initializer) setupGatewayInterface() error { gatewayIface, portExists := i.ifaceStore.GetInterface(i.hostGateway) if !portExists { klog.V(2).Infof("Creating gateway port %s on OVS bridge", i.hostGateway) - gwPortUUID, err := i.ovsBridgeClient.CreateInternalPort(i.hostGateway, config.HostGatewayOFPort, nil) + externalIDs := map[string]interface{}{ + interfacestore.AntreaInterfaceTypeKey: interfacestore.AntreaGateway, + } + gwPortUUID, err := i.ovsBridgeClient.CreateInternalPort(i.hostGateway, config.HostGatewayOFPort, externalIDs) if err != nil { klog.Errorf("Failed to create gateway port %s on OVS bridge: %v", i.hostGateway, err) return err @@ -683,7 +739,10 @@ func (i *Initializer) setupDefaultTunnelInterface() error { tunnelPortName = defaultTunInterfaceName i.nodeConfig.DefaultTunName = tunnelPortName } - tunnelPortUUID, err := i.ovsBridgeClient.CreateTunnelPortExt(tunnelPortName, i.networkConfig.TunnelType, config.DefaultTunOFPort, shouldEnableCsum, localIPStr, "", "", nil) + externalIDs := map[string]interface{}{ + interfacestore.AntreaInterfaceTypeKey: interfacestore.AntreaTunnel, + } + tunnelPortUUID, err := i.ovsBridgeClient.CreateTunnelPortExt(tunnelPortName, i.networkConfig.TunnelType, config.DefaultTunOFPort, shouldEnableCsum, localIPStr, "", "", externalIDs) if err != nil { klog.Errorf("Failed to create tunnel port %s type %s on OVS bridge: %v", tunnelPortName, i.networkConfig.TunnelType, err) return err diff --git a/pkg/agent/agent_linux.go b/pkg/agent/agent_linux.go index 8ec39a7d714..33c189f59de 100644 --- a/pkg/agent/agent_linux.go +++ b/pkg/agent/agent_linux.go @@ -114,7 +114,10 @@ func (i *Initializer) prepareOVSBridge() error { } else { // OVS does not receive "ofport_request" param when creating local port, so here use config.AutoAssignedOFPort=0 // to ignore this param. - if _, err = i.ovsBridgeClient.CreateInternalPort(brName, config.AutoAssignedOFPort, nil); err != nil { + externalIDs := map[string]interface{}{ + interfacestore.AntreaInterfaceTypeKey: interfacestore.AntreaHost, + } + if _, err = i.ovsBridgeClient.CreateInternalPort(brName, config.AutoAssignedOFPort, externalIDs); err != nil { return err } } @@ -208,7 +211,7 @@ func (i *Initializer) BridgeUplinkToOVSBridge() error { return err } // Create uplink port. - uplinkPortUUId, err := i.ovsBridgeClient.CreateUplinkPort(uplink, config.UplinkOFPort, map[string]interface{}{"antrea-uplink": "true"}) + uplinkPortUUId, err := i.ovsBridgeClient.CreateUplinkPort(uplink, config.UplinkOFPort, map[string]interface{}{interfacestore.AntreaInterfaceTypeKey: interfacestore.AntreaUplink}) if err != nil { return fmt.Errorf("failed to add uplink port %s: err=%w", uplink, err) } diff --git a/pkg/agent/agent_test.go b/pkg/agent/agent_test.go index bf97ce2dea1..0d4f8570fe4 100644 --- a/pkg/agent/agent_test.go +++ b/pkg/agent/agent_test.go @@ -107,6 +107,23 @@ func TestInitstore(t *testing.T) { if !found2 { t.Errorf("Failed to load OVS port into local store") } + + // OVS port external_ids should be updated to set AntreaInterfaceTypeKey if it doesn't exist in OVSPortData. + delete(ovsPort1.ExternalIDs, interfacestore.AntreaInterfaceTypeKey) + delete(ovsPort2.ExternalIDs, interfacestore.AntreaInterfaceTypeKey) + initOVSPorts2 := []ovsconfig.OVSPortData{ovsPort1, ovsPort2} + mockOVSBridgeClient.EXPECT().GetPortList().Return(initOVSPorts2, nil) + updateExtIDsFunc := func(p ovsconfig.OVSPortData) map[string]interface{} { + extIDs := make(map[string]interface{}) + for k, v := range p.ExternalIDs { + extIDs[k] = v + } + extIDs[interfacestore.AntreaInterfaceTypeKey] = interfacestore.AntreaContainer + return extIDs + } + mockOVSBridgeClient.EXPECT().SetPortExternalIDs(ovsPort1.Name, updateExtIDsFunc(ovsPort1)).Return(nil) + mockOVSBridgeClient.EXPECT().SetPortExternalIDs(ovsPort2.Name, updateExtIDsFunc(ovsPort2)).Return(nil) + initializer.initInterfaceStore() } func TestPersistRoundNum(t *testing.T) { diff --git a/pkg/agent/agent_windows.go b/pkg/agent/agent_windows.go index c932fa0b218..187d5268bd7 100644 --- a/pkg/agent/agent_windows.go +++ b/pkg/agent/agent_windows.go @@ -144,7 +144,10 @@ func (i *Initializer) prepareOVSBridge() error { } else { // OVS does not receive "ofport_request" param when creating local port, so here use config.AutoAssignedOFPort=0 // to ignore this param. - if _, err = i.ovsBridgeClient.CreateInternalPort(brName, config.AutoAssignedOFPort, nil); err != nil { + externalIDs := map[string]interface{}{ + interfacestore.AntreaInterfaceTypeKey: interfacestore.AntreaHost, + } + if _, err = i.ovsBridgeClient.CreateInternalPort(brName, config.AutoAssignedOFPort, externalIDs); err != nil { return err } } diff --git a/pkg/agent/cniserver/pod_configuration.go b/pkg/agent/cniserver/pod_configuration.go index 5b3ff95bc6b..0f8300761df 100644 --- a/pkg/agent/cniserver/pod_configuration.go +++ b/pkg/agent/cniserver/pod_configuration.go @@ -131,6 +131,7 @@ func BuildOVSPortExternalIDs(containerConfig *interfacestore.InterfaceConfig) ma externalIDs[ovsExternalIDIP] = getContainerIPsString(containerConfig.IPs) externalIDs[ovsExternalIDPodName] = containerConfig.PodName externalIDs[ovsExternalIDPodNamespace] = containerConfig.PodNamespace + externalIDs[interfacestore.AntreaInterfaceTypeKey] = interfacestore.AntreaContainer return externalIDs } diff --git a/pkg/agent/interfacestore/types.go b/pkg/agent/interfacestore/types.go index 5bfdd8455d4..b9c3d011178 100644 --- a/pkg/agent/interfacestore/types.go +++ b/pkg/agent/interfacestore/types.go @@ -31,6 +31,14 @@ const ( TunnelInterface // UplinkInterface is used to mark current interface is for uplink port UplinkInterface + + AntreaInterfaceTypeKey = "antrea-type" + AntreaGateway = "gateway" + AntreaContainer = "container" + AntreaTunnel = "tunnel" + AntreaUplink = "uplink" + AntreaHost = "host" + AntreaUnset = "" ) type InterfaceType uint8 diff --git a/pkg/ovs/ovsconfig/interfaces.go b/pkg/ovs/ovsconfig/interfaces.go index 9fde8260e07..b49fbc4c023 100644 --- a/pkg/ovs/ovsconfig/interfaces.go +++ b/pkg/ovs/ovsconfig/interfaces.go @@ -55,4 +55,5 @@ type OVSBridgeClient interface { IsHardwareOffloadEnabled() bool GetOVSDatapathType() OVSDatapathType SetInterfaceType(name, ifType string) Error + SetPortExternalIDs(portName string, externalIDs map[string]interface{}) Error } diff --git a/pkg/ovs/ovsconfig/ovs_client.go b/pkg/ovs/ovsconfig/ovs_client.go index 695158b259f..936dfff1a73 100644 --- a/pkg/ovs/ovsconfig/ovs_client.go +++ b/pkg/ovs/ovsconfig/ovs_client.go @@ -911,3 +911,20 @@ func (br *OVSBridge) SetInterfaceType(name, ifType string) Error { } return nil } + +func (br *OVSBridge) SetPortExternalIDs(portName string, externalIDs map[string]interface{}) Error { + tx := br.ovsdb.Transaction(openvSwitchSchema) + tx.Update(dbtransaction.Update{ + Table: "Port", + Where: [][]interface{}{{"name", "==", portName}}, + Row: map[string]interface{}{ + "external_ids": helpers.MakeOVSDBMap(externalIDs), + }, + }) + _, err, temporary := tx.Commit() + if err != nil { + klog.Error("Transaction failed", err) + return NewTransactionError(err, temporary) + } + return nil +} diff --git a/pkg/ovs/ovsconfig/testing/mock_ovsconfig.go b/pkg/ovs/ovsconfig/testing/mock_ovsconfig.go index 9dc0216e47f..fd09f4a1e22 100644 --- a/pkg/ovs/ovsconfig/testing/mock_ovsconfig.go +++ b/pkg/ovs/ovsconfig/testing/mock_ovsconfig.go @@ -423,3 +423,17 @@ func (mr *MockOVSBridgeClientMockRecorder) SetInterfaceType(arg0, arg1 interface mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetInterfaceType", reflect.TypeOf((*MockOVSBridgeClient)(nil).SetInterfaceType), arg0, arg1) } + +// SetPortExternalIDs mocks base method +func (m *MockOVSBridgeClient) SetPortExternalIDs(arg0 string, arg1 map[string]interface{}) ovsconfig.Error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SetPortExternalIDs", arg0, arg1) + ret0, _ := ret[0].(ovsconfig.Error) + return ret0 +} + +// SetPortExternalIDs indicates an expected call of SetPortExternalIDs +func (mr *MockOVSBridgeClientMockRecorder) SetPortExternalIDs(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetPortExternalIDs", reflect.TypeOf((*MockOVSBridgeClient)(nil).SetPortExternalIDs), arg0, arg1) +}