Skip to content

Commit

Permalink
Unify some functions in pkg/agent/route (#3889)
Browse files Browse the repository at this point in the history
This commit unifies the following functions in route_linux.go and
route_windows.go:
  - AddLoadBalancer
  - DeleteLoadBalancer

Removes:
  - DeleteClusterIPRoute
  - AddClusterIPRoute

Adds a fix to restore the ClusterIP CIDR route after it is deleted, by saving
the route to the Service route cache which will be re-sync'd periodically.

Signed-off-by: Hongliang Liu <lhongliang@vmware.com>
  • Loading branch information
hongliangl authored Apr 4, 2023
1 parent 21fd096 commit 6143e78
Show file tree
Hide file tree
Showing 16 changed files with 519 additions and 431 deletions.
2 changes: 0 additions & 2 deletions cmd/antrea-agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,8 +257,6 @@ func run(o *Options) error {
stopCh,
o.nodeType,
o.config.ExternalNode.ExternalNodeNamespace,
features.DefaultFeatureGate.Enabled(features.AntreaProxy),
o.config.AntreaProxy.ProxyAll,
connectUplinkToBridge,
l7NetworkPolicyEnabled)
err = agentInitializer.Initialize()
Expand Down
8 changes: 1 addition & 7 deletions pkg/agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,9 @@ type Initializer struct {
serviceConfig *config.ServiceConfig
l7NetworkPolicyConfig *config.L7NetworkPolicyConfig
enableL7NetworkPolicy bool
enableProxy bool
connectUplinkToBridge bool
// networkReadyCh should be closed once the Node's network is ready.
// The CNI server will wait for it before handling any CNI Add requests.
proxyAll bool
networkReadyCh chan<- struct{}
stopCh <-chan struct{}
nodeType config.NodeType
Expand All @@ -140,8 +138,6 @@ func NewInitializer(
stopCh <-chan struct{},
nodeType config.NodeType,
externalNodeNamespace string,
enableProxy bool,
proxyAll bool,
connectUplinkToBridge bool,
enableL7NetworkPolicy bool,
) *Initializer {
Expand All @@ -165,8 +161,6 @@ func NewInitializer(
stopCh: stopCh,
nodeType: nodeType,
externalNodeNamespace: externalNodeNamespace,
enableProxy: enableProxy,
proxyAll: proxyAll,
connectUplinkToBridge: connectUplinkToBridge,
enableL7NetworkPolicy: enableL7NetworkPolicy,
}
Expand All @@ -177,7 +171,7 @@ func (i *Initializer) GetNodeConfig() *config.NodeConfig {
return i.nodeConfig
}

// GetNodeConfig returns the NodeConfig.
// GetWireGuardClient returns the Wireguard client.
func (i *Initializer) GetWireGuardClient() wireguard.Interface {
return i.wireGuardClient
}
Expand Down
2 changes: 0 additions & 2 deletions pkg/agent/controller/noderoute/node_route_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,6 @@ func (c *Controller) removeStaleGatewayRoutes() error {
}

// routeClient will remove orphaned routes whose destinations are not in desiredPodCIDRs.
// If proxyAll enabled, it will also remove routes that are for Windows ClusterIP Services
// which no longer exist.
if err := c.routeClient.Reconcile(desiredPodCIDRs); err != nil {
return err
}
Expand Down
32 changes: 22 additions & 10 deletions pkg/agent/nodeportlocal/rules/netnat_rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,13 @@ package rules

import (
"fmt"
"net"

"k8s.io/klog/v2"

"antrea.io/antrea/pkg/agent/route"
"antrea.io/antrea/pkg/agent/util"
binding "antrea.io/antrea/pkg/ovs/openflow"
)

// Use antrea-nat netnatstaticmapping rules as NPL implementation
Expand Down Expand Up @@ -68,13 +70,18 @@ func (nn *netnatRules) initRules() error {

// AddRule appends a NetNatStaticMapping rule.
func (nn *netnatRules) AddRule(nodePort int, podIP string, podPort int, protocol string) error {
nodePort16 := util.PortToUint16(nodePort)
podPort16 := util.PortToUint16(podPort)
podAddr := fmt.Sprintf("%s:%d", podIP, podPort16)
if err := util.ReplaceNetNatStaticMapping(antreaNatNPL, "0.0.0.0", nodePort16, podIP, podPort16, protocol); err != nil {
netNatStaticMapping := &util.NetNatStaticMapping{
Name: antreaNatNPL,
ExternalIP: net.ParseIP("0.0.0.0"),
ExternalPort: util.PortToUint16(nodePort),
InternalIP: net.ParseIP(podIP),
InternalPort: util.PortToUint16(podPort),
Protocol: binding.Protocol(protocol),
}
if err := util.ReplaceNetNatStaticMapping(netNatStaticMapping); err != nil {
return err
}
klog.InfoS("Successfully added NetNat rule", "podAddr", podAddr, "nodePort", nodePort16, "protocol", protocol)
klog.InfoS("Successfully added NetNatStaticMapping", "NetNatStaticMapping", netNatStaticMapping)
return nil
}

Expand All @@ -90,13 +97,18 @@ func (nn *netnatRules) AddAllRules(nplList []PodNodePort) error {

// DeleteRule deletes a specific NPL rule from NetNatStaticMapping table
func (nn *netnatRules) DeleteRule(nodePort int, podIP string, podPort int, protocol string) error {
nodePort16 := util.PortToUint16(nodePort)
podPort16 := util.PortToUint16(podPort)
podAddr := fmt.Sprintf("%s:%d", podIP, podPort16)
if err := util.RemoveNetNatStaticMappingByNPLTuples(antreaNatNPL, "0.0.0.0", nodePort16, podIP, podPort16, protocol); err != nil {
netNatStaticMapping := &util.NetNatStaticMapping{
Name: antreaNatNPL,
ExternalIP: net.ParseIP("0.0.0.0"),
ExternalPort: util.PortToUint16(nodePort),
InternalIP: net.ParseIP(podIP),
InternalPort: util.PortToUint16(podPort),
Protocol: binding.Protocol(protocol),
}
if err := util.RemoveNetNatStaticMappingByNPLTuples(netNatStaticMapping); err != nil {
return err
}
klog.InfoS("Successfully deleted NetNatStaticMapping rule", "podAddr", podAddr, "nodePort", nodePort16, "protocol", protocol)
klog.InfoS("Successfully deleted NetNatStaticMapping", "NetNatStaticMapping", netNatStaticMapping)
return nil
}

Expand Down
22 changes: 0 additions & 22 deletions pkg/agent/proxy/proxier.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,12 +177,6 @@ func (p *proxier) removeStaleServices() {
continue
}
}
if svcInfo.ClusterIP() != nil {
if err := p.deleteRouteForServiceIP(svcInfoStr, svcInfo.ClusterIP(), p.routeClient.DeleteClusterIPRoute); err != nil {
klog.ErrorS(err, "Failed to remove ClusterIP Service routes", "Service", svcPortName)
continue
}
}
}
// Remove LoadBalancer flows and configurations.
if p.proxyLoadBalancerIPs && len(svcInfo.LoadBalancerIPStrings()) > 0 {
Expand Down Expand Up @@ -599,13 +593,6 @@ func (p *proxier) installServices() {
continue
}
}
// If previous Service which has ClusterIP should be removed, remove ClusterIP routes.
if pSvcInfo.ClusterIP() != nil {
if err := p.deleteRouteForServiceIP(pSvcInfo.String(), pSvcInfo.ClusterIP(), p.routeClient.DeleteClusterIPRoute); err != nil {
klog.ErrorS(err, "Error when uninstalling ClusterIP route for Service", "ServicePortName", svcPortName, "ServiceInfo", svcInfoStr)
continue
}
}
}
}

Expand All @@ -628,15 +615,6 @@ func (p *proxier) installServices() {
}

if p.proxyAll {
// Install ClusterIP route on Node so that ClusterIP can be accessed on Node. Every time a new ClusterIP
// is created, the routing target IP block will be recalculated for expansion to be able to route the new
// created ClusterIP. Deleting a ClusterIP will not shrink the target routing IP block. The Service CIDR
// can be finally calculated after creating enough ClusterIPs.
if err := p.addRouteForServiceIP(svcInfo.String(), svcInfo.ClusterIP(), p.routeClient.AddClusterIPRoute); err != nil {
klog.ErrorS(err, "Error when installing ClusterIP route for Service", "ServicePortName", svcPortName, "ServiceInfo", svcInfoStr)
continue
}

// If previous Service is nil or NodePort flows and configurations of previous Service have been removed,
// install NodePort flows and configurations for current Service.
if svcInfo.NodePort() > 0 && (pSvcInfo == nil || needRemoval) {
Expand Down
Loading

0 comments on commit 6143e78

Please sign in to comment.