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

## This Improvement request (usability, performance, tech debt, etc.) affects these Traffic Control components:

- Traffic Monitor

## Current behavior:

https://github.com/apache/trafficcontrol/blob/9516de39fffa3ddda0346720ef98edabc8efa07e/traffic_monitor/towrap/towrap.go#L679-L756
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 https://github.com/apache/trafficcontrol/pull/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.

	// 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
	}

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

Current behavior:

New behavior:

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

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

Description

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

Current behavior:

New behavior:

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions