Merge pull request #123105 from danwinship/nodeport-addresses-primary

Add `--nodeport-addresses primary`, warn on empty `--nodeport-addresses`
This commit is contained in:
Kubernetes Prow Robot
2024-04-18 08:49:21 -07:00
committed by GitHub
20 changed files with 214 additions and 238 deletions

View File

@@ -197,7 +197,7 @@ func (o *Options) AddFlags(fs *pflag.FlagSet) {
"This parameter is ignored if a config file is specified by --config.")
fs.StringSliceVar(&o.config.NodePortAddresses, "nodeport-addresses", o.config.NodePortAddresses,
"A list of CIDR ranges that contain valid node IPs. If set, connections to NodePort services will only be accepted on node IPs in one of the indicated ranges. If unset, NodePort connections will be accepted on all local IPs. This parameter is ignored if a config file is specified by --config.")
"A list of CIDR ranges that contain valid node IPs, or alternatively, the single string 'primary'. If set to a list of CIDRs, connections to NodePort services will only be accepted on node IPs in one of the indicated ranges. If set to 'primary', NodePort services will only be accepted on the node's primary IP(s) according to the Node object. If unset, NodePort connections will be accepted on all local IPs. This parameter is ignored if a config file is specified by --config.")
fs.Int32Var(o.config.OOMScoreAdj, "oom-score-adj", ptr.Deref(o.config.OOMScoreAdj, int32(qos.KubeProxyOOMScoreAdj)), "The oom-score-adj value for kube-proxy process. Values must be within the range [-1000, 1000]. This parameter is ignored if a config file is specified by --config.")
fs.Int32Var(o.config.Conntrack.MaxPerCore, "conntrack-max-per-core", *o.config.Conntrack.MaxPerCore,
@@ -631,6 +631,17 @@ func newProxyServer(logger klog.Logger, config *kubeproxyconfig.KubeProxyConfigu
rawNodeIPs := getNodeIPs(logger, s.Client, s.Hostname)
s.PrimaryIPFamily, s.NodeIPs = detectNodeIPs(logger, rawNodeIPs, config.BindAddress)
if len(config.NodePortAddresses) == 1 && config.NodePortAddresses[0] == kubeproxyconfig.NodePortAddressesPrimary {
var nodePortAddresses []string
if nodeIP := s.NodeIPs[v1.IPv4Protocol]; nodeIP != nil && !nodeIP.IsLoopback() {
nodePortAddresses = append(nodePortAddresses, fmt.Sprintf("%s/32", nodeIP.String()))
}
if nodeIP := s.NodeIPs[v1.IPv6Protocol]; nodeIP != nil && !nodeIP.IsLoopback() {
nodePortAddresses = append(nodePortAddresses, fmt.Sprintf("%s/128", nodeIP.String()))
}
config.NodePortAddresses = nodePortAddresses
}
s.Broadcaster = events.NewBroadcaster(&events.EventSinkImpl{Interface: s.Client.EventsV1()})
s.Recorder = s.Broadcaster.NewRecorder(proxyconfigscheme.Scheme, "kube-proxy")
@@ -650,6 +661,11 @@ func newProxyServer(logger klog.Logger, config *kubeproxyconfig.KubeProxyConfigu
return nil, err
}
err = checkBadConfig(s)
if err != nil {
logger.Error(err, "Kube-proxy configuration may be incomplete or incorrect")
}
ipv4Supported, ipv6Supported, dualStackSupported, err := s.platformCheckSupported()
if err != nil {
return nil, err
@@ -661,7 +677,7 @@ func newProxyServer(logger klog.Logger, config *kubeproxyconfig.KubeProxyConfigu
logger.Info("kube-proxy running in single-stack mode", "ipFamily", s.PrimaryIPFamily)
}
err, fatal := checkIPConfig(s, dualStackSupported)
err, fatal := checkBadIPConfig(s, dualStackSupported)
if err != nil {
if fatal {
return nil, fmt.Errorf("kube-proxy configuration is incorrect: %v", err)
@@ -677,8 +693,42 @@ func newProxyServer(logger klog.Logger, config *kubeproxyconfig.KubeProxyConfigu
return s, nil
}
// checkIPConfig confirms that s has proper configuration for its primary IP family.
func checkIPConfig(s *ProxyServer, dualStackSupported bool) (error, bool) {
// checkBadConfig checks for bad/deprecated configuation
func checkBadConfig(s *ProxyServer) error {
var errors []error
// At this point we haven't seen any actual Services or EndpointSlices, so we
// don't really know if the cluster is expected to be single- or dual-stack. But
// we can at least take note of whether there is any explicitly-dual-stack
// configuration.
anyDualStackConfig := false
clusterCIDRs := strings.Split(s.Config.ClusterCIDR, ",")
for _, config := range [][]string{clusterCIDRs, s.Config.NodePortAddresses, s.Config.IPVS.ExcludeCIDRs, s.podCIDRs} {
if dual, _ := netutils.IsDualStackCIDRStrings(config); dual {
anyDualStackConfig = true
break
}
}
// Warn if NodePortAddresses does not limit connections on all IP families that
// seem to be in use.
cidrsByFamily := proxyutil.MapCIDRsByIPFamily(s.Config.NodePortAddresses)
if len(s.Config.NodePortAddresses) == 0 {
errors = append(errors, fmt.Errorf("nodePortAddresses is unset; NodePort connections will be accepted on all local IPs. Consider using `--nodeport-addresses primary`"))
} else if anyDualStackConfig && len(cidrsByFamily[s.PrimaryIPFamily]) == len(s.Config.NodePortAddresses) {
errors = append(errors, fmt.Errorf("cluster appears to be dual-stack but nodePortAddresses contains only %s addresses; NodePort connections will be accepted on all local %s IPs", s.PrimaryIPFamily, proxyutil.OtherIPFamily(s.PrimaryIPFamily)))
} else if len(cidrsByFamily[s.PrimaryIPFamily]) == 0 {
errors = append(errors, fmt.Errorf("cluster appears to be %s-primary but nodePortAddresses contains only %s addresses; NodePort connections will be accepted on all local %s IPs", s.PrimaryIPFamily, proxyutil.OtherIPFamily(s.PrimaryIPFamily), s.PrimaryIPFamily))
}
return utilerrors.NewAggregate(errors)
}
// checkBadIPConfig checks for bad configuration relative to s.PrimaryIPFamily.
// Historically, we did not check most of the config options, so we cannot retroactively
// make IP family mismatches in those options be fatal. When we add new options to check
// here, we should make problems with those options be fatal.
func checkBadIPConfig(s *ProxyServer, dualStackSupported bool) (err error, fatal bool) {
var errors []error
var badFamily netutils.IPFamily
@@ -695,11 +745,6 @@ func checkIPConfig(s *ProxyServer, dualStackSupported bool) (error, bool) {
clusterType = fmt.Sprintf("%s-only", s.PrimaryIPFamily)
}
// Historically, we did not check most of the config options, so we cannot
// retroactively make IP family mismatches in those options be fatal. When we add
// new options to check here, we should make problems with those options be fatal.
fatal := false
if s.Config.ClusterCIDR != "" {
clusterCIDRs := strings.Split(s.Config.ClusterCIDR, ",")
if badCIDRs(clusterCIDRs, badFamily) {
@@ -711,10 +756,6 @@ func checkIPConfig(s *ProxyServer, dualStackSupported bool) (error, bool) {
}
}
if badCIDRs(s.Config.NodePortAddresses, badFamily) {
errors = append(errors, fmt.Errorf("cluster is %s but nodePortAddresses contains only IPv%s addresses", clusterType, badFamily))
}
if badCIDRs(s.podCIDRs, badFamily) {
errors = append(errors, fmt.Errorf("cluster is %s but node.spec.podCIDRs contains only IPv%s addresses", clusterType, badFamily))
if s.Config.DetectLocalMode == kubeproxyconfig.LocalModeNodeCIDR {
@@ -742,6 +783,9 @@ func checkIPConfig(s *ProxyServer, dualStackSupported bool) (error, bool) {
}
}
// Note that s.Config.NodePortAddresses gets checked as part of checkBadConfig()
// so it doesn't need to be checked here.
return utilerrors.NewAggregate(errors), fatal
}

View File

@@ -70,6 +70,10 @@ func (o *Options) platformApplyDefaults(config *proxyconfigapi.KubeProxyConfigur
config.Mode = proxyconfigapi.ProxyModeIPTables
}
if config.Mode == proxyconfigapi.ProxyModeNFTables && len(config.NodePortAddresses) == 0 {
config.NodePortAddresses = []string{proxyconfigapi.NodePortAddressesPrimary}
}
if config.DetectLocalMode == "" {
o.logger.V(4).Info("Defaulting detect-local-mode", "localModeClusterCIDR", string(proxyconfigapi.LocalModeClusterCIDR))
config.DetectLocalMode = proxyconfigapi.LocalModeClusterCIDR

View File

@@ -849,7 +849,81 @@ func Test_detectNodeIPs(t *testing.T) {
}
}
func Test_checkIPConfig(t *testing.T) {
func Test_checkBadConfig(t *testing.T) {
cases := []struct {
name string
proxy *ProxyServer
err bool
}{
{
name: "single-stack NodePortAddresses with single-stack config",
proxy: &ProxyServer{
Config: &kubeproxyconfig.KubeProxyConfiguration{
ClusterCIDR: "10.0.0.0/8",
NodePortAddresses: []string{"192.168.0.0/24"},
},
PrimaryIPFamily: v1.IPv4Protocol,
},
err: false,
},
{
name: "dual-stack NodePortAddresses with dual-stack config",
proxy: &ProxyServer{
Config: &kubeproxyconfig.KubeProxyConfiguration{
ClusterCIDR: "10.0.0.0/8,fd09::/64",
NodePortAddresses: []string{"192.168.0.0/24", "fd03::/64"},
},
PrimaryIPFamily: v1.IPv4Protocol,
},
err: false,
},
{
name: "empty NodePortAddresses",
proxy: &ProxyServer{
Config: &kubeproxyconfig.KubeProxyConfiguration{
NodePortAddresses: []string{},
},
PrimaryIPFamily: v1.IPv4Protocol,
},
err: true,
},
{
name: "single-stack NodePortAddresses with dual-stack config",
proxy: &ProxyServer{
Config: &kubeproxyconfig.KubeProxyConfiguration{
ClusterCIDR: "10.0.0.0/8,fd09::/64",
NodePortAddresses: []string{"192.168.0.0/24"},
},
PrimaryIPFamily: v1.IPv4Protocol,
},
err: true,
},
{
name: "wrong-single-stack NodePortAddresses",
proxy: &ProxyServer{
Config: &kubeproxyconfig.KubeProxyConfiguration{
ClusterCIDR: "fd09::/64",
NodePortAddresses: []string{"192.168.0.0/24"},
},
PrimaryIPFamily: v1.IPv6Protocol,
},
err: true,
},
}
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
err := checkBadConfig(c.proxy)
if err != nil && !c.err {
t.Errorf("unexpected error: %v", err)
} else if err == nil && c.err {
t.Errorf("unexpected lack of error")
}
})
}
}
func Test_checkBadIPConfig(t *testing.T) {
cases := []struct {
name string
proxy *ProxyServer
@@ -929,53 +1003,6 @@ func Test_checkIPConfig(t *testing.T) {
dsFatal: false,
},
{
name: "ok single-stack nodePortAddresses",
proxy: &ProxyServer{
Config: &kubeproxyconfig.KubeProxyConfiguration{
NodePortAddresses: []string{"10.0.0.0/8", "192.168.0.0/24"},
},
PrimaryIPFamily: v1.IPv4Protocol,
},
ssErr: false,
dsErr: false,
},
{
name: "ok dual-stack nodePortAddresses",
proxy: &ProxyServer{
Config: &kubeproxyconfig.KubeProxyConfiguration{
NodePortAddresses: []string{"10.0.0.0/8", "fd01:2345::/64", "fd01:abcd::/64"},
},
PrimaryIPFamily: v1.IPv4Protocol,
},
ssErr: false,
dsErr: false,
},
{
name: "ok reversed dual-stack nodePortAddresses",
proxy: &ProxyServer{
Config: &kubeproxyconfig.KubeProxyConfiguration{
NodePortAddresses: []string{"fd01:2345::/64", "fd01:abcd::/64", "10.0.0.0/8"},
},
PrimaryIPFamily: v1.IPv4Protocol,
},
ssErr: false,
dsErr: false,
},
{
name: "wrong-family nodePortAddresses",
proxy: &ProxyServer{
Config: &kubeproxyconfig.KubeProxyConfiguration{
NodePortAddresses: []string{"10.0.0.0/8"},
},
PrimaryIPFamily: v1.IPv6Protocol,
},
ssErr: true,
ssFatal: false,
dsErr: true,
dsFatal: false,
},
{
name: "ok single-stack node.spec.podCIDRs",
proxy: &ProxyServer{
@@ -1133,7 +1160,7 @@ func Test_checkIPConfig(t *testing.T) {
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
err, fatal := checkIPConfig(c.proxy, false)
err, fatal := checkBadIPConfig(c.proxy, false)
if err != nil && !c.ssErr {
t.Errorf("unexpected error in single-stack case: %v", err)
} else if err == nil && c.ssErr {
@@ -1142,7 +1169,7 @@ func Test_checkIPConfig(t *testing.T) {
t.Errorf("expected fatal=%v, got %v", c.ssFatal, fatal)
}
err, fatal = checkIPConfig(c.proxy, true)
err, fatal = checkBadIPConfig(c.proxy, true)
if err != nil && !c.dsErr {
t.Errorf("unexpected error in dual-stack case: %v", err)
} else if err == nil && c.dsErr {