mirror of
https://github.com/lukaszraczylo/kportal.git
synced 2026-06-15 02:45:33 +00:00
fixup! Update go.mod and go.sum (#48)
This commit is contained in:
@@ -20,12 +20,12 @@ import (
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
"github.com/nvm/kportal/internal/config"
|
||||
"github.com/nvm/kportal/internal/events"
|
||||
"github.com/nvm/kportal/internal/healthcheck"
|
||||
"github.com/nvm/kportal/internal/k8s"
|
||||
"github.com/nvm/kportal/internal/logger"
|
||||
"github.com/nvm/kportal/internal/mdns"
|
||||
"github.com/lukaszraczylo/kportal/internal/config"
|
||||
"github.com/lukaszraczylo/kportal/internal/events"
|
||||
"github.com/lukaszraczylo/kportal/internal/healthcheck"
|
||||
"github.com/lukaszraczylo/kportal/internal/k8s"
|
||||
"github.com/lukaszraczylo/kportal/internal/logger"
|
||||
"github.com/lukaszraczylo/kportal/internal/mdns"
|
||||
)
|
||||
|
||||
// StatusUpdater is an interface for updating forward status
|
||||
@@ -241,12 +241,17 @@ func (m *Manager) Stop() {
|
||||
}
|
||||
m.workersMu.Unlock()
|
||||
|
||||
// Stop all workers
|
||||
// Stop all workers with limited concurrency to avoid unbounded goroutine creation
|
||||
var wg sync.WaitGroup
|
||||
sem := make(chan struct{}, 10) // Limit to 10 concurrent stops
|
||||
|
||||
for _, worker := range workers {
|
||||
wg.Add(1)
|
||||
sem <- struct{}{} // Acquire semaphore
|
||||
|
||||
go func(w *ForwardWorker) {
|
||||
defer wg.Done()
|
||||
defer func() { <-sem }() // Release semaphore
|
||||
w.Stop()
|
||||
}(worker)
|
||||
}
|
||||
|
||||
@@ -1,11 +1,12 @@
|
||||
package forward
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/nvm/kportal/internal/config"
|
||||
"github.com/nvm/kportal/internal/events"
|
||||
"github.com/lukaszraczylo/kportal/internal/config"
|
||||
"github.com/lukaszraczylo/kportal/internal/events"
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
@@ -331,3 +332,45 @@ func TestManager_EventBusIntegration(t *testing.T) {
|
||||
// Handler
|
||||
})
|
||||
}
|
||||
|
||||
// TestManager_Stop_WithManyWorkers tests that shutdown limits concurrent stops
|
||||
func TestManager_Stop_WithManyWorkers(t *testing.T) {
|
||||
manager, err := NewManager(false)
|
||||
if err != nil {
|
||||
t.Skip("Skipping test - no kubeconfig available")
|
||||
}
|
||||
|
||||
// Create and add mock workers directly to test shutdown behavior
|
||||
numWorkers := 25
|
||||
manager.workersMu.Lock()
|
||||
for i := 0; i < numWorkers; i++ {
|
||||
fwd := config.Forward{
|
||||
Resource: fmt.Sprintf("pod/app-%d", i),
|
||||
Port: 8080,
|
||||
LocalPort: 10000 + i,
|
||||
}
|
||||
worker := NewForwardWorker(fwd, manager.portForwarder, false, nil, manager.healthChecker, manager.watchdog)
|
||||
manager.workers[fwd.ID()] = worker
|
||||
}
|
||||
manager.workersMu.Unlock()
|
||||
|
||||
// Stop should complete successfully with limited concurrency
|
||||
done := make(chan bool)
|
||||
go func() {
|
||||
manager.Stop()
|
||||
done <- true
|
||||
}()
|
||||
|
||||
select {
|
||||
case <-done:
|
||||
// Success - all workers stopped
|
||||
case <-time.After(10 * time.Second):
|
||||
t.Fatal("Stop timed out with many workers")
|
||||
}
|
||||
|
||||
// Verify workers map is cleared
|
||||
manager.workersMu.RLock()
|
||||
workerCount := len(manager.workers)
|
||||
manager.workersMu.RUnlock()
|
||||
assert.Equal(t, 0, workerCount, "Workers map should be empty after Stop")
|
||||
}
|
||||
|
||||
@@ -7,7 +7,7 @@ import (
|
||||
"runtime"
|
||||
"strings"
|
||||
|
||||
"github.com/nvm/kportal/internal/logger"
|
||||
"github.com/lukaszraczylo/kportal/internal/logger"
|
||||
)
|
||||
|
||||
const (
|
||||
|
||||
@@ -5,8 +5,8 @@ import (
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
"github.com/nvm/kportal/internal/events"
|
||||
"github.com/nvm/kportal/internal/logger"
|
||||
"github.com/lukaszraczylo/kportal/internal/events"
|
||||
"github.com/lukaszraczylo/kportal/internal/logger"
|
||||
)
|
||||
|
||||
const (
|
||||
|
||||
@@ -8,12 +8,12 @@ import (
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
"github.com/nvm/kportal/internal/config"
|
||||
"github.com/nvm/kportal/internal/healthcheck"
|
||||
"github.com/nvm/kportal/internal/httplog"
|
||||
"github.com/nvm/kportal/internal/k8s"
|
||||
"github.com/nvm/kportal/internal/logger"
|
||||
"github.com/nvm/kportal/internal/retry"
|
||||
"github.com/lukaszraczylo/kportal/internal/config"
|
||||
"github.com/lukaszraczylo/kportal/internal/healthcheck"
|
||||
"github.com/lukaszraczylo/kportal/internal/httplog"
|
||||
"github.com/lukaszraczylo/kportal/internal/k8s"
|
||||
"github.com/lukaszraczylo/kportal/internal/logger"
|
||||
"github.com/lukaszraczylo/kportal/internal/retry"
|
||||
)
|
||||
|
||||
const (
|
||||
@@ -132,8 +132,16 @@ func (w *ForwardWorker) GetForwardID() string {
|
||||
|
||||
// run is the main worker loop that handles retries.
|
||||
func (w *ForwardWorker) run() {
|
||||
defer close(w.doneChan)
|
||||
defer w.stopHTTPProxy() // Ensure proxy is stopped on exit
|
||||
// Use a combined defer with sync.Once to ensure doneChan is closed
|
||||
// even if stopHTTPProxy() panics. This prevents the worker from
|
||||
// getting stuck if cleanup operations fail.
|
||||
var closeDoneOnce sync.Once
|
||||
defer func() {
|
||||
w.stopHTTPProxy() // Ensure proxy is stopped on exit
|
||||
closeDoneOnce.Do(func() {
|
||||
close(w.doneChan)
|
||||
})
|
||||
}()
|
||||
|
||||
// Note: Heartbeat management is now centralized in the Watchdog.
|
||||
// The watchdog polls workers via the HeartbeatResponder interface (IsAlive method)
|
||||
@@ -266,14 +274,16 @@ func (w *ForwardWorker) establishForward(podName string) error {
|
||||
|
||||
// Create a context for this forward attempt
|
||||
forwardCtx, forwardCancel := context.WithCancel(w.ctx)
|
||||
defer forwardCancel()
|
||||
|
||||
// Store cancel function so TriggerReconnect can use it
|
||||
w.forwardCancelMu.Lock()
|
||||
w.forwardCancel = forwardCancel
|
||||
w.forwardCancelMu.Unlock()
|
||||
|
||||
// Combined cleanup: cancel context and clear the cancel function reference.
|
||||
// Using a single defer ensures both operations happen atomically.
|
||||
defer func() {
|
||||
forwardCancel()
|
||||
w.forwardCancelMu.Lock()
|
||||
w.forwardCancel = nil
|
||||
w.forwardCancelMu.Unlock()
|
||||
|
||||
@@ -5,7 +5,7 @@ import (
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/nvm/kportal/internal/config"
|
||||
"github.com/lukaszraczylo/kportal/internal/config"
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
|
||||
@@ -1,9 +1,11 @@
|
||||
package forward
|
||||
|
||||
import (
|
||||
"sync"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/nvm/kportal/internal/config"
|
||||
"github.com/lukaszraczylo/kportal/internal/config"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
@@ -284,3 +286,93 @@ func TestWorkerVerboseMode(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestWorkerCleanupWithPanic verifies that doneChan is properly closed
|
||||
// even when cleanup functions panic. This tests the fix for the defer
|
||||
// ordering issue where stopHTTPProxy() could prevent doneChan from closing.
|
||||
func TestWorkerCleanupWithPanic(t *testing.T) {
|
||||
t.Run("doneChan closed after panic in cleanup", func(t *testing.T) {
|
||||
doneChan := make(chan struct{})
|
||||
|
||||
// Simulate the cleanup pattern used in run() with sync.Once
|
||||
var closeDoneOnce sync.Once
|
||||
cleanupWithPanic := func() {
|
||||
// Simulate stopHTTPProxy() that panics
|
||||
panic("simulated panic in cleanup")
|
||||
}
|
||||
|
||||
// Use defer with recovery to test the pattern
|
||||
func() {
|
||||
defer func() {
|
||||
if r := recover(); r != nil {
|
||||
// Expected panic - doneChan should still be closed
|
||||
_ = r // Suppress SA9003: empty branch warning
|
||||
}
|
||||
closeDoneOnce.Do(func() {
|
||||
close(doneChan)
|
||||
})
|
||||
}()
|
||||
|
||||
cleanupWithPanic()
|
||||
}()
|
||||
|
||||
// Verify doneChan was closed even though cleanup panicked
|
||||
select {
|
||||
case <-doneChan:
|
||||
// Success: channel was closed
|
||||
case <-time.After(100 * time.Millisecond):
|
||||
t.Fatal("doneChan should be closed even when cleanup panics")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("doneChan closed normally without panic", func(t *testing.T) {
|
||||
doneChan := make(chan struct{})
|
||||
|
||||
var closeDoneOnce sync.Once
|
||||
cleanupNormal := func() {
|
||||
// Normal cleanup, no panic
|
||||
}
|
||||
|
||||
func() {
|
||||
defer func() {
|
||||
cleanupNormal()
|
||||
closeDoneOnce.Do(func() {
|
||||
close(doneChan)
|
||||
})
|
||||
}()
|
||||
// Normal function execution
|
||||
}()
|
||||
|
||||
// Verify doneChan was closed
|
||||
select {
|
||||
case <-doneChan:
|
||||
// Success
|
||||
case <-time.After(100 * time.Millisecond):
|
||||
t.Fatal("doneChan should be closed after normal execution")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("sync.Once prevents double close", func(t *testing.T) {
|
||||
doneChan := make(chan struct{})
|
||||
|
||||
var closeDoneOnce sync.Once
|
||||
closeFunc := func() {
|
||||
closeDoneOnce.Do(func() {
|
||||
close(doneChan)
|
||||
})
|
||||
}
|
||||
|
||||
// Call closeFunc multiple times
|
||||
closeFunc()
|
||||
closeFunc()
|
||||
closeFunc()
|
||||
|
||||
// Should not panic - sync.Once ensures close() is only called once
|
||||
select {
|
||||
case <-doneChan:
|
||||
// Success
|
||||
case <-time.After(100 * time.Millisecond):
|
||||
t.Fatal("doneChan should be closed")
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user