From 13f0449e4cace8bd3a6dd1f0da642f0713433707 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Sat, 11 Jan 2025 09:17:37 -0500 Subject: [PATCH 1/6] Fix up kube-proxy import ordering/organization. --- pkg/proxy/iptables/proxier_test.go | 1 + pkg/proxy/ipvs/graceful_termination_test.go | 3 +-- pkg/proxy/ipvs/ipset.go | 7 +++---- pkg/proxy/ipvs/netlink_linux.go | 6 +++--- pkg/proxy/ipvs/proxier.go | 7 +++---- pkg/proxy/ipvs/proxier_test.go | 1 + pkg/proxy/ipvs/testing/fake.go | 2 +- pkg/proxy/ipvs/util/ipvs_linux.go | 2 +- pkg/proxy/ipvs/util/ipvs_linux_test.go | 5 ++--- pkg/proxy/nftables/proxier.go | 5 +++-- pkg/proxy/nftables/proxier_test.go | 2 +- pkg/proxy/winkernel/hcnutils.go | 1 + pkg/proxy/winkernel/hns.go | 4 ++-- pkg/proxy/winkernel/hns_test.go | 6 ++---- pkg/proxy/winkernel/proxier.go | 1 + pkg/proxy/winkernel/proxier_test.go | 1 + 16 files changed, 27 insertions(+), 27 deletions(-) diff --git a/pkg/proxy/iptables/proxier_test.go b/pkg/proxy/iptables/proxier_test.go index bcbdbe0f9da..29e4456457a 100644 --- a/pkg/proxy/iptables/proxier_test.go +++ b/pkg/proxy/iptables/proxier_test.go @@ -35,6 +35,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/lithammer/dedent" "github.com/stretchr/testify/assert" + v1 "k8s.io/api/core/v1" discovery "k8s.io/api/discovery/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" diff --git a/pkg/proxy/ipvs/graceful_termination_test.go b/pkg/proxy/ipvs/graceful_termination_test.go index 9d04894731f..3b3d0d6c58e 100644 --- a/pkg/proxy/ipvs/graceful_termination_test.go +++ b/pkg/proxy/ipvs/graceful_termination_test.go @@ -24,10 +24,9 @@ import ( "reflect" "testing" - netutils "k8s.io/utils/net" - utilipvs "k8s.io/kubernetes/pkg/proxy/ipvs/util" utilipvstest "k8s.io/kubernetes/pkg/proxy/ipvs/util/testing" + netutils "k8s.io/utils/net" ) func Test_GracefulDeleteRS(t *testing.T) { diff --git a/pkg/proxy/ipvs/ipset.go b/pkg/proxy/ipvs/ipset.go index 056f80ca03d..193fc634b63 100644 --- a/pkg/proxy/ipvs/ipset.go +++ b/pkg/proxy/ipvs/ipset.go @@ -20,14 +20,13 @@ limitations under the License. package ipvs import ( - "k8s.io/apimachinery/pkg/util/sets" - utilversion "k8s.io/apimachinery/pkg/util/version" - utilipset "k8s.io/kubernetes/pkg/proxy/ipvs/ipset" - "fmt" "strings" + "k8s.io/apimachinery/pkg/util/sets" + utilversion "k8s.io/apimachinery/pkg/util/version" "k8s.io/klog/v2" + utilipset "k8s.io/kubernetes/pkg/proxy/ipvs/ipset" ) const ( diff --git a/pkg/proxy/ipvs/netlink_linux.go b/pkg/proxy/ipvs/netlink_linux.go index 98c96bc7031..632b5078ad3 100644 --- a/pkg/proxy/ipvs/netlink_linux.go +++ b/pkg/proxy/ipvs/netlink_linux.go @@ -23,13 +23,13 @@ import ( "fmt" "net" + "github.com/vishvananda/netlink" + "golang.org/x/sys/unix" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/klog/v2" proxyutil "k8s.io/kubernetes/pkg/proxy/util" netutils "k8s.io/utils/net" - - "github.com/vishvananda/netlink" - "golang.org/x/sys/unix" ) type netlinkHandle struct { diff --git a/pkg/proxy/ipvs/proxier.go b/pkg/proxy/ipvs/proxier.go index 11ec8c8d0d5..8f96e692b70 100644 --- a/pkg/proxy/ipvs/proxier.go +++ b/pkg/proxy/ipvs/proxier.go @@ -33,10 +33,6 @@ import ( "sync/atomic" "time" - "k8s.io/klog/v2" - utilexec "k8s.io/utils/exec" - netutils "k8s.io/utils/net" - v1 "k8s.io/api/core/v1" discovery "k8s.io/api/discovery/v1" "k8s.io/apimachinery/pkg/types" @@ -45,6 +41,7 @@ import ( "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/tools/events" utilsysctl "k8s.io/component-helpers/node/util/sysctl" + "k8s.io/klog/v2" "k8s.io/kubernetes/pkg/proxy" "k8s.io/kubernetes/pkg/proxy/conntrack" "k8s.io/kubernetes/pkg/proxy/healthcheck" @@ -56,6 +53,8 @@ import ( "k8s.io/kubernetes/pkg/util/async" utiliptables "k8s.io/kubernetes/pkg/util/iptables" utilkernel "k8s.io/kubernetes/pkg/util/kernel" + utilexec "k8s.io/utils/exec" + netutils "k8s.io/utils/net" ) const ( diff --git a/pkg/proxy/ipvs/proxier_test.go b/pkg/proxy/ipvs/proxier_test.go index b28ab05f4e3..86aacbd3634 100644 --- a/pkg/proxy/ipvs/proxier_test.go +++ b/pkg/proxy/ipvs/proxier_test.go @@ -31,6 +31,7 @@ import ( "time" "github.com/stretchr/testify/assert" + v1 "k8s.io/api/core/v1" discovery "k8s.io/api/discovery/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" diff --git a/pkg/proxy/ipvs/testing/fake.go b/pkg/proxy/ipvs/testing/fake.go index cd5958ccd53..7b7544fa9c7 100644 --- a/pkg/proxy/ipvs/testing/fake.go +++ b/pkg/proxy/ipvs/testing/fake.go @@ -21,9 +21,9 @@ package testing import ( "fmt" - "k8s.io/utils/net" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/utils/net" ) // FakeNetlinkHandle mock implementation of proxy NetlinkHandle diff --git a/pkg/proxy/ipvs/util/ipvs_linux.go b/pkg/proxy/ipvs/util/ipvs_linux.go index e9513a64156..e038fad07b6 100644 --- a/pkg/proxy/ipvs/util/ipvs_linux.go +++ b/pkg/proxy/ipvs/util/ipvs_linux.go @@ -28,8 +28,8 @@ import ( "time" libipvs "github.com/moby/ipvs" - "golang.org/x/sys/unix" + "k8s.io/klog/v2" ) diff --git a/pkg/proxy/ipvs/util/ipvs_linux_test.go b/pkg/proxy/ipvs/util/ipvs_linux_test.go index e604cefbea6..f3595f260aa 100644 --- a/pkg/proxy/ipvs/util/ipvs_linux_test.go +++ b/pkg/proxy/ipvs/util/ipvs_linux_test.go @@ -24,11 +24,10 @@ import ( "reflect" "testing" - netutils "k8s.io/utils/net" - libipvs "github.com/moby/ipvs" - "golang.org/x/sys/unix" + + netutils "k8s.io/utils/net" ) func Test_toVirtualServer(t *testing.T) { diff --git a/pkg/proxy/nftables/proxier.go b/pkg/proxy/nftables/proxier.go index b12ecc9b956..36f4b2ee764 100644 --- a/pkg/proxy/nftables/proxier.go +++ b/pkg/proxy/nftables/proxier.go @@ -28,7 +28,6 @@ import ( "crypto/sha256" "encoding/base32" "fmt" - "golang.org/x/time/rate" "net" "os" "os/exec" @@ -39,7 +38,9 @@ import ( "sync/atomic" "time" - v1 "k8s.io/api/core/v1" + "golang.org/x/time/rate" + + "k8s.io/api/core/v1" discovery "k8s.io/api/discovery/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" diff --git a/pkg/proxy/nftables/proxier_test.go b/pkg/proxy/nftables/proxier_test.go index c42e597a498..112d67997ba 100644 --- a/pkg/proxy/nftables/proxier_test.go +++ b/pkg/proxy/nftables/proxier_test.go @@ -21,7 +21,6 @@ package nftables import ( "fmt" - "golang.org/x/time/rate" "net" "reflect" "testing" @@ -29,6 +28,7 @@ import ( "github.com/lithammer/dedent" "github.com/stretchr/testify/assert" + "golang.org/x/time/rate" v1 "k8s.io/api/core/v1" discovery "k8s.io/api/discovery/v1" diff --git a/pkg/proxy/winkernel/hcnutils.go b/pkg/proxy/winkernel/hcnutils.go index b6e2a400dcc..afb513b3fed 100644 --- a/pkg/proxy/winkernel/hcnutils.go +++ b/pkg/proxy/winkernel/hcnutils.go @@ -21,6 +21,7 @@ package winkernel import ( "github.com/Microsoft/hnslib/hcn" + "k8s.io/klog/v2" ) diff --git a/pkg/proxy/winkernel/hns.go b/pkg/proxy/winkernel/hns.go index fae5b1abc8a..13f1ea868fb 100644 --- a/pkg/proxy/winkernel/hns.go +++ b/pkg/proxy/winkernel/hns.go @@ -23,11 +23,11 @@ import ( "crypto/sha1" "encoding/json" "fmt" + "strings" "github.com/Microsoft/hnslib/hcn" - "k8s.io/klog/v2" - "strings" + "k8s.io/klog/v2" ) type HostNetworkService interface { diff --git a/pkg/proxy/winkernel/hns_test.go b/pkg/proxy/winkernel/hns_test.go index 6e676016fd5..74a51b68f31 100644 --- a/pkg/proxy/winkernel/hns_test.go +++ b/pkg/proxy/winkernel/hns_test.go @@ -21,14 +21,12 @@ package winkernel import ( "encoding/json" - - "github.com/Microsoft/hnslib/hcn" - "github.com/stretchr/testify/assert" - "strings" "testing" + "github.com/Microsoft/hnslib/hcn" "github.com/google/go-cmp/cmp" + "github.com/stretchr/testify/assert" ) const ( diff --git a/pkg/proxy/winkernel/proxier.go b/pkg/proxy/winkernel/proxier.go index b60bfe1e701..3e038683109 100644 --- a/pkg/proxy/winkernel/proxier.go +++ b/pkg/proxy/winkernel/proxier.go @@ -31,6 +31,7 @@ import ( "github.com/Microsoft/hnslib" "github.com/Microsoft/hnslib/hcn" + v1 "k8s.io/api/core/v1" discovery "k8s.io/api/discovery/v1" "k8s.io/apimachinery/pkg/util/intstr" diff --git a/pkg/proxy/winkernel/proxier_test.go b/pkg/proxy/winkernel/proxier_test.go index c13f0694e18..e78a8003686 100644 --- a/pkg/proxy/winkernel/proxier_test.go +++ b/pkg/proxy/winkernel/proxier_test.go @@ -28,6 +28,7 @@ import ( "time" "github.com/Microsoft/hnslib/hcn" + v1 "k8s.io/api/core/v1" discovery "k8s.io/api/discovery/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" From 36f5820ad156c2353149b1cfd20f38d7be6a0943 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Sat, 11 Jan 2025 09:20:53 -0500 Subject: [PATCH 2/6] Remove some unused proxy args/fields Remove the utilexec.Interface args from the iptables/ipvs constructors (which have been unused since the conntrack cleanup code was ported to netlink). Remove the EventRecorder fields from the iptables/ipvs Proxiers, which have been unused since we removed the port-opener code in 2022. Remove the strictARP field from the ipvs Proxier, which has apparently always been unused (strictARP is only looked at at construct time). --- cmd/kube-proxy/app/server_linux.go | 4 ---- pkg/proxy/iptables/proxier.go | 9 ++------- pkg/proxy/ipvs/proxier.go | 12 +++--------- pkg/proxy/ipvs/proxier_test.go | 1 - pkg/proxy/nftables/proxier.go | 2 -- pkg/proxy/winkernel/proxier.go | 2 -- 6 files changed, 5 insertions(+), 25 deletions(-) diff --git a/cmd/kube-proxy/app/server_linux.go b/cmd/kube-proxy/app/server_linux.go index 56fb9d5e45e..d9a704860e9 100644 --- a/cmd/kube-proxy/app/server_linux.go +++ b/cmd/kube-proxy/app/server_linux.go @@ -175,7 +175,6 @@ func (s *ProxyServer) createProxier(ctx context.Context, config *proxyconfigapi. ctx, ipt, utilsysctl.New(), - exec.New(), config.SyncPeriod.Duration, config.MinSyncPeriod.Duration, config.Linux.MasqueradeAll, @@ -199,7 +198,6 @@ func (s *ProxyServer) createProxier(ctx context.Context, config *proxyconfigapi. s.PrimaryIPFamily, iptInterface, utilsysctl.New(), - exec.New(), config.SyncPeriod.Duration, config.MinSyncPeriod.Duration, config.Linux.MasqueradeAll, @@ -235,7 +233,6 @@ func (s *ProxyServer) createProxier(ctx context.Context, config *proxyconfigapi. ipvsInterface, ipsetInterface, utilsysctl.New(), - execer, config.SyncPeriod.Duration, config.MinSyncPeriod.Duration, config.IPVS.ExcludeCIDRs, @@ -263,7 +260,6 @@ func (s *ProxyServer) createProxier(ctx context.Context, config *proxyconfigapi. ipvsInterface, ipsetInterface, utilsysctl.New(), - execer, config.SyncPeriod.Duration, config.MinSyncPeriod.Duration, config.IPVS.ExcludeCIDRs, diff --git a/pkg/proxy/iptables/proxier.go b/pkg/proxy/iptables/proxier.go index 9c9e5f0c5cd..9e3ea0ade9e 100644 --- a/pkg/proxy/iptables/proxier.go +++ b/pkg/proxy/iptables/proxier.go @@ -54,7 +54,6 @@ import ( "k8s.io/kubernetes/pkg/proxy/util/nfacct" "k8s.io/kubernetes/pkg/util/async" utiliptables "k8s.io/kubernetes/pkg/util/iptables" - utilexec "k8s.io/utils/exec" ) const ( @@ -101,7 +100,6 @@ func NewDualStackProxier( ctx context.Context, ipt [2]utiliptables.Interface, sysctl utilsysctl.Interface, - exec utilexec.Interface, syncPeriod time.Duration, minSyncPeriod time.Duration, masqueradeAll bool, @@ -117,7 +115,7 @@ func NewDualStackProxier( ) (proxy.Provider, error) { // Create an ipv4 instance of the single-stack proxier ipv4Proxier, err := NewProxier(ctx, v1.IPv4Protocol, ipt[0], sysctl, - exec, syncPeriod, minSyncPeriod, masqueradeAll, localhostNodePorts, masqueradeBit, + syncPeriod, minSyncPeriod, masqueradeAll, localhostNodePorts, masqueradeBit, localDetectors[v1.IPv4Protocol], hostname, nodeIPs[v1.IPv4Protocol], recorder, healthzServer, nodePortAddresses, initOnly) if err != nil { @@ -125,7 +123,7 @@ func NewDualStackProxier( } ipv6Proxier, err := NewProxier(ctx, v1.IPv6Protocol, ipt[1], sysctl, - exec, syncPeriod, minSyncPeriod, masqueradeAll, false, masqueradeBit, + syncPeriod, minSyncPeriod, masqueradeAll, false, masqueradeBit, localDetectors[v1.IPv6Protocol], hostname, nodeIPs[v1.IPv6Protocol], recorder, healthzServer, nodePortAddresses, initOnly) if err != nil { @@ -175,7 +173,6 @@ type Proxier struct { localDetector proxyutil.LocalTrafficDetector hostname string nodeIP net.IP - recorder events.EventRecorder serviceHealthServer healthcheck.ServiceHealthServer healthzServer *healthcheck.ProxyHealthServer @@ -230,7 +227,6 @@ func NewProxier(ctx context.Context, ipFamily v1.IPFamily, ipt utiliptables.Interface, sysctl utilsysctl.Interface, - exec utilexec.Interface, syncPeriod time.Duration, minSyncPeriod time.Duration, masqueradeAll bool, @@ -300,7 +296,6 @@ func NewProxier(ctx context.Context, localDetector: localDetector, hostname: hostname, nodeIP: nodeIP, - recorder: recorder, serviceHealthServer: serviceHealthServer, healthzServer: healthzServer, precomputedProbabilities: make([]string, 0, 1001), diff --git a/pkg/proxy/ipvs/proxier.go b/pkg/proxy/ipvs/proxier.go index 8f96e692b70..c035b150491 100644 --- a/pkg/proxy/ipvs/proxier.go +++ b/pkg/proxy/ipvs/proxier.go @@ -53,7 +53,6 @@ import ( "k8s.io/kubernetes/pkg/util/async" utiliptables "k8s.io/kubernetes/pkg/util/iptables" utilkernel "k8s.io/kubernetes/pkg/util/kernel" - utilexec "k8s.io/utils/exec" netutils "k8s.io/utils/net" ) @@ -115,7 +114,6 @@ func NewDualStackProxier( ipvs utilipvs.Interface, ipset utilipset.Interface, sysctl utilsysctl.Interface, - exec utilexec.Interface, syncPeriod time.Duration, minSyncPeriod time.Duration, excludeCIDRs []string, @@ -136,7 +134,7 @@ func NewDualStackProxier( ) (proxy.Provider, error) { // Create an ipv4 instance of the single-stack proxier ipv4Proxier, err := NewProxier(ctx, v1.IPv4Protocol, ipt[0], ipvs, ipset, sysctl, - exec, syncPeriod, minSyncPeriod, filterCIDRs(false, excludeCIDRs), strictARP, + syncPeriod, minSyncPeriod, filterCIDRs(false, excludeCIDRs), strictARP, tcpTimeout, tcpFinTimeout, udpTimeout, masqueradeAll, masqueradeBit, localDetectors[v1.IPv4Protocol], hostname, nodeIPs[v1.IPv4Protocol], recorder, healthzServer, scheduler, nodePortAddresses, initOnly) @@ -145,7 +143,7 @@ func NewDualStackProxier( } ipv6Proxier, err := NewProxier(ctx, v1.IPv6Protocol, ipt[1], ipvs, ipset, sysctl, - exec, syncPeriod, minSyncPeriod, filterCIDRs(true, excludeCIDRs), strictARP, + syncPeriod, minSyncPeriod, filterCIDRs(true, excludeCIDRs), strictARP, tcpTimeout, tcpFinTimeout, udpTimeout, masqueradeAll, masqueradeBit, localDetectors[v1.IPv6Protocol], hostname, nodeIPs[v1.IPv6Protocol], recorder, healthzServer, scheduler, nodePortAddresses, initOnly) @@ -197,8 +195,7 @@ type Proxier struct { minSyncPeriod time.Duration // Values are CIDR's to exclude when cleaning up IPVS rules. excludeCIDRs []*net.IPNet - // Set to true to set sysctls arp_ignore and arp_announce - strictARP bool + iptables utiliptables.Interface ipvs utilipvs.Interface ipset utilipset.Interface @@ -208,7 +205,6 @@ type Proxier struct { localDetector proxyutil.LocalTrafficDetector hostname string nodeIP net.IP - recorder events.EventRecorder serviceHealthServer healthcheck.ServiceHealthServer healthzServer *healthcheck.ProxyHealthServer @@ -270,7 +266,6 @@ func NewProxier( ipvs utilipvs.Interface, ipset utilipset.Interface, sysctl utilsysctl.Interface, - exec utilexec.Interface, syncPeriod time.Duration, minSyncPeriod time.Duration, excludeCIDRs []string, @@ -388,7 +383,6 @@ func NewProxier( localDetector: localDetector, hostname: hostname, nodeIP: nodeIP, - recorder: recorder, serviceHealthServer: serviceHealthServer, healthzServer: healthzServer, ipvs: ipvs, diff --git a/pkg/proxy/ipvs/proxier_test.go b/pkg/proxy/ipvs/proxier_test.go index 86aacbd3634..c835ade5c46 100644 --- a/pkg/proxy/ipvs/proxier_test.go +++ b/pkg/proxy/ipvs/proxier_test.go @@ -149,7 +149,6 @@ func NewFakeProxier(ctx context.Context, ipt utiliptables.Interface, ipvs utilip ipvs: ipvs, ipset: ipset, conntrack: conntrack.NewFake(), - strictARP: false, localDetector: proxyutil.NewNoOpLocalDetector(), hostname: testHostname, serviceHealthServer: healthcheck.NewFakeServiceHealthServer(), diff --git a/pkg/proxy/nftables/proxier.go b/pkg/proxy/nftables/proxier.go index 36f4b2ee764..d7d8976664e 100644 --- a/pkg/proxy/nftables/proxier.go +++ b/pkg/proxy/nftables/proxier.go @@ -180,7 +180,6 @@ type Proxier struct { localDetector proxyutil.LocalTrafficDetector hostname string nodeIP net.IP - recorder events.EventRecorder serviceHealthServer healthcheck.ServiceHealthServer healthzServer *healthcheck.ProxyHealthServer @@ -265,7 +264,6 @@ func NewProxier(ctx context.Context, localDetector: localDetector, hostname: hostname, nodeIP: nodeIP, - recorder: recorder, serviceHealthServer: serviceHealthServer, healthzServer: healthzServer, nodePortAddresses: nodePortAddresses, diff --git a/pkg/proxy/winkernel/proxier.go b/pkg/proxy/winkernel/proxier.go index 3e038683109..a4e57dd36e2 100644 --- a/pkg/proxy/winkernel/proxier.go +++ b/pkg/proxy/winkernel/proxier.go @@ -651,7 +651,6 @@ type Proxier struct { // These are effectively const and do not need the mutex to be held. hostname string nodeIP net.IP - recorder events.EventRecorder serviceHealthServer healthcheck.ServiceHealthServer healthzServer *healthcheck.ProxyHealthServer @@ -813,7 +812,6 @@ func NewProxier( endpointsMap: make(proxy.EndpointsMap), hostname: hostname, nodeIP: nodeIP, - recorder: recorder, serviceHealthServer: serviceHealthServer, healthzServer: healthzServer, hns: hns, From b5e9a8262e4a45393d13dfc2ba01e810c6c45dfb Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Sat, 11 Jan 2025 07:44:39 -0500 Subject: [PATCH 3/6] Remove exec arg from utilipset.New Historically it took an exec argument so you could pass a FakeExec to mock its behavior in unit tests, but it has a fake implementation now that is much more useful for unit tests than trying to use the real implementation with a fake exec. (The unit tests still use fake execs, but they don't need to use a public constructor.) So remove the exec args from the public constructors. --- cmd/kube-proxy/app/server_linux.go | 6 ++---- pkg/proxy/ipvs/ipset/ipset.go | 4 ++-- pkg/proxy/ipvs/ipset/ipset_test.go | 24 ++++++++++++++---------- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/cmd/kube-proxy/app/server_linux.go b/cmd/kube-proxy/app/server_linux.go index d9a704860e9..19e5170d339 100644 --- a/cmd/kube-proxy/app/server_linux.go +++ b/cmd/kube-proxy/app/server_linux.go @@ -217,8 +217,7 @@ func (s *ProxyServer) createProxier(ctx context.Context, config *proxyconfigapi. return nil, fmt.Errorf("unable to create proxier: %v", err) } } else if config.Mode == proxyconfigapi.ProxyModeIPVS { - execer := exec.New() - ipsetInterface := utilipset.New(execer) + ipsetInterface := utilipset.New() ipvsInterface := utilipvs.New() if err := ipvs.CanUseIPVSProxier(ctx, ipvsInterface, ipsetInterface, config.IPVS.Scheduler); err != nil { return nil, fmt.Errorf("can't use the IPVS proxier: %v", err) @@ -511,8 +510,7 @@ func platformCleanup(ctx context.Context, mode proxyconfigapi.ProxyMode, cleanup // Clean up iptables and ipvs rules if switching to nftables, or if cleanupAndExit if !isIPTablesBased(mode) || cleanupAndExit { ipts, _ := getIPTables(v1.IPFamilyUnknown) - execer := exec.New() - ipsetInterface := utilipset.New(execer) + ipsetInterface := utilipset.New() ipvsInterface := utilipvs.New() for _, ipt := range ipts { diff --git a/pkg/proxy/ipvs/ipset/ipset.go b/pkg/proxy/ipvs/ipset/ipset.go index 2957340d937..041e9278167 100644 --- a/pkg/proxy/ipvs/ipset/ipset.go +++ b/pkg/proxy/ipvs/ipset/ipset.go @@ -278,9 +278,9 @@ type runner struct { } // New returns a new Interface which will exec ipset. -func New(exec utilexec.Interface) Interface { +func New() Interface { return &runner{ - exec: exec, + exec: utilexec.New(), } } diff --git a/pkg/proxy/ipvs/ipset/ipset_test.go b/pkg/proxy/ipvs/ipset/ipset_test.go index 00a270262d1..a2e45327519 100644 --- a/pkg/proxy/ipvs/ipset/ipset_test.go +++ b/pkg/proxy/ipvs/ipset/ipset_test.go @@ -28,6 +28,10 @@ import ( fakeexec "k8s.io/utils/exec/testing" ) +func newInternal(fexec *fakeexec.FakeExec) Interface { + return &runner{fexec} +} + func TestCheckIPSetVersion(t *testing.T) { testCases := []struct { vstring string @@ -83,7 +87,7 @@ func TestFlushSet(t *testing.T) { func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } - runner := New(fexec) + runner := newInternal(fexec) // Success. err := runner.FlushSet("FOOBAR") if err != nil { @@ -119,7 +123,7 @@ func TestDestroySet(t *testing.T) { func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } - runner := New(fexec) + runner := newInternal(fexec) // Success err := runner.DestroySet("FOOBAR") if err != nil { @@ -153,7 +157,7 @@ func TestDestroyAllSets(t *testing.T) { func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } - runner := New(fexec) + runner := newInternal(fexec) // Success err := runner.DestroyAllSets() if err != nil { @@ -198,7 +202,7 @@ func TestCreateSet(t *testing.T) { func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } - runner := New(fexec) + runner := newInternal(fexec) // Create with ignoreExistErr = false, expect success err := runner.CreateSet(&testSet, false) if err != nil { @@ -388,7 +392,7 @@ func TestAddEntry(t *testing.T) { func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } - runner := New(fexec) + runner := newInternal(fexec) // Create with ignoreExistErr = false, expect success err := runner.AddEntry(testCases[i].entry.String(), testCases[i].set, false) if err != nil { @@ -437,7 +441,7 @@ func TestDelEntry(t *testing.T) { func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } - runner := New(fexec) + runner := newInternal(fexec) err := runner.DelEntry(testCases[i].entry.String(), testCases[i].set.Name) if err != nil { @@ -482,7 +486,7 @@ func TestTestEntry(t *testing.T) { func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } - runner := New(fexec) + runner := newInternal(fexec) // Success ok, err := runner.TestEntry(testEntry.String(), setName) if err != nil { @@ -530,7 +534,7 @@ func TestTestEntryIPv6(t *testing.T) { func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } - runner := New(fexec) + runner := newInternal(fexec) // Success ok, err := runner.TestEntry(testEntry.String(), setName) if err != nil { @@ -604,7 +608,7 @@ Members: }, }, } - runner := New(fexec) + runner := newInternal(fexec) // Success entries, err := runner.ListEntries("foobar") if err != nil { @@ -643,7 +647,7 @@ baz` func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } - runner := New(fexec) + runner := newInternal(fexec) // Success list, err := runner.ListSets() if err != nil { From 303593cafe1163f0813948f42c9e0f7561017c8f Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Sun, 15 Dec 2024 10:49:39 -0500 Subject: [PATCH 4/6] Fix some pkg/proxy comments Remove a bunch of comments that are either inaccurate ("the proxier can only be tested by e2e tests") or weirdly overspecific about obvious details ("the proxier will not exit if an iptables call fails"). --- pkg/proxy/iptables/proxier.go | 13 ++----------- pkg/proxy/ipvs/proxier.go | 9 ++------- pkg/proxy/nftables/proxier.go | 10 ++-------- pkg/proxy/winkernel/proxier.go | 5 ++--- 4 files changed, 8 insertions(+), 29 deletions(-) diff --git a/pkg/proxy/iptables/proxier.go b/pkg/proxy/iptables/proxier.go index 9e3ea0ade9e..80c00cd53a5 100644 --- a/pkg/proxy/iptables/proxier.go +++ b/pkg/proxy/iptables/proxier.go @@ -19,10 +19,6 @@ limitations under the License. package iptables -// -// NOTE: this needs to be tested in e2e since it uses iptables for everything. -// - import ( "bytes" "context" @@ -135,8 +131,7 @@ func NewDualStackProxier( return metaproxier.NewMetaProxier(ipv4Proxier, ipv6Proxier), nil } -// Proxier is an iptables based proxy for connections between a localhost:lport -// and services that provide the actual backends. +// Proxier is an iptables-based proxy type Proxier struct { // ipFamily defines the IP family which this proxier is tracking. ipFamily v1.IPFamily @@ -218,11 +213,7 @@ type Proxier struct { // Proxier implements proxy.Provider var _ proxy.Provider = &Proxier{} -// NewProxier returns a new Proxier given an iptables Interface instance. -// Because of the iptables logic, it is assumed that there is only a single Proxier active on a machine. -// An error will be returned if iptables fails to update or acquire the initial lock. -// Once a proxier is created, it will keep iptables up to date in the background and -// will not terminate if a particular iptables call fails. +// NewProxier returns a new single-stack IPTables proxier. func NewProxier(ctx context.Context, ipFamily v1.IPFamily, ipt utiliptables.Interface, diff --git a/pkg/proxy/ipvs/proxier.go b/pkg/proxy/ipvs/proxier.go index c035b150491..c83a3cad5ee 100644 --- a/pkg/proxy/ipvs/proxier.go +++ b/pkg/proxy/ipvs/proxier.go @@ -159,8 +159,7 @@ func NewDualStackProxier( return metaproxier.NewMetaProxier(ipv4Proxier, ipv6Proxier), nil } -// Proxier is an ipvs based proxy for connections between a localhost:lport -// and services that provide the actual backends. +// Proxier is an ipvs-based proxy type Proxier struct { // the ipfamily on which this proxy is operating on. ipFamily v1.IPFamily @@ -254,11 +253,7 @@ type Proxier struct { // Proxier implements proxy.Provider var _ proxy.Provider = &Proxier{} -// NewProxier returns a new Proxier given an iptables and ipvs Interface instance. -// Because of the iptables and ipvs logic, it is assumed that there is only a single Proxier active on a machine. -// An error will be returned if it fails to update or acquire the initial lock. -// Once a proxier is created, it will keep iptables and ipvs rules up to date in the background and -// will not terminate if a particular iptables or ipvs call fails. +// NewProxier returns a new single-stack IPVS proxier. func NewProxier( ctx context.Context, ipFamily v1.IPFamily, diff --git a/pkg/proxy/nftables/proxier.go b/pkg/proxy/nftables/proxier.go index d7d8976664e..a5f2ce96334 100644 --- a/pkg/proxy/nftables/proxier.go +++ b/pkg/proxy/nftables/proxier.go @@ -19,10 +19,6 @@ limitations under the License. package nftables -// -// NOTE: this needs to be tested in e2e since it uses nftables for everything. -// - import ( "context" "crypto/sha256" @@ -143,7 +139,7 @@ func NewDualStackProxier( return metaproxier.NewMetaProxier(ipv4Proxier, ipv6Proxier), nil } -// Proxier is an nftables based proxy +// Proxier is an nftables-based proxy type Proxier struct { // ipFamily defines the IP family which this proxier is tracking. ipFamily v1.IPFamily @@ -211,9 +207,7 @@ type Proxier struct { // Proxier implements proxy.Provider var _ proxy.Provider = &Proxier{} -// NewProxier returns a new nftables Proxier. Once a proxier is created, it will keep -// nftables up to date in the background and will not terminate if a particular nftables -// call fails. +// NewProxier returns a new single-stack NFTables proxier. func NewProxier(ctx context.Context, ipFamily v1.IPFamily, syncPeriod time.Duration, diff --git a/pkg/proxy/winkernel/proxier.go b/pkg/proxy/winkernel/proxier.go index a4e57dd36e2..4558014331f 100644 --- a/pkg/proxy/winkernel/proxier.go +++ b/pkg/proxy/winkernel/proxier.go @@ -623,8 +623,7 @@ func (network hnsNetworkInfo) findRemoteSubnetProviderAddress(ip string) string type endPointsReferenceCountMap map[string]*uint16 -// Proxier is an hns based proxy for connections between a localhost:lport -// and services that provide the actual backends. +// Proxier is an HNS-based proxy type Proxier struct { // ipFamily defines the IP family which this proxier is tracking. ipFamily v1.IPFamily @@ -701,7 +700,7 @@ type closeable interface { // Proxier implements proxy.Provider var _ proxy.Provider = &Proxier{} -// NewProxier returns a new Proxier +// NewProxier returns a new single-stack winkernel proxier. func NewProxier( ipFamily v1.IPFamily, syncPeriod time.Duration, From f001b3916d4775b75910c6ef65578cfeb4fa90a7 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Tue, 14 Jan 2025 20:12:15 -0500 Subject: [PATCH 5/6] Remove a stale comment in metaproxier.go (NodeHandler was implemented in metaProxier a long time ago.) --- pkg/proxy/metaproxier/meta_proxier.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/pkg/proxy/metaproxier/meta_proxier.go b/pkg/proxy/metaproxier/meta_proxier.go index 08c7b6f6cc7..df56bd8f8a6 100644 --- a/pkg/proxy/metaproxier/meta_proxier.go +++ b/pkg/proxy/metaproxier/meta_proxier.go @@ -21,7 +21,6 @@ import ( discovery "k8s.io/api/discovery/v1" "k8s.io/klog/v2" "k8s.io/kubernetes/pkg/proxy" - "k8s.io/kubernetes/pkg/proxy/config" ) type metaProxier struct { @@ -29,8 +28,6 @@ type metaProxier struct { ipv4Proxier proxy.Provider // actual, wrapped ipv6Proxier proxy.Provider - // TODO(imroc): implement node handler for meta proxier. - config.NoopNodeHandler } // NewMetaProxier returns a dual-stack "meta-proxier". Proxier API From b62503dd66ecee08022055a6c1b7c657440f5130 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Tue, 31 Dec 2024 20:05:20 -0500 Subject: [PATCH 6/6] Remove a dead error check in winkernel The cmd/kube-proxy code never passes nil for the node IP any more. --- pkg/proxy/winkernel/proxier.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/pkg/proxy/winkernel/proxier.go b/pkg/proxy/winkernel/proxier.go index 4558014331f..57c4a908b6f 100644 --- a/pkg/proxy/winkernel/proxier.go +++ b/pkg/proxy/winkernel/proxier.go @@ -712,11 +712,6 @@ func NewProxier( healthzBindAddress string, config config.KubeProxyWinkernelConfiguration, ) (*Proxier, error) { - if nodeIP == nil { - klog.InfoS("Invalid nodeIP, initializing kube-proxy with 127.0.0.1 as nodeIP") - nodeIP = netutils.ParseIPSloppy("127.0.0.1") - } - // windows listens to all node addresses nodePortAddresses := proxyutil.NewNodePortAddresses(ipFamily, nil) serviceHealthServer := healthcheck.NewServiceHealthServer(hostname, recorder, nodePortAddresses, healthzServer)