Merge pull request #47187 from leblancd/v6_only_proxy_server

Automatic merge from submit-queue (batch tested with PRs 47309, 47187)

Add IPv6 test cases to kube-proxy server test.

**What this PR does / why we need it**: This change adds some IPv6 test cases for the kube-proxy server. Also adds some test cases for negative conditions for better test coverage.

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #47313

**Special notes for your reviewer**:

**Release note**:

```release-note
```

cmd/kube-proxy/app/BUILD

@@ -73,10 +73,13 @@ go_test(
     deps = [
         "//pkg/api:go_default_library",
         "//pkg/apis/componentconfig:go_default_library",
+        "//pkg/apis/componentconfig/v1alpha1:go_default_library",
         "//pkg/util:go_default_library",
+        "//pkg/util/configz:go_default_library",
         "//pkg/util/iptables:go_default_library",
         "//vendor/github.com/stretchr/testify/assert:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/diff:go_default_library",
     ],
 )

cmd/kube-proxy/app/server_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package app
 
 import (
+	"errors"
 	"fmt"
 	"reflect"
 	"runtime"
@@ -27,10 +28,13 @@ import (
 	"github.com/stretchr/testify/assert"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	k8sRuntime "k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/diff"
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/apis/componentconfig"
+	"k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1"
 	"k8s.io/kubernetes/pkg/util"
+	"k8s.io/kubernetes/pkg/util/configz"
 	"k8s.io/kubernetes/pkg/util/iptables"
 )
 
@@ -134,23 +138,67 @@ func Test_getProxyMode(t *testing.T) {
 	}
 }
 
-// This test verifies that Proxy Server does not crash when CleanupAndExit is true.
+// TestNewOptionsFailures tests failure modes for NewOptions()
+func TestNewOptionsFailures(t *testing.T) {
+
+	// Create a fake scheme builder that generates an error
+	errString := fmt.Sprintf("Simulated error")
+	genError := func(scheme *k8sRuntime.Scheme) error {
+		return errors.New(errString)
+	}
+	fakeSchemeBuilder := k8sRuntime.NewSchemeBuilder(genError)
+
+	simulatedErrorTest := func(target string) {
+		var addToScheme *func(s *k8sRuntime.Scheme) error
+		if target == "componentconfig" {
+			addToScheme = &componentconfig.AddToScheme
+		} else {
+			addToScheme = &v1alpha1.AddToScheme
+		}
+		restoreValue := *addToScheme
+		restore := func() {
+			*addToScheme = restoreValue
+		}
+		defer restore()
+		*addToScheme = fakeSchemeBuilder.AddToScheme
+		_, err := NewOptions()
+		assert.Error(t, err, fmt.Sprintf("Simulated error in component %s", target))
+	}
+
+	// Simulate errors in calls to AddToScheme()
+	faultTargets := []string{"componentconfig", "v1alpha1"}
+	for _, target := range faultTargets {
+		simulatedErrorTest(target)
+	}
+}
+
+// This test verifies that NewProxyServer does not crash when CleanupAndExit is true.
 func TestProxyServerWithCleanupAndExit(t *testing.T) {
-	options, err := NewOptions()
+	// Each bind address below is a separate test case
-	if err != nil {
+	bindAddresses := []string{
-		t.Fatal(err)
+		"0.0.0.0",
+		"2001:db8::1",
 	}
+	for _, addr := range bindAddresses {
+		options, err := NewOptions()
+		if err != nil {
+			t.Fatalf("Unexpected error with address %s: %v", addr, err)
+		}
 
-	options.config = &componentconfig.KubeProxyConfiguration{
+		options.config = &componentconfig.KubeProxyConfiguration{
-		BindAddress: "0.0.0.0",
+			BindAddress: addr,
+		}
+		options.CleanupAndExit = true
+
+		proxyserver, err := NewProxyServer(options.config, options.CleanupAndExit, options.scheme, options.master)
+
+		assert.Nil(t, err, "unexpected error in NewProxyServer, addr: %s", addr)
+		assert.NotNil(t, proxyserver, "nil proxy server obj, addr: %s", addr)
+		assert.NotNil(t, proxyserver.IptInterface, "nil iptables intf, addr: %s", addr)
+
+		// Clean up config for next test case
+		configz.Delete("componentconfig")
 	}
-	options.CleanupAndExit = true
-
-	proxyserver, err := NewProxyServer(options.config, options.CleanupAndExit, options.scheme, options.master)
-
-	assert.Nil(t, err)
-	assert.NotNil(t, proxyserver)
-	assert.NotNil(t, proxyserver.IptInterface)
 }
 
 func TestGetConntrackMax(t *testing.T) {
@@ -211,16 +259,18 @@ func TestGetConntrackMax(t *testing.T) {
 	}
 }
 
+// TestLoadConfig tests proper operation of loadConfig()
 func TestLoadConfig(t *testing.T) {
-	yaml := `apiVersion: componentconfig/v1alpha1
+
-bindAddress: 9.8.7.6
+	yamlTemplate := `apiVersion: componentconfig/v1alpha1
+bindAddress: %s
 clientConnection:
   acceptContentTypes: "abc"
   burst: 100
   contentType: content-type
   kubeconfig: "/path/to/kubeconfig"
   qps: 7
-clusterCIDR: "1.2.3.0/24"
+clusterCIDR: "%s"
 configSyncPeriod: 15s
 conntrack:
   max: 4
@@ -229,7 +279,7 @@ conntrack:
   tcpCloseWaitTimeout: 10s
   tcpEstablishedTimeout: 20s
 featureGates: "all"
-healthzBindAddress: 1.2.3.4:12345
+healthzBindAddress: "%s"
 hostnameOverride: "foo"
 iptables:
   masqueradeAll: true
@@ -237,7 +287,7 @@ iptables:
   minSyncPeriod: 10s
   syncPeriod: 60s
 kind: KubeProxyConfiguration
-metricsBindAddress: 2.3.4.5:23456
+metricsBindAddress: "%s"
 mode: "iptables"
 oomScoreAdj: 17
 portRange: "2-7"
@@ -245,47 +295,104 @@ resourceContainer: /foo
 udpTimeoutMilliseconds: 123ms
 `
 
-	expected := &componentconfig.KubeProxyConfiguration{
+	testCases := []struct {
-		BindAddress: "9.8.7.6",
+		name               string
-		ClientConnection: componentconfig.ClientConnectionConfiguration{
+		bindAddress        string
-			AcceptContentTypes: "abc",
+		clusterCIDR        string
-			Burst:              100,
+		healthzBindAddress string
-			ContentType:        "content-type",
+		metricsBindAddress string
-			KubeConfigFile:     "/path/to/kubeconfig",
+	}{
-			QPS:                7,
+		{
+			name:               "IPv4 config",
+			bindAddress:        "9.8.7.6",
+			clusterCIDR:        "1.2.3.0/24",
+			healthzBindAddress: "1.2.3.4:12345",
+			metricsBindAddress: "2.3.4.5:23456",
 		},
-		ClusterCIDR:      "1.2.3.0/24",
+		{
-		ConfigSyncPeriod: metav1.Duration{Duration: 15 * time.Second},
+			name:               "IPv6 config",
-		Conntrack: componentconfig.KubeProxyConntrackConfiguration{
+			bindAddress:        "2001:db8::1",
-			Max:                   4,
+			clusterCIDR:        "fd00:1::0/64",
-			MaxPerCore:            2,
+			healthzBindAddress: "[fd00:1::5]:12345",
-			Min:                   1,
+			metricsBindAddress: "[fd00:2::5]:23456",
-			TCPCloseWaitTimeout:   metav1.Duration{Duration: 10 * time.Second},
-			TCPEstablishedTimeout: metav1.Duration{Duration: 20 * time.Second},
 		},
-		FeatureGates:       "all",
-		HealthzBindAddress: "1.2.3.4:12345",
-		HostnameOverride:   "foo",
-		IPTables: componentconfig.KubeProxyIPTablesConfiguration{
-			MasqueradeAll: true,
-			MasqueradeBit: util.Int32Ptr(17),
-			MinSyncPeriod: metav1.Duration{Duration: 10 * time.Second},
-			SyncPeriod:    metav1.Duration{Duration: 60 * time.Second},
-		},
-		MetricsBindAddress: "2.3.4.5:23456",
-		Mode:               "iptables",
-		OOMScoreAdj:        util.Int32Ptr(17),
-		PortRange:          "2-7",
-		ResourceContainer:  "/foo",
-		UDPIdleTimeout:     metav1.Duration{Duration: 123 * time.Millisecond},
 	}
 
-	options, err := NewOptions()
+	for _, tc := range testCases {
-	assert.NoError(t, err)
+		expected := &componentconfig.KubeProxyConfiguration{
+			BindAddress: tc.bindAddress,
+			ClientConnection: componentconfig.ClientConnectionConfiguration{
+				AcceptContentTypes: "abc",
+				Burst:              100,
+				ContentType:        "content-type",
+				KubeConfigFile:     "/path/to/kubeconfig",
+				QPS:                7,
+			},
+			ClusterCIDR:      tc.clusterCIDR,
+			ConfigSyncPeriod: metav1.Duration{Duration: 15 * time.Second},
+			Conntrack: componentconfig.KubeProxyConntrackConfiguration{
+				Max:                   4,
+				MaxPerCore:            2,
+				Min:                   1,
+				TCPCloseWaitTimeout:   metav1.Duration{Duration: 10 * time.Second},
+				TCPEstablishedTimeout: metav1.Duration{Duration: 20 * time.Second},
+			},
+			FeatureGates:       "all",
+			HealthzBindAddress: tc.healthzBindAddress,
+			HostnameOverride:   "foo",
+			IPTables: componentconfig.KubeProxyIPTablesConfiguration{
+				MasqueradeAll: true,
+				MasqueradeBit: util.Int32Ptr(17),
+				MinSyncPeriod: metav1.Duration{Duration: 10 * time.Second},
+				SyncPeriod:    metav1.Duration{Duration: 60 * time.Second},
+			},
+			MetricsBindAddress: tc.metricsBindAddress,
+			Mode:               "iptables",
+			OOMScoreAdj:        util.Int32Ptr(17),
+			PortRange:          "2-7",
+			ResourceContainer:  "/foo",
+			UDPIdleTimeout:     metav1.Duration{Duration: 123 * time.Millisecond},
+		}
 
-	config, err := options.loadConfig([]byte(yaml))
+		options, err := NewOptions()
-	assert.NoError(t, err)
+		assert.NoError(t, err, "unexpected error for %s: %v", tc.name, err)
-	if !reflect.DeepEqual(expected, config) {
+
-		t.Fatalf("unexpected config, diff = %s", diff.ObjectDiff(config, expected))
+		yaml := fmt.Sprintf(
+			yamlTemplate, tc.bindAddress, tc.clusterCIDR,
+			tc.healthzBindAddress, tc.metricsBindAddress)
+		config, err := options.loadConfig([]byte(yaml))
+		assert.NoError(t, err, "unexpected error for %s: %v", tc.name, err)
+		if !reflect.DeepEqual(expected, config) {
+			t.Fatalf("unexpected config for %s test, diff = %s", tc.name, diff.ObjectDiff(config, expected))
+		}
+	}
+}
+
+// TestLoadConfigFailures tests failure modes for loadConfig()
+func TestLoadConfigFailures(t *testing.T) {
+	testCases := []struct {
+		name   string
+		config string
+		expErr string
+	}{
+		{
+			name:   "Decode error test",
+			config: "Twas bryllyg, and ye slythy toves",
+			expErr: "could not find expected ':'",
+		},
+		{
+			name:   "Bad config type test",
+			config: "kind: KubeSchedulerConfiguration",
+			expErr: "unexpected config type",
+		},
+	}
+	version := "apiVersion: componentconfig/v1alpha1"
+	for _, tc := range testCases {
+		options, _ := NewOptions()
+		config := fmt.Sprintf("%s\n%s", version, tc.config)
+		_, err := options.loadConfig([]byte(config))
+		if assert.Error(t, err, tc.name) {
+			assert.Contains(t, err.Error(), tc.expErr, tc.name)
+		}
 	}
 }