You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by zr...@apache.org on 2020/12/22 18:06:00 UTC
[trafficcontrol] branch master updated: Topology based DS's + reqd
capabilities + ORG servers should show ORG servers,
alongwith the other servers when queried for the linked servers (#5389)
This is an automated email from the ASF dual-hosted git repository.
zrhoffman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git
The following commit(s) were added to refs/heads/master by this push:
new bef0008 Topology based DS's + reqd capabilities + ORG servers should show ORG servers, alongwith the other servers when queried for the linked servers (#5389)
bef0008 is described below
commit bef00082a6b0d06ca1dc3d25996feff9acd3e362
Author: Srijeet Chatterjee <30...@users.noreply.github.com>
AuthorDate: Tue Dec 22 10:57:21 2020 -0700
Topology based DS's + reqd capabilities + ORG servers should show ORG servers, alongwith the other servers when queried for the linked servers (#5389)
* wip
* wip
* modified query
* adding test
* Adding new test
* adding test in the right file
* refactoring
* changing error msg
* Adding changelog
* code review fixes
* code review fixes
---
CHANGELOG.md | 3 +-
.../deliveryservices_required_capabilities_test.go | 36 ++++++++++++++++
traffic_ops/traffic_ops_golang/server/servers.go | 48 +++++++++++++++-------
.../traffic_ops_golang/server/servers_test.go | 4 ++
4 files changed, 75 insertions(+), 16 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index b7ab9ab..49f073d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,12 +7,13 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
### Added
- Traffic Ops: added a feature so that the user can specify `maxRequestHeaderBytes` on a per delivery service basis
- Traffic Router: log warnings when requests to Traffic Monitor return a 503 status code
-- #5344 - Add a page that addresses migrating from Traffic Ops API v1 for each endpoint
+- [#5344](https://github.com/apache/trafficcontrol/issues/5344) - Add a page that addresses migrating from Traffic Ops API v1 for each endpoint
- [#5296](https://github.com/apache/trafficcontrol/issues/5296) - Fixed a bug where users couldn't update any regex in Traffic Ops/ Traffic Portal
- Added API endpoints for ACME accounts
- Traffic Ops: Added validation to ensure that the cachegroups of a delivery services' assigned ORG servers are present in the topology
### Fixed
+- [#5380](https://github.com/apache/trafficcontrol/issues/5380) - Show the correct servers (including ORGs) when a topology based DS with required capabilities + ORG servers is queried for the assigned servers
- [#5195](https://github.com/apache/trafficcontrol/issues/5195) - Correctly show CDN ID in Changelog during Snap
- Fixed Traffic Router logging unnecessary warnings for IPv6-only caches
- Fixed parent.config generation for topology-based delivery services (inline comments not supported)
diff --git a/traffic_ops/testing/api/v3/deliveryservices_required_capabilities_test.go b/traffic_ops/testing/api/v3/deliveryservices_required_capabilities_test.go
index ed9ef4b..6b09461 100644
--- a/traffic_ops/testing/api/v3/deliveryservices_required_capabilities_test.go
+++ b/traffic_ops/testing/api/v3/deliveryservices_required_capabilities_test.go
@@ -19,6 +19,7 @@ import (
"fmt"
"net/http"
"net/url"
+ "strconv"
"strings"
"testing"
"time"
@@ -45,9 +46,44 @@ func TestDeliveryServicesRequiredCapabilities(t *testing.T) {
func TestTopologyBasedDeliveryServicesRequiredCapabilities(t *testing.T) {
WithObjs(t, []TCObj{CDNs, Types, Tenants, Users, Parameters, Profiles, Statuses, Divisions, Regions, PhysLocations, CacheGroups, Servers, ServerCapabilities, ServerServerCapabilitiesForTopologies, Topologies, DeliveryServices, TopologyBasedDeliveryServiceRequiredCapabilities}, func() {
GetTestDeliveryServicesRequiredCapabilities(t)
+ OriginAssignTopologyBasedDeliveryServiceWithRequiredCapabilities(t)
})
}
+func OriginAssignTopologyBasedDeliveryServiceWithRequiredCapabilities(t *testing.T) {
+ resp, _, err := TOSession.GetDeliveryServiceByXMLIDNullableWithHdr("ds-top-req-cap2", nil)
+ if err != nil {
+ t.Errorf("getting delivery service by xml ID: %v", err.Error())
+ }
+ if len(resp) != 1 {
+ t.Fatalf("expected to get only one delivery service in the response, but got %d", len(resp))
+ }
+ if resp[0].ID == nil {
+ t.Fatalf("no ID in the resulting delivery service")
+ }
+ dsID := *resp[0].ID
+ params := url.Values{}
+ _, _, err = TOSession.AssignServersToDeliveryService([]string{"denver-mso-org-01"}, "ds-top-req-cap2")
+ if err != nil {
+ t.Errorf("assigning ORG server to ds-top delivery service: %v", err.Error())
+ }
+ params.Add("dsId", strconv.Itoa(dsID))
+ params.Add("type", tc.OriginTypeName)
+ responseServers, _, err := TOSession.GetServersWithHdr(¶ms, nil)
+ if err != nil {
+ t.Fatalf("getting servers for ds-top-req-cap2 delivery service: %v", err.Error())
+ }
+ if len(responseServers.Response) != 1 {
+ t.Fatalf("expected just one ORG server in the response, but got %d", len(responseServers.Response))
+ }
+ if responseServers.Response[0].HostName == nil {
+ t.Fatal("expected a valid host name for the resulting ORG server, but got nothing")
+ }
+ if *responseServers.Response[0].HostName != "denver-mso-org-01" {
+ t.Errorf("expected host name of the resulting ORG server to be %v, but got %v", "denver-mso-org-01", *responseServers.Response[0].HostName)
+ }
+}
+
func GetTestDeliveryServicesRequiredCapabilitiesIMSAfterChange(t *testing.T, header http.Header) {
data := testData.DeliveryServicesRequiredCapabilities
ds1 := helperGetDeliveryServiceID(t, data[0].XMLID)
diff --git a/traffic_ops/traffic_ops_golang/server/servers.go b/traffic_ops/traffic_ops_golang/server/servers.go
index 9ed4df1..f2d8a65 100644
--- a/traffic_ops/traffic_ops_golang/server/servers.go
+++ b/traffic_ops/traffic_ops_golang/server/servers.go
@@ -399,6 +399,12 @@ RETURNING
upd_pending
`
+const originServerQuery = `
+JOIN deliveryservice_server dsorg
+ON dsorg.server = s.id
+WHERE t.name = '` + tc.OriginTypeName + `'
+AND dsorg.deliveryservice=:dsId
+`
const deleteServerQuery = `DELETE FROM server WHERE id=$1`
const deleteInterfacesQuery = `DELETE FROM interface WHERE server=$1`
const deleteIPsQuery = `DELETE FROM ip_address WHERE server = $1`
@@ -767,6 +773,19 @@ JOIN type t ON s.type = t.id ` +
select max(last_updated) as t from last_deleted l where l.table_name='server') as res`
}
+func getServerCount(tx *sqlx.Tx, query string, queryValues map[string]interface{}) (uint64, error) {
+ var serverCount uint64
+ ns, err := tx.PrepareNamed(query)
+ if err != nil {
+ return 0, fmt.Errorf("couldn't prepare the query to get server count : %v", err)
+ }
+ err = tx.NamedStmt(ns).QueryRow(queryValues).Scan(&serverCount)
+ if err != nil {
+ return 0, fmt.Errorf("failed to get servers count: %v", err)
+ }
+ return serverCount, nil
+}
+
func getServers(h http.Header, params map[string]string, tx *sqlx.Tx, user *auth.CurrentUser, useIMS bool, version api.Version) ([]tc.ServerNullable, uint64, error, error, int, *time.Time) {
var maxTime time.Time
var runSecond bool
@@ -797,6 +816,9 @@ func getServers(h http.Header, params map[string]string, tx *sqlx.Tx, user *auth
queryAddition := ""
dsHasRequiredCapabilities := false
var cdnID int
+ var serverCount uint64
+ var err error
+
if dsIDStr, ok := params[`dsId`]; ok {
// don't allow query on ds outside user's tenant
dsID, err := strconv.Atoi(dsIDStr)
@@ -853,23 +875,15 @@ func getServers(h http.Header, params map[string]string, tx *sqlx.Tx, user *auth
return nil, 0, util.JoinErrs(errs), nil, http.StatusBadRequest, nil
}
- // TODO there's probably a cleaner way to do this by preparing a NamedStmt first and using its QueryRow method
- var serverCount uint64
- countRows, err := tx.NamedQuery(serverCountQuery+queryAddition+where, queryValues)
+ countQuery := serverCountQuery + queryAddition + where
+ // If we are querying for a DS that has reqd capabilities, we need to make sure that we also include all the ORG servers directly assigned to this DS
+ if _, ok := params["dsId"]; ok && dsHasRequiredCapabilities {
+ countQuery = `SELECT (` + countQuery + `) + (` + serverCountQuery + originServerQuery + `) AS total`
+ }
+ serverCount, err = getServerCount(tx, countQuery, queryValues)
if err != nil {
return nil, 0, nil, fmt.Errorf("failed to get servers count: %v", err), http.StatusInternalServerError, nil
}
- defer countRows.Close()
- rowsAffected := 0
- for countRows.Next() {
- if err = countRows.Scan(&serverCount); err != nil {
- return nil, 0, nil, fmt.Errorf("failed to read servers count: %v", err), http.StatusInternalServerError, nil
- }
- rowsAffected++
- }
- if rowsAffected != 1 {
- return nil, 0, nil, fmt.Errorf("incorrect rows returned for server count, want: 1 got: %v", rowsAffected), http.StatusInternalServerError, nil
- }
serversList := []tc.ServerNullable{}
if useIMS {
@@ -884,8 +898,12 @@ func getServers(h http.Header, params map[string]string, tx *sqlx.Tx, user *auth
}
query := selectQuery + queryAddition + where + orderBy + pagination
- log.Debugln("Query is ", query)
+ // If you're looking to get the servers for a particular delivery service, make sure you're also querying the ORG servers from the deliveryservice_server table
+ if _, ok := params[`dsId`]; ok {
+ query = `(` + selectQuery + queryAddition + where + orderBy + pagination + `) UNION ` + selectQuery + originServerQuery
+ }
+ log.Debugln("Query is ", query)
rows, err := tx.NamedQuery(query, queryValues)
if err != nil {
return nil, serverCount, nil, errors.New("querying: " + err.Error()), http.StatusInternalServerError, nil
diff --git a/traffic_ops/traffic_ops_golang/server/servers_test.go b/traffic_ops/traffic_ops_golang/server/servers_test.go
index df2c454..0ea0d02 100644
--- a/traffic_ops/traffic_ops_golang/server/servers_test.go
+++ b/traffic_ops/traffic_ops_golang/server/servers_test.go
@@ -248,6 +248,8 @@ func TestGetServersByCachegroup(t *testing.T) {
}
mock.ExpectBegin()
+ mock.ExpectPrepare("SELECT COUNT\\(s.id\\) FROM s")
+ mock.ExpectPrepare("SELECT COUNT\\(s.id\\) FROM s")
mock.ExpectQuery("SELECT COUNT\\(s.id\\) FROM s").WillReturnRows(unfilteredRows)
mock.ExpectQuery("SELECT").WillReturnRows(rows)
mock.ExpectQuery("SELECT").WillReturnRows(interfaceRows)
@@ -356,6 +358,8 @@ func TestGetMidServers(t *testing.T) {
}
}
mock.ExpectBegin()
+ mock.ExpectPrepare("SELECT COUNT\\(s.id\\) FROM s")
+ mock.ExpectPrepare("SELECT COUNT\\(s.id\\) FROM s")
mock.ExpectQuery("SELECT COUNT\\(s.id\\) FROM s").WillReturnRows(unfilteredRows)
mock.ExpectQuery("SELECT").WillReturnRows(rows)
mock.ExpectQuery("SELECT").WillReturnRows(interfaceRows)