backport: ci: remove logger field from portforwarder to avoid race with goroutine (#2959) (#2975)

ci: remove logger field from portforwarder to avoid race with goroutine (#2959)

* do not pass testing logger into goroutine to avoid race

* fix port forwarder

Author: QxBytes

test/e2e/framework/kubernetes/port-forward.go

@@ -86,7 +86,7 @@ func (p *PortForward) Run() error {
 
 		log.Printf("attempting port forward to pod name \"%s\" with label \"%s\", in namespace \"%s\"...\n", targetPodName, p.LabelSelector, p.Namespace)
 
-		p.pf, err = k8s.NewPortForwarder(config, &logger{}, opts)
+		p.pf, err = k8s.NewPortForwarder(config, opts)
 		if err != nil {
 			return fmt.Errorf("could not create port forwarder: %w", err)
 		}
@@ -161,9 +161,3 @@ func (p *PortForward) Stop() error {
 	p.pf.Stop()
 	return nil
 }
-
-type logger struct{}
-
-func (l *logger) Logf(format string, args ...interface{}) {
-	log.Printf(format, args...)
-}

test/integration/datapath/datapath_linux_test.go

@@ -221,7 +221,7 @@ func TestDatapathLinux(t *testing.T) {
 				DestPort:      8080,
 			}
 
-			pf, err := k8s.NewPortForwarder(restConfig, t, pfOpts)
+			pf, err := k8s.NewPortForwarder(restConfig, pfOpts)
 			if err != nil {
 				t.Fatal(err)
 			}

test/integration/k8s_test.go

@@ -174,7 +174,7 @@ func TestPodScaling(t *testing.T) {
 			}
 
 			pingCheckFn := func() error {
-				pf, err := NewPortForwarder(restConfig, t, pfOpts)
+				pf, err := NewPortForwarder(restConfig, pfOpts)
 				if err != nil {
 					t.Fatalf("could not build port forwarder: %v", err)
 				}

test/integration/networkobservability/hubble_test.go

@@ -220,7 +220,7 @@ func TestEndpoints(t *testing.T) {
 	defer cancel()
 	pingCheckFn := func() error {
 		var pf *k8s.PortForwarder
-		pf, err := k8s.NewPortForwarder(config, t, k8s.PortForwardingOpts{
+		pf, err := k8s.NewPortForwarder(config, k8s.PortForwardingOpts{
 			Namespace:     namespace,
 			LabelSelector: labelSelector,
 			LocalPort:     9965,

test/integration/portforward.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"io"
+	"log"
 	"math/rand"
 	"net/http"
 	"sync"
@@ -26,7 +27,6 @@ type PortForwarder struct {
 	clientset *kubernetes.Clientset
 	transport http.RoundTripper
 	upgrader  spdy.Upgrader
-	logger    logger
 
 	opts PortForwardingOpts
 
@@ -45,7 +45,7 @@ type PortForwardingOpts struct {
 }
 
 // NewPortForwarder creates a PortForwarder.
-func NewPortForwarder(restConfig *rest.Config, logger logger, opts PortForwardingOpts) (*PortForwarder, error) {
+func NewPortForwarder(restConfig *rest.Config, opts PortForwardingOpts) (*PortForwarder, error) {
 	clientset, err := kubernetes.NewForConfig(restConfig)
 	if err != nil {
 		return nil, fmt.Errorf("could not create clientset: %w", err)
@@ -60,7 +60,6 @@ func NewPortForwarder(restConfig *rest.Config, logger logger, opts PortForwardin
 		clientset: clientset,
 		transport: transport,
 		upgrader:  upgrader,
-		logger:    logger,
 		opts:      opts,
 		stopChan:  make(chan struct{}, 1),
 	}, nil
@@ -173,7 +172,7 @@ func (p *PortForwarder) KeepAlive(ctx context.Context) {
 	for {
 		select {
 		case <-ctx.Done():
-			p.logger.Logf("port forwarder: keep alive cancelled: %v", ctx.Err())
+			log.Printf("port forwarder: keep alive cancelled: %v", ctx.Err())
 			return
 		case pfErr := <-p.errChan:
 			// as of client-go v0.26.1, if the connection is successful at first but then fails,
@@ -182,14 +181,14 @@ func (p *PortForwarder) KeepAlive(ctx context.Context) {
 			//
 			// see https://github.com/kubernetes/client-go/commit/d0842249d3b92ea67c446fe273f84fe74ebaed9f
 			// for the relevant change.
-			p.logger.Logf("port forwarder: received error signal: %v. restarting session", pfErr)
+			log.Printf("port forwarder: received error signal: %v. restarting session", pfErr)
 			p.Stop()
 			if err := p.Forward(ctx); err != nil {
-				p.logger.Logf("port forwarder: could not restart session: %v. retrying", err)
+				log.Printf("port forwarder: could not restart session: %v. retrying", err)
 
 				select {
 				case <-ctx.Done():
-					p.logger.Logf("port forwarder: keep alive cancelled: %v", ctx.Err())
+					log.Printf("port forwarder: keep alive cancelled: %v", ctx.Err())
 					return
 				case <-time.After(time.Second): // todo: make configurable?
 					continue