Skip to content

Commit

Permalink
Validate serviceCIDR configuration if AntreaProxy is disabled
Browse files Browse the repository at this point in the history 
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 committed Nov 1, 2021
1 parent 11d1370 commit d42bd33
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 38 deletions.
2 changes: 1 addition & 1 deletion cmd/antrea-agent/agent.go
View file
Expand Up @@ -144,7 +144,7 @@ func run(o *Options) error {
egressConfig := &config.EgressConfig{
ExceptCIDRs: exceptCIDRs,
}
routeClient, err := route.NewClient(serviceCIDRNet, networkConfig, o.config.NoSNAT, o.config.AntreaProxy.ProxyAll)
routeClient, err := route.NewClient(networkConfig, o.config.NoSNAT, o.config.AntreaProxy.ProxyAll)
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
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 @@ -216,8 +204,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 @@ -257,8 +247,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
Expand Up @@ -84,7 +84,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 @@ -110,11 +109,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 bool) (*Client, error) {
func NewClient(networkConfig *config.NetworkConfig, noSNAT, proxyAll 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
Expand Up @@ -48,7 +48,6 @@ var (
type Client struct {
nodeConfig *config.NodeConfig
networkConfig *config.NetworkConfig
serviceCIDR *net.IPNet
hostRoutes *sync.Map
fwClient *winfirewall.Client
bridgeInfIndex int
Expand All @@ -57,11 +56,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 bool) (*Client, error) {
func NewClient(networkConfig *config.NetworkConfig, noSNAT, proxyAll 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
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)
client, err := NewClient(&config.NetworkConfig{}, true, false)
svcStr1 := "1.1.0.10"
svcIP1 := net.ParseIP(svcStr1)
svcIPNet1 := util.NewIPNet(svcIP1)
Expand Down
17 changes: 8 additions & 9 deletions test/integration/agent/route_test.go
View file
Expand Up @@ -59,7 +59,6 @@ var (
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"
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)
routeClient, err := route.NewClient(tc.networkConfig, tc.noSNAT, 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)
routeClient, err := route.NewClient(&config.NetworkConfig{TrafficEncapMode: config.TrafficEncapModeEncap}, 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)
routeClient, err := route.NewClient(&config.NetworkConfig{TrafficEncapMode: config.TrafficEncapModeEncap}, 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)
routeClient, err := route.NewClient(&config.NetworkConfig{TrafficEncapMode: tc.mode}, 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)
routeClient, err := route.NewClient(&config.NetworkConfig{TrafficEncapMode: tc.mode}, 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)
routeClient, err := route.NewClient(&config.NetworkConfig{TrafficEncapMode: tc.mode}, 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)
routeClient, err := route.NewClient(&config.NetworkConfig{TrafficEncapMode: config.TrafficEncapModeNetworkPolicyOnly}, 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)
routeClient, err := route.NewClient(&config.NetworkConfig{TrafficEncapMode: config.TrafficEncapModeEncap}, 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 d42bd33

Please sign in to comment.