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 1202163 Log when getting errors from TO but backup configs exist
(#5513)
1202163 is described below
commit 12021638a87b35aad5207d6cb38c955729bbb616
Author: Rawlin Peters <[email protected]>
AuthorDate: Thu Feb 11 15:27:08 2021 -0700
Log when getting errors from TO but backup configs exist (#5513)
* Log when getting errors from TO but backup configs exist
* Add deliveryServices monitoring config validation
* Add changelog
---
CHANGELOG.md | 1 +
lib/go-tc/traffic_monitor.go | 8 +++----
lib/go-tc/traffic_monitor_test.go | 44 ++++++++++++++++++++++-----------------
traffic_monitor/towrap/towrap.go | 6 +++---
4 files changed, 32 insertions(+), 27 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 057f3af..be19364 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -34,6 +34,7 @@ The format is based on [Keep a
Changelog](http://keepachangelog.com/en/1.0.0/).
- [#5396](https://github.com/apache/trafficcontrol/issues/5396) - Return the
correct error type if user tries to update the root tenant
- [#5378](https://github.com/apache/trafficcontrol/issues/5378) - Updating a
non existent DS should return a 404, instead of a 500
- Fixed a potential Traffic Router race condition that could cause erroneous
503s for CLIENT_STEERING delivery services when loading new steering changes
+- Fixed a logging bug in Traffic Monitor where it wouldn't log errors in
certain cases where a backup file could be used instead. Also, Traffic Monitor
now rejects monitoring snapshots that have no delivery services.
- [#5195](https://github.com/apache/trafficcontrol/issues/5195) - Correctly
show CDN ID in Changelog during Snap
- [#5438](https://github.com/apache/trafficcontrol/issues/5438) - Correctly
specify nodejs version requirements in traffic_portal.spec
- Fixed Traffic Router logging unnecessary warnings for IPv6-only caches
diff --git a/lib/go-tc/traffic_monitor.go b/lib/go-tc/traffic_monitor.go
index 6f7f1b2..52be683 100644
--- a/lib/go-tc/traffic_monitor.go
+++ b/lib/go-tc/traffic_monitor.go
@@ -303,11 +303,9 @@ func (cfg *TrafficMonitorConfigMap) Valid() error {
if len(cfg.TrafficMonitor) == 0 {
return errors.New("MonitorConfig.TrafficMonitor empty")
}
- // TODO uncomment this, when TO is fixed to include DeliveryServices.
- // See https://github.com/apache/trafficcontrol/issues/3528
- // if len(cfg.DeliveryService) == 0 {
- // return errors.New("MonitorConfig.DeliveryService empty")
- // }
+ if len(cfg.DeliveryService) == 0 {
+ return errors.New("MonitorConfig.DeliveryService empty")
+ }
if len(cfg.Profile) == 0 {
return errors.New("MonitorConfig.Profile empty")
}
diff --git a/lib/go-tc/traffic_monitor_test.go
b/lib/go-tc/traffic_monitor_test.go
index bf6d0f4..343603d 100644
--- a/lib/go-tc/traffic_monitor_test.go
+++ b/lib/go-tc/traffic_monitor_test.go
@@ -473,24 +473,23 @@ func TestTrafficMonitorConfigMap_Valid(t *testing.T) {
}
mc.Config["peers.polling.interval"] = 42.0
- // TODO: uncomment these tests when #3528 is resolved
- // mc.DeliveryService = nil
- // err = mc.Valid()
- // if err == nil {
- // t.Error("Didn't get expected error checking validity of config
map with nil DeliveryService")
- // } else {
- // t.Logf("Got expected error: checking validity of config map
with nil DeliveryService: %v", err)
- // }
-
- // mc.DeliveryService = map[string]TMDeliveryService{}
- // err = mc.Valid()
- // if err == nil {
- // t.Error("Didn't get expected error checking validity of config
map with no DeliveryServices")
- // } else {
- // t.Logf("Got expected error: checking validity of config map
with no DeliveryServices: %v", err)
- // }
-
- // mc.DeliveryService["a"] = TMDeliveryService{}
+ mc.DeliveryService = nil
+ err = mc.Valid()
+ if err == nil {
+ t.Error("Didn't get expected error checking validity of config
map with nil DeliveryService")
+ } else {
+ t.Logf("Got expected error: checking validity of config map
with nil DeliveryService: %v", err)
+ }
+
+ mc.DeliveryService = map[string]TMDeliveryService{}
+ err = mc.Valid()
+ if err == nil {
+ t.Error("Didn't get expected error checking validity of config
map with no DeliveryServices")
+ } else {
+ t.Logf("Got expected error: checking validity of config map
with no DeliveryServices: %v", err)
+ }
+
+ mc.DeliveryService["a"] = TMDeliveryService{}
mc.Profile = nil
err = mc.Valid()
if err == nil {
@@ -570,7 +569,7 @@ func TestTrafficMonitorTransformToMap(t *testing.T) {
TrafficMonitors: []TrafficMonitor{
TrafficMonitor{},
},
- DeliveryServices: []TMDeliveryService{},
+ DeliveryServices: []TMDeliveryService{{XMLID: "foo"}},
Profiles: []TMProfile{
{
Name: "test",
@@ -598,6 +597,13 @@ func TestTrafficMonitorTransformToMap(t *testing.T) {
t.Errorf("Incorrect number of traffic servers after conversion;
expected: 1, got: %d", len(converted.TrafficServer))
}
+ if len(converted.DeliveryService) != 1 {
+ t.Errorf("Incorrect number of deliveryServices after
conversion; expected: 1, got: %d", len(converted.DeliveryService))
+ }
+ if _, ok := converted.DeliveryService["foo"]; !ok {
+ t.Error("Expected delivery service 'foo' to exist in map after
conversion, but it didn't")
+ }
+
if _, ok := converted.TrafficServer["testHostname"]; !ok {
t.Error("Expected server 'testHostname' to exist in map after
conversion, but it didn't")
} else if len(converted.TrafficServer["testHostname"].Interfaces) != 1 {
diff --git a/traffic_monitor/towrap/towrap.go b/traffic_monitor/towrap/towrap.go
index 1b6b541..f7ef5b7 100644
--- a/traffic_monitor/towrap/towrap.go
+++ b/traffic_monitor/towrap/towrap.go
@@ -363,12 +363,12 @@ func (s TrafficOpsSessionThreadsafe) CRConfigRaw(cdn
string) ([]byte, error) {
ioutil.WriteFile(s.CRConfigBackupFile, data, 0644)
} else {
if s.BackupFileExists() {
+ log.Errorln("using backup file for CRConfig snapshot
due to error fetching CRConfig snapshot from Traffic Ops: " + err.Error())
data, err = ioutil.ReadFile(s.CRConfigBackupFile)
if err != nil {
return nil, fmt.Errorf("file Read Error: %v",
err)
}
remoteAddr = localHostIP
- log.Errorln("Error getting CRConfig from traffic_ops,
backup file exists, reading from file")
err = nil
} else {
return nil, fmt.Errorf("Failed to get CRConfig from
Traffic Ops (%v), and there is no backup file", err)
@@ -473,17 +473,17 @@ func (s TrafficOpsSessionThreadsafe)
trafficMonitorConfigMapRaw(cdn string) (*tc
if !s.BackupFileExists() {
return nil, err
}
+ log.Errorln("using backup file for monitoring config snapshot
due to invalid monitoring config snapshot from Traffic Ops: " + err.Error())
b, err := ioutil.ReadFile(s.TMConfigBackupFile)
if err != nil {
return nil, errors.New("reading TMConfigBackupFile: " +
err.Error())
}
- log.Errorln("Error getting configMap from traffic_ops, backup
file exists, reading from file")
json := jsoniter.ConfigFastest
var tmConfig tc.TrafficMonitorConfig
if err := json.Unmarshal(b, &tmConfig); err != nil {
- return nil, errors.New("unmarhsalling backup file
monitoring.json: " + err.Error())
+ return nil, errors.New("unmarshalling backup file
monitoring.json: " + err.Error())
}
return tc.TrafficMonitorTransformToMap(&tmConfig)
}
