Fix max_origin_connections calculation for topology-based DSes

CHANGELOG.md

@@ -42,6 +42,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - Only `operations` and `admin` roles should have the `DELIVERY-SERVICE:UPDATE` permission.
 - [#6369](https://github.com/apache/trafficcontrol/pull/6369) Fixed `/acme_accounts` endpoint to validate email and URL fields
 - Fixed searching of the ds parameter merge_parent_groups slice.
+- [#6806](https://github.com/apache/trafficcontrol/issues/6806) t3c calculates max_origin_connections incorrectly for topology-based delivery services
 - Fixed TO API `PUT /servers/:id/status` to only queue updates on the same CDN as the updated server
 - t3c-generate fix for combining remapconfig and cachekeyconfig parameters for MakeRemapDotConfig call.
 - [#6780](https://github.com/apache/trafficcontrol/issues/6780) Fixed t3c to use secondary parents when there are no primary parents available.

lib/go-atscfg/headerrewritedotconfig.go

@@ -154,7 +154,7 @@ func MakeHeaderRewriteDotConfig(
 	atsMajorVersion, verWarns := getATSMajorVersion(tcServerParams)
 	warnings = append(warnings, verWarns...)
 
-	assignedTierPeers, assignWarns := getAssignedTierPeers(server, ds, servers, deliveryServiceServers, cacheGroupsArr, serverCapabilities, requiredCapabilities[*ds.ID])
+	assignedTierPeers, assignWarns := getAssignedTierPeers(server, ds, topology, servers, deliveryServiceServers, cacheGroupsArr, serverCapabilities, requiredCapabilities[*ds.ID])
 	warnings = append(warnings, assignWarns...)
 
 	dsOnlinePeerCount := 0
@@ -282,14 +282,15 @@ func serverIsMid(server *Server) bool {
 func getAssignedTierPeers(
 	server *Server,
 	ds *DeliveryService,
+	topology tc.Topology,
 	servers []Server,
 	deliveryServiceServers []DeliveryServiceServer,
 	cacheGroups []tc.CacheGroupNullable,
 	serverCapabilities map[int]map[ServerCapability]struct{},
 	dsRequiredCapabilities map[ServerCapability]struct{},
 ) ([]Server, []string) {
 	if ds.Topology != nil {
-		return getTopologyTierServers(dsRequiredCapabilities, tc.CacheGroupName(*server.Cachegroup), servers, serverCapabilities)
+		return getTopologyTierServers(dsRequiredCapabilities, tc.CacheGroupName(*server.Cachegroup), topology, cacheGroups, servers, serverCapabilities)
 	}
 	if serverIsMid(server) {
 		return getAssignedMids(server, ds, servers, deliveryServiceServers, cacheGroups)
@@ -422,14 +423,14 @@ func getAssignedMids(
 	return assignedMids, warnings
 }
 
-// getTopologyDSServerCount returns the servers in cg which will be used to serve ds.
+// getTopologyTierServers returns the servers in the same tier as cg which will be used to serve ds.
 // This should only be used for DSes with Topologies.
-// It returns all servers in CG with the Capabilities of ds in cg.
-// It will not be the number of servers for Delivery Services not using Topologies, which use DeliveryService-Server assignments instead.
+// It returns all servers in with the Capabilities of ds in the same tier as cg.
 // Returns the servers, and any warnings.
-func getTopologyTierServers(dsRequiredCapabilities map[ServerCapability]struct{}, cg tc.CacheGroupName, servers []Server, serverCapabilities map[int]map[ServerCapability]struct{}) ([]Server, []string) {
+func getTopologyTierServers(dsRequiredCapabilities map[ServerCapability]struct{}, cg tc.CacheGroupName, topology tc.Topology, cacheGroups []tc.CacheGroupNullable, servers []Server, serverCapabilities map[int]map[ServerCapability]struct{}) ([]Server, []string) {
 	warnings := []string{}
 	topoServers := []Server{}
+	cacheGroupsInSameTier := getCachegroupsInSameTopologyTier(string(cg), cacheGroups, topology)
 	for _, sv := range servers {
 		if sv.Cachegroup == nil {
 			warnings = append(warnings, "Servers had server with nil cachegroup, skipping!")
@@ -439,7 +440,7 @@ func getTopologyTierServers(dsRequiredCapabilities map[ServerCapability]struct{}
 			continue
 		}
 
-		if *sv.Cachegroup != string(cg) {
+		if !cacheGroupsInSameTier[*sv.Cachegroup] {
 			continue
 		}
 		if !hasRequiredCapabilities(serverCapabilities[*sv.ID], dsRequiredCapabilities) {
@@ -450,6 +451,73 @@ func getTopologyTierServers(dsRequiredCapabilities map[ServerCapability]struct{}
 	return topoServers, warnings
 }
 
+func getCachegroupsInSameTopologyTier(cg string, cacheGroups []tc.CacheGroupNullable, topology tc.Topology) map[string]bool {
+	cacheGroupMap := make(map[string]tc.CacheGroupNullable)
+	originCacheGroups := make(map[string]bool)
+	for _, cg := range cacheGroups {
+		if cg.Name == nil || cg.Type == nil {
+			continue
+		}
+		cacheGroupMap[*cg.Name] = cg
+		if *cg.Type == tc.CacheGroupOriginTypeName {
+			originCacheGroups[*cg.Name] = true
+		}
+	}
+	originNodes := make(map[int]bool)
+	nodeIndex := -1
+	for i, node := range topology.Nodes {
+		if node.Cachegroup == cg {
+			nodeIndex = i
+		}
+		if originCacheGroups[node.Cachegroup] {
+			originNodes[i] = true
+		}
+	}
+	nodesWithSameOriginDistances := getNodesWithSameOriginDistances(nodeIndex, topology, originNodes)
+	cacheGroupsInSameTopologyTier := make(map[string]bool)
+	for _, nodeI := range nodesWithSameOriginDistances {
+		cacheGroupsInSameTopologyTier[topology.Nodes[nodeI].Cachegroup] = true
+	}
+	return cacheGroupsInSameTopologyTier
+}
+
+func getNodesWithSameOriginDistances(nodeIndex int, topology tc.Topology, originNodes map[int]bool) []int {
+	originDistances := make(map[int]int)
+	nodeDistance := -1
+	for i := range topology.Nodes {
+		d := getOriginDistance(topology, i, originNodes, originDistances)
+		if nodeIndex == i {
+			nodeDistance = d
+		}
+	}
+	sameDistances := make([]int, 0)
+	for i, d := range originDistances {
+		if d == nodeDistance {
+			sameDistances = append(sameDistances, i)
+		}
+	}
+	return sameDistances
+}
+
+func getOriginDistance(topology tc.Topology, nodeIndex int, originNodes map[int]bool, originDistances map[int]int) int {
+	if originDistance, ok := originDistances[nodeIndex]; ok {
+		return originDistance
+	}
+	parents := topology.Nodes[nodeIndex].Parents
+	if len(parents) == 0 {
+		originDistances[nodeIndex] = 1
+		return originDistances[nodeIndex]
+	}
+	for _, p := range parents {
+		if originNodes[p] {
+			originDistances[nodeIndex] = 1
+			return originDistances[nodeIndex]
+		}
+	}
+	originDistances[nodeIndex] = 1 + getOriginDistance(topology, parents[0], originNodes, originDistances)
+	return originDistances[nodeIndex]
+}
+
 var returnRe = regexp.MustCompile(`\s*__RETURN__\s*`)
 
 // makeATCHeaderRewriteDirectives returns the Header Rewrite text for all per-Delivery-Service Traffic Control directives, such as MaxOriginConnections and ServiceCategory.

lib/go-atscfg/headerrewritedotconfig_test.go

@@ -20,6 +20,7 @@ package atscfg
  */
 
 import (
+	"reflect"
 	"strings"
 	"testing"
 
@@ -170,6 +171,199 @@ func TestMakeHeaderRewriteDotConfigNoMaxOriginConnections(t *testing.T) {
 	}
 }
 
+func TestGetCachegroupsInSameTopologyTier(t *testing.T) {
+	allCachegroups := []tc.CacheGroupNullable{
+		{
+			Name: util.StrPtr("edge1"),
+			Type: util.StrPtr(tc.CacheGroupEdgeTypeName),
+		},
+		{
+			Name: util.StrPtr("edge2"),
+			Type: util.StrPtr(tc.CacheGroupEdgeTypeName),
+		},
+		{
+			Name: util.StrPtr("deep1"),
+			Type: util.StrPtr(tc.CacheGroupEdgeTypeName),
+		},
+		{
+			Name: util.StrPtr("mid1"),
+			Type: util.StrPtr(tc.CacheGroupMidTypeName),
+		},
+		{
+			Name: util.StrPtr("mid2"),
+			Type: util.StrPtr(tc.CacheGroupMidTypeName),
+		},
+		{
+			Name: util.StrPtr("org1"),
+			Type: util.StrPtr(tc.CacheGroupOriginTypeName),
+		},
+		{
+			Name: util.StrPtr("org2"),
+			Type: util.StrPtr(tc.CacheGroupOriginTypeName),
+		},
+	}
+	type testCase struct {
+		cachegroup  string
+		cachegroups []tc.CacheGroupNullable
+		topology    tc.Topology
+		expected    map[string]bool
+	}
+	testCases := []testCase{
+		{
+			cachegroup:  "edge1",
+			cachegroups: allCachegroups,
+			topology: tc.Topology{
+				Nodes: []tc.TopologyNode{
+					{
+						// 0
+						Cachegroup: "edge1",
+						Parents:    []int{3},
+					},
+					{
+						// 1
+						Cachegroup: "deep1",
+						Parents:    []int{0},
+					},
+					{
+						// 2
+						Cachegroup: "edge2",
+						Parents:    []int{4},
+					},
+					{
+						// 3
+						Cachegroup: "mid1",
+						Parents:    []int{},
+					},
+					{
+						// 4
+						Cachegroup: "mid2",
+						Parents:    []int{},
+					},
+				},
+			},
+			expected: map[string]bool{"edge1": true, "edge2": true},
+		},
+		{
+			cachegroup:  "deep1",
+			cachegroups: allCachegroups,
+			topology: tc.Topology{
+				Nodes: []tc.TopologyNode{
+					{
+						// 0
+						Cachegroup: "edge1",
+						Parents:    []int{3},
+					},
+					{
+						// 1
+						Cachegroup: "deep1",
+						Parents:    []int{0},
+					},
+					{
+						// 2
+						Cachegroup: "edge2",
+						Parents:    []int{4},
+					},
+					{
+						// 3
+						Cachegroup: "mid1",
+						Parents:    []int{},
+					},
+					{
+						// 4
+						Cachegroup: "mid2",
+						Parents:    []int{},
+					},
+				},
+			},
+			expected: map[string]bool{"deep1": true},
+		},
+		{
+			cachegroup:  "mid1",
+			cachegroups: allCachegroups,
+			topology: tc.Topology{
+				Nodes: []tc.TopologyNode{
+					{
+						// 0
+						Cachegroup: "edge1",
+						Parents:    []int{3},
+					},
+					{
+						// 1
+						Cachegroup: "deep1",
+						Parents:    []int{0},
+					},
+					{
+						// 2
+						Cachegroup: "edge2",
+						Parents:    []int{4},
+					},
+					{
+						// 3
+						Cachegroup: "mid1",
+						Parents:    []int{},
+					},
+					{
+						// 4
+						Cachegroup: "mid2",
+						Parents:    []int{},
+					},
+				},
+			},
+			expected: map[string]bool{"mid1": true, "mid2": true},
+		},
+		{
+			cachegroup:  "edge2",
+			cachegroups: allCachegroups,
+			topology: tc.Topology{
+				Nodes: []tc.TopologyNode{
+					{
+						// 0
+						Cachegroup: "edge1",
+						Parents:    []int{3},
+					},
+					{
+						// 1
+						Cachegroup: "deep1",
+						Parents:    []int{0},
+					},
+					{
+						// 2
+						Cachegroup: "edge2",
+						Parents:    []int{4},
+					},
+					{
+						// 3
+						Cachegroup: "mid1",
+						Parents:    []int{5},
+					},
+					{
+						// 4
+						Cachegroup: "mid2",
+						Parents:    []int{6},
+					},
+					{
+						// 5
+						Cachegroup: "org1",
+						Parents:    []int{},
+					},
+					{
+						// 5
+						Cachegroup: "org2",
+						Parents:    []int{},
+					},
+				},
+			},
+			expected: map[string]bool{"edge1": true, "edge2": true},
+		},
+	}
+	for _, tc := range testCases {
+		actual := getCachegroupsInSameTopologyTier(tc.cachegroup, tc.cachegroups, tc.topology)
+		if !reflect.DeepEqual(tc.expected, actual) {
+			t.Errorf("getting cachegroups in same topology tier -- expected: %v, actual: %v", tc.expected, actual)
+		}
+	}
+}
+
 func TestMakeHeaderRewriteMidDotConfig(t *testing.T) {
 	cdnName := "mycdn"
 	hdr := "myHeaderComment"