Merge pull request #3779 from openziti/bug-fixes

Fix a panic and a deadlock

Author: plorenz

.github/workflows/backward-compatibility.yml

@@ -64,6 +64,8 @@ jobs:
           go install -tags=all,tests ./...
 
       - name: Create Test Environment
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
         shell: bash
         run: |
           echo "ZITI_ROOT=$(go env GOPATH)/bin" >> "$GITHUB_ENV"

.github/workflows/validation-test.yml

@@ -62,6 +62,8 @@ jobs:
           go install -tags=all,tests ./...
 
       - name: Create Test Environment
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
         shell: bash
         run: |
           echo "ZITI_ROOT=$(go env GOPATH)/bin" >> "$GITHUB_ENV"

controller/model/path.go

@@ -31,6 +31,10 @@ type Path struct {
 	TerminatorRemoteAddr string
 }
 
+func (self *Path) IsValid() bool {
+	return self != nil && len(self.Nodes) > 0
+}
+
 func (self *Path) Cost(minRouterCost uint16) int64 {
 	var cost int64
 	for _, l := range self.Links {

controller/network/network.go

@@ -589,6 +589,7 @@ func (network *Network) CreateCircuit(params model.CreateCircuitParams) (*model.
 		Id:        circuitId,
 		ClientId:  clientId.Token,
 		ServiceId: serviceId,
+		Path:      &model.Path{}, // empty until the circuit is built
 	}
 
 	// Reserve the circuit ID in the map to prevent collisions and protect routing.
@@ -1093,14 +1094,17 @@ func (network *Network) rerouteLink(l *model.Link, deadline time.Time) error {
 func (network *Network) rerouteCircuitWithTries(circuit *model.Circuit, retries int) bool {
 	log := pfxlog.Logger().WithField("circuitId", circuit.Id)
 
-	for i := 0; i < retries; i++ {
-		deadline := time.Now().Add(config.DefaultOptionsRouteTimeout)
-		err := network.rerouteCircuit(circuit, deadline)
-		if err == nil {
-			return true
-		}
+	// Path is nil for reserved circuits that haven't been built yet
+	if circuit.Path.IsValid() {
+		for i := 0; i < retries; i++ {
+			deadline := time.Now().Add(config.DefaultOptionsRouteTimeout)
+			err := network.rerouteCircuit(circuit, deadline)
+			if err == nil {
+				return true
+			}
 
-		log.WithError(err).WithField("attempt", i).Error("error re-routing circuit")
+			log.WithError(err).WithField("attempt", i).Error("error re-routing circuit")
+		}
 	}
 
 	if err := network.RemoveCircuit(circuit.Id, true); err != nil {

controller/network/smart.go

@@ -71,8 +71,10 @@ func (network *Network) getRerouteCandidates() []*newCircuitPath {
 	circuitCosts := make(map[string]int64)
 	var orderedCircuits []string
 	for _, circuit := range circuits {
-		circuitCosts[circuit.Id] = network.calculateCircuitCost(circuit.Path)
-		orderedCircuits = append(orderedCircuits, circuit.Id)
+		if circuit.Path.IsValid() {
+			circuitCosts[circuit.Id] = network.calculateCircuitCost(circuit.Path)
+			orderedCircuits = append(orderedCircuits, circuit.Id)
+		}
 	}
 
 	sort.SliceStable(orderedCircuits, func(i, j int) bool {

router/state/manager.go

@@ -957,6 +957,14 @@ func (self *ManagerImpl) SetRouterDataModel(model *common.RouterDataModel, reset
 
 	self.routerDataModel.Store(model)
 
+	// Clear the replace-in-progress flag before syncing subscribers. The model is
+	// fully stored at this point, so RouterDataModel() can safely return it.
+	// SyncAllSubscribers triggers subscriber notifications (e.g., service hosting)
+	// that call RouterDataModel(). If the flag is still set, those calls spin-wait
+	// for it to clear, which can't happen until this function returns, causing a
+	// deadlock.
+	self.rmdReplaceInProgress.Store(false)
+
 	if index < existingIndex {
 		self.env.GetIndexWatchers().NotifyOfIndexReset()
 	}

zititest/models/circuit-test/main.go

@@ -36,7 +36,7 @@ import (
 	"github.com/openziti/ziti/zititest/zitilab/validations"
 )
 
-var DefaultVersion = "v1.7.0"
+var DefaultVersion = ""
 var ClientRoutersVersion = ""
 var HostRoutersVersion = ""
 
@@ -48,7 +48,7 @@ const (
 	TestModeHostBackwardsCompatibility   = "host-backwards-compatibility"
 )
 
-var mode testMode = TestModeHostBackwardsCompatibility
+var mode testMode = TestModeDefault
 
 var entityCounts = map[string]uint32{
 	"loop-client":    1,