Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support ipv6 in bind address #76320

Merged
merged 1 commit into from Apr 27, 2019
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -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",
@@ -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).

This comment has been minimized.

Copy link
@MrHohn

MrHohn Apr 15, 2019

Member

Would be great to preserve some of the original comments:

// addressFromDeprecatedFlags returns server address from flags
// passed on the command line based on the following rules:
// ...

This comment has been minimized.

Copy link
@JieJhih

JieJhih Apr 16, 2019

Author Member

Updated.
Thanks

// 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
@@ -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)
}

}
}
@@ -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",
@@ -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]"
}
@@ -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 {

This comment has been minimized.

Copy link
@MrHohn

MrHohn Apr 15, 2019

Member

Would be great to have a unit test for this helper func :)

This comment has been minimized.

Copy link
@JieJhih

JieJhih Apr 16, 2019

Author Member

Done.
Thanks

// 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)
}
@@ -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)
}
}
}
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.