Skip to content

Commit

Permalink
fix(options): make clusterIP specification similar to other options
Browse files Browse the repository at this point in the history
  • Loading branch information
aauren committed Oct 7, 2023
1 parent a31511d commit b3e0768
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 43 deletions.
2 changes: 1 addition & 1 deletion docs/user-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ Usage of kube-router:
--run-router Enables Pod Networking -- Advertises and learns the routes to Pods via iBGP. (default true)
--run-service-proxy Enables Service Proxy -- sets up IPVS for Kubernetes Services. (default true)
--runtime-endpoint string Path to CRI compatible container runtime socket (used for DSR mode). Currently known working with containerd.
--service-cluster-ip-range string CIDR value from which service cluster IPs are assigned. If dual-stack is used, this can be a comma-separated list of CIDR value. Default: 10.96.0.0/12 (default "10.96.0.0/12")
--service-cluster-ip-range strings CIDR values from which service cluster IPs are assigned (can be specified up to 2 times) (default [10.96.0.0/12])
--service-external-ip-range strings Specify external IP CIDRs that are used for inter-cluster communication (can be specified multiple times)
--service-node-port-range string NodePort range specified with either a hyphen or colon (default "30000-32767")
-v, --v string log level for V logs (default "0")
Expand Down
15 changes: 5 additions & 10 deletions pkg/controllers/netpol/network_policy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -741,16 +741,11 @@ func NewNetworkPolicyController(clientset kubernetes.Interface,
npc.fullSyncRequestChan = make(chan struct{}, 1)

// Validate and parse ClusterIP service range
if config.ClusterIPCIDR == "" {
return nil, fmt.Errorf("parameter --service-cluster-ip-range is empty")
}
clusterIPCIDRList := strings.Split(config.ClusterIPCIDR, ",")

if len(clusterIPCIDRList) == 0 {
if len(config.ClusterIPCIDRs) == 0 {
return nil, fmt.Errorf("failed to get parse --service-cluster-ip-range parameter, the list is empty")
}

_, primaryIpnet, err := net.ParseCIDR(strings.TrimSpace(clusterIPCIDRList[0]))
_, primaryIpnet, err := net.ParseCIDR(strings.TrimSpace(config.ClusterIPCIDRs[0]))
if err != nil {
return nil, fmt.Errorf("failed to get parse --service-cluster-ip-range parameter: %w", err)
}
Expand All @@ -770,9 +765,9 @@ func NewNetworkPolicyController(clientset kubernetes.Interface,
}
}

if len(clusterIPCIDRList) > 1 {
if len(config.ClusterIPCIDRs) > 1 {
if config.EnableIPv4 && config.EnableIPv6 {
_, secondaryIpnet, err := net.ParseCIDR(strings.TrimSpace(clusterIPCIDRList[1]))
_, secondaryIpnet, err := net.ParseCIDR(strings.TrimSpace(config.ClusterIPCIDRs[1]))
if err != nil {
return nil, fmt.Errorf("failed to get parse --service-cluster-ip-range parameter: %v", err)
}
Expand All @@ -791,7 +786,7 @@ func NewNetworkPolicyController(clientset kubernetes.Interface,
"dual-stack must be enabled to provide two addresses")
}
}
if len(clusterIPCIDRList) > 2 {
if len(config.ClusterIPCIDRs) > 2 {
return nil, fmt.Errorf("too many CIDRs provided in --service-cluster-ip-range parameter, only two " +
"addresses are allowed at once for dual-stack")
}
Expand Down
54 changes: 27 additions & 27 deletions pkg/controllers/netpol/network_policy_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,10 +275,10 @@ func testForMissingOrUnwanted(t *testing.T, targetMsg string, got []podInfo, wan
}
}

func newMinimalKubeRouterConfig(clusterIPCIDR string, nodePortRange string, hostNameOverride string, externalIPs []string, enableIPv6 bool) *options.KubeRouterConfig {
func newMinimalKubeRouterConfig(clusterIPCIDRs []string, nodePortRange string, hostNameOverride string, externalIPs []string, enableIPv6 bool) *options.KubeRouterConfig {
kubeConfig := options.NewKubeRouterConfig()
if clusterIPCIDR != "" {
kubeConfig.ClusterIPCIDR = clusterIPCIDR
if len(clusterIPCIDRs) > 0 && clusterIPCIDRs[0] != "" {
kubeConfig.ClusterIPCIDRs = clusterIPCIDRs
}
if nodePortRange != "" {
kubeConfig.NodePortRange = nodePortRange
Expand Down Expand Up @@ -750,145 +750,145 @@ func TestNetworkPolicyController(t *testing.T) {
testCases := []tNetPolConfigTestCase{
{
"Default options are successful",
newMinimalKubeRouterConfig("", "", "node", nil, false),
newMinimalKubeRouterConfig([]string{""}, "", "node", nil, false),
false,
"",
},
{
"Missing nodename fails appropriately",
newMinimalKubeRouterConfig("", "", "", nil, false),
newMinimalKubeRouterConfig([]string{""}, "", "", nil, false),
true,
"failed to identify the node by NODE_NAME, hostname or --hostname-override",
},
{
"Test bad cluster CIDR (not properly formatting ip address)",
newMinimalKubeRouterConfig("10.10.10", "", "node", nil, false),
newMinimalKubeRouterConfig([]string{"10.10.10"}, "", "node", nil, false),
true,
"failed to get parse --service-cluster-ip-range parameter: invalid CIDR address: 10.10.10",
},
{
"Test bad cluster CIDR (not using an ip address)",
newMinimalKubeRouterConfig("foo", "", "node", nil, false),
newMinimalKubeRouterConfig([]string{"foo"}, "", "node", nil, false),
true,
"failed to get parse --service-cluster-ip-range parameter: invalid CIDR address: foo",
},
{
"Test bad cluster CIDR (using an ip address that is not a CIDR)",
newMinimalKubeRouterConfig("10.10.10.10", "", "node", nil, false),
newMinimalKubeRouterConfig([]string{"10.10.10.10"}, "", "node", nil, false),
true,
"failed to get parse --service-cluster-ip-range parameter: invalid CIDR address: 10.10.10.10",
},
{
"Test bad cluster CIDRs (using more than 2 ip addresses, including 2 ipv4)",
newMinimalKubeRouterConfig("10.96.0.0/12,10.244.0.0/16,2001:db8:42:1::/112", "", "node", nil, false),
newMinimalKubeRouterConfig([]string{"10.96.0.0/12", "10.244.0.0/16", "2001:db8:42:1::/112"}, "", "node", nil, false),
true,
"too many CIDRs provided in --service-cluster-ip-range parameter: dual-stack must be enabled to provide two addresses",
},
{
"Test bad cluster CIDRs (using more than 2 ip addresses, including 2 ipv6)",
newMinimalKubeRouterConfig("10.96.0.0/12,2001:db8:42:0::/56,2001:db8:42:1::/112", "", "node", nil, false),
newMinimalKubeRouterConfig([]string{"10.96.0.0/12", "2001:db8:42:0::/56", "2001:db8:42:1::/112"}, "", "node", nil, false),
true,
"too many CIDRs provided in --service-cluster-ip-range parameter: dual-stack must be enabled to provide two addresses",
},
{
"Test good cluster CIDR (using single IP with a /32)",
newMinimalKubeRouterConfig("10.10.10.10/32", "", "node", nil, false),
newMinimalKubeRouterConfig([]string{"10.10.10.10/32"}, "", "node", nil, false),
false,
"",
},
{
"Test good cluster CIDR (using normal range with /24)",
newMinimalKubeRouterConfig("10.10.10.0/24", "", "node", nil, false),
newMinimalKubeRouterConfig([]string{"10.10.10.0/24"}, "", "node", nil, false),
false,
"",
},
{
"Test good cluster CIDR (using ipv6)",
newMinimalKubeRouterConfig("2001:db8:42:1::/112", "", "node", []string{"2001:db8:42:1::/112"}, true),
newMinimalKubeRouterConfig([]string{"2001:db8:42:1::/112"}, "", "node", []string{"2001:db8:42:1::/112"}, true),
false,
"",
},
{
"Test good cluster CIDRs (with dual-stack)",
newMinimalKubeRouterConfig("10.96.0.0/12,2001:db8:42:1::/112", "", "node", []string{"10.96.0.0/12", "2001:db8:42:1::/112"}, true),
newMinimalKubeRouterConfig([]string{"10.96.0.0/12", "2001:db8:42:1::/112"}, "", "node", []string{"10.96.0.0/12", "2001:db8:42:1::/112"}, true),
false,
"",
},
{
"Test bad node port specification (using commas)",
newMinimalKubeRouterConfig("", "8080,8081", "node", nil, false),
newMinimalKubeRouterConfig([]string{""}, "8080,8081", "node", nil, false),
true,
"failed to parse node port range given: '8080,8081' please see specification in help text",
},
{
"Test bad node port specification (not using numbers)",
newMinimalKubeRouterConfig("", "foo:bar", "node", nil, false),
newMinimalKubeRouterConfig([]string{""}, "foo:bar", "node", nil, false),
true,
"failed to parse node port range given: 'foo:bar' please see specification in help text",
},
{
"Test bad node port specification (using anything in addition to range)",
newMinimalKubeRouterConfig("", "8080,8081-8090", "node", nil, false),
newMinimalKubeRouterConfig([]string{""}, "8080,8081-8090", "node", nil, false),
true,
"failed to parse node port range given: '8080,8081-8090' please see specification in help text",
},
{
"Test bad node port specification (using reversed range)",
newMinimalKubeRouterConfig("", "8090-8080", "node", nil, false),
newMinimalKubeRouterConfig([]string{""}, "8090-8080", "node", nil, false),
true,
"port 1 is greater than or equal to port 2 in range given: '8090-8080'",
},
{
"Test bad node port specification (port out of available range)",
newMinimalKubeRouterConfig("", "132000-132001", "node", nil, false),
newMinimalKubeRouterConfig([]string{""}, "132000-132001", "node", nil, false),
true,
"could not parse first port number from range given: '132000-132001'",
},
{
"Test good node port specification (using colon separator)",
newMinimalKubeRouterConfig("", "8080:8090", "node", nil, false),
newMinimalKubeRouterConfig([]string{""}, "8080:8090", "node", nil, false),
false,
"",
},
{
"Test good node port specification (using hyphen separator)",
newMinimalKubeRouterConfig("", "8080-8090", "node", nil, false),
newMinimalKubeRouterConfig([]string{""}, "8080-8090", "node", nil, false),
false,
"",
},
{
"Test bad external IP CIDR (not properly formatting ip address)",
newMinimalKubeRouterConfig("", "", "node", []string{"199.10.10"}, false),
newMinimalKubeRouterConfig([]string{""}, "", "node", []string{"199.10.10"}, false),
true,
"failed to get parse --service-external-ip-range parameter: '199.10.10'. Error: invalid CIDR address: 199.10.10",
},
{
"Test bad external IP CIDR (not using an ip address)",
newMinimalKubeRouterConfig("", "", "node", []string{"foo"}, false),
newMinimalKubeRouterConfig([]string{""}, "", "node", []string{"foo"}, false),
true,
"failed to get parse --service-external-ip-range parameter: 'foo'. Error: invalid CIDR address: foo",
},
{
"Test bad external IP CIDR (using an ip address that is not a CIDR)",
newMinimalKubeRouterConfig("", "", "node", []string{"199.10.10.10"}, false),
newMinimalKubeRouterConfig([]string{""}, "", "node", []string{"199.10.10.10"}, false),
true,
"failed to get parse --service-external-ip-range parameter: '199.10.10.10'. Error: invalid CIDR address: 199.10.10.10",
},
{
"Test bad external IP CIDR (making sure that it processes all items in the list)",
newMinimalKubeRouterConfig("", "", "node", []string{"199.10.10.10/32", "199.10.10.11"}, false),
newMinimalKubeRouterConfig([]string{""}, "", "node", []string{"199.10.10.10/32", "199.10.10.11"}, false),
true,
"failed to get parse --service-external-ip-range parameter: '199.10.10.11'. Error: invalid CIDR address: 199.10.10.11",
},
{
"Test good external IP CIDR (using single IP with a /32)",
newMinimalKubeRouterConfig("", "", "node", []string{"199.10.10.10/32"}, false),
newMinimalKubeRouterConfig([]string{""}, "", "node", []string{"199.10.10.10/32"}, false),
false,
"",
},
{
"Test good external IP CIDR (using normal range with /24)",
newMinimalKubeRouterConfig("", "", "node", []string{"199.10.10.10/24"}, false),
newMinimalKubeRouterConfig([]string{""}, "", "node", []string{"199.10.10.10/24"}, false),
false,
"",
},
Expand Down
9 changes: 4 additions & 5 deletions pkg/options/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ type KubeRouterConfig struct {
CacheSyncTimeout time.Duration
CleanupConfig bool
ClusterAsn uint
ClusterIPCIDR string
ClusterIPCIDRs []string
DisableSrcDstCheck bool
EnableCNI bool
EnableiBGP bool
Expand Down Expand Up @@ -84,7 +84,7 @@ func NewKubeRouterConfig() *KubeRouterConfig {
BGPGracefulRestartTime: 90 * time.Second,
BGPHoldTime: 90 * time.Second,
CacheSyncTimeout: 1 * time.Minute,
ClusterIPCIDR: "10.96.0.0/12",
ClusterIPCIDRs: []string{"10.96.0.0/12"},
EnableOverlay: true,
IPTablesSyncPeriod: 5 * time.Minute,
IpvsGracefulPeriod: 30 * time.Second,
Expand Down Expand Up @@ -215,9 +215,8 @@ func (s *KubeRouterConfig) AddFlags(fs *pflag.FlagSet) {
fs.StringVar(&s.RuntimeEndpoint, "runtime-endpoint", "",
"Path to CRI compatible container runtime socket (used for DSR mode). Currently known working with "+
"containerd.")
fs.StringVar(&s.ClusterIPCIDR, "service-cluster-ip-range", s.ClusterIPCIDR,
"CIDR value from which service cluster IPs are assigned. "+
"If dual-stack is used, this can be a comma-separated list of CIDR value. Default: 10.96.0.0/12")
fs.StringSliceVar(&s.ClusterIPCIDRs, "service-cluster-ip-range", s.ClusterIPCIDRs,
"CIDR values from which service cluster IPs are assigned (can be specified up to 2 times)")
fs.StringSliceVar(&s.ExternalIPCIDRs, "service-external-ip-range", s.ExternalIPCIDRs,
"Specify external IP CIDRs that are used for inter-cluster communication "+
"(can be specified multiple times)")
Expand Down

0 comments on commit b3e0768

Please sign in to comment.