You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by oc...@apache.org on 2022/05/23 21:52:42 UTC
[trafficcontrol] branch master updated: `api/{{version}/deliveryservices/{id}/health` returns no info if the delivery service uses a topology (#6827)
This is an automated email from the ASF dual-hosted git repository.
ocket8888 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git
The following commit(s) were added to refs/heads/master by this push:
new f039b78cd2 `api/{{version}/deliveryservices/{id}/health` returns no info if the delivery service uses a topology (#6827)
f039b78cd2 is described below
commit f039b78cd2328f36b0314ca00bd06d31cf5ee8f3
Author: Srijeet Chatterjee <30...@users.noreply.github.com>
AuthorDate: Mon May 23 15:52:36 2022 -0600
`api/{{version}/deliveryservices/{id}/health` returns no info if the delivery service uses a topology (#6827)
* initial changes
* add tests. changelog
* code review fixes
* return error as last param
---
CHANGELOG.md | 1 +
.../traffic_ops_golang/deliveryservice/health.go | 59 +++++++--
.../deliveryservice/health_test.go | 136 +++++++++++++++++++++
3 files changed, 189 insertions(+), 7 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index da7fc76b6a..6ee6b7c859 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 c5676fc730..73c235519d 100644
--- a/traffic_ops/traffic_ops_golang/deliveryservice/health.go
+++ b/traffic_ops/traffic_ops_golang/deliveryservice/health.go
@@ -22,6 +22,7 @@ package deliveryservice
import (
"database/sql"
"errors"
+ "fmt"
"net/http"
"strings"
@@ -102,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)
-
+ cgData, totalOnline, totalOffline, err = 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)
@@ -114,14 +118,38 @@ 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) (map[tc.CacheGroupName]tc.HealthDataCacheGroup, uint64, uint64, error) {
+
+ var deliveryService tc.CRConfigDeliveryService
+ var ok bool
+ var topology string
+ var cacheGroupNameMap = make(map[string]bool)
+ var skip bool
+
+ if deliveryService, ok = crConfig.DeliveryServices[string(ds)]; !ok {
+ 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 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
+ }
+ }
+ }
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 +160,24 @@ 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]struct{}, len(cache.Capabilities))
+ for _, cap := range cache.Capabilities {
+ cacheCapabilities[cap] = struct{}{}
+ }
+ for _, rc := range deliveryService.RequiredCapabilities {
+ if _, ok = cacheCapabilities[rc]; !ok {
+ skip = true
+ break
+ }
+ }
+ if skip {
+ continue
+ }
+ }
cgHealth := data[tc.CacheGroupName(*cache.CacheGroup)]
cgHealth.Name = tc.CacheGroupName(*cache.CacheGroup)
if avail.IsAvailable {
@@ -144,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 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
new file mode 100644
index 0000000000..64516e7561
--- /dev/null
+++ b/traffic_ops/traffic_ops_golang/deliveryservice/health_test.go
@@ -0,0 +1,136 @@
+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 (
+ "testing"
+ "time"
+
+ "github.com/apache/trafficcontrol/lib/go-tc"
+ "github.com/apache/trafficcontrol/lib/go-util"
+)
+
+func TestAddHealth(t *testing.T) {
+ 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,
+ },
+ },
+ }
+
+ 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{
+ 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, 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)
+ }
+ 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, 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)
+ }
+ 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)
+ }
+}