From 08e320fa4ef96880f0c8ac7ee69f118306c95702 Mon Sep 17 00:00:00 2001 From: JieJhih Jhang Date: Tue, 9 Apr 2019 15:30:11 +0800 Subject: [PATCH] support ipv6 in bind address use split host port func instead trim specific character add unit test for metrics and healthz bind address recover import package refactor set default kube proxy configuration fix ipv4 condition fix set default port condition rewrite call function occasion to reduce error set ipv6 default value move get GetBindAddressHostPort to util use one func to handle deprecated series update bazel define address type return earlier in the error case refactor set default kube proxy configuration logic recover import package preserve some of the original comments add get default address func add append port if needed unit test rewrite unit test for deprecated flags remove unused codes --- cmd/kube-proxy/app/BUILD | 1 + cmd/kube-proxy/app/server.go | 44 ++++--------- cmd/kube-proxy/app/server_test.go | 76 ++++++++++++++++++++++ pkg/proxy/apis/config/v1alpha1/BUILD | 1 + pkg/proxy/apis/config/v1alpha1/defaults.go | 31 +++++++-- pkg/proxy/util/utils.go | 21 ++++++ pkg/proxy/util/utils_test.go | 47 +++++++++++++ 7 files changed, 181 insertions(+), 40 deletions(-) diff --git a/cmd/kube-proxy/app/BUILD b/cmd/kube-proxy/app/BUILD index 38459a7bcec..667faf2f26b 100644 --- a/cmd/kube-proxy/app/BUILD +++ b/cmd/kube-proxy/app/BUILD @@ -31,6 +31,7 @@ go_library( "//pkg/proxy/iptables:go_default_library", "//pkg/proxy/ipvs:go_default_library", "//pkg/proxy/userspace:go_default_library", + "//pkg/proxy/util:go_default_library", "//pkg/util/configz:go_default_library", "//pkg/util/filesystem:go_default_library", "//pkg/util/flag:go_default_library", diff --git a/cmd/kube-proxy/app/server.go b/cmd/kube-proxy/app/server.go index c91f93244fc..efa85e4116c 100644 --- a/cmd/kube-proxy/app/server.go +++ b/cmd/kube-proxy/app/server.go @@ -61,6 +61,7 @@ import ( "k8s.io/kubernetes/pkg/proxy/iptables" "k8s.io/kubernetes/pkg/proxy/ipvs" "k8s.io/kubernetes/pkg/proxy/userspace" + proxyutil "k8s.io/kubernetes/pkg/proxy/util" "k8s.io/kubernetes/pkg/util/configz" "k8s.io/kubernetes/pkg/util/filesystem" utilflag "k8s.io/kubernetes/pkg/util/flag" @@ -212,8 +213,8 @@ func NewOptions() *Options { func (o *Options) Complete() error { if len(o.ConfigFile) == 0 && len(o.WriteConfigTo) == 0 { klog.Warning("WARNING: all flags other than --config, --write-config-to, and --cleanup are deprecated. Please begin using a config file ASAP.") - o.applyDeprecatedHealthzPortToConfig() - o.applyDeprecatedMetricsPortToConfig() + o.config.HealthzBindAddress = addressFromDeprecatedFlags(o.config.HealthzBindAddress, o.healthzPort) + o.config.MetricsBindAddress = addressFromDeprecatedFlags(o.config.MetricsBindAddress, o.metricsPort) } // Load the config file here in Complete, so that Validate validates the fully-resolved config. @@ -357,38 +358,15 @@ func (o *Options) writeConfigFile() error { return nil } -// applyDeprecatedHealthzPortToConfig sets o.config.HealthzBindAddress from -// flags passed on the command line based on the following rules: -// -// 1. If --healthz-port is 0, disable the healthz server. -// 2. Otherwise, use the value of --healthz-port for the port portion of -// o.config.HealthzBindAddress -func (o *Options) applyDeprecatedHealthzPortToConfig() { - if o.healthzPort == 0 { - o.config.HealthzBindAddress = "" - return +// addressFromDeprecatedFlags returns server address from flags +// passed on the command line based on the following rules: +// 1. If port is 0, disable the server (e.g. set address to empty). +// 2. Otherwise, set the port portion of the config accordingly. +func addressFromDeprecatedFlags(addr string, port int32) string { + if port == 0 { + return "" } - - index := strings.Index(o.config.HealthzBindAddress, ":") - if index != -1 { - o.config.HealthzBindAddress = o.config.HealthzBindAddress[0:index] - } - - o.config.HealthzBindAddress = fmt.Sprintf("%s:%d", o.config.HealthzBindAddress, o.healthzPort) -} - -func (o *Options) applyDeprecatedMetricsPortToConfig() { - if o.metricsPort == 0 { - o.config.MetricsBindAddress = "" - return - } - - index := strings.Index(o.config.MetricsBindAddress, ":") - if index != -1 { - o.config.MetricsBindAddress = o.config.MetricsBindAddress[0:index] - } - - o.config.MetricsBindAddress = fmt.Sprintf("%s:%d", o.config.MetricsBindAddress, o.metricsPort) + return proxyutil.AppendPortIfNeeded(addr, port) } // loadConfigFromFile loads the contents of file and decodes it as a diff --git a/cmd/kube-proxy/app/server_test.go b/cmd/kube-proxy/app/server_test.go index 345e783cf20..dabafa56df3 100644 --- a/cmd/kube-proxy/app/server_test.go +++ b/cmd/kube-proxy/app/server_test.go @@ -549,3 +549,79 @@ func (s *fakeProxyServerError) Run() error { return fmt.Errorf("mocking error from ProxyServer.Run()") } } + +func TestAddressFromDeprecatedFlags(t *testing.T) { + testCases := []struct { + name string + healthzPort int32 + healthzBindAddress string + metricsPort int32 + metricsBindAddress string + expHealthz string + expMetrics string + }{ + { + name: "IPv4 bind address", + healthzBindAddress: "1.2.3.4", + healthzPort: 12345, + metricsBindAddress: "2.3.4.5", + metricsPort: 23456, + expHealthz: "1.2.3.4:12345", + expMetrics: "2.3.4.5:23456", + }, + { + name: "IPv4 bind address has port", + healthzBindAddress: "1.2.3.4:12345", + healthzPort: 23456, + metricsBindAddress: "2.3.4.5:12345", + metricsPort: 23456, + expHealthz: "1.2.3.4:12345", + expMetrics: "2.3.4.5:12345", + }, + { + name: "IPv6 bind address", + healthzBindAddress: "fd00:1::5", + healthzPort: 12345, + metricsBindAddress: "fd00:1::6", + metricsPort: 23456, + expHealthz: "[fd00:1::5]:12345", + expMetrics: "[fd00:1::6]:23456", + }, + { + name: "IPv6 bind address has port", + healthzBindAddress: "[fd00:1::5]:12345", + healthzPort: 56789, + metricsBindAddress: "[fd00:1::6]:56789", + metricsPort: 12345, + expHealthz: "[fd00:1::5]:12345", + expMetrics: "[fd00:1::6]:56789", + }, + { + name: "Invalid IPv6 Config", + healthzBindAddress: "[fd00:1::5]", + healthzPort: 12345, + metricsBindAddress: "[fd00:1::6]", + metricsPort: 56789, + expHealthz: "[fd00:1::5]", + expMetrics: "[fd00:1::6]", + }, + } + + for i := range testCases { + gotHealthz := addressFromDeprecatedFlags(testCases[i].healthzBindAddress, testCases[i].healthzPort) + gotMetrics := addressFromDeprecatedFlags(testCases[i].metricsBindAddress, testCases[i].metricsPort) + + errFn := func(name, except, got string) { + t.Errorf("case %s: expected %v, got %v", name, except, got) + } + + if gotHealthz != testCases[i].expHealthz { + errFn(testCases[i].name, testCases[i].expHealthz, gotHealthz) + } + + if gotMetrics != testCases[i].expMetrics { + errFn(testCases[i].name, testCases[i].expMetrics, gotMetrics) + } + + } +} diff --git a/pkg/proxy/apis/config/v1alpha1/BUILD b/pkg/proxy/apis/config/v1alpha1/BUILD index aed068d8458..d1f27332a41 100644 --- a/pkg/proxy/apis/config/v1alpha1/BUILD +++ b/pkg/proxy/apis/config/v1alpha1/BUILD @@ -20,6 +20,7 @@ go_library( "//pkg/kubelet/qos:go_default_library", "//pkg/master/ports:go_default_library", "//pkg/proxy/apis/config:go_default_library", + "//pkg/proxy/util:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/conversion:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", diff --git a/pkg/proxy/apis/config/v1alpha1/defaults.go b/pkg/proxy/apis/config/v1alpha1/defaults.go index b6417744a23..5c7489d7e02 100644 --- a/pkg/proxy/apis/config/v1alpha1/defaults.go +++ b/pkg/proxy/apis/config/v1alpha1/defaults.go @@ -18,14 +18,16 @@ package v1alpha1 import ( "fmt" - "strings" + "net" "time" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" kruntime "k8s.io/apimachinery/pkg/runtime" kubeproxyconfigv1alpha1 "k8s.io/kube-proxy/config/v1alpha1" + "k8s.io/kubernetes/pkg/kubelet/qos" "k8s.io/kubernetes/pkg/master/ports" + proxyutil "k8s.io/kubernetes/pkg/proxy/util" "k8s.io/utils/pointer" ) @@ -34,19 +36,24 @@ func addDefaultingFuncs(scheme *kruntime.Scheme) error { } func SetDefaults_KubeProxyConfiguration(obj *kubeproxyconfigv1alpha1.KubeProxyConfiguration) { + if len(obj.BindAddress) == 0 { obj.BindAddress = "0.0.0.0" } + + defaultHealthzAddress, defaultMetricsAddress := getDefaultAddresses(obj.BindAddress) + if obj.HealthzBindAddress == "" { - obj.HealthzBindAddress = fmt.Sprintf("0.0.0.0:%v", ports.ProxyHealthzPort) - } else if !strings.Contains(obj.HealthzBindAddress, ":") { - obj.HealthzBindAddress += fmt.Sprintf(":%v", ports.ProxyHealthzPort) + obj.HealthzBindAddress = fmt.Sprintf("%s:%v", defaultHealthzAddress, ports.ProxyHealthzPort) + } else { + obj.HealthzBindAddress = proxyutil.AppendPortIfNeeded(obj.HealthzBindAddress, ports.ProxyHealthzPort) } if obj.MetricsBindAddress == "" { - obj.MetricsBindAddress = fmt.Sprintf("127.0.0.1:%v", ports.ProxyStatusPort) - } else if !strings.Contains(obj.MetricsBindAddress, ":") { - obj.MetricsBindAddress += fmt.Sprintf(":%v", ports.ProxyStatusPort) + obj.MetricsBindAddress = fmt.Sprintf("%s:%v", defaultMetricsAddress, ports.ProxyStatusPort) + } else { + obj.MetricsBindAddress = proxyutil.AppendPortIfNeeded(obj.MetricsBindAddress, ports.ProxyStatusPort) } + if obj.OOMScoreAdj == nil { temp := int32(qos.KubeProxyOOMScoreAdj) obj.OOMScoreAdj = &temp @@ -121,3 +128,13 @@ func SetDefaults_KubeProxyConfiguration(obj *kubeproxyconfigv1alpha1.KubeProxyCo obj.FeatureGates = make(map[string]bool) } } + +// getDefaultAddresses returns default address of healthz and metrics server +// based on the given bind address. IPv6 addresses are enclosed in square +// brackets for appending port. +func getDefaultAddresses(bindAddress string) (defaultHealthzAddress, defaultMetricsAddress string) { + if net.ParseIP(bindAddress).To4() != nil { + return "0.0.0.0", "127.0.0.1" + } + return "[::]", "[::1]" +} diff --git a/pkg/proxy/util/utils.go b/pkg/proxy/util/utils.go index c513d26a5a9..822da8534d3 100644 --- a/pkg/proxy/util/utils.go +++ b/pkg/proxy/util/utils.go @@ -214,3 +214,24 @@ func filterWithCondition(strs []string, expectedCondition bool, conditionFunc fu } return corrects, incorrects } + +// AppendPortIfNeeded appends the given port to IP address unless it is already in +// "ipv4:port" or "[ipv6]:port" format. +func AppendPortIfNeeded(addr string, port int32) string { + // Return if address is already in "ipv4:port" or "[ipv6]:port" format. + if _, _, err := net.SplitHostPort(addr); err == nil { + return addr + } + + // Simply return for invalid case. This should be caught by validation instead. + ip := net.ParseIP(addr) + if ip == nil { + return addr + } + + // Append port to address. + if ip.To4() != nil { + return fmt.Sprintf("%s:%d", addr, port) + } + return fmt.Sprintf("[%s]:%d", addr, port) +} diff --git a/pkg/proxy/util/utils_test.go b/pkg/proxy/util/utils_test.go index 891a3520f1c..9abf40fed4e 100644 --- a/pkg/proxy/util/utils_test.go +++ b/pkg/proxy/util/utils_test.go @@ -396,3 +396,50 @@ func TestGetNodeAddressses(t *testing.T) { } } } + +func TestAppendPortIfNeeded(t *testing.T) { + testCases := []struct { + name string + addr string + port int32 + expect string + }{ + { + name: "IPv4 all-zeros bind address has port", + addr: "0.0.0.0:12345", + port: 23456, + expect: "0.0.0.0:12345", + }, + { + name: "non-zeros IPv4 config", + addr: "9.8.7.6", + port: 12345, + expect: "9.8.7.6:12345", + }, + { + name: "IPv6 \"[::]\" bind address has port", + addr: "[::]:12345", + port: 23456, + expect: "[::]:12345", + }, + { + name: "IPv6 config", + addr: "fd00:1::5", + port: 23456, + expect: "[fd00:1::5]:23456", + }, + { + name: "Invalid IPv6 Config", + addr: "[fd00:1::5]", + port: 12345, + expect: "[fd00:1::5]", + }, + } + + for i := range testCases { + got := AppendPortIfNeeded(testCases[i].addr, testCases[i].port) + if testCases[i].expect != got { + t.Errorf("case %s: expected %v, got %v", testCases[i].name, testCases[i].expect, got) + } + } +}