Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

api/{{version}/deliveryservices/{id}/health returns no info if the delivery service uses a topology #6827

Merged
merged 4 commits into from
May 23, 2022
Merged

api/{{version}/deliveryservices/{id}/health returns no info if the delivery service uses a topology #6827

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
aeab520
initial changes
srijeet0406 May 12, 2022
dc8f479
add tests. changelog
srijeet0406 May 12, 2022
197addb
code review fixes
srijeet0406 May 20, 2022
ac431d1
return error as last param
srijeet0406 May 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 18 additions & 18 deletions traffic_ops/traffic_ops_golang/deliveryservice/health.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand All @@ -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) {
srijeet0406 marked this conversation as resolved.
Show resolved Hide resolved

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
}
}
}
Expand All @@ -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 {
Expand All @@ -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
}
205 changes: 88 additions & 117 deletions traffic_ops/traffic_ops_golang/deliveryservice/health_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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)
}
Expand Down