[4.1.x backport] Remove unnecessary JOIN (#5184)

CHANGELOG.md

@@ -11,6 +11,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 ### Fixed
 - Fixed #5188 - DSR (delivery service request) incorrectly marked as complete and error message not displaying when DSR fulfilled and DS update fails in Traffic Portal. [Related Github issues](https://github.com/apache/trafficcontrol/issues/5188)
 - Fixed #5006 - Traffic Ops now generates the Monitoring on-the-fly if the snapshot doesn't exist, and logs an error. This fixes upgrading to 4.x to not break the CDN until a Snapshot is done.
+- Fixed #5180 - Global Max Mbps and Tps is not send to TM
+- Fixed #3528 - Fix Traffic Ops monitoring.json missing DeliveryServices
 - Fixed #5074 - Traffic Monitor logging "CreateStats not adding availability data for server: not found in DeliveryServices" for MID cachces
 - Fixed an issue that causes Traffic Router to mistakenly route to caches that had recently been set from ADMIN_DOWN to OFFLINE
 - Traffic Ops Ort: Disabled ntpd verification (ntpd is deprecated in CentOS)

traffic_ops/traffic_ops_golang/monitoring/monitoring.go

@@ -1,3 +1,5 @@
+// Package monitoring contains handlers and supporting logic for the
+// /cdns/{{CDN Name}}/configs/monitoring Traffic Ops API endpoint.
 package monitoring
 
 /*
@@ -130,7 +132,7 @@ func GetMonitoringJSON(tx *sql.Tx, cdnName string) (*Monitoring, error) {
 		return nil, fmt.Errorf("error getting profiles: %v", err)
 	}
 
-	deliveryServices, err := getDeliveryServices(tx, routers)
+	deliveryServices, err := getDeliveryServices(tx)
 	if err != nil {
 		return nil, fmt.Errorf("error getting deliveryservices: %v", err)
 	}
@@ -337,20 +339,13 @@ WHERE pr.config_file = $2;
 	return profilesArr, nil
 }
 
-func getDeliveryServices(tx *sql.Tx, routers []Router) ([]DeliveryService, error) {
-	profileNames := []string{}
-	for _, router := range routers {
-		profileNames = append(profileNames, router.Profile)
-	}
-
+func getDeliveryServices(tx *sql.Tx) ([]DeliveryService, error) {
 	query := `
-SELECT ds.xml_id, ds.global_max_tps, ds.global_max_mbps
-FROM deliveryservice ds
-JOIN profile profile ON profile.id = ds.profile
-WHERE profile.name = ANY($1)
-AND ds.active = true
-`
-	rows, err := tx.Query(query, pq.Array(profileNames))
+	SELECT ds.xml_id, ds.global_max_tps, ds.global_max_mbps
+	FROM deliveryservice ds
+	WHERE ds.active = true
+	`
+	rows, err := tx.Query(query)
 	if err != nil {
 		return nil, err
 	}

traffic_ops/traffic_ops_golang/monitoring/monitoring_test.go

@@ -151,7 +151,8 @@ func TestGetCachegroups(t *testing.T) {
 
 	mock.ExpectQuery("SELECT").WithArgs(cdn).WillReturnRows(rows)
 
-	dbCtx, _ := context.WithTimeout(context.TODO(), time.Duration(10)*time.Second)
+	dbCtx, f := context.WithTimeout(context.TODO(), time.Duration(10)*time.Second)
+	defer f()
 	tx, err := db.BeginTx(dbCtx, nil)
 	if err != nil {
 		t.Fatalf("creating transaction: %v", err)
@@ -325,7 +326,8 @@ func TestGetProfiles(t *testing.T) {
 
 	mock.ExpectQuery("SELECT").WithArgs(pq.Array(profileNames), CacheMonitorConfigFile).WillReturnRows(rows)
 
-	dbCtx, _ := context.WithTimeout(context.TODO(), time.Duration(10)*time.Second)
+	dbCtx, f := context.WithTimeout(context.TODO(), time.Duration(10)*time.Second)
+	defer f()
 	tx, err := db.BeginTx(dbCtx, nil)
 	if err != nil {
 		t.Fatalf("creating transaction: %v", err)
@@ -373,11 +375,6 @@ func TestGetDeliveryServices(t *testing.T) {
 	db := sqlx.NewDb(mockDB, "sqlmock")
 	defer db.Close()
 
-	router := Router{
-		Type:    RouterType,
-		Profile: "routerProfile",
-	}
-
 	deliveryservice := DeliveryService{
 		XMLID:              "myDsid",
 		TotalTPSThreshold:  42.42,
@@ -386,31 +383,29 @@ func TestGetDeliveryServices(t *testing.T) {
 	}
 
 	deliveryservices := []DeliveryService{deliveryservice}
-	routers := []Router{router}
 
 	mock.ExpectBegin()
 	rows := sqlmock.NewRows([]string{"xml_id", "global_max_tps", "global_max_mbps"})
 	for _, deliveryservice := range deliveryservices {
 		rows = rows.AddRow(deliveryservice.XMLID, deliveryservice.TotalTPSThreshold, deliveryservice.TotalKBPSThreshold/KilobitsPerMegabit)
 	}
 
-	profileNames := []string{router.Profile}
-
-	mock.ExpectQuery("SELECT").WithArgs(pq.Array(profileNames)).WillReturnRows(rows)
+	mock.ExpectQuery("SELECT").WillReturnRows(rows)
 
-	dbCtx, _ := context.WithTimeout(context.TODO(), time.Duration(10)*time.Second)
+	dbCtx, f := context.WithTimeout(context.TODO(), time.Duration(10)*time.Second)
+	defer f()
 	tx, err := db.BeginTx(dbCtx, nil)
 	if err != nil {
 		t.Fatalf("creating transaction: %v", err)
 	}
 
-	sqlDeliveryservices, err := getDeliveryServices(tx, routers)
+	sqlDeliveryservices, err := getDeliveryServices(tx)
 	if err != nil {
-		t.Errorf("getProfiles expected: nil error, actual: %v", err)
+		t.Errorf("getDeliveryServices expected: nil error, actual: %v", err)
 	}
 
 	if len(deliveryservices) != len(sqlDeliveryservices) {
-		t.Errorf("getProfiles expected: %+v actual: %+v", deliveryservices, sqlDeliveryservices)
+		t.Fatalf("getDeliveryServices expected: %+v actual: %+v", deliveryservices, sqlDeliveryservices)
 	}
 
 	for i, sqlDeliveryservice := range sqlDeliveryservices {
@@ -448,7 +443,8 @@ func TestGetConfig(t *testing.T) {
 
 	mock.ExpectQuery("SELECT").WillReturnRows(rows)
 
-	dbCtx, _ := context.WithTimeout(context.TODO(), time.Duration(10)*time.Second)
+	dbCtx, f := context.WithTimeout(context.TODO(), time.Duration(10)*time.Second)
+	defer f()
 	tx, err := db.BeginTx(dbCtx, nil)
 	if err != nil {
 		t.Fatalf("creating transaction: %v", err)
@@ -610,10 +606,6 @@ func TestGetMonitoringJSON(t *testing.T) {
 		//
 		// getDeliveryServices
 		//
-		router := Router{
-			Type:    RouterType,
-			Profile: "routerProfile",
-		}
 
 		deliveryservice := DeliveryService{
 			XMLID:              "myDsid",
@@ -630,9 +622,7 @@ func TestGetMonitoringJSON(t *testing.T) {
 			rows = rows.AddRow(deliveryservice.XMLID, deliveryservice.TotalTPSThreshold, deliveryservice.TotalKBPSThreshold/KilobitsPerMegabit)
 		}
 
-		profileNames := []string{router.Profile}
-
-		mock.ExpectQuery("SELECT").WithArgs(pq.Array(profileNames)).WillReturnRows(rows)
+		mock.ExpectQuery("SELECT").WillReturnRows(rows)
 		resp.Response.DeliveryServices = deliveryservices
 	}
 	{
@@ -653,15 +643,16 @@ func TestGetMonitoringJSON(t *testing.T) {
 		resp.Response.Config = config
 	}
 
-	dbCtx, _ := context.WithTimeout(context.TODO(), time.Duration(10)*time.Second)
+	dbCtx, f := context.WithTimeout(context.TODO(), time.Duration(10)*time.Second)
+	defer f()
 	tx, err := db.BeginTx(dbCtx, nil)
 	if err != nil {
 		t.Fatalf("creating transaction: %v", err)
 	}
 
 	sqlResp, err := GetMonitoringJSON(tx, cdn)
 	if err != nil {
-		t.Errorf("GetMonitoringJSON expected: nil error, actual: %v", err)
+		t.Fatalf("GetMonitoringJSON expected: nil error, actual: %v", err)
 	}
 
 	resp.Response.TrafficServers = sortCaches(resp.Response.TrafficServers)