Use common code for Start and setting ports from control server

internal/portforward/service/service.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"net/http"
+	"slices"
 	"sync"
 )
 
@@ -53,39 +54,25 @@ func (s *Service) GetPortsForwarded() (ports []uint16) {
 }
 
 func (s *Service) SetPortsForwarded(ctx context.Context, ports []uint16) (err error) {
-	for i, port := range s.ports {
-		err := s.portAllower.RemoveAllowedPort(ctx, port)
-		if err != nil {
-			for j := range i {
-				_ = s.portAllower.SetAllowedPort(ctx, s.ports[j], s.settings.Interface)
-			}
-			return fmt.Errorf("removing allowed port: %w", err)
-		}
-	}
+	s.startStopMutex.Lock()
+	defer s.startStopMutex.Unlock()
 
-	for i, port := range ports {
-		err := s.portAllower.SetAllowedPort(ctx, port, s.settings.Interface)
-		if err != nil {
-			for j := range i {
-				_ = s.portAllower.RemoveAllowedPort(ctx, s.ports[j])
-			}
-			for _, port := range s.ports {
-				_ = s.portAllower.SetAllowedPort(ctx, port, s.settings.Interface)
-			}
-			return fmt.Errorf("setting allowed port: %w", err)
-		}
+	s.portMutex.Lock()
+	defer s.portMutex.Unlock()
+	slices.Sort(ports)
+	if slices.Equal(s.ports, ports) {
+		return nil
 	}
 
-	err = s.writePortForwardedFile(ports)
+	err = s.cleanup()
 	if err != nil {
-		_ = s.cleanup()
-		return fmt.Errorf("writing port forwarded file: %w", err)
+		return fmt.Errorf("cleaning up: %w", err)
 	}
 
-	s.portMutex.RLock()
-	defer s.portMutex.RUnlock()
-	s.ports = make([]uint16, len(ports))
-	copy(s.ports, ports)
+	err = s.onNewPorts(ctx, ports)
+	if err != nil {
+		return fmt.Errorf("handling new ports: %w", err)
+	}
 
 	s.logger.Info("updated: " + portsToString(s.ports))
 

internal/portforward/service/start.go

@@ -3,6 +3,7 @@ package service
 import (
 	"context"
 	"fmt"
+	"slices"
 
 	"github.com/qdm12/gluetun/internal/netlink"
 	"github.com/qdm12/gluetun/internal/provider/utils"
@@ -47,30 +48,67 @@ func (s *Service) Start(ctx context.Context) (runError <-chan error, err error)
 		return nil, fmt.Errorf("port forwarding for the first time: %w", err)
 	}
 
+	err = s.onNewPorts(ctx, ports)
+	if err != nil {
+		return nil, err
+	}
+
+	keepPortCtx, keepPortCancel := context.WithCancel(context.Background())
+	s.keepPortCancel = keepPortCancel
+	runErrorCh := make(chan error)
+	keepPortDoneCh := make(chan struct{})
+	s.keepPortDoneCh = keepPortDoneCh
+
+	readyCh := make(chan struct{})
+	go func(ctx context.Context, portForwarder PortForwarder,
+		obj utils.PortForwardObjects, readyCh chan<- struct{},
+		runError chan<- error, doneCh chan<- struct{},
+	) {
+		defer close(doneCh)
+		close(readyCh)
+		err = portForwarder.KeepPortForward(ctx, obj)
+		crashed := ctx.Err() == nil
+		if !crashed { // stopped by Stop call
+			return
+		}
+		s.startStopMutex.Lock()
+		defer s.startStopMutex.Unlock()
+		_ = s.cleanup()
+		runError <- err
+	}(keepPortCtx, s.settings.PortForwarder, obj, readyCh, runErrorCh, keepPortDoneCh)
+	<-readyCh
+
+	return runErrorCh, nil
+}
+
+func (s *Service) onNewPorts(ctx context.Context, ports []uint16) (err error) {
+	slices.Sort(ports)
+
 	s.logger.Info(portsToString(ports))
 
 	for _, port := range ports {
 		err = s.portAllower.SetAllowedPort(ctx, port, s.settings.Interface)
 		if err != nil {
-			return nil, fmt.Errorf("allowing port in firewall: %w", err)
+			return fmt.Errorf("allowing port in firewall: %w", err)
 		}
 
 		if s.settings.ListeningPort != 0 {
 			err = s.portAllower.RedirectPort(ctx, s.settings.Interface, port, s.settings.ListeningPort)
 			if err != nil {
-				return nil, fmt.Errorf("redirecting port in firewall: %w", err)
+				return fmt.Errorf("redirecting port in firewall: %w", err)
 			}
 		}
 	}
 
 	err = s.writePortForwardedFile(ports)
 	if err != nil {
 		_ = s.cleanup()
-		return nil, fmt.Errorf("writing port file: %w", err)
+		return fmt.Errorf("writing port file: %w", err)
 	}
 
 	s.portMutex.Lock()
-	s.ports = ports
+	s.ports = make([]uint16, len(ports))
+	copy(s.ports, ports)
 	s.portMutex.Unlock()
 
 	if s.settings.UpCommand != "" {
@@ -81,30 +119,5 @@ func (s *Service) Start(ctx context.Context) (runError <-chan error, err error)
 		}
 	}
 
-	keepPortCtx, keepPortCancel := context.WithCancel(context.Background())
-	s.keepPortCancel = keepPortCancel
-	runErrorCh := make(chan error)
-	keepPortDoneCh := make(chan struct{})
-	s.keepPortDoneCh = keepPortDoneCh
-
-	readyCh := make(chan struct{})
-	go func(ctx context.Context, portForwarder PortForwarder,
-		obj utils.PortForwardObjects, readyCh chan<- struct{},
-		runError chan<- error, doneCh chan<- struct{},
-	) {
-		defer close(doneCh)
-		close(readyCh)
-		err = portForwarder.KeepPortForward(ctx, obj)
-		crashed := ctx.Err() == nil
-		if !crashed { // stopped by Stop call
-			return
-		}
-		s.startStopMutex.Lock()
-		defer s.startStopMutex.Unlock()
-		_ = s.cleanup()
-		runError <- err
-	}(keepPortCtx, s.settings.PortForwarder, obj, readyCh, runErrorCh, keepPortDoneCh)
-	<-readyCh
-
-	return runErrorCh, nil
+	return nil
 }