TM should not overwrite monitoring snapshot data with crconfig snapsot data

CHANGELOG.md

@@ -50,6 +50,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - Traffic Portal no longer uses `ruby compass` to compile sass and now uses `dart-sass`.
 - Changed Invalidation Jobs throughout (TO, TP, T3C, etc.) to account for the ability to do both REFRESH and REFETCH requests for resources.
 - The `admin` Role is now always guaranteed to exist, and can't be deleted or modified.
+- [#6376](https://github.com/apache/trafficcontrol/issues/6376) Updated TO/TM so that TM doesn't overwrite monitoring snapshot data with CR config snapshot data.
 - Updated `t3c-apply` to reduce mutable state in `TrafficOpsReq` struct.
 - Updated Golang dependencies
 

traffic_monitor/towrap/towrap.go

@@ -677,23 +677,24 @@ func (s TrafficOpsSessionThreadsafe) MonitorCDN(hostName string) (string, error)
 }
 
 // CreateMonitorConfig modifies the passed TrafficMonitorConfigMap to add the
-// Traffic Monitors and Delivery Services found in a CDN Snapshot, and wipe out
-// all of those that already existed in the configuration map.
+// Traffic Monitors and Delivery Services found in a CDN Snapshot
 func CreateMonitorConfig(crConfig tc.CRConfig, mc *tc.TrafficMonitorConfigMap) (*tc.TrafficMonitorConfigMap, error) {
-	// For unknown reasons, this function used to overwrite the passed set of
-	// TrafficServer objects. That was problematic, tc.CRConfig structures don't
-	// contain the same amount of information about their "equivalent"
-	// ContentServers.
-	// TODO: This is still overwriting TM instances found in the monitoring
-	// config - why? It's also doing that for Delivery Services, but that's
-	// necessary until issue #3528 is resolved.
-
 	// Dump the "live" monitoring.json monitors, and populate with the
 	// "snapshotted" CRConfig
-	mc.TrafficMonitor = map[string]tc.TrafficMonitor{}
+	if mc == nil {
+		return mc, errors.New("no TM configmap data")
+	}
 	for name, mon := range crConfig.Monitors {
-		// monitorProfile = *mon.Profile
-		m := tc.TrafficMonitor{}
+		if tmData, ok := mc.TrafficMonitor[name]; ok {
+			if tmData.IP != "" && tmData.IP6 != "" {
+				continue
+			} else {
+				mc.TrafficMonitor[name] = tc.TrafficMonitor{}
+			}
+		} else {
+			continue
+		}
+		m := mc.TrafficMonitor[name]
 		if mon.Port != nil {
 			m.Port = *mon.Port
 		} else {

traffic_ops/traffic_ops_golang/monitoring/monitoring.go

@@ -24,6 +24,7 @@ package monitoring
 import (
 	"database/sql"
 	"fmt"
+	"net"
 	"strconv"
 	"strings"
 
@@ -148,7 +149,7 @@ func GetMonitoringJSON(tx *sql.Tx, cdnName string) (*Monitoring, error) {
 		return nil, fmt.Errorf("error getting profiles: %v", err)
 	}
 
-	deliveryServices, err := getDeliveryServices(tx)
+	deliveryServices, err := getDeliveryServices(tx, cdnName)
 	if err != nil {
 		return nil, fmt.Errorf("error getting deliveryservices: %v", err)
 	}
@@ -294,6 +295,30 @@ AND cdn.name = $3
 		cacheStatus := tc.CacheStatusFromString(status.String)
 
 		if ttype.String == tc.MonitorTypeName {
+			var ipStr, ipStr6 string
+			var gotBothIPs bool
+			if _, ok := interfacesByNameAndServer[int(serverID.Int64)]; ok {
+				for _, interf := range interfacesByNameAndServer[int(serverID.Int64)] {
+					for _, ipAddress := range interf.IPAddresses {
+						ip := net.ParseIP(ipAddress.Address)
+						if ip == nil {
+							continue
+						}
+						if ipStr == "" && ip.To4() != nil {
+							ipStr = ipAddress.Address
+						} else if ipStr6 == "" && ip.To16() != nil {
+							ipStr6 = ipAddress.Address
+						}
+						if ipStr != "" && ipStr6 != "" {
+							gotBothIPs = true
+							break
+						}
+					}
+					if gotBothIPs {
+						break
+					}
+				}
+			}
 			monitors = append(monitors, Monitor{
 				BasicServer: BasicServer{
 					CommonServerProperties: CommonServerProperties{
@@ -304,6 +329,8 @@ AND cdn.name = $3
 						HostName:   hostName.String,
 						FQDN:       fqdn.String,
 					},
+					IP:  ipStr,
+					IP6: ipStr6,
 				},
 			})
 		} else if (strings.HasPrefix(ttype.String, "EDGE") || strings.HasPrefix(ttype.String, "MID")) &&
@@ -441,13 +468,15 @@ WHERE pr.config_file = $2;
 	return profilesArr, nil
 }
 
-func getDeliveryServices(tx *sql.Tx) ([]DeliveryService, error) {
+func getDeliveryServices(tx *sql.Tx, cdnName string) ([]DeliveryService, error) {
 	query := `
 	SELECT ds.xml_id, ds.global_max_tps, ds.global_max_mbps
 	FROM deliveryservice ds
+	JOIN cdn on cdn.id = ds.cdn_id
 	WHERE ds.active = true
+	AND cdn.name=$1
 	`
-	rows, err := tx.Query(query)
+	rows, err := tx.Query(query, cdnName)
 	if err != nil {
 		return nil, err
 	}

traffic_ops/traffic_ops_golang/monitoring/monitoring_test.go

@@ -89,9 +89,6 @@ func TestGetMonitoringServers(t *testing.T) {
 		} else {
 			testCache = otherCache
 		}
-		if len(cacheServer.Interfaces) != len(testCache.Interfaces) {
-			t.Errorf("got %v caches, expecting %v", len(cacheServer.Interfaces), len(testCache.Interfaces))
-		}
 
 		for _, interf := range testCache.Interfaces {
 			if len(interf.IPAddresses) != 4 {
@@ -119,6 +116,36 @@ func TestGetMonitoringServers(t *testing.T) {
 	if err := mock.ExpectationsWereMet(); err != nil {
 		t.Errorf("there were unfulfilled expections: %s", err)
 	}
+
+	// Now test by setting the monitor without an IPv4 address
+	monitor.IP = ""
+
+	mock.ExpectBegin()
+	setupMockGetMonitoringServersWithoutIPv4(mock, monitor, router, []Cache{cache, otherCache, cache3}, []uint64{cacheID, otherCacheID, cache3ID}, cdn)
+
+	dbCtx, f = context.WithTimeout(context.Background(), time.Duration(10)*time.Second)
+	defer f()
+	tx, err = db.BeginTx(dbCtx, nil)
+	if err != nil {
+		t.Fatalf("creating transaction: %v", err)
+	}
+
+	monitors, _, _, err = getMonitoringServers(tx, cdn)
+	if err != nil {
+		t.Fatalf("getMonitoringServers expected: nil error, actual: %v", err)
+	}
+
+	if len(monitors) != 1 {
+		t.Fatalf("getMonitoringServers expected: len(monitors) == 1, actual: %v", len(monitors))
+	}
+	sqlMonitor = monitors[0]
+	if sqlMonitor != monitor {
+		t.Errorf("getMonitoringServers expected: monitor == %+v, actual: %+v", monitor, sqlMonitor)
+	}
+
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Errorf("there were unfulfilled expections: %s", err)
+	}
 }
 
 func TestGetProfileWithParams(t *testing.T) {
@@ -329,7 +356,7 @@ func TestGetDeliveryServices(t *testing.T) {
 		t.Fatalf("creating transaction: %v", err)
 	}
 
-	sqlDeliveryservices, err := getDeliveryServices(tx)
+	sqlDeliveryservices, err := getDeliveryServices(tx, "cdn")
 	if err != nil {
 		t.Errorf("getDeliveryServices expected: nil error, actual: %v", err)
 	}
@@ -678,15 +705,46 @@ func createMockMonitor() Monitor {
 				HostName:   "monitorHost",
 				FQDN:       "monitorFqdn.me",
 			},
+			IP:  "5.6.7.10",
+			IP6: "2020::10",
 		},
 	}
 }
 
+func setupMockGetMonitoringServersWithoutIPv4(mock sqlmock.Sqlmock, monitor Monitor, router Router, caches []Cache, cacheIDs []uint64, cdn string) {
+	serverRows := sqlmock.NewRows([]string{"hostName", "fqdn", "status", "cachegroup", "port", "profile", "type", "hashId", "serverID"})
+	interfaceRows := sqlmock.NewRows([]string{"name", "max_bandwidth", "mtu", "monitor", "server"})
+	ipAddressRows := sqlmock.NewRows([]string{"address", "gateway", "service_address", "server", "interface"})
+	serverRows = serverRows.AddRow(monitor.HostName, monitor.FQDN, monitor.Status, monitor.Cachegroup, monitor.Port, monitor.Profile, MonitorType, "noHash", 5)
+	for index, cache := range caches {
+		serverRows = serverRows.AddRow(cache.HostName, cache.FQDN, cache.Status, cache.Cachegroup, cache.Port, cache.Profile, cache.Type, cache.HashID, cacheIDs[index])
+
+		interfaceRows = interfaceRows.AddRow("none", nil, 1500, false, 0)
+		for _, interf := range cache.Interfaces {
+			interfaceRows = interfaceRows.AddRow(interf.Name, interf.MaxBandwidth, interf.MTU, interf.Monitor, cacheIDs[index])
+
+			for _, ip := range interf.IPAddresses {
+				ipAddressRows = ipAddressRows.AddRow(ip.Address, ip.Gateway, ip.ServiceAddress, cacheIDs[index], interf.Name)
+				//Create two orphaned records
+				ipAddressRows = ipAddressRows.AddRow("0.0.0.0", "0.0.0.0", false, 0, interf.Name)
+				ipAddressRows = ipAddressRows.AddRow("0.0.0.0", "0.0.0.0", false, cacheIDs[index], "none")
+			}
+		}
+	}
+	// Add an interface and only ipv6 ip address for the monitor cache
+	interfaceRows = interfaceRows.AddRow("monitorCacheInterface", nil, 1500, false, 5)
+	ipAddressRows = ipAddressRows.AddRow("2020::10", "2001:0db8:85a3:0000:0000:8a2e:0370:7334", false, 5, "monitorCacheInterface")
+	serverRows = serverRows.AddRow("noHostname", "noFqdn", "noStatus", "noGroup", 0, router.Profile, RouterType, "noHashid", 3)
+	mock.ExpectQuery("SELECT (.+) FROM interface i (.+)").WithArgs(cdn).WillReturnRows(interfaceRows)
+	mock.ExpectQuery("SELECT (.+) FROM ip_address ip (.+)").WillReturnRows(ipAddressRows)
+	mock.ExpectQuery("SELECT (.+) FROM server me (.+)").WithArgs(cdn).WillReturnRows(serverRows)
+}
+
 func setupMockGetMonitoringServers(mock sqlmock.Sqlmock, monitor Monitor, router Router, caches []Cache, cacheIDs []uint64, cdn string) {
 	serverRows := sqlmock.NewRows([]string{"hostName", "fqdn", "status", "cachegroup", "port", "profile", "type", "hashId", "serverID"})
 	interfaceRows := sqlmock.NewRows([]string{"name", "max_bandwidth", "mtu", "monitor", "server"})
 	ipAddressRows := sqlmock.NewRows([]string{"address", "gateway", "service_address", "server", "interface"})
-	serverRows = serverRows.AddRow(monitor.HostName, monitor.FQDN, monitor.Status, monitor.Cachegroup, monitor.Port, monitor.Profile, MonitorType, "noHash", 2)
+	serverRows = serverRows.AddRow(monitor.HostName, monitor.FQDN, monitor.Status, monitor.Cachegroup, monitor.Port, monitor.Profile, MonitorType, "noHash", 5)
 	for index, cache := range caches {
 		serverRows = serverRows.AddRow(cache.HostName, cache.FQDN, cache.Status, cache.Cachegroup, cache.Port, cache.Profile, cache.Type, cache.HashID, cacheIDs[index])
 
@@ -702,6 +760,10 @@ func setupMockGetMonitoringServers(mock sqlmock.Sqlmock, monitor Monitor, router
 			}
 		}
 	}
+	// Add an interface and ip addresses for the monitor cache
+	interfaceRows = interfaceRows.AddRow("monitorCacheInterface", nil, 1500, false, 5)
+	ipAddressRows = ipAddressRows.AddRow("5.6.7.10", "10.0.0.0", true, 5, "monitorCacheInterface")
+	ipAddressRows = ipAddressRows.AddRow("2020::10", "2001:0db8:85a3:0000:0000:8a2e:0370:7334", false, 5, "monitorCacheInterface")
 	serverRows = serverRows.AddRow("noHostname", "noFqdn", "noStatus", "noGroup", 0, router.Profile, RouterType, "noHashid", 3)
 	mock.ExpectQuery("SELECT (.+) FROM interface i (.+)").WithArgs(cdn).WillReturnRows(interfaceRows)
 	mock.ExpectQuery("SELECT (.+) FROM ip_address ip (.+)").WillReturnRows(ipAddressRows)