TM should not need to overwrite monitoring snapshot data with CRConfig snapshot data

[rawlinp]

This Improvement request (usability, performance, tech debt, etc.) affects these Traffic Control components:

• Traffic Monitor

Current behavior:

trafficcontrol/traffic_monitor/towrap/towrap.go

Lines 679 to 756 in 9516de3

	// 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.
	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{}
	for name, mon := range crConfig.Monitors {
	// monitorProfile = *mon.Profile
	m := tc.TrafficMonitor{}
	if mon.Port != nil {
	m.Port = *mon.Port
	} else {
	log.Warnf("Creating monitor config: CRConfig monitor %s missing Port field\n", name)
	}
	if mon.IP6 != nil {
	m.IP6 = *mon.IP6
	} else {
	log.Warnf("Creating monitor config: CRConfig monitor %s missing IP6 field\n", name)
	}
	if mon.IP != nil {
	m.IP = *mon.IP
	} else {
	log.Warnf("Creating monitor config: CRConfig monitor %s missing IP field\n", name)
	}
	m.HostName = name
	if mon.FQDN != nil {
	m.FQDN = *mon.FQDN
	} else {
	log.Warnf("Creating monitor config: CRConfig monitor %s missing FQDN field\n", name)
	}
	if mon.Profile != nil {
	m.Profile = *mon.Profile
	} else {
	log.Warnf("Creating monitor config: CRConfig monitor %s missing Profile field\n", name)
	}
	if mon.Location != nil {
	m.Location = *mon.Location
	} else {
	log.Warnf("Creating monitor config: CRConfig monitor %s missing Location field\n", name)
	}
	if mon.ServerStatus != nil {
	m.ServerStatus = string(*mon.ServerStatus)
	} else {
	log.Warnf("Creating monitor config: CRConfig monitor %s missing ServerStatus field\n", name)
	}
	mc.TrafficMonitor[name] = m
	}
	// Dump the "live" monitoring.json DeliveryServices, and populate with the
	// "snapshotted" CRConfig but keep using the monitoring.json thresholds,
	// because they're not in the CRConfig.
	rawDeliveryServices := mc.DeliveryService
	mc.DeliveryService = map[string]tc.TMDeliveryService{}
	for name, _ := range crConfig.DeliveryServices {
	if rawDS, ok := rawDeliveryServices[name]; ok {
	// use the raw DS if it exists, because the CRConfig doesn't have
	// thresholds or statuses
	mc.DeliveryService[name] = rawDS
	} else {
	mc.DeliveryService[name] = tc.TMDeliveryService{
	XMLID: name,
	TotalTPSThreshold: 0,
	ServerStatus: "REPORTED",
	TotalKbpsThreshold: 0,
	}
	}
	}
	return mc, nil
	}

TM first requests the monitoring config snapshot then requests the CRConfig snapshot in order to create its internal TrafficMonitorConfigMap representation. It currently overwrites the Traffic Monitor data in the monitoring snapshot with data from the CRConfig snapshot, most likely due to the monitoring config missing Traffic Monitor IPv4 and IPv6 addresses. It will also fall back to using CRConfig delivery service data if not present in the monitoring snapshot (although this should no longer be an issue as of #5184). Since the monitoring config and CRConfig are now snapshotted together, this is entirely unnecessary.

New behavior:

1. Traffic Ops should populate the ip and ip6 fields in the trafficMonitors array of the monitoring config snapshot instead of leaving them blank:

      {
        "profile": "MY_TM_PROFILE",
        "status": "ONLINE",
        "port": 80,
        "cachegroup": "foo",
        "hostname": "tm1",
        "fqdn": "example.com",
        "ip": "",
        "ip6": ""
      },

2. Traffic Ops should exclude delivery service data from the monitoring config snapshot that does not belong to the given CDN being snapshotted. It currently includes all delivery services across all CDNs.
3. TM should be updated to only overwrite the Traffic Monitor data in its monitoring config snapshot (in the code linked above) with CRConfig data if the monitoring snapshot is still missing IPv4 and IPv6 data. Otherwise, it should never have to overwrite the monitoring config data. Eventually, we can remove that fallback code entirely, once it's assumed that TM config snapshots will always have IP data going forward.

[srijeet0406]

@rawlinp I can take this one if no one from the distriibuted TM group is planning on workng on it.

[rawlinp]

Go for it! This is totally unrelated to distributed TM anyways -- just a little bit of tech debt I noticed along the way.