From ce52f70c6610cd8e905c71b6d008608161bee9fc Mon Sep 17 00:00:00 2001 From: Daniel Canter Date: Mon, 13 Sep 2021 00:56:31 -0700 Subject: [PATCH] Replace custom dualstack support logic in Windows Kube-proxy Due to an incorrect version range definition in hcsshim for dualstack support, the Windows kubeproxy had to define it's own version range logic to check if dualstack was supported on the host. This was remedied in hcsshim (https://github.com/microsoft/hcsshim/pull/1003) and this work has been vendored into K8s as well (https://github.com/kubernetes/kubernetes/pull/104880). This change simply makes use of the now correct version range to check if dualstack is supported, and gets rid of the old custom logic. Signed-off-by: Daniel Canter --- pkg/proxy/winkernel/proxier.go | 34 ++++++------------- pkg/proxy/winkernel/proxier_test.go | 52 ----------------------------- 2 files changed, 11 insertions(+), 75 deletions(-) diff --git a/pkg/proxy/winkernel/proxier.go b/pkg/proxy/winkernel/proxier.go index 9cb9e8716c1..dc73dbd6a0b 100644 --- a/pkg/proxy/winkernel/proxier.go +++ b/pkg/proxy/winkernel/proxier.go @@ -39,7 +39,6 @@ import ( "k8s.io/apimachinery/pkg/util/intstr" apiutil "k8s.io/apimachinery/pkg/util/net" "k8s.io/apimachinery/pkg/util/sets" - "k8s.io/apimachinery/pkg/util/version" "k8s.io/apimachinery/pkg/util/wait" utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/tools/events" @@ -188,20 +187,22 @@ func (t DualStackCompatTester) DualStackCompatible(networkName string) bool { return false } - globals, err := hcn.GetGlobals() - if err != nil { - klog.ErrorS(err, "Unable to determine networking stack version. Falling back to single-stack") - return false - } - - if !kernelSupportsDualstack(globals.Version) { - klog.InfoS("This version of Windows does not support dual-stack. Falling back to single-stack") + // First tag of hcsshim that has a proper check for dual stack support is v0.8.22 due to a bug. + if err := hcn.IPv6DualStackSupported(); err != nil { + // Hcn *can* fail the query to grab the version of hcn itself (which this call will do internally before parsing + // to see if dual stack is supported), but the only time this can happen, at least that can be discerned, is if the host + // is pre-1803 and hcn didn't exist. hcsshim should truthfully return a known error if this happened that we can + // check against, and the case where 'err != this known error' would be the 'this feature isn't supported' case, as is being + // used here. For now, seeming as how nothing before ws2019 (1809) is listed as supported for k8s we can pretty much assume + // any error here isn't because the query failed, it's just that dualstack simply isn't supported on the host. With all + // that in mind, just log as info and not error to let the user know we're falling back. + klog.InfoS("This version of Windows does not support dual-stack. Falling back to single-stack", "err", err.Error()) return false } // check if network is using overlay hns, _ := newHostNetworkService() - networkName, err = getNetworkName(networkName) + networkName, err := getNetworkName(networkName) if err != nil { klog.ErrorS(err, "unable to determine dual-stack status %v. Falling back to single-stack") return false @@ -221,19 +222,6 @@ func (t DualStackCompatTester) DualStackCompatible(networkName string) bool { return true } -// The hcsshim version logic has a bug that did not calculate the versioning of DualStack correctly. -// DualStack is supported in WS 2004+ (10.0.19041+) where HCN component version is 11.10+ -// https://github.com/microsoft/hcsshim/pull/1003#issuecomment-827930358 -func kernelSupportsDualstack(currentVersion hcn.Version) bool { - hnsVersion := fmt.Sprintf("%d.%d.0", currentVersion.Major, currentVersion.Minor) - v, err := version.ParseSemantic(hnsVersion) - if err != nil { - return false - } - - return v.AtLeast(version.MustParseSemantic("11.10.0")) -} - func Log(v interface{}, message string, level klog.Level) { klog.V(level).InfoS("%s", message, "spewConfig", spewSdump(v)) } diff --git a/pkg/proxy/winkernel/proxier_test.go b/pkg/proxy/winkernel/proxier_test.go index 51c4840542b..4be9db6b6d8 100644 --- a/pkg/proxy/winkernel/proxier_test.go +++ b/pkg/proxy/winkernel/proxier_test.go @@ -26,7 +26,6 @@ import ( "testing" "time" - "github.com/Microsoft/hcsshim/hcn" v1 "k8s.io/api/core/v1" discovery "k8s.io/api/discovery/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -931,54 +930,3 @@ func makeTestEndpointSlice(namespace, name string, sliceNum int, epsFunc func(*d epsFunc(eps) return eps } - -func Test_kernelSupportsDualstack(t *testing.T) { - tests := []struct { - currentVersion hcn.Version - name string - want bool - }{ - { - hcn.Version{Major: 10, Minor: 10}, - "Less than minimal should not be supported", - false, - }, - { - hcn.Version{Major: 9, Minor: 11}, - "Less than minimal should not be supported", - false, - }, - { - hcn.Version{Major: 11, Minor: 1}, - "Less than minimal should not be supported", - false, - }, - { - hcn.Version{Major: 11, Minor: 10}, - "Current version should be supported", - true, - }, - { - hcn.Version{Major: 11, Minor: 11}, - "Greater than minimal version should be supported", - true, - }, - { - hcn.Version{Major: 12, Minor: 1}, - "Greater than minimal version should be supported", - true, - }, - { - hcn.Version{Major: 12, Minor: 12}, - "Greater than minimal should be supported", - true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := kernelSupportsDualstack(tt.currentVersion); got != tt.want { - t.Errorf("kernelSupportsDualstack on version %v: got %v, want %v", tt.currentVersion, got, tt.want) - } - }) - } -}