Skip to content

Commit

Permalink
backport: ci: remove logger field from portforwarder to avoid race wi…
Browse files Browse the repository at this point in the history
…th goroutine (#2959) (#3264)

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
  • Loading branch information
QxBytes authored Dec 13, 2024
1 parent 20dd164 commit 8e70c85
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 8 deletions.
2 changes: 1 addition & 1 deletion test/integration/k8s_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,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)
}
Expand Down
13 changes: 6 additions & 7 deletions test/integration/portforward.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"io"
"log"
"math/rand"
"net/http"
"sync"
Expand All @@ -26,7 +27,6 @@ type PortForwarder struct {
clientset *kubernetes.Clientset
transport http.RoundTripper
upgrader spdy.Upgrader
logger logger

opts PortForwardingOpts

Expand All @@ -44,7 +44,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)
Expand All @@ -59,7 +59,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
Expand Down Expand Up @@ -156,7 +155,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,
Expand All @@ -165,14 +164,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
Expand Down

0 comments on commit 8e70c85

Please sign in to comment.