From aeab5206d479f892236a695e534c45f73d493f53 Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Wed, 11 May 2022 19:21:24 -0600 Subject: [PATCH 1/4] initial changes --- .../deliveryservice/health.go | 46 +++++++++++++++++-- 1 file changed, 43 insertions(+), 3 deletions(-) diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/health.go b/traffic_ops/traffic_ops_golang/deliveryservice/health.go index c5676fc730..9d6c267530 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/health.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/health.go @@ -25,6 +25,7 @@ import ( "net/http" "strings" + "github.com/apache/trafficcontrol/lib/go-log" "github.com/apache/trafficcontrol/lib/go-tc" "github.com/apache/trafficcontrol/lib/go-util" "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api" @@ -115,13 +116,39 @@ func getMonitorHealth(tx *sql.Tx, ds tc.DeliveryServiceName, monitorFQDNs []stri // addHealth adds the given cache states to the given data and totals, and returns the new data and totals func addHealth(ds tc.DeliveryServiceName, data map[tc.CacheGroupName]tc.HealthDataCacheGroup, totalOnline uint64, totalOffline uint64, crStates tc.CRStates, crConfig tc.CRConfig) (map[tc.CacheGroupName]tc.HealthDataCacheGroup, uint64, uint64) { + + var deliveryService tc.CRConfigDeliveryService + var ok bool + var topology string + var cacheGroupNameMap = make(map[string]bool) + var cacheCapabilities = make(map[string]bool) + + if deliveryService, ok = crConfig.DeliveryServices[string(ds)]; !ok { + log.Errorln("delivery service not found in CRConfig") + return map[tc.CacheGroupName]tc.HealthDataCacheGroup{}, 0, 0 + } + + if deliveryService.Topology != nil { + topology = *deliveryService.Topology + if topology != "" { + if top, ok := crConfig.Topologies[topology]; !ok { + log.Errorf("CRConfig topologies does not contain DS topology: %s", topology) + } else { + for _, n := range top.Nodes { + cacheGroupNameMap[n] = true + } + } + } + } for cacheName, avail := range crStates.Caches { cache, ok := crConfig.ContentServers[string(cacheName)] if !ok { continue // TODO warn? } - if _, ok := cache.DeliveryServices[string(ds)]; !ok { - continue + if topology == "" { + if _, ok := cache.DeliveryServices[string(ds)]; !ok { + continue + } } if cache.ServerStatus == nil || *cache.ServerStatus != tc.CRConfigServerStatus(tc.CacheStatusReported) { continue @@ -132,7 +159,20 @@ func addHealth(ds tc.DeliveryServiceName, data map[tc.CacheGroupName]tc.HealthDa if cache.CacheGroup == nil { continue // TODO warn? } - + if topology != "" { + if _, ok := cacheGroupNameMap[*cache.CacheGroup]; !ok { + continue + } + } + cacheCapabilities = make(map[string]bool) + for _, cap := range cache.Capabilities { + cacheCapabilities[cap] = true + } + for _, rc := range deliveryService.RequiredCapabilities { + if _, ok = cacheCapabilities[rc]; !ok { + continue + } + } cgHealth := data[tc.CacheGroupName(*cache.CacheGroup)] cgHealth.Name = tc.CacheGroupName(*cache.CacheGroup) if avail.IsAvailable { From dc8f4793e787c88b1a154231a1803f76721656c3 Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Thu, 12 May 2022 12:12:15 -0600 Subject: [PATCH 2/4] add tests. changelog --- CHANGELOG.md | 1 + .../deliveryservice/health.go | 19 +- .../deliveryservice/health_test.go | 165 ++++++++++++++++++ 3 files changed, 178 insertions(+), 7 deletions(-) create mode 100644 traffic_ops/traffic_ops_golang/deliveryservice/health_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 341ce78450..6a551b2673 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - Fixed Traffic Ops ignoring the configured database port value, which was prohibiting the use of anything other than port 5432 (the PostgreSQL default) - [#6580](https://github.com/apache/trafficcontrol/issues/6580) Fixed cache config generation remap.config targets for MID-type servers in a Topology with other caches as parents and HTTPS origins. - Traffic Router: fixed a null pointer exception that caused snapshots to be rejected if a topology cachegroup did not have any online/reported/admin\_down caches +- [#6271](https://github.com/apache/trafficcontrol/issues/6271) `api/{{version}/deliveryservices/{id}/health` returns no info if the delivery service uses a topology. - [#6549](https://github.com/apache/trafficcontrol/issues/6549) Fixed internal server error while deleting a delivery service created from a DSR (Traafic Ops). - [#6538](https://github.com/apache/trafficcontrol/pull/6538) Fixed the incorrect use of secure.port on TrafficRouter and corrected to the httpsPort value from the TR server configuration. - [#6562](https://github.com/apache/trafficcontrol/pull/6562) Fixed incorrect template in Ansible dataset loader role when fallbackToClosest is defined. diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/health.go b/traffic_ops/traffic_ops_golang/deliveryservice/health.go index 9d6c267530..ce9bf383a5 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/health.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/health.go @@ -122,6 +122,7 @@ func addHealth(ds tc.DeliveryServiceName, data map[tc.CacheGroupName]tc.HealthDa var topology string var cacheGroupNameMap = make(map[string]bool) var cacheCapabilities = make(map[string]bool) + var skip bool if deliveryService, ok = crConfig.DeliveryServices[string(ds)]; !ok { log.Errorln("delivery service not found in CRConfig") @@ -163,13 +164,17 @@ func addHealth(ds tc.DeliveryServiceName, data map[tc.CacheGroupName]tc.HealthDa if _, ok := cacheGroupNameMap[*cache.CacheGroup]; !ok { continue } - } - cacheCapabilities = make(map[string]bool) - for _, cap := range cache.Capabilities { - cacheCapabilities[cap] = true - } - for _, rc := range deliveryService.RequiredCapabilities { - if _, ok = cacheCapabilities[rc]; !ok { + cacheCapabilities = make(map[string]bool) + for _, cap := range cache.Capabilities { + cacheCapabilities[cap] = true + } + for _, rc := range deliveryService.RequiredCapabilities { + if _, ok = cacheCapabilities[rc]; !ok { + skip = true + continue + } + } + if skip { continue } } diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/health_test.go b/traffic_ops/traffic_ops_golang/deliveryservice/health_test.go new file mode 100644 index 0000000000..f1aab39996 --- /dev/null +++ b/traffic_ops/traffic_ops_golang/deliveryservice/health_test.go @@ -0,0 +1,165 @@ +package deliveryservice + +/* + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +import ( + "github.com/apache/trafficcontrol/lib/go-tc" + + "encoding/json" + "testing" +) + +const crConfigJson = ` +{ + "config": {}, + "contentServers": { + "cache1": { + "cacheGroup": "cg1", + "capabilities": [ + "cap1", + "cap2" + ], + "status": "REPORTED", + "type": "EDGE", + "deliveryServices": { + "ds1": [ + "edge.ds1.test.net" + ] + } + }, + "cache2": { + "cacheGroup": "cg2", + "capabilities": [ + "cap2", + "cap3" + ], + "status": "REPORTED", + "type": "EDGE" + }, + "cache3": { + "cacheGroup": "cg2", + "capabilities": [ + "cap3", + "cap4" + ], + "status": "REPORTED", + "type": "EDGE" + } + }, + "deliveryServices": { + "ds1": {}, + "ds2-topology": { + "topology": "test_topology", + "requiredCapabilities": ["cap2"] + } + }, + "contentRouters": { + "tr1": {} + }, + "edgeLocations": { + "edge1": {} + }, + "trafficRouterLocations": { + "tr-loc": {} + }, + "monitors": { + "tm-host": {} + }, + "stats": {}, + "topologies": { + "test_topology": { + "nodes": [ + "cg2" + ] + } + } +} +` +const crStatesJson = ` +{ + "caches": { + "cache1": { + "isAvailable": true, + "ipv4Available": true, + "ipv6Available": true, + "status": "REPORTED - available", + "lastPoll": "2022-05-11T19:50:55.036253631Z" + }, + "cache2": { + "isAvailable": true, + "ipv4Available": true, + "ipv6Available": true, + "status": "REPORTED - available", + "lastPoll": "2022-05-11T19:51:06.965095596Z" + }, + "cache3": { + "isAvailable": true, + "ipv4Available": true, + "ipv6Available": true, + "status": "REPORTED - available", + "lastPoll": "2022-05-11T19:51:06.965095596Z" + } + }, + "deliveryServices": { + "ds1": { + "disabledLocations": [], + "isAvailable": true + }, + "ds2-topology": { + "disabledLocations": [], + "isAvailable": true + } + } +} +` + +func TestAddHealth(t *testing.T) { + crStates := tc.CRStates{} + crConfig := tc.CRConfig{} + err := json.Unmarshal([]byte(crStatesJson), &crStates) + if err != nil { + t.Fatalf("error unmarshalling crStates: %v", err) + } + err = json.Unmarshal([]byte(crConfigJson), &crConfig) + if err != nil { + t.Fatalf("error unmarshalling crConfig: %v", err) + } + data := make(map[tc.CacheGroupName]tc.HealthDataCacheGroup) + data[tc.CacheGroupName("cache1")] = tc.HealthDataCacheGroup{ + Offline: 0, + Online: 0, + Name: "cg1", + } + data[tc.CacheGroupName("cache2")] = tc.HealthDataCacheGroup{ + Offline: 0, + Online: 0, + Name: "cg2", + } + data[tc.CacheGroupName("cache3")] = tc.HealthDataCacheGroup{ + Offline: 0, + Online: 0, + Name: "cg2", + } + _, available, unAvailable := addHealth("ds1", data, 0, 0, crStates, crConfig) + if available != 1 || unAvailable != 0 { + t.Errorf("expected ds1 to have 1 online and 0 offline caches, but got %d online and %d offline instead", available, unAvailable) + } + // Even though there are 2 REPORTED EDGE caches in cg2, the result should just include 1, because one of them should get filtered out because it's missing a required capability (cap2) + _, available, unAvailable = addHealth("ds2-topology", data, 0, 0, crStates, crConfig) + if available != 1 || unAvailable != 0 { + t.Errorf("expected ds2-topology to have 1 online and 0 offline caches, but got %d online and %d offline instead", available, unAvailable) + } +} From 197addb080e50412c1dd45707dd52174fb09e0d8 Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Fri, 20 May 2022 15:34:05 -0600 Subject: [PATCH 3/4] code review fixes --- .../deliveryservice/health.go | 36 +-- .../deliveryservice/health_test.go | 205 ++++++++---------- 2 files changed, 106 insertions(+), 135 deletions(-) diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/health.go b/traffic_ops/traffic_ops_golang/deliveryservice/health.go index ce9bf383a5..13e8862d68 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/health.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/health.go @@ -22,10 +22,10 @@ package deliveryservice import ( "database/sql" "errors" + "fmt" "net/http" "strings" - "github.com/apache/trafficcontrol/lib/go-log" "github.com/apache/trafficcontrol/lib/go-tc" "github.com/apache/trafficcontrol/lib/go-util" "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api" @@ -103,8 +103,11 @@ func getMonitorHealth(tx *sql.Tx, ds tc.DeliveryServiceName, monitorFQDNs []stri errs = append(errs, errors.New("getting CRConfig for delivery service '"+string(ds)+"' monitor '"+monitorFQDN+"': "+err.Error())) continue } - cgData, totalOnline, totalOffline = addHealth(ds, cgData, totalOnline, totalOffline, crStates, crConfig) - + err, cgData, totalOnline, totalOffline = addHealth(ds, cgData, totalOnline, totalOffline, crStates, crConfig) + if err != nil { + errs = append(errs, err) + continue + } healthData := tc.HealthData{TotalOffline: totalOffline, TotalOnline: totalOnline, CacheGroups: []tc.HealthDataCacheGroup{}} for _, health := range cgData { healthData.CacheGroups = append(healthData.CacheGroups, health) @@ -115,29 +118,26 @@ func getMonitorHealth(tx *sql.Tx, ds tc.DeliveryServiceName, monitorFQDNs []stri } // addHealth adds the given cache states to the given data and totals, and returns the new data and totals -func addHealth(ds tc.DeliveryServiceName, data map[tc.CacheGroupName]tc.HealthDataCacheGroup, totalOnline uint64, totalOffline uint64, crStates tc.CRStates, crConfig tc.CRConfig) (map[tc.CacheGroupName]tc.HealthDataCacheGroup, uint64, uint64) { +func addHealth(ds tc.DeliveryServiceName, data map[tc.CacheGroupName]tc.HealthDataCacheGroup, totalOnline uint64, totalOffline uint64, crStates tc.CRStates, crConfig tc.CRConfig) (error, map[tc.CacheGroupName]tc.HealthDataCacheGroup, uint64, uint64) { var deliveryService tc.CRConfigDeliveryService var ok bool var topology string var cacheGroupNameMap = make(map[string]bool) - var cacheCapabilities = make(map[string]bool) var skip bool if deliveryService, ok = crConfig.DeliveryServices[string(ds)]; !ok { - log.Errorln("delivery service not found in CRConfig") - return map[tc.CacheGroupName]tc.HealthDataCacheGroup{}, 0, 0 + return errors.New("delivery service not found in CRConfig"), map[tc.CacheGroupName]tc.HealthDataCacheGroup{}, 0, 0 } - if deliveryService.Topology != nil { + var top tc.CRConfigTopology topology = *deliveryService.Topology if topology != "" { - if top, ok := crConfig.Topologies[topology]; !ok { - log.Errorf("CRConfig topologies does not contain DS topology: %s", topology) - } else { - for _, n := range top.Nodes { - cacheGroupNameMap[n] = true - } + if top, ok = crConfig.Topologies[topology]; !ok { + return fmt.Errorf("CRConfig topologies does not contain DS topology: %s", topology), map[tc.CacheGroupName]tc.HealthDataCacheGroup{}, 0, 0 + } + for _, n := range top.Nodes { + cacheGroupNameMap[n] = true } } } @@ -164,14 +164,14 @@ func addHealth(ds tc.DeliveryServiceName, data map[tc.CacheGroupName]tc.HealthDa if _, ok := cacheGroupNameMap[*cache.CacheGroup]; !ok { continue } - cacheCapabilities = make(map[string]bool) + cacheCapabilities := make(map[string]struct{}, len(cache.Capabilities)) for _, cap := range cache.Capabilities { - cacheCapabilities[cap] = true + cacheCapabilities[cap] = struct{}{} } for _, rc := range deliveryService.RequiredCapabilities { if _, ok = cacheCapabilities[rc]; !ok { skip = true - continue + break } } if skip { @@ -189,5 +189,5 @@ func addHealth(ds tc.DeliveryServiceName, data map[tc.CacheGroupName]tc.HealthDa } data[tc.CacheGroupName(*cache.CacheGroup)] = cgHealth } - return data, totalOnline, totalOffline + return nil, data, totalOnline, totalOffline } diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/health_test.go b/traffic_ops/traffic_ops_golang/deliveryservice/health_test.go index f1aab39996..a9d6d3ec68 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/health_test.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/health_test.go @@ -16,126 +16,91 @@ package deliveryservice */ import ( - "github.com/apache/trafficcontrol/lib/go-tc" - - "encoding/json" "testing" -) + "time" -const crConfigJson = ` -{ - "config": {}, - "contentServers": { - "cache1": { - "cacheGroup": "cg1", - "capabilities": [ - "cap1", - "cap2" - ], - "status": "REPORTED", - "type": "EDGE", - "deliveryServices": { - "ds1": [ - "edge.ds1.test.net" - ] - } - }, - "cache2": { - "cacheGroup": "cg2", - "capabilities": [ - "cap2", - "cap3" - ], - "status": "REPORTED", - "type": "EDGE" - }, - "cache3": { - "cacheGroup": "cg2", - "capabilities": [ - "cap3", - "cap4" - ], - "status": "REPORTED", - "type": "EDGE" - } - }, - "deliveryServices": { - "ds1": {}, - "ds2-topology": { - "topology": "test_topology", - "requiredCapabilities": ["cap2"] - } - }, - "contentRouters": { - "tr1": {} - }, - "edgeLocations": { - "edge1": {} - }, - "trafficRouterLocations": { - "tr-loc": {} - }, - "monitors": { - "tm-host": {} - }, - "stats": {}, - "topologies": { - "test_topology": { - "nodes": [ - "cg2" - ] - } - } -} -` -const crStatesJson = ` -{ - "caches": { - "cache1": { - "isAvailable": true, - "ipv4Available": true, - "ipv6Available": true, - "status": "REPORTED - available", - "lastPoll": "2022-05-11T19:50:55.036253631Z" - }, - "cache2": { - "isAvailable": true, - "ipv4Available": true, - "ipv6Available": true, - "status": "REPORTED - available", - "lastPoll": "2022-05-11T19:51:06.965095596Z" - }, - "cache3": { - "isAvailable": true, - "ipv4Available": true, - "ipv6Available": true, - "status": "REPORTED - available", - "lastPoll": "2022-05-11T19:51:06.965095596Z" - } - }, - "deliveryServices": { - "ds1": { - "disabledLocations": [], - "isAvailable": true - }, - "ds2-topology": { - "disabledLocations": [], - "isAvailable": true - } - } -} -` + "github.com/apache/trafficcontrol/lib/go-tc" + "github.com/apache/trafficcontrol/lib/go-util" +) func TestAddHealth(t *testing.T) { - crStates := tc.CRStates{} - crConfig := tc.CRConfig{} - err := json.Unmarshal([]byte(crStatesJson), &crStates) - if err != nil { - t.Fatalf("error unmarshalling crStates: %v", err) + crStates := tc.CRStates{ + Caches: map[tc.CacheName]tc.IsAvailable{ + "cache1": { + IsAvailable: true, + Ipv4Available: true, + Ipv6Available: true, + Status: "REPORTED - available", + LastPoll: time.Now(), + }, + "cache2": { + IsAvailable: true, + Ipv4Available: true, + Ipv6Available: true, + Status: "REPORTED - available", + LastPoll: time.Now(), + }, + "cache3": { + IsAvailable: true, + Ipv4Available: true, + Ipv6Available: true, + Status: "REPORTED - available", + LastPoll: time.Now(), + }, + }, + DeliveryService: map[tc.DeliveryServiceName]tc.CRStatesDeliveryService{ + "ds1": { + DisabledLocations: []tc.CacheGroupName{}, + IsAvailable: true, + }, + "ds2-topology": { + DisabledLocations: []tc.CacheGroupName{}, + IsAvailable: true, + }, + }, } - err = json.Unmarshal([]byte(crConfigJson), &crConfig) - if err != nil { - t.Fatalf("error unmarshalling crConfig: %v", err) + + status := tc.CRConfigServerStatus("REPORTED") + crConfig := tc.CRConfig{ + Config: nil, + ContentServers: map[string]tc.CRConfigTrafficOpsServer{ + "cache1": { + CacheGroup: util.StrPtr("cg1"), + Capabilities: []string{"cap1", "cap2"}, + ServerStatus: &status, + ServerType: util.StrPtr("EDGE"), + DeliveryServices: map[string][]string{ + "ds1": {"edge.ds1.test.net"}, + }, + }, + "cache2": { + CacheGroup: util.StrPtr("cg2"), + Capabilities: []string{"cap2", "cap3"}, + ServerStatus: &status, + ServerType: util.StrPtr("EDGE"), + }, + "cache3": { + CacheGroup: util.StrPtr("cg2"), + Capabilities: []string{"cap3", "cap4"}, + ServerStatus: &status, + ServerType: util.StrPtr("EDGE"), + }, + }, + ContentRouters: nil, + DeliveryServices: map[string]tc.CRConfigDeliveryService{ + "ds1": {}, + "ds2-topology": { + Topology: util.StrPtr("test_topology"), + RequiredCapabilities: []string{"cap2"}, + }, + }, + EdgeLocations: nil, + RouterLocations: nil, + Monitors: nil, + Stats: tc.CRConfigStats{}, + Topologies: map[string]tc.CRConfigTopology{ + "test_topology": {Nodes: []string{"cg2"}}, + }, } data := make(map[tc.CacheGroupName]tc.HealthDataCacheGroup) data[tc.CacheGroupName("cache1")] = tc.HealthDataCacheGroup{ @@ -153,12 +118,18 @@ func TestAddHealth(t *testing.T) { Online: 0, Name: "cg2", } - _, available, unAvailable := addHealth("ds1", data, 0, 0, crStates, crConfig) + err, _, available, unAvailable := addHealth("ds1", data, 0, 0, crStates, crConfig) + if err != nil { + t.Fatalf("expected no error while adding health of ds1, but got %v", err) + } if available != 1 || unAvailable != 0 { t.Errorf("expected ds1 to have 1 online and 0 offline caches, but got %d online and %d offline instead", available, unAvailable) } // Even though there are 2 REPORTED EDGE caches in cg2, the result should just include 1, because one of them should get filtered out because it's missing a required capability (cap2) - _, available, unAvailable = addHealth("ds2-topology", data, 0, 0, crStates, crConfig) + err, _, available, unAvailable = addHealth("ds2-topology", data, 0, 0, crStates, crConfig) + if err != nil { + t.Fatalf("expected no error while adding health of ds2, but got %v", err) + } if available != 1 || unAvailable != 0 { t.Errorf("expected ds2-topology to have 1 online and 0 offline caches, but got %d online and %d offline instead", available, unAvailable) } From ac431d1503a69078a3d449bab35997b8936bbf68 Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Sun, 22 May 2022 18:04:56 -0600 Subject: [PATCH 4/4] return error as last param --- .../traffic_ops_golang/deliveryservice/health.go | 10 +++++----- .../traffic_ops_golang/deliveryservice/health_test.go | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/health.go b/traffic_ops/traffic_ops_golang/deliveryservice/health.go index 13e8862d68..73c235519d 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/health.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/health.go @@ -103,7 +103,7 @@ func getMonitorHealth(tx *sql.Tx, ds tc.DeliveryServiceName, monitorFQDNs []stri errs = append(errs, errors.New("getting CRConfig for delivery service '"+string(ds)+"' monitor '"+monitorFQDN+"': "+err.Error())) continue } - err, cgData, totalOnline, totalOffline = addHealth(ds, cgData, totalOnline, totalOffline, crStates, crConfig) + cgData, totalOnline, totalOffline, err = addHealth(ds, cgData, totalOnline, totalOffline, crStates, crConfig) if err != nil { errs = append(errs, err) continue @@ -118,7 +118,7 @@ func getMonitorHealth(tx *sql.Tx, ds tc.DeliveryServiceName, monitorFQDNs []stri } // addHealth adds the given cache states to the given data and totals, and returns the new data and totals -func addHealth(ds tc.DeliveryServiceName, data map[tc.CacheGroupName]tc.HealthDataCacheGroup, totalOnline uint64, totalOffline uint64, crStates tc.CRStates, crConfig tc.CRConfig) (error, map[tc.CacheGroupName]tc.HealthDataCacheGroup, uint64, uint64) { +func addHealth(ds tc.DeliveryServiceName, data map[tc.CacheGroupName]tc.HealthDataCacheGroup, totalOnline uint64, totalOffline uint64, crStates tc.CRStates, crConfig tc.CRConfig) (map[tc.CacheGroupName]tc.HealthDataCacheGroup, uint64, uint64, error) { var deliveryService tc.CRConfigDeliveryService var ok bool @@ -127,14 +127,14 @@ func addHealth(ds tc.DeliveryServiceName, data map[tc.CacheGroupName]tc.HealthDa var skip bool if deliveryService, ok = crConfig.DeliveryServices[string(ds)]; !ok { - return errors.New("delivery service not found in CRConfig"), map[tc.CacheGroupName]tc.HealthDataCacheGroup{}, 0, 0 + return map[tc.CacheGroupName]tc.HealthDataCacheGroup{}, 0, 0, errors.New("delivery service not found in CRConfig") } if deliveryService.Topology != nil { var top tc.CRConfigTopology topology = *deliveryService.Topology if topology != "" { if top, ok = crConfig.Topologies[topology]; !ok { - return fmt.Errorf("CRConfig topologies does not contain DS topology: %s", topology), map[tc.CacheGroupName]tc.HealthDataCacheGroup{}, 0, 0 + return map[tc.CacheGroupName]tc.HealthDataCacheGroup{}, 0, 0, fmt.Errorf("CRConfig topologies does not contain DS topology: %s", topology) } for _, n := range top.Nodes { cacheGroupNameMap[n] = true @@ -189,5 +189,5 @@ func addHealth(ds tc.DeliveryServiceName, data map[tc.CacheGroupName]tc.HealthDa } data[tc.CacheGroupName(*cache.CacheGroup)] = cgHealth } - return nil, data, totalOnline, totalOffline + return data, totalOnline, totalOffline, nil } diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/health_test.go b/traffic_ops/traffic_ops_golang/deliveryservice/health_test.go index a9d6d3ec68..64516e7561 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/health_test.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/health_test.go @@ -118,7 +118,7 @@ func TestAddHealth(t *testing.T) { Online: 0, Name: "cg2", } - err, _, available, unAvailable := addHealth("ds1", data, 0, 0, crStates, crConfig) + _, available, unAvailable, err := addHealth("ds1", data, 0, 0, crStates, crConfig) if err != nil { t.Fatalf("expected no error while adding health of ds1, but got %v", err) } @@ -126,7 +126,7 @@ func TestAddHealth(t *testing.T) { t.Errorf("expected ds1 to have 1 online and 0 offline caches, but got %d online and %d offline instead", available, unAvailable) } // Even though there are 2 REPORTED EDGE caches in cg2, the result should just include 1, because one of them should get filtered out because it's missing a required capability (cap2) - err, _, available, unAvailable = addHealth("ds2-topology", data, 0, 0, crStates, crConfig) + _, available, unAvailable, err = addHealth("ds2-topology", data, 0, 0, crStates, crConfig) if err != nil { t.Fatalf("expected no error while adding health of ds2, but got %v", err) }