[trafficcontrol] branch 5.0.x updated: Tm csv nilcheck (#5358)

[oc...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

ocket8888 pushed a commit to branch 5.0.x
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git


The following commit(s) were added to refs/heads/5.0.x by this push:
     new 3abf9c8  Tm csv nilcheck (#5358)
3abf9c8 is described below

commit 3abf9c842c552bfa7607e0ecf91542e9325a8aa1
Author: Evan Zelkowitz <ez...@apache.org>
AuthorDate: Mon Dec 7 20:29:57 2020 -0700

    Tm csv nilcheck (#5358)
    
    * Add map interface existence checks for all system entries. It is possible to have machines that dont return these in which case the existing code will crash
    
    * Add changelog
    
    (cherry picked from commit d628398b393f78211297e5213cebd985dcce677a)
---
 CHANGELOG.md                        |  2 +-
 traffic_monitor/cache/astats_csv.go | 25 +++++++++++++++++--------
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index d3fd9f1..97bf1b6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -136,7 +136,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - Fixed #5274 - CDN in a Box's Traffic Vault image failed to build due to Basho's repo responding with 402 Payment Required. The repo has been removed from the image.
 - #5069 - For LetsEncryptDnsChallengerWatcher in Traffic Router, the cr-config location is configurable instead of only looking at `/opt/traffic_router/db/cr-config.json`
 - #5191 - Error from IMS requests to /federations/all
-
+- Fixed Astats csv issue where it could crash if caches dont return proc data
 
 ### Changed
 - Changed some Traffic Ops Go Client methods to use `DeliveryServiceNullable` inputs and outputs.
diff --git a/traffic_monitor/cache/astats_csv.go b/traffic_monitor/cache/astats_csv.go
index cce0342..d36495d 100644
--- a/traffic_monitor/cache/astats_csv.go
+++ b/traffic_monitor/cache/astats_csv.go
@@ -79,26 +79,35 @@ func astatsCsvParseCsv(cacheName string, data io.Reader) (Statistics, map[string
 	statMap := atsData.Ats
 
 	// Handle system specific values and remove them from the map for precomputing to not have issues
-	if stats.Loadavg, err = LoadavgFromRawLine(statMap["proc.loadavg"].(string)); err != nil {
+	if loadAvg, hadLoadAvg := statMap["proc.loadavg"].(string); !hadLoadAvg {
+		return stats, nil, fmt.Errorf("failed to parse loadavg line for cache '%s': %v", cacheName, "no proc.loadavg in Astats response")
+	} else if stats.Loadavg, err = LoadavgFromRawLine(loadAvg); err != nil {
 		return stats, nil, fmt.Errorf("parsing loadavg for cache '%s': %v", cacheName, err)
 	} else {
 		delete(statMap, "proc.loadavg")
 	}
 
-	if err := stats.AddInterfaceFromRawLine(statMap["proc.net.dev"].(string)); err != nil {
+	if procNetDev, hadProcNetDev := statMap["proc.net.dev"].(string); !hadProcNetDev {
+		return stats, nil, fmt.Errorf("failed to parse interface line for cache '%s': %v", cacheName, "no proc.net.dev in Astats response")
+	} else if err := stats.AddInterfaceFromRawLine(procNetDev); err != nil {
 		return stats, nil, fmt.Errorf("failed to parse interface line for cache '%s': %v", cacheName, err)
 	} else {
 		delete(statMap, "proc.net.dev")
 	}
 
-	if inf, ok := stats.Interfaces[statMap["inf.name"].(string)]; !ok {
+	if infName, hadinf := statMap["inf.name"].(string); !hadinf {
+		return stats, nil, fmt.Errorf("failed to parse inf line for cache '%s': %v", cacheName, "no inf.name in Astats response")
+	} else if inf, err := stats.Interfaces[infName]; !err {
 		return stats, nil, errors.New("/proc/net/dev line didn't match reported interface line")
 	} else {
-		inf.Speed = int64(statMap["inf.speed"].(float64)) //strconv.ParseInt(statMap["inf.speed"].(string), 10, 64)
-		stats.Interfaces[statMap["inf.name"].(string)] = inf
-		delete(statMap, "inf.speed")
-		delete(statMap, "inf.name")
-
+		if infSpeed, hadspeed := statMap["inf.speed"].(float64); !hadspeed {
+			return stats, nil, fmt.Errorf("failed to parse interface speed line for cache '%s': %v", cacheName, "no inf.speed in Astats response")
+		} else {
+			inf.Speed = int64(infSpeed)
+			stats.Interfaces[infName] = inf
+			delete(statMap, "inf.speed")
+			delete(statMap, "inf.name")
+		}
 	}
 
 	// Clean up other non-stats entries