[trafficcontrol] 15/21: Make only CDN specific queries for FQDNs (#6288)

[zr...@apache.org]

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

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

commit f2885e88907789d353f490854a25d52a70523c54
Author: Srijeet Chatterjee <30...@users.noreply.github.com>
AuthorDate: Tue Oct 26 08:29:41 2021 -0600

    Make only CDN specific queries for FQDNs (#6288)
    
    * Make only CDN specific queries for FQDNs
    
    * fix ISE
    
    (cherry picked from commit 82e8db6457d8257eb865ae8cab63fa21243352e4)
---
 CHANGELOG.md                                       |   1 +
 .../traffic_ops_golang/crstats/dsrouting.go        |   4 +-
 traffic_ops/traffic_ops_golang/crstats/routing.go  |  12 ++-
 .../traffic_ops_golang/crstats/routing_test.go     | 105 +++++++++++++++++++++
 .../traffic_ops_golang/dbhelpers/db_helpers.go     |  13 +--
 traffic_ops/traffic_ops_golang/server/servers.go   |   2 +-
 6 files changed, 126 insertions(+), 11 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6fda29d..3aabc45 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,6 +8,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - [#6125](https://github.com/apache/trafficcontrol/issues/6125) - Fix `/cdns/{name}/federations?id=#` to search for CDN.
 - [#6283](https://github.com/apache/trafficcontrol/issues/6283) - The Traffic Ops Postinstall script will work in CentOS 7, even if Python 3 is installed
 - [#5373](https://github.com/apache/trafficcontrol/issues/5373) - Traffic Monitor logs not consistent
+- [#6197](https://github.com/apache/trafficcontrol/issues/6197) - TO `/deliveryservices/:id/routing` makes requests to all TRs instead of by CDN.
 
 ### Changed
 - [#5927](https://github.com/apache/trafficcontrol/issues/5927) Updated CDN-in-a-Box to not run a Riak container by default but instead only run it if the optional flag is provided.
diff --git a/traffic_ops/traffic_ops_golang/crstats/dsrouting.go b/traffic_ops/traffic_ops_golang/crstats/dsrouting.go
index c7ac010..55c9454 100644
--- a/traffic_ops/traffic_ops_golang/crstats/dsrouting.go
+++ b/traffic_ops/traffic_ops_golang/crstats/dsrouting.go
@@ -53,7 +53,7 @@ func GetDSRouting(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	dsType, exists, err := dbhelpers.GetDeliveryServiceType(dsID, tx)
+	dsType, cdnName, exists, err := dbhelpers.GetDeliveryServiceTypeAndCDNName(dsID, tx)
 	if err != nil {
 		api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, fmt.Errorf("getting delivery service type: %v", err))
 		return
@@ -85,7 +85,7 @@ func GetDSRouting(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	routers, err := getCDNRouterFQDNs(tx, nil)
+	routers, err := getCDNRouterFQDNs(tx, &cdnName)
 	if err != nil {
 		api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, errors.New("getting monitors: "+err.Error()))
 		return
diff --git a/traffic_ops/traffic_ops_golang/crstats/routing.go b/traffic_ops/traffic_ops_golang/crstats/routing.go
index bd720c0..20e9a99 100644
--- a/traffic_ops/traffic_ops_golang/crstats/routing.go
+++ b/traffic_ops/traffic_ops_golang/crstats/routing.go
@@ -207,7 +207,8 @@ func getCRSStats(respond chan<- RouterResp, wg *sync.WaitGroup, routerFQDN, cdn
 
 // getCDNRouterFQDNs returns an FQDN, including port, of an online router for each CDN, for each router. If a CDN has no online routers, that CDN will not have an entry in the map. The port returned is the API port.
 func getCDNRouterFQDNs(tx *sql.Tx, requiredCDN *string) (map[tc.CDNName][]string, error) {
-	rows, err := tx.Query(`
+	var args interface{}
+	query := `
 SELECT s.host_name, s.domain_name, max(pa.value) as port, c.name as cdn
 FROM server as s
 JOIN type as t ON s.type = t.id
@@ -218,8 +219,15 @@ JOIN profile_parameter as pp ON pp.profile = pr.id
 LEFT JOIN parameter as pa ON (pp.parameter = pa.id AND pa.name = 'api.port' AND pa.config_file = 'server.xml')
 WHERE t.name = '` + tc.RouterTypeName + `'
 AND st.name = '` + RouterOnlineStatus + `'
+`
+	if requiredCDN != nil {
+		query += `AND c.name = $1`
+		args = *requiredCDN
+	}
+	query += `
 GROUP BY s.host_name, s.domain_name, c.name
-`)
+`
+	rows, err := tx.Query(query, args)
 	if err != nil {
 		return nil, errors.New("querying routers: " + err.Error())
 	}
diff --git a/traffic_ops/traffic_ops_golang/crstats/routing_test.go b/traffic_ops/traffic_ops_golang/crstats/routing_test.go
new file mode 100644
index 0000000..7329516
--- /dev/null
+++ b/traffic_ops/traffic_ops_golang/crstats/routing_test.go
@@ -0,0 +1,105 @@
+package crstats
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/apache/trafficcontrol/lib/go-util"
+
+	"gopkg.in/DATA-DOG/go-sqlmock.v1"
+)
+
+func TestGetCDNRouterFQDNs(t *testing.T) {
+	db, mock, err := sqlmock.New()
+	if err != nil {
+		t.Fatalf("an error '%s' was not expected when opening a stub database connection", err)
+	}
+
+	mock.ExpectBegin()
+	rows := sqlmock.NewRows([]string{
+		"host_name",
+		"domain_name",
+		"port",
+		"cdn"})
+
+	rows.AddRow("host1", "test", 2500, "ott")
+	mock.ExpectQuery("SELECT").WithArgs("ott").WillReturnRows(rows)
+	mock.ExpectCommit()
+
+	dbCtx, cancelTx := context.WithTimeout(context.TODO(), 10*time.Second)
+	defer cancelTx()
+	tx, err := db.BeginTx(dbCtx, nil)
+	if err != nil {
+		t.Fatalf("creating transaction: %v", err)
+	}
+	result, err := getCDNRouterFQDNs(tx, util.StrPtr("ott"))
+	if err != nil {
+		t.Fatalf("error Getting CDN router FQDN: %v", err)
+	}
+	if len(result) != 1 {
+		t.Fatalf("expected to receive one item in the response, but got %d", len(result))
+	}
+	if _, ok := result["ott"]; !ok {
+		t.Fatal("expected to get an item with 'ott' key, but got nothing")
+	}
+}
+
+func TestGetCDNRouterFQDNsWithoutCDN(t *testing.T) {
+	db, mock, err := sqlmock.New()
+	if err != nil {
+		t.Fatalf("an error '%s' was not expected when opening a stub database connection", err)
+	}
+
+	mock.ExpectBegin()
+	rows := sqlmock.NewRows([]string{
+		"host_name",
+		"domain_name",
+		"port",
+		"cdn"})
+
+	rows.AddRow("host1", "test", 2500, "ott")
+	rows.AddRow("host2", "test2", 3500, "newCDN")
+	mock.ExpectQuery("SELECT").WithArgs(nil).WillReturnRows(rows)
+	mock.ExpectCommit()
+
+	dbCtx, cancelTx := context.WithTimeout(context.TODO(), 10*time.Second)
+	defer cancelTx()
+	tx, err := db.BeginTx(dbCtx, nil)
+	if err != nil {
+		t.Fatal("creating transaction: ", err)
+	}
+
+	result, err := getCDNRouterFQDNs(tx, nil)
+	if err != nil {
+		t.Fatalf("error Getting CDN router FQDN: %v", err)
+	}
+	if len(result) != 2 {
+		t.Fatalf("expected to receive two items in the response, but got %d", len(result))
+	}
+	if _, ok := result["ott"]; !ok {
+		t.Fatal("expected to get an item with 'ott' key, but got nothing")
+	}
+	if _, ok := result["newCDN"]; !ok {
+		t.Fatal("expected to get an item with 'newCDN' key, but got nothing")
+	}
+}
diff --git a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
index f7ff623..89f5c6b 100644
--- a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
+++ b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
@@ -1358,16 +1358,17 @@ func CachegroupParameterAssociationExists(id int, cachegroup int, tx *sql.Tx) (b
 	return count > 0, nil
 }
 
-// GetDeliveryServiceType returns the type of the deliveryservice.
-func GetDeliveryServiceType(dsID int, tx *sql.Tx) (tc.DSType, bool, error) {
+// GetDeliveryServiceTypeAndCDNName returns the type and the CDN name of the deliveryservice.
+func GetDeliveryServiceTypeAndCDNName(dsID int, tx *sql.Tx) (tc.DSType, string, bool, error) {
 	var dsType tc.DSType
-	if err := tx.QueryRow(`SELECT t.name FROM deliveryservice as ds JOIN type t ON ds.type = t.id WHERE ds.id=$1`, dsID).Scan(&dsType); err != nil {
+	var cdnName string
+	if err := tx.QueryRow(`SELECT t.name, c.name as cdn FROM deliveryservice as ds JOIN type t ON ds.type = t.id JOIN cdn c ON c.id = ds.cdn_id WHERE ds.id=$1`, dsID).Scan(&dsType, &cdnName); err != nil {
 		if err == sql.ErrNoRows {
-			return tc.DSTypeInvalid, false, nil
+			return tc.DSTypeInvalid, cdnName, false, nil
 		}
-		return tc.DSTypeInvalid, false, errors.New("querying type from delivery service: " + err.Error())
+		return tc.DSTypeInvalid, cdnName, false, errors.New("querying type from delivery service: " + err.Error())
 	}
-	return dsType, true, nil
+	return dsType, cdnName, true, nil
 }
 
 // GetDeliveryServiceTypeAndTopology returns the type of the deliveryservice and the name of its topology.
diff --git a/traffic_ops/traffic_ops_golang/server/servers.go b/traffic_ops/traffic_ops_golang/server/servers.go
index 133e5c8..e87c0fe 100644
--- a/traffic_ops/traffic_ops_golang/server/servers.go
+++ b/traffic_ops/traffic_ops_golang/server/servers.go
@@ -982,7 +982,7 @@ func getServers(h http.Header, params map[string]string, tx *sqlx.Tx, user *auth
 		queryAddition = fmt.Sprintf(deliveryServiceServersJoin, joinSubQuery)
 
 		// depending on ds type, also need to add mids
-		dsType, _, err := dbhelpers.GetDeliveryServiceType(dsID, tx.Tx)
+		dsType, _, _, err := dbhelpers.GetDeliveryServiceTypeAndCDNName(dsID, tx.Tx)
 		if err != nil {
 			return nil, 0, nil, err, http.StatusInternalServerError, nil
 		}