Skip to content

Commit

Permalink
daemon: Remove defunct --single-cluster-route flag
Browse files Browse the repository at this point in the history 
The `single-cluster-route` flag was intended to instruct Cilium to use a
single route covering the entire cluster CIDR to point to the
`cilium_host` interface in tunnel mode, instead of per-host or
per-endpoint routes.

However, the underlying concept of a cluster prefix used for routing was
removed in PR cilium#10194. It seems the `single-cluster-route`
flag was likely even broken prior to that and accidentally used the
per-node mask instead of the per-cluster mask for the route. In summary,
this flag has not been functional for almost two years. As no one seems
to have noticed, it should be safe to remove this defunct flag without a
deprecation period.

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
  • Loading branch information
@gandro
gandro committed Jan 10, 2022
1 parent f98ffa5 commit 6ea28d4
Show file tree
Hide file tree
Showing 7 changed files with 7 additions and 153 deletions.
1 change: 0 additions & 1 deletion Documentation/cmdref/cilium-agent.md
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 0 additions & 4 deletions daemon/cmd/daemon_main.go
View file
Expand Up @@ -752,10 +752,6 @@ func initializeFlags() {
"Regular expression matching compatible Istio sidecar istio-proxy container image names")
option.BindEnv(option.SidecarIstioProxyImage)

flags.Bool(option.SingleClusterRouteName, false,
"Use a single cluster route instead of per node routes")
option.BindEnv(option.SingleClusterRouteName)

flags.String(option.SocketPath, defaults.SockPath, "Sets daemon's socket path to listen for connections")
option.BindEnv(option.SocketPath)

Expand Down
37 changes: 6 additions & 31 deletions pkg/datapath/linux/node.go
View file
Expand Up @@ -1097,10 +1097,8 @@ func (n *linuxNodeHandler) nodeUpdate(oldNode, newNode *nodeTypes.Node, firstAdd
// Not a typo, the IPv4 host IP is used to build the IPv6 overlay
updateTunnelMapping(oldIP6Cidr, newNode.IPv6AllocCIDR, oldIP4, newIP4, firstAddition, n.nodeConfig.EnableIPv6, oldKey, newKey)

if !n.nodeConfig.UseSingleClusterRoute {
n.updateOrRemoveNodeRoutes([]*cidr.CIDR{oldIP4Cidr}, []*cidr.CIDR{newNode.IPv4AllocCIDR}, isLocalNode)
n.updateOrRemoveNodeRoutes([]*cidr.CIDR{oldIP6Cidr}, []*cidr.CIDR{newNode.IPv6AllocCIDR}, isLocalNode)
}
n.updateOrRemoveNodeRoutes([]*cidr.CIDR{oldIP4Cidr}, []*cidr.CIDR{newNode.IPv4AllocCIDR}, isLocalNode)
n.updateOrRemoveNodeRoutes([]*cidr.CIDR{oldIP6Cidr}, []*cidr.CIDR{newNode.IPv6AllocCIDR}, isLocalNode)

return nil
} else if firstAddition {
Expand Down Expand Up @@ -1148,11 +1146,8 @@ func (n *linuxNodeHandler) nodeDelete(oldNode *nodeTypes.Node) error {
if n.nodeConfig.EnableEncapsulation {
deleteTunnelMapping(oldNode.IPv4AllocCIDR, false)
deleteTunnelMapping(oldNode.IPv6AllocCIDR, false)

if !n.nodeConfig.UseSingleClusterRoute {
n.deleteNodeRoute(oldNode.IPv4AllocCIDR, false)
n.deleteNodeRoute(oldNode.IPv6AllocCIDR, false)
}
n.deleteNodeRoute(oldNode.IPv4AllocCIDR, false)
n.deleteNodeRoute(oldNode.IPv6AllocCIDR, false)
}

if n.enableNeighDiscovery {
Expand All @@ -1172,15 +1167,6 @@ func (n *linuxNodeHandler) nodeDelete(oldNode *nodeTypes.Node) error {
return nil
}

func (n *linuxNodeHandler) updateOrRemoveClusterRoute(addressing datapath.NodeAddressingFamily, addressFamilyEnabled bool) {
allocCIDR := addressing.AllocationCIDR()
if addressFamilyEnabled {
n.updateNodeRoute(allocCIDR, addressFamilyEnabled, false)
} else if rt, _ := n.lookupNodeRoute(allocCIDR, false); rt != nil {
n.deleteNodeRoute(allocCIDR, false)
}
}

func (n *linuxNodeHandler) replaceHostRules() error {
rule := route.Rule{
Priority: 1,
Expand Down Expand Up @@ -1544,21 +1530,10 @@ func (n *linuxNodeHandler) NodeConfigurationChanged(newConfig datapath.LocalNode
ipsec.DeleteXfrm()
}

if newConfig.UseSingleClusterRoute {
n.updateOrRemoveClusterRoute(n.nodeAddressing.IPv4(), newConfig.EnableIPv4)
n.updateOrRemoveClusterRoute(n.nodeAddressing.IPv6(), newConfig.EnableIPv6)
} else if prevConfig.UseSingleClusterRoute {
// single cluster route has been disabled, remove route
n.deleteNodeRoute(n.nodeAddressing.IPv4().AllocationCIDR(), false)
n.deleteNodeRoute(n.nodeAddressing.IPv6().AllocationCIDR(), false)
}

if !n.isInitialized {
n.isInitialized = true
if !n.nodeConfig.UseSingleClusterRoute {
for _, unlinkedNode := range n.nodes {
n.nodeUpdate(nil, unlinkedNode, true)
}
for _, unlinkedNode := range n.nodes {
n.nodeUpdate(nil, unlinkedNode, true)
}
}

Expand Down
82 changes: 0 additions & 82 deletions pkg/datapath/linux/node_linux_test.go
View file
Expand Up @@ -232,70 +232,6 @@ func (s *linuxPrivilegedBaseTestSuite) TestUpdateNodeRoute(c *check.C) {
}
}

func (s *linuxPrivilegedBaseTestSuite) TestSingleClusterPrefix(c *check.C) {
dpConfig := DatapathConfiguration{HostDevice: dummyHostDeviceName}

linuxNodeHandler := NewNodeHandler(dpConfig, s.nodeAddressing, nil).(*linuxNodeHandler)
c.Assert(linuxNodeHandler, check.Not(check.IsNil))

// enable as per test definition
err := linuxNodeHandler.NodeConfigurationChanged(datapath.LocalNodeConfiguration{
UseSingleClusterRoute: true,
EnableIPv4: s.enableIPv4,
EnableIPv6: s.enableIPv6,
})
c.Assert(err, check.IsNil)

if s.enableIPv4 {
foundRoute, err := linuxNodeHandler.lookupNodeRoute(s.nodeAddressing.IPv4().AllocationCIDR(), false)
c.Assert(err, check.IsNil)
c.Assert(foundRoute, check.Not(check.IsNil))
}

if s.enableIPv6 {
foundRoute, err := linuxNodeHandler.lookupNodeRoute(s.nodeAddressing.IPv6().AllocationCIDR(), false)
c.Assert(err, check.IsNil)
c.Assert(foundRoute, check.Not(check.IsNil))
}

// disable ipv4, enable ipv6. addressing may not be available for IPv6
err = linuxNodeHandler.NodeConfigurationChanged(datapath.LocalNodeConfiguration{
UseSingleClusterRoute: true,
EnableIPv6: true,
})
c.Assert(err, check.IsNil)

foundRoute, err := linuxNodeHandler.lookupNodeRoute(s.nodeAddressing.IPv4().AllocationCIDR(), false)
c.Assert(err, check.IsNil)
c.Assert(foundRoute, check.IsNil)

if s.enableIPv6 {
foundRoute, err := linuxNodeHandler.lookupNodeRoute(s.nodeAddressing.IPv6().AllocationCIDR(), false)
c.Assert(err, check.IsNil)
c.Assert(foundRoute, check.Not(check.IsNil))
}

// enable ipv4, enable ipv6, addressing may not be available
err = linuxNodeHandler.NodeConfigurationChanged(datapath.LocalNodeConfiguration{
UseSingleClusterRoute: true,
EnableIPv6: true,
EnableIPv4: true,
})
c.Assert(err, check.IsNil)

if s.enableIPv4 {
foundRoute, err := linuxNodeHandler.lookupNodeRoute(s.nodeAddressing.IPv4().AllocationCIDR(), false)
c.Assert(err, check.IsNil)
c.Assert(foundRoute, check.Not(check.IsNil))
}

if s.enableIPv6 {
foundRoute, err := linuxNodeHandler.lookupNodeRoute(s.nodeAddressing.IPv6().AllocationCIDR(), false)
c.Assert(err, check.IsNil)
c.Assert(foundRoute, check.Not(check.IsNil))
}
}

func (s *linuxPrivilegedBaseTestSuite) TestAuxiliaryPrefixes(c *check.C) {
net1 := cidr.MustParseCIDR("30.30.0.0/24")
net2 := cidr.MustParseCIDR("cafe:f00d::/112")
Expand Down Expand Up @@ -2647,15 +2583,6 @@ func (s *linuxPrivilegedBaseTestSuite) BenchmarkNodeUpdateEncap(c *check.C) {
})
}

func (s *linuxPrivilegedBaseTestSuite) BenchmarkNodeUpdateEncapSingleClusterRoute(c *check.C) {
s.benchmarkNodeUpdate(c, datapath.LocalNodeConfiguration{
EnableIPv4: s.enableIPv4,
EnableIPv6: s.enableIPv6,
EnableEncapsulation: true,
UseSingleClusterRoute: true,
})
}

func (s *linuxPrivilegedBaseTestSuite) BenchmarkNodeUpdateDirectRoute(c *check.C) {
s.benchmarkNodeUpdate(c, datapath.LocalNodeConfiguration{
EnableIPv4: s.enableIPv4,
Expand Down Expand Up @@ -2794,15 +2721,6 @@ func (s *linuxPrivilegedBaseTestSuite) BenchmarkNodeValidateImplementationEncap(
})
}

func (s *linuxPrivilegedBaseTestSuite) BenchmarkNodeValidateImplementationEncapSingleCluster(c *check.C) {
s.benchmarkNodeValidateImplementation(c, datapath.LocalNodeConfiguration{
EnableIPv4: s.enableIPv4,
EnableIPv6: s.enableIPv6,
EnableEncapsulation: true,
UseSingleClusterRoute: true,
})
}

func (s *linuxPrivilegedBaseTestSuite) BenchmarkNodeValidateImplementationDirectRoute(c *check.C) {
s.benchmarkNodeValidateImplementation(c, datapath.LocalNodeConfiguration{
EnableIPv4: s.enableIPv4,
Expand Down
15 changes: 0 additions & 15 deletions pkg/datapath/node.go
View file
Expand Up @@ -41,21 +41,6 @@ type LocalNodeConfiguration struct {
// subsequent calls to NodeConfigurationChanged().
EnableIPv6 bool

// UseSingleClusterRoute enables the use of a single cluster-wide route
// to direct traffic from the host into the Cilium datapath. This
// avoids the requirement to install a separate route for each node
// CIDR and can thus improve the overhead when operating large clusters
// with significant node event churn due to auto-scaling.
//
// Use of UseSingleClusterRoute must be compatible with
// EnableAutoDirectRouting. When both are enabled, any direct node
// route must take precedence over the cluster-wide route as per LPM
// routing definition.
//
// This field is mutable. The implementation of
// NodeConfigurationChanged() must adjust the routes accordingly.
UseSingleClusterRoute bool

// EnableEncapsulation enables use of encapsulation in communication
// between nodes.
//
Expand Down
1 change: 0 additions & 1 deletion pkg/nodediscovery/nodediscovery.go
View file
Expand Up @@ -102,7 +102,6 @@ func NewNodeDiscovery(manager *nodemanager.Manager, mtuConfig mtu.Configuration,
Manager: manager,
LocalConfig: datapath.LocalNodeConfiguration{
MtuConfig: mtuConfig,
UseSingleClusterRoute: option.Config.UseSingleClusterRoute,
EnableIPv4: option.Config.EnableIPv4,
EnableIPv6: option.Config.EnableIPv6,
EnableEncapsulation: option.Config.Tunnel != option.TunnelDisabled,
Expand Down
20 changes: 1 addition & 19 deletions pkg/option/config.go
View file
Expand Up @@ -476,14 +476,6 @@ const (
// TunnelPortName is the name of the TunnelPort option
TunnelPortName = "tunnel-port"

// SingleClusterRouteName is the name of the SingleClusterRoute option
//
// SingleClusterRoute enables use of a single route covering the entire
// cluster CIDR to point to the cilium_host interface instead of using
// a separate route for each cluster node CIDR. This option is not
// compatible with Tunnel=TunnelDisabled
SingleClusterRouteName = "single-cluster-route"

// MonitorAggregationName specifies the MonitorAggregationLevel on the
// comandline.
MonitorAggregationName = "monitor-aggregation"
Expand Down Expand Up @@ -1357,10 +1349,6 @@ type DaemonConfig struct {
// RunInterval. Zero means unlimited.
MaxControllerInterval int

// UseSingleClusterRoute specifies whether to use a single cluster route
// instead of per-node routes.
UseSingleClusterRoute bool

// HTTPNormalizePath switches on Envoy HTTP path normalization options, which currently
// includes RFC 3986 path normalization, Envoy merge slashes option, and unescaping and
// redirecting for paths that contain escaped slashes. These are necessary to keep path based
Expand Down Expand Up @@ -2319,12 +2307,7 @@ func (c *DaemonConfig) Validate() error {
}

switch c.Tunnel {
case TunnelVXLAN, TunnelGeneve, "":
case TunnelDisabled:
if c.UseSingleClusterRoute {
return fmt.Errorf("option --%s cannot be used in combination with --%s=%s",
SingleClusterRouteName, TunnelName, TunnelDisabled)
}
case TunnelVXLAN, TunnelGeneve, TunnelDisabled, "":
default:
return fmt.Errorf("invalid tunnel mode '%s', valid modes = {%s}", c.Tunnel, GetTunnelModes())
}
Expand Down Expand Up @@ -2617,7 +2600,6 @@ func (c *DaemonConfig) Populate() {
c.RouteMetric = viper.GetInt(RouteMetric)
c.RunDir = viper.GetString(StateDir)
c.SidecarIstioProxyImage = viper.GetString(SidecarIstioProxyImage)
c.UseSingleClusterRoute = viper.GetBool(SingleClusterRouteName)
c.SocketPath = viper.GetString(SocketPath)
c.SockopsEnable = viper.GetBool(SockopsEnableName)
c.TracePayloadlen = viper.GetInt(TracePayloadlen)
Expand Down

0 comments on commit 6ea28d4

Please sign in to comment.