From 56c60c458bcc7c7f5b0456cceab18028f466e09b Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Wed, 21 Oct 2020 16:07:40 -0600 Subject: [PATCH 1/3] Remove unnecessary JOIN --- .../monitoring/monitoring.go | 43 ++++++++----------- 1 file changed, 19 insertions(+), 24 deletions(-) diff --git a/traffic_ops/traffic_ops_golang/monitoring/monitoring.go b/traffic_ops/traffic_ops_golang/monitoring/monitoring.go index 6cf5b54c78..d118218862 100644 --- a/traffic_ops/traffic_ops_golang/monitoring/monitoring.go +++ b/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 /* @@ -27,8 +29,8 @@ import ( "strings" "github.com/apache/trafficcontrol/lib/go-log" - "github.com/apache/trafficcontrol/lib/go-tc" + "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api" "github.com/lib/pq" @@ -159,7 +161,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) } @@ -201,25 +203,25 @@ WHERE cdn.name = $1 ` interfacesQuery := ` -SELECT +SELECT i.name, i.max_bandwidth, i.mtu, i.monitor, i.server FROM interface i WHERE i.server in ( - SELECT - s.id - FROM "server" s - JOIN cdn c - on c.id = s.cdn_id + SELECT + s.id + FROM "server" s + JOIN cdn c + on c.id = s.cdn_id WHERE c.name = $1 )` ipAddressQuery := ` -SELECT +SELECT ip.address, ip.gateway, ip.service_address, ip.server, ip.interface FROM ip_address ip -JOIN server s +JOIN server s ON s.id = ip.server -JOIN cdn cdn +JOIN cdn cdn ON cdn.id = s.cdn_id WHERE ip.server = ANY($1) AND ip.interface = ANY($2) @@ -452,20 +454,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 } From e89e479704cc85408950530f491edd18035a9f20 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Wed, 21 Oct 2020 16:30:15 -0600 Subject: [PATCH 2/3] Fix tests, also some context leaks --- .../monitoring/monitoring_test.go | 52 +++++++++---------- 1 file changed, 24 insertions(+), 28 deletions(-) diff --git a/traffic_ops/traffic_ops_golang/monitoring/monitoring_test.go b/traffic_ops/traffic_ops_golang/monitoring/monitoring_test.go index fe758142fb..128af3be82 100644 --- a/traffic_ops/traffic_ops_golang/monitoring/monitoring_test.go +++ b/traffic_ops/traffic_ops_golang/monitoring/monitoring_test.go @@ -8,14 +8,14 @@ import ( "testing" "time" - "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api" - "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/parameter" - "github.com/lib/pq" - "github.com/apache/trafficcontrol/lib/go-tc" "github.com/apache/trafficcontrol/lib/go-util" + "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api" + "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/parameter" + "github.com/jmoiron/sqlx" + "github.com/lib/pq" "gopkg.in/DATA-DOG/go-sqlmock.v1" ) @@ -66,7 +66,8 @@ func TestGetMonitoringServers(t *testing.T) { mock.ExpectBegin() setupMockGetMonitoringServers(mock, monitor, router, []Cache{cache, otherCache, cache3}, []uint64{cacheID, otherCacheID, cache3ID}, cdn) - dbCtx, _ := context.WithTimeout(context.Background(), time.Duration(10)*time.Second) + 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) @@ -133,7 +134,8 @@ func TestGetProfileWithParams(t *testing.T) { mockGetParams(mock, expected, cdn) mock.ExpectCommit() - 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) @@ -182,7 +184,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) @@ -253,7 +256,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) @@ -301,11 +305,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, @@ -314,7 +313,6 @@ 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"}) @@ -322,23 +320,22 @@ func TestGetDeliveryServices(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) - 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 { @@ -376,7 +373,8 @@ func TestGetConfig(t *testing.T) { mock.ExpectQuery("SELECT").WithArgs(tc.MonitorProfilePrefix+"%%", MonitorConfigFile, "mycdn").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) @@ -482,7 +480,6 @@ func TestGetMonitoringJSON(t *testing.T) { // // getDeliveryServices // - router := createMockRouter() deliveryservice := DeliveryService{ XMLID: "myDsid", @@ -499,9 +496,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 } { @@ -522,7 +517,8 @@ 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) @@ -530,7 +526,7 @@ func TestGetMonitoringJSON(t *testing.T) { 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) } for _, cache := range resp.Response.TrafficServers { cache.Interfaces = sortInterfaces(cache.Interfaces) From 2c11cce581e412b2d9339dd5d32349e0c2d6717a Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Wed, 21 Oct 2020 16:32:19 -0600 Subject: [PATCH 3/3] Add to CHANGELOG --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ca4f9f10d..ff299ea1dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -109,6 +109,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - Fixed #5102 - Python client scripts fail silently on authentication failures - Fixed #5103 - Python client scripts crash on connection errors - Fixed matching of wildcards in subjectAlternateNames when loading TLS certificates +- Fixed #5180 - Global Max Mbps and Tps is not send to TM +- Fixed #3528 - Fix Traffic Ops monitoring.json missing DeliveryServices ### Changed - Changed some Traffic Ops Go Client methods to use `DeliveryServiceNullable` inputs and outputs.