Skip to content

Commit

Permalink
Validate serviceCIDR configuration if AntreaProxy is disabled (antrea…
Browse files Browse the repository at this point in the history 
…-io#2936)

Move the validation for serviceCIDR under the condition that AntreaProxy
is disabled. So that the user could not ignore the item when using
AntreaProxy instead of kube-proxy.

Signed-off-by: wenyingd <wenyingd@vmware.com>
  • Loading branch information
@wenyingd
wenyingd authored Nov 9, 2021
1 parent 201e71b commit 6097f37
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 46 deletions.
2 changes: 1 addition & 1 deletion cmd/antrea-agent/agent.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ func run(o *Options) error {
egressConfig := &config.EgressConfig{
ExceptCIDRs: exceptCIDRs,
}
routeClient, err := route.NewClient(serviceCIDRNet, networkConfig, o.config.NoSNAT, o.config.AntreaProxy.ProxyAll, connectUplinkToBridge)
routeClient, err := route.NewClient(networkConfig, o.config.NoSNAT, o.config.AntreaProxy.ProxyAll, connectUplinkToBridge)
if err != nil {
return fmt.Errorf("error creating route client: %v", err)
}
Expand Down
35 changes: 18 additions & 17 deletions cmd/antrea-agent/options.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,17 +103,6 @@ func (o *Options) validate(args []string) error {
return fmt.Errorf("no positional arguments are supported")
}

// Validate service CIDR configuration
_, _, err := net.ParseCIDR(o.config.ServiceCIDR)
if err != nil {
return fmt.Errorf("Service CIDR %s is invalid", o.config.ServiceCIDR)
}
if o.config.ServiceCIDRv6 != "" {
_, _, err := net.ParseCIDR(o.config.ServiceCIDRv6)
if err != nil {
return fmt.Errorf("Service CIDR v6 %s is invalid", o.config.ServiceCIDRv6)
}
}
if o.config.TunnelType != ovsconfig.VXLANTunnel && o.config.TunnelType != ovsconfig.GeneveTunnel &&
o.config.TunnelType != ovsconfig.GRETunnel && o.config.TunnelType != ovsconfig.STTTunnel {
return fmt.Errorf("tunnel type %s is invalid", o.config.TunnelType)
Expand All @@ -131,8 +120,7 @@ func (o *Options) validate(args []string) error {
}

// Check if the enabled features are supported on the OS.
err = o.checkUnsupportedFeatures()
if err != nil {
if err := o.checkUnsupportedFeatures(); err != nil {
return err
}

Expand Down Expand Up @@ -219,8 +207,10 @@ func (o *Options) setDefaults() {
if o.config.HostProcPathPrefix == "" {
o.config.HostProcPathPrefix = defaultHostProcPathPrefix
}
if o.config.ServiceCIDR == "" {
o.config.ServiceCIDR = defaultServiceCIDR
if !features.DefaultFeatureGate.Enabled(features.AntreaProxy) {
if o.config.ServiceCIDR == "" {
o.config.ServiceCIDR = defaultServiceCIDR
}
}
if o.config.APIPort == 0 {
o.config.APIPort = apis.AntreaAgentAPIPort
Expand Down Expand Up @@ -264,8 +254,19 @@ func (o *Options) setDefaults() {
}

func (o *Options) validateAntreaProxyConfig() error {
if !features.DefaultFeatureGate.Enabled(features.AntreaProxy) && len(o.config.AntreaProxy.SkipServices) > 0 {
klog.InfoS("skipServices will be ignored because AntreaProxy is disabled", "skipServices", o.config.AntreaProxy.SkipServices)
if !features.DefaultFeatureGate.Enabled(features.AntreaProxy) {
// Validate service CIDR configuration if AntreaProxy is not enabled.
if _, _, err := net.ParseCIDR(o.config.ServiceCIDR); err != nil {
return fmt.Errorf("Service CIDR %s is invalid", o.config.ServiceCIDR)
}
if o.config.ServiceCIDRv6 != "" {
if _, _, err := net.ParseCIDR(o.config.ServiceCIDRv6); err != nil {
return fmt.Errorf("Service CIDR v6 %s is invalid", o.config.ServiceCIDRv6)
}
}
if len(o.config.AntreaProxy.SkipServices) > 0 {
klog.InfoS("skipServices will be ignored because AntreaProxy is disabled", "skipServices", o.config.AntreaProxy.SkipServices)
}
}

if o.config.AntreaProxy.ProxyAll {
Expand Down
6 changes: 1 addition & 5 deletions pkg/agent/route/route_linux.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ type Client struct {
nodeConfig *config.NodeConfig
networkConfig *config.NetworkConfig
noSNAT bool
serviceCIDR *net.IPNet
ipt *iptables.Client
// nodeRoutes caches ip routes to remote Pods. It's a map of podCIDR to routes.
nodeRoutes sync.Map
Expand All @@ -117,11 +116,8 @@ type Client struct {
}

// NewClient returns a route client.
// TODO: remove param serviceCIDR after kube-proxy is replaced by Antrea Proxy. This param is not used in this file;
// leaving it here is to be compatible with the implementation on Windows.
func NewClient(serviceCIDR *net.IPNet, networkConfig *config.NetworkConfig, noSNAT, proxyAll, connectUplinkToBridge bool) (*Client, error) {
func NewClient(networkConfig *config.NetworkConfig, noSNAT, proxyAll, connectUplinkToBridge bool) (*Client, error) {
return &Client{
serviceCIDR: serviceCIDR,
networkConfig: networkConfig,
noSNAT: noSNAT,
proxyAll: proxyAll,
Expand Down
5 changes: 1 addition & 4 deletions pkg/agent/route/route_windows.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ var (
type Client struct {
nodeConfig *config.NodeConfig
networkConfig *config.NetworkConfig
serviceCIDR *net.IPNet
hostRoutes *sync.Map
fwClient *winfirewall.Client
bridgeInfIndex int
Expand All @@ -58,11 +57,9 @@ type Client struct {
}

// NewClient returns a route client.
// Todo: remove param serviceCIDR after kube-proxy is replaced by Antrea Proxy completely.
func NewClient(serviceCIDR *net.IPNet, networkConfig *config.NetworkConfig, noSNAT, proxyAll, connectUplinkToBridge bool) (*Client, error) {
func NewClient(networkConfig *config.NetworkConfig, noSNAT, proxyAll, connectUplinkToBridge bool) (*Client, error) {
return &Client{
networkConfig: networkConfig,
serviceCIDR: serviceCIDR,
hostRoutes: &sync.Map{},
fwClient: winfirewall.NewClient(),
noSNAT: noSNAT,
Expand Down
3 changes: 1 addition & 2 deletions pkg/agent/route/route_windows_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ func TestRouteOperation(t *testing.T) {
hostGateway := "Loopback Pseudo-Interface 1"
gwLink := getNetLinkIndex("Loopback Pseudo-Interface 1")

_, serviceCIDR, _ := net.ParseCIDR("1.1.0.0/16")
peerNodeIP1 := net.ParseIP("10.0.0.2")
peerNodeIP2 := net.ParseIP("10.0.0.3")
gwIP1 := net.ParseIP("192.168.2.1")
Expand All @@ -51,7 +50,7 @@ func TestRouteOperation(t *testing.T) {
gwIP2 := net.ParseIP("192.168.3.1")
_, destCIDR2, _ := net.ParseCIDR(dest2)

client, err := NewClient(serviceCIDR, &config.NetworkConfig{}, true, false, false)
client, err := NewClient(&config.NetworkConfig{}, true, false, false)
svcStr1 := "1.1.0.10"
svcIP1 := net.ParseIP(svcStr1)
svcIPNet1 := util.NewIPNet(svcIP1)
Expand Down
33 changes: 16 additions & 17 deletions test/integration/agent/route_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,14 @@ var (
defer conn.Close()
return &utilip.DualStackIPs{IPv4: conn.LocalAddr().(*net.UDPAddr).IP}
}())
nodeLink, _ = netlink.LinkByName(nodeIntf.Name)
localPeerIP = ip.NextIP(nodeIPv4.IP)
remotePeerIP = net.ParseIP("50.50.50.1")
_, serviceCIDR, _ = net.ParseCIDR("200.200.0.0/16")
gwIP = net.ParseIP("10.10.10.1")
gwMAC, _ = net.ParseMAC("12:34:56:78:bb:cc")
gwName = "antrea-gw0"
gwConfig = &config.GatewayConfig{IPv4: gwIP, MAC: gwMAC, Name: gwName}
nodeConfig = &config.NodeConfig{
nodeLink, _ = netlink.LinkByName(nodeIntf.Name)
localPeerIP = ip.NextIP(nodeIPv4.IP)
remotePeerIP = net.ParseIP("50.50.50.1")
gwIP = net.ParseIP("10.10.10.1")
gwMAC, _ = net.ParseMAC("12:34:56:78:bb:cc")
gwName = "antrea-gw0"
gwConfig = &config.GatewayConfig{IPv4: gwIP, MAC: gwMAC, Name: gwName}
nodeConfig = &config.NodeConfig{
Name: "test",
PodIPv4CIDR: podCIDR,
NodeIPv4Addr: nodeIPv4,
Expand Down Expand Up @@ -140,7 +139,7 @@ func TestInitialize(t *testing.T) {

for _, tc := range tcs {
t.Logf("Running Initialize test with mode %s node config %s", tc.networkConfig.TrafficEncapMode, nodeConfig)
routeClient, err := route.NewClient(serviceCIDR, tc.networkConfig, tc.noSNAT, false, false)
routeClient, err := route.NewClient(tc.networkConfig, tc.noSNAT, false, false)
assert.NoError(t, err)

var xtablesReleasedTime, initializedTime time.Time
Expand Down Expand Up @@ -246,7 +245,7 @@ func TestIpTablesSync(t *testing.T) {
gwLink := createDummyGW(t)
defer netlink.LinkDel(gwLink)

routeClient, err := route.NewClient(serviceCIDR, &config.NetworkConfig{TrafficEncapMode: config.TrafficEncapModeEncap}, false, false, false)
routeClient, err := route.NewClient(&config.NetworkConfig{TrafficEncapMode: config.TrafficEncapModeEncap}, false, false, false)
assert.Nil(t, err)

inited := make(chan struct{})
Expand Down Expand Up @@ -297,7 +296,7 @@ func TestAddAndDeleteSNATRule(t *testing.T) {
gwLink := createDummyGW(t)
defer netlink.LinkDel(gwLink)

routeClient, err := route.NewClient(serviceCIDR, &config.NetworkConfig{TrafficEncapMode: config.TrafficEncapModeEncap}, false, false, false)
routeClient, err := route.NewClient(&config.NetworkConfig{TrafficEncapMode: config.TrafficEncapModeEncap}, false, false, false)
assert.Nil(t, err)

inited := make(chan struct{})
Expand Down Expand Up @@ -351,7 +350,7 @@ func TestAddAndDeleteRoutes(t *testing.T) {

for _, tc := range tcs {
t.Logf("Running test with mode %s peer cidr %s peer ip %s node config %s", tc.mode, tc.peerCIDR, tc.peerIP, nodeConfig)
routeClient, err := route.NewClient(serviceCIDR, &config.NetworkConfig{TrafficEncapMode: tc.mode}, false, false, false)
routeClient, err := route.NewClient(&config.NetworkConfig{TrafficEncapMode: tc.mode}, false, false, false)
assert.NoError(t, err)
err = routeClient.Initialize(nodeConfig, func() {})
assert.NoError(t, err)
Expand Down Expand Up @@ -414,7 +413,7 @@ func TestSyncRoutes(t *testing.T) {

for _, tc := range tcs {
t.Logf("Running test with mode %s peer cidr %s peer ip %s node config %s", tc.mode, tc.peerCIDR, tc.peerIP, nodeConfig)
routeClient, err := route.NewClient(serviceCIDR, &config.NetworkConfig{TrafficEncapMode: tc.mode}, false, false, false)
routeClient, err := route.NewClient(&config.NetworkConfig{TrafficEncapMode: tc.mode}, false, false, false)
assert.NoError(t, err)
err = routeClient.Initialize(nodeConfig, func() {})
assert.NoError(t, err)
Expand Down Expand Up @@ -508,7 +507,7 @@ func TestReconcile(t *testing.T) {

for _, tc := range tcs {
t.Logf("Running test with mode %s added routes %v desired routes %v", tc.mode, tc.addedRoutes, tc.desiredPeerCIDRs)
routeClient, err := route.NewClient(serviceCIDR, &config.NetworkConfig{TrafficEncapMode: tc.mode}, false, false, false)
routeClient, err := route.NewClient(&config.NetworkConfig{TrafficEncapMode: tc.mode}, false, false, false)
assert.NoError(t, err)
err = routeClient.Initialize(nodeConfig, func() {})
assert.NoError(t, err)
Expand Down Expand Up @@ -546,7 +545,7 @@ func TestRouteTablePolicyOnly(t *testing.T) {
gwLink := createDummyGW(t)
defer netlink.LinkDel(gwLink)

routeClient, err := route.NewClient(serviceCIDR, &config.NetworkConfig{TrafficEncapMode: config.TrafficEncapModeNetworkPolicyOnly}, false, false, false)
routeClient, err := route.NewClient(&config.NetworkConfig{TrafficEncapMode: config.TrafficEncapModeNetworkPolicyOnly}, false, false, false)
assert.NoError(t, err)
err = routeClient.Initialize(nodeConfig, func() {})
assert.NoError(t, err)
Expand Down Expand Up @@ -602,7 +601,7 @@ func TestIPv6RoutesAndNeighbors(t *testing.T) {
gwLink := createDummyGW(t)
defer netlink.LinkDel(gwLink)

routeClient, err := route.NewClient(serviceCIDR, &config.NetworkConfig{TrafficEncapMode: config.TrafficEncapModeEncap}, false, false, false)
routeClient, err := route.NewClient(&config.NetworkConfig{TrafficEncapMode: config.TrafficEncapModeEncap}, false, false, false)
assert.Nil(t, err)
_, ipv6Subnet, _ := net.ParseCIDR("fd74:ca9b:172:19::/64")
gwIPv6 := net.ParseIP("fd74:ca9b:172:19::1")
Expand Down

0 comments on commit 6097f37

Please sign in to comment.