Merge pull request #19611 from thockin/proxy-sysctl-decouple

Auto commit by PR queue bot

cmd/kube-proxy/app/server.go

@@ -188,7 +188,7 @@ func NewProxyServerDefault(config *options.ProxyServerConfig) (*ProxyServer, err
 	var proxier proxy.ProxyProvider
 	var endpointsHandler proxyconfig.EndpointsConfigHandler
 
-	proxyMode := getProxyMode(string(config.Mode), client.Nodes(), hostname, iptInterface)
+	proxyMode := getProxyMode(string(config.Mode), client.Nodes(), hostname, iptInterface, iptables.LinuxKernelCompatTester{})
 	if proxyMode == proxyModeIptables {
 		glog.V(2).Info("Using iptables Proxier.")
 		proxierIptables, err := iptables.NewProxier(iptInterface, execer, config.IPTablesSyncPeriod.Duration, config.MasqueradeAll)
@@ -308,28 +308,28 @@ type nodeGetter interface {
 	Get(hostname string) (*api.Node, error)
 }
 
-func getProxyMode(proxyMode string, client nodeGetter, hostname string, iptver iptables.IptablesVersioner) string {
+func getProxyMode(proxyMode string, client nodeGetter, hostname string, iptver iptables.IptablesVersioner, kcompat iptables.KernelCompatTester) string {
 	if proxyMode == proxyModeUserspace {
 		return proxyModeUserspace
 	} else if proxyMode == proxyModeIptables {
-		return tryIptablesProxy(iptver)
+		return tryIptablesProxy(iptver, kcompat)
 	} else if proxyMode != "" {
 		glog.V(1).Infof("Flag proxy-mode=%q unknown, assuming iptables proxy", proxyMode)
-		return tryIptablesProxy(iptver)
+		return tryIptablesProxy(iptver, kcompat)
 	}
 	// proxyMode == "" - choose the best option.
 	if client == nil {
 		glog.Errorf("nodeGetter is nil: assuming iptables proxy")
-		return tryIptablesProxy(iptver)
+		return tryIptablesProxy(iptver, kcompat)
 	}
 	node, err := client.Get(hostname)
 	if err != nil {
 		glog.Errorf("Can't get Node %q, assuming iptables proxy: %v", hostname, err)
-		return tryIptablesProxy(iptver)
+		return tryIptablesProxy(iptver, kcompat)
 	}
 	if node == nil {
 		glog.Errorf("Got nil Node %q, assuming iptables proxy: %v", hostname)
-		return tryIptablesProxy(iptver)
+		return tryIptablesProxy(iptver, kcompat)
 	}
 	proxyMode, found := node.Annotations[betaProxyModeAnnotation]
 	if found {
@@ -345,13 +345,13 @@ func getProxyMode(proxyMode string, client nodeGetter, hostname string, iptver i
 		glog.V(1).Infof("Annotation demands userspace proxy")
 		return proxyModeUserspace
 	}
-	return tryIptablesProxy(iptver)
+	return tryIptablesProxy(iptver, kcompat)
 }
 
-func tryIptablesProxy(iptver iptables.IptablesVersioner) string {
+func tryIptablesProxy(iptver iptables.IptablesVersioner, kcompat iptables.KernelCompatTester) string {
 	var err error
 	// guaranteed false on error, error only necessary for debugging
-	useIptablesProxy, err := iptables.CanUseIptablesProxier(iptver)
+	useIptablesProxy, err := iptables.CanUseIptablesProxier(iptver, kcompat)
 	if err != nil {
 		glog.Errorf("Can't determine whether to use iptables proxy, using userspace proxier: %v", err)
 		return proxyModeUserspace

cmd/kube-proxy/app/server_test.go

@@ -45,6 +45,17 @@ func (fake *fakeIptablesVersioner) GetVersion() (string, error) {
 	return fake.version, fake.err
 }
 
+type fakeKernelCompatTester struct {
+	ok bool
+}
+
+func (fake *fakeKernelCompatTester) IsCompatible() error {
+	if !fake.ok {
+		return fmt.Errorf("error")
+	}
+	return nil
+}
+
 func Test_getProxyMode(t *testing.T) {
 	if runtime.GOOS != "linux" {
 		t.Skip("skipping on non-Linux")
@@ -54,6 +65,7 @@ func Test_getProxyMode(t *testing.T) {
 		annotationKey   string
 		annotationVal   string
 		iptablesVersion string
+		kernelCompat    bool
 		iptablesError   error
 		expected        string
 	}{
@@ -71,9 +83,16 @@ func Test_getProxyMode(t *testing.T) {
 			iptablesVersion: "0.0.0",
 			expected:        proxyModeUserspace,
 		},
-		{ // flag says iptables, version ok
+		{ // flag says iptables, version ok, kernel not compatible
 			flag:            "iptables",
 			iptablesVersion: iptables.MinCheckVersion,
+			kernelCompat:    false,
+			expected:        proxyModeUserspace,
+		},
+		{ // flag says iptables, version ok, kernel is compatible
+			flag:            "iptables",
+			iptablesVersion: iptables.MinCheckVersion,
+			kernelCompat:    true,
 			expected:        proxyModeIptables,
 		},
 		{ // detect, error
@@ -86,9 +105,16 @@ func Test_getProxyMode(t *testing.T) {
 			iptablesVersion: "0.0.0",
 			expected:        proxyModeUserspace,
 		},
-		{ // detect, version ok
+		{ // detect, version ok, kernel not compatible
 			flag:            "",
 			iptablesVersion: iptables.MinCheckVersion,
+			kernelCompat:    false,
+			expected:        proxyModeUserspace,
+		},
+		{ // detect, version ok, kernel is compatible
+			flag:            "",
+			iptablesVersion: iptables.MinCheckVersion,
+			kernelCompat:    true,
 			expected:        proxyModeIptables,
 		},
 		{ // annotation says userspace
@@ -111,11 +137,20 @@ func Test_getProxyMode(t *testing.T) {
 			iptablesVersion: "0.0.0",
 			expected:        proxyModeUserspace,
 		},
-		{ // annotation says iptables, version ok
+		{ // annotation says iptables, version ok, kernel not compatible
 			flag:            "",
 			annotationKey:   "net.experimental.kubernetes.io/proxy-mode",
 			annotationVal:   "iptables",
 			iptablesVersion: iptables.MinCheckVersion,
+			kernelCompat:    false,
+			expected:        proxyModeUserspace,
+		},
+		{ // annotation says iptables, version ok, kernel is compatible
+			flag:            "",
+			annotationKey:   "net.experimental.kubernetes.io/proxy-mode",
+			annotationVal:   "iptables",
+			iptablesVersion: iptables.MinCheckVersion,
+			kernelCompat:    true,
 			expected:        proxyModeIptables,
 		},
 		{ // annotation says something else, version ok
@@ -123,6 +158,7 @@ func Test_getProxyMode(t *testing.T) {
 			annotationKey:   "net.experimental.kubernetes.io/proxy-mode",
 			annotationVal:   "other",
 			iptablesVersion: iptables.MinCheckVersion,
+			kernelCompat:    true,
 			expected:        proxyModeIptables,
 		},
 		{ // annotation says nothing, version ok
@@ -130,6 +166,7 @@ func Test_getProxyMode(t *testing.T) {
 			annotationKey:   "net.experimental.kubernetes.io/proxy-mode",
 			annotationVal:   "",
 			iptablesVersion: iptables.MinCheckVersion,
+			kernelCompat:    true,
 			expected:        proxyModeIptables,
 		},
 		{ // annotation says userspace
@@ -152,11 +189,20 @@ func Test_getProxyMode(t *testing.T) {
 			iptablesVersion: "0.0.0",
 			expected:        proxyModeUserspace,
 		},
-		{ // annotation says iptables, version ok
+		{ // annotation says iptables, version ok, kernel not compatible
 			flag:            "",
 			annotationKey:   "net.beta.kubernetes.io/proxy-mode",
 			annotationVal:   "iptables",
 			iptablesVersion: iptables.MinCheckVersion,
+			kernelCompat:    false,
+			expected:        proxyModeUserspace,
+		},
+		{ // annotation says iptables, version ok, kernel is compatible
+			flag:            "",
+			annotationKey:   "net.beta.kubernetes.io/proxy-mode",
+			annotationVal:   "iptables",
+			iptablesVersion: iptables.MinCheckVersion,
+			kernelCompat:    true,
 			expected:        proxyModeIptables,
 		},
 		{ // annotation says something else, version ok
@@ -164,6 +210,7 @@ func Test_getProxyMode(t *testing.T) {
 			annotationKey:   "net.beta.kubernetes.io/proxy-mode",
 			annotationVal:   "other",
 			iptablesVersion: iptables.MinCheckVersion,
+			kernelCompat:    true,
 			expected:        proxyModeIptables,
 		},
 		{ // annotation says nothing, version ok
@@ -171,6 +218,7 @@ func Test_getProxyMode(t *testing.T) {
 			annotationKey:   "net.beta.kubernetes.io/proxy-mode",
 			annotationVal:   "",
 			iptablesVersion: iptables.MinCheckVersion,
+			kernelCompat:    true,
 			expected:        proxyModeIptables,
 		},
 		{ // flag says userspace, annotation disagrees
@@ -185,6 +233,7 @@ func Test_getProxyMode(t *testing.T) {
 			annotationKey:   "net.experimental.kubernetes.io/proxy-mode",
 			annotationVal:   "userspace",
 			iptablesVersion: iptables.MinCheckVersion,
+			kernelCompat:    true,
 			expected:        proxyModeIptables,
 		},
 		{ // flag says userspace, annotation disagrees
@@ -199,6 +248,7 @@ func Test_getProxyMode(t *testing.T) {
 			annotationKey:   "net.beta.kubernetes.io/proxy-mode",
 			annotationVal:   "userspace",
 			iptablesVersion: iptables.MinCheckVersion,
+			kernelCompat:    true,
 			expected:        proxyModeIptables,
 		},
 	}
@@ -206,7 +256,8 @@ func Test_getProxyMode(t *testing.T) {
 		getter := &fakeNodeInterface{}
 		getter.node.Annotations = map[string]string{c.annotationKey: c.annotationVal}
 		versioner := &fakeIptablesVersioner{c.iptablesVersion, c.iptablesError}
-		r := getProxyMode(c.flag, getter, "host", versioner)
+		kcompater := &fakeKernelCompatTester{c.kernelCompat}
+		r := getProxyMode(c.flag, getter, "host", versioner, kcompater)
 		if r != c.expected {
 			t.Errorf("Case[%d] Expected %q, got %q", i, c.expected, r)
 		}

pkg/proxy/iptables/proxier.go

@@ -68,12 +68,18 @@ type IptablesVersioner interface {
 	GetVersion() (string, error)
 }
 
+// KernelCompatTester tests whether the required kernel capabilities are
+// present to run the iptables proxier.
+type KernelCompatTester interface {
+	IsCompatible() error
+}
+
 // CanUseIptablesProxier returns true if we should use the iptables Proxier
 // instead of the "classic" userspace Proxier.  This is determined by checking
 // the iptables version and for the existence of kernel features. It may return
 // an error if it fails to get the iptables version without error, in which
 // case it will also return false.
-func CanUseIptablesProxier(iptver IptablesVersioner) (bool, error) {
+func CanUseIptablesProxier(iptver IptablesVersioner, kcompat KernelCompatTester) (bool, error) {
 	minVersion, err := semver.NewVersion(iptablesMinVersion)
 	if err != nil {
 		return false, err
@@ -91,18 +97,23 @@ func CanUseIptablesProxier(iptver IptablesVersioner) (bool, error) {
 		return false, nil
 	}
 
-	// Check for the required sysctls.  We don't care about the value, just
+	// Check that the kernel supports what we need.
-	// that it exists.  If this Proxier is chosen, we'll iniialize it as we
+	if err := kcompat.IsCompatible(); err != nil {
-	// need.
-	// TODO: we should inject a sysctl.Interface like we do for iptables
-	_, err = utilsysctl.GetSysctl(sysctlRouteLocalnet)
-	if err != nil {
 		return false, err
 	}
-
 	return true, nil
 }
 
+type LinuxKernelCompatTester struct{}
+
+func (lkct LinuxKernelCompatTester) IsCompatible() error {
+	// Check for the required sysctls.  We don't care about the value, just
+	// that it exists.  If this Proxier is chosen, we'll initialize it as we
+	// need.
+	_, err := utilsysctl.GetSysctl(sysctlRouteLocalnet)
+	return err
+}
+
 const sysctlRouteLocalnet = "net/ipv4/conf/all/route_localnet"
 const sysctlBridgeCallIptables = "net/bridge/bridge-nf-call-iptables"