From cbae4117480ba133a38b8b531a2a18f41ff12e83 Mon Sep 17 00:00:00 2001 From: Zach Hoffman Date: Wed, 26 Oct 2022 01:04:00 -0600 Subject: [PATCH 1/4] Consider DS Regional field for MaxOriginConnections of Edge Caches --- lib/go-atscfg/headerrewritedotconfig.go | 22 +- lib/go-atscfg/headerrewritedotconfig_test.go | 207 ++++++++++--------- 2 files changed, 122 insertions(+), 107 deletions(-) diff --git a/lib/go-atscfg/headerrewritedotconfig.go b/lib/go-atscfg/headerrewritedotconfig.go index 83cf556578..611ba5dcb8 100644 --- a/lib/go-atscfg/headerrewritedotconfig.go +++ b/lib/go-atscfg/headerrewritedotconfig.go @@ -302,7 +302,7 @@ func getAssignedTierPeers( if serverIsMid(server) { return getAssignedMids(server, ds, servers, deliveryServiceServers, cacheGroups) } - return getAssignedEdges(ds, servers, deliveryServiceServers) + return getAssignedEdges(ds, server, servers, deliveryServiceServers) } // getAssignedEdges returns all EDGE caches assigned to ds via DeliveryService-Service. Does not consider Topologies. @@ -310,6 +310,7 @@ func getAssignedTierPeers( // Returns the list of assigned servers, and any warnings. func getAssignedEdges( ds *DeliveryService, + server *Server, servers []Server, deliveryServiceServers []DeliveryServiceServer, ) ([]Server, []string) { @@ -326,22 +327,25 @@ func getAssignedEdges( } assignedEdges := []Server{} - for _, server := range servers { - if server.CDNName == nil { - warnings = append(warnings, "servers had server with missing cdnName, skipping!") + for _, sv := range servers { + if sv.CDNName == nil { + warnings = append(warnings, "servers had sv with missing cdnName, skipping!") continue } - if server.ID == nil { - warnings = append(warnings, "servers had server with missing id, skipping!") + if sv.ID == nil { + warnings = append(warnings, "servers had sv with missing id, skipping!") continue } - if *server.CDNName != *ds.CDNName { + if *sv.CDNName != *ds.CDNName { + continue + } + if _, ok := dsServerIDs[*sv.ID]; !ok && ds.Topology == nil { continue } - if _, ok := dsServerIDs[*server.ID]; !ok && ds.Topology == nil { + if ds != nil && ds.Regional && *sv.Cachegroup != *server.Cachegroup { continue } - assignedEdges = append(assignedEdges, server) + assignedEdges = append(assignedEdges, sv) } return assignedEdges, warnings } diff --git a/lib/go-atscfg/headerrewritedotconfig_test.go b/lib/go-atscfg/headerrewritedotconfig_test.go index 4cc362d9ab..e4675d9630 100644 --- a/lib/go-atscfg/headerrewritedotconfig_test.go +++ b/lib/go-atscfg/headerrewritedotconfig_test.go @@ -364,96 +364,7 @@ func TestGetCachegroupsInSameTopologyTier(t *testing.T) { } } -func TestGetTopologyTierServers(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("org1"), - Type: util.StrPtr(tc.CacheGroupOriginTypeName), - }, - } - - allServers := []Server{ - { - Cachegroup: util.Ptr("edge1"), - HostName: util.Ptr("edgeCache1"), - ID: util.Ptr(0), - }, - { - Cachegroup: util.Ptr("edge2"), - HostName: util.Ptr("edgeCache2"), - ID: util.Ptr(0), - }, - } - - topology := tc.Topology{ - Nodes: []tc.TopologyNode{ - { - Cachegroup: "edge1", - Parents: []int{2}, - }, - { - Cachegroup: "edge2", - Parents: []int{2}, - }, - { - Cachegroup: "org1", - }, - }, - } - - type testCase struct { - ds *DeliveryService - dsRequiredCapabilities map[ServerCapability]struct{} - cg tc.CacheGroupName - topology tc.Topology - cacheGroups []tc.CacheGroupNullable - servers []Server - serverCapabilities map[int]map[ServerCapability]struct{} - - expectedHostnames []string - } - testCases := []testCase{ - { - ds: &DeliveryService{}, - cg: tc.CacheGroupName("edge1"), - topology: topology, - cacheGroups: allCachegroups, - servers: allServers, - - expectedHostnames: []string{"edgeCache1", "edgeCache2"}, - }, - { - ds: &DeliveryService{Regional: true}, - cg: tc.CacheGroupName("edge1"), - topology: topology, - cacheGroups: allCachegroups, - servers: allServers, - - expectedHostnames: []string{"edgeCache1"}, - }, - } - - for _, tc := range testCases { - actualServers, _ := getTopologyTierServers(tc.ds, tc.dsRequiredCapabilities, tc.cg, tc.topology, tc.cacheGroups, tc.servers, tc.serverCapabilities) - actualHostnames := []string{} - for _, as := range actualServers { - actualHostnames = append(actualHostnames, *as.HostName) - } - if !reflect.DeepEqual(tc.expectedHostnames, actualHostnames) { - t.Errorf("getting servers in same topology tier -- expected: %v, actual: %v", tc.expectedHostnames, actualHostnames) - } - } -} - -func TestGetAssignedMids(t *testing.T) { +func TestGetAssignedTierPeers(t *testing.T) { allCachegroups := []tc.CacheGroupNullable{ { Name: util.StrPtr("edge1"), @@ -481,7 +392,7 @@ func TestGetAssignedMids(t *testing.T) { }, } - allServers := []Server{ + edges := []Server{ { Cachegroup: util.Ptr("edge1"), CDNName: util.Ptr("mycdn"), @@ -496,7 +407,8 @@ func TestGetAssignedMids(t *testing.T) { ID: util.Ptr(2), Status: util.Ptr(string(tc.CacheStatusReported)), }, - + } + mids := []Server{ { Cachegroup: util.Ptr("mid1"), CDNName: util.Ptr("mycdn"), @@ -512,28 +424,74 @@ func TestGetAssignedMids(t *testing.T) { Status: util.Ptr(string(tc.CacheStatusReported)), }, } + allServers := append(edges, mids...) + _ = allServers - allDeliveryServices := []DeliveryService{{}, {}} + topology := tc.Topology{ + Name: "mytopology", + Nodes: []tc.TopologyNode{ + { + Cachegroup: "edge1", + Parents: []int{2}, + }, + { + Cachegroup: "edge2", + Parents: []int{2}, + }, + { + Cachegroup: "org1", + }, + }, + } + _ = topology + + allDeliveryServices := []DeliveryService{{}, {}, {}, {}} allDeliveryServices[0].ID = util.Ptr(1) allDeliveryServices[0].CDNName = util.Ptr("mycdn") allDeliveryServices[1].ID = util.Ptr(2) allDeliveryServices[1].Regional = true allDeliveryServices[1].CDNName = util.Ptr("mycdn") + allDeliveryServices[2].Topology = util.Ptr(topology.Name) + allDeliveryServices[3].Topology = util.Ptr(topology.Name) + allDeliveryServices[3].Regional = true type testCase struct { server *Server ds *DeliveryService + topology tc.Topology deliveryServiceServers []DeliveryServiceServer + dsRequiredCapabilities map[ServerCapability]struct{} servers []Server cacheGroups []tc.CacheGroupNullable + serverCapabilities map[int]map[ServerCapability]struct{} expectedHostnames []string } testCases := []testCase{ + // topology + { + ds: &allDeliveryServices[2], + server: &allServers[0], + topology: topology, + cacheGroups: allCachegroups, + servers: allServers, + + expectedHostnames: []string{"edgeCache1", "edgeCache2"}, + }, + { + ds: &allDeliveryServices[3], + server: &allServers[0], + topology: topology, + cacheGroups: allCachegroups, + servers: allServers, + + expectedHostnames: []string{"edgeCache1"}, + }, + // mid { - server: &allServers[0], + server: &mids[0], ds: &allDeliveryServices[0], - servers: allServers, + servers: mids, deliveryServiceServers: []DeliveryServiceServer{ { Server: 1, @@ -557,9 +515,9 @@ func TestGetAssignedMids(t *testing.T) { expectedHostnames: []string{"midCache1", "midCache2"}, }, { - server: &allServers[0], + server: &mids[0], ds: &allDeliveryServices[1], - servers: allServers, + servers: mids, deliveryServiceServers: []DeliveryServiceServer{ { Server: 1, @@ -582,10 +540,63 @@ func TestGetAssignedMids(t *testing.T) { expectedHostnames: []string{"midCache1"}, }, + // edge + { + server: &edges[0], + ds: &allDeliveryServices[0], + servers: edges, + deliveryServiceServers: []DeliveryServiceServer{ + { + Server: 1, + DeliveryService: 1, + }, + { + Server: 2, + DeliveryService: 1, + }, + { + Server: 3, + DeliveryService: 1, + }, + { + Server: 4, + DeliveryService: 1, + }, + }, + cacheGroups: allCachegroups, + + expectedHostnames: []string{"edgeCache1", "edgeCache2"}, + }, + { + server: &edges[0], + ds: &allDeliveryServices[1], + servers: edges, + deliveryServiceServers: []DeliveryServiceServer{ + { + Server: 1, + DeliveryService: 2, + }, + { + Server: 2, + DeliveryService: 2, + }, + { + Server: 3, + DeliveryService: 2, + }, + { + Server: 4, + DeliveryService: 2, + }, + }, + cacheGroups: allCachegroups, + + expectedHostnames: []string{"edgeCache1"}, + }, } for _, tc := range testCases { - actualServers, _ := getAssignedMids(tc.server, tc.ds, tc.servers, tc.deliveryServiceServers, tc.cacheGroups) + actualServers, _ := getAssignedTierPeers(tc.server, tc.ds, tc.topology, tc.servers, tc.deliveryServiceServers, tc.cacheGroups, tc.serverCapabilities, tc.dsRequiredCapabilities) actualHostnames := []string{} for _, as := range actualServers { actualHostnames = append(actualHostnames, *as.HostName) From faa7c4baee16c777bc96fd517b377a0ae849d2fc Mon Sep 17 00:00:00 2001 From: Zach Hoffman Date: Wed, 26 Oct 2022 02:06:26 -0600 Subject: [PATCH 2/4] Move Regional Delivery Service-handling case for Mid Caches --- lib/go-atscfg/headerrewritedotconfig.go | 26 ++++++++++---------- lib/go-atscfg/headerrewritedotconfig_test.go | 10 +++++--- 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/lib/go-atscfg/headerrewritedotconfig.go b/lib/go-atscfg/headerrewritedotconfig.go index 611ba5dcb8..7711ab4e3d 100644 --- a/lib/go-atscfg/headerrewritedotconfig.go +++ b/lib/go-atscfg/headerrewritedotconfig.go @@ -372,16 +372,16 @@ func getAssignedMids( serverCGs := map[tc.CacheGroupName]struct{}{} for _, sv := range servers { if sv.CDNName == nil { - warnings = append(warnings, "TO returned Servers server with missing CDNName, skipping!") + warnings = append(warnings, "TO returned Servers sv with missing CDNName, skipping!") continue } else if sv.ID == nil { - warnings = append(warnings, "TO returned Servers server with missing ID, skipping!") + warnings = append(warnings, "TO returned Servers sv with missing ID, skipping!") continue } else if sv.Status == nil { - warnings = append(warnings, "TO returned Servers server with missing Status, skipping!") + warnings = append(warnings, "TO returned Servers sv with missing Status, skipping!") continue } else if sv.Cachegroup == nil { - warnings = append(warnings, "TO returned Servers server with missing Cachegroup, skipping!") + warnings = append(warnings, "TO returned Servers sv with missing Cachegroup, skipping!") continue } @@ -394,9 +394,6 @@ func getAssignedMids( if tc.CacheStatus(*sv.Status) != tc.CacheStatusReported && tc.CacheStatus(*sv.Status) != tc.CacheStatusOnline { continue } - if ds != nil && ds.Regional && *sv.Cachegroup != *server.Cachegroup { - continue - } serverCGs[tc.CacheGroupName(*sv.Cachegroup)] = struct{}{} } @@ -416,22 +413,25 @@ func getAssignedMids( } assignedMids := []Server{} - for _, server := range servers { - if server.CDNName == nil { + for _, sv := range servers { + if sv.CDNName == nil { warnings = append(warnings, "TO returned Servers server with missing CDNName, skipping!") continue } - if server.Cachegroup == nil { + if sv.Cachegroup == nil { warnings = append(warnings, "TO returned Servers server with missing Cachegroup, skipping!") continue } - if *server.CDNName != *ds.CDNName { + if *sv.CDNName != *ds.CDNName { + continue + } + if _, ok := parentCGs[*sv.Cachegroup]; !ok { continue } - if _, ok := parentCGs[*server.Cachegroup]; !ok { + if ds != nil && ds.Regional && *sv.Cachegroup != *server.Cachegroup { continue } - assignedMids = append(assignedMids, server) + assignedMids = append(assignedMids, sv) } return assignedMids, warnings diff --git a/lib/go-atscfg/headerrewritedotconfig_test.go b/lib/go-atscfg/headerrewritedotconfig_test.go index e4675d9630..78d3852992 100644 --- a/lib/go-atscfg/headerrewritedotconfig_test.go +++ b/lib/go-atscfg/headerrewritedotconfig_test.go @@ -415,6 +415,7 @@ func TestGetAssignedTierPeers(t *testing.T) { HostName: util.Ptr("midCache1"), ID: util.Ptr(3), Status: util.Ptr(string(tc.CacheStatusReported)), + Type: tc.MidTypePrefix, }, { Cachegroup: util.Ptr("mid2"), @@ -422,6 +423,7 @@ func TestGetAssignedTierPeers(t *testing.T) { HostName: util.Ptr("midCache2"), ID: util.Ptr(4), Status: util.Ptr(string(tc.CacheStatusReported)), + Type: tc.MidTypePrefix, }, } allServers := append(edges, mids...) @@ -489,9 +491,9 @@ func TestGetAssignedTierPeers(t *testing.T) { }, // mid { - server: &mids[0], + server: &allServers[2], ds: &allDeliveryServices[0], - servers: mids, + servers: allServers, deliveryServiceServers: []DeliveryServiceServer{ { Server: 1, @@ -515,9 +517,9 @@ func TestGetAssignedTierPeers(t *testing.T) { expectedHostnames: []string{"midCache1", "midCache2"}, }, { - server: &mids[0], + server: &allServers[2], ds: &allDeliveryServices[1], - servers: mids, + servers: allServers, deliveryServiceServers: []DeliveryServiceServer{ { Server: 1, From 85f1f04fed4634eebed5396fe189fa6e1e2b184e Mon Sep 17 00:00:00 2001 From: Zach Hoffman Date: Wed, 26 Oct 2022 14:55:29 -0600 Subject: [PATCH 3/4] Revert false positive from variable rename --- lib/go-atscfg/headerrewritedotconfig.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/go-atscfg/headerrewritedotconfig.go b/lib/go-atscfg/headerrewritedotconfig.go index 7711ab4e3d..e47a87568a 100644 --- a/lib/go-atscfg/headerrewritedotconfig.go +++ b/lib/go-atscfg/headerrewritedotconfig.go @@ -329,11 +329,11 @@ func getAssignedEdges( assignedEdges := []Server{} for _, sv := range servers { if sv.CDNName == nil { - warnings = append(warnings, "servers had sv with missing cdnName, skipping!") + warnings = append(warnings, "servers had server with missing cdnName, skipping!") continue } if sv.ID == nil { - warnings = append(warnings, "servers had sv with missing id, skipping!") + warnings = append(warnings, "servers had server with missing id, skipping!") continue } if *sv.CDNName != *ds.CDNName { From 3b25d1aeaefc164dc9d34faf17b5745a272de7bf Mon Sep 17 00:00:00 2001 From: Zach Hoffman Date: Thu, 27 Oct 2022 10:19:50 -0600 Subject: [PATCH 4/4] Remove no-op variable assignments --- lib/go-atscfg/headerrewritedotconfig_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/go-atscfg/headerrewritedotconfig_test.go b/lib/go-atscfg/headerrewritedotconfig_test.go index 78d3852992..28f2725df8 100644 --- a/lib/go-atscfg/headerrewritedotconfig_test.go +++ b/lib/go-atscfg/headerrewritedotconfig_test.go @@ -427,7 +427,6 @@ func TestGetAssignedTierPeers(t *testing.T) { }, } allServers := append(edges, mids...) - _ = allServers topology := tc.Topology{ Name: "mytopology", @@ -445,7 +444,6 @@ func TestGetAssignedTierPeers(t *testing.T) { }, }, } - _ = topology allDeliveryServices := []DeliveryService{{}, {}, {}, {}} allDeliveryServices[0].ID = util.Ptr(1)