You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by oc...@apache.org on 2020/11/18 17:26:31 UTC
[trafficcontrol] 09/09: Add validation to topology updates and
server updates/deletions (#5299)
This is an automated email from the ASF dual-hosted git repository.
ocket8888 pushed a commit to branch 5.0.x
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git
commit 3d0a90f9918dfa1a0158a3e04da3a2440197555c
Author: Rawlin Peters <ra...@apache.org>
AuthorDate: Wed Nov 18 06:47:14 2020 -0700
Add validation to topology updates and server updates/deletions (#5299)
Topologies must contain at least 1 server per cachegroup in each of the
CDNs of any assigned delivery services. Otherwise, a delivery service's
topology is not truly representative of reality.
(cherry picked from commit be43e76bef2e60b2a5df2049ab8633f035ecd995)
---
CHANGELOG.md | 1 +
traffic_ops/testing/api/v3/servers_test.go | 63 +++-
traffic_ops/testing/api/v3/tc-fixtures.json | 335 +++++++++++++++++++++
traffic_ops/testing/api/v3/topologies_test.go | 66 +++-
.../traffic_ops_golang/dbhelpers/db_helpers.go | 45 +++
traffic_ops/traffic_ops_golang/server/servers.go | 64 ++--
.../traffic_ops_golang/topology/topologies.go | 95 ++++--
7 files changed, 602 insertions(+), 67 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 07861c4..1d6f591 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -67,6 +67,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
- Added default sort logic to GET API calls using Read()
- Traffic Ops: added validation for assigning ORG servers to topology-based delivery services
- Added locationByDeepCoverageZone to the `crs/stats/ip/{ip}` endpoint in the Traffic Router API
+- Traffic Ops: added validation for topology updates and server updates/deletions to ensure that topologies have at least one server per cachegroup in each CDN of any assigned delivery services
### 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 issue](https://github.com/apache/trafficcontrol/issues/5188)
diff --git a/traffic_ops/testing/api/v3/servers_test.go b/traffic_ops/testing/api/v3/servers_test.go
index 1af5300..9d0d4d1 100644
--- a/traffic_ops/testing/api/v3/servers_test.go
+++ b/traffic_ops/testing/api/v3/servers_test.go
@@ -60,16 +60,23 @@ func LastServerInTopologyCacheGroup(t *testing.T) {
const cacheGroupName = "topology-mid-cg-01"
const moveToCacheGroup = "topology-mid-cg-02"
const topologyName = "forked-topology"
+ const cdnName = "cdn2"
const expectedLength = 1
+ cdns, _, err := TOSession.GetCDNByNameWithHdr(cdnName, nil)
+ if err != nil {
+ t.Fatalf("unable to GET CDN: %v", err)
+ }
+ cdnID := cdns[0].ID
params := url.Values{}
params.Add("cachegroupName", cacheGroupName)
params.Add("topology", topologyName)
+ params.Add("cdn", strconv.Itoa(cdnID))
servers, _, err := TOSession.GetServersWithHdr(¶ms, nil)
if err != nil {
- t.Fatalf("getting server from cachegroup %s in topology %s: %s", cacheGroupName, topologyName, err.Error())
+ t.Fatalf("getting server from cdn %s from cachegroup %s in topology %s: %s", cdnName, cacheGroupName, topologyName, err.Error())
}
if len(servers.Response) != expectedLength {
- t.Fatalf("expected to get %d server from cachegroup %s in topology %s, got %d servers", expectedLength, cacheGroupName, topologyName, len(servers.Response))
+ t.Fatalf("expected to get %d server from cdn %s from cachegroup %s in topology %s, got %d servers", expectedLength, cdnName, cacheGroupName, topologyName, len(servers.Response))
}
server := servers.Response[0]
_, reqInf, err := TOSession.DeleteServerByID(*server.ID)
@@ -80,6 +87,28 @@ func LastServerInTopologyCacheGroup(t *testing.T) {
t.Fatalf("expected a 400-level error deleting server with id %d, got status code %d: %s", *server.ID, reqInf.StatusCode, err.Error())
}
+ // attempt to move it to another CDN while it's the last server in the cachegroup in its CDN
+ cdns, _, err = TOSession.GetCDNByNameWithHdr("cdn1", nil)
+ if err != nil {
+ t.Fatalf("unable to GET CDN: %v", err)
+ }
+ newCDNID := cdns[0].ID
+ oldCDNID := *server.CDNID
+ server.CDNID = &newCDNID
+ profiles, _, err := TOSession.GetProfileByNameWithHdr("MID1", nil)
+ if err != nil {
+ t.Fatalf("unable to GET profile: %v", err)
+ }
+ newProfile := profiles[0].ID
+ oldProfile := *server.ProfileID
+ server.ProfileID = &newProfile
+ _, _, err = TOSession.UpdateServerByIDWithHdr(*server.ID, server, nil)
+ if err == nil {
+ t.Fatalf("changing the CDN of the last server (%s) in a CDN in a cachegroup used by a topology assigned to a delivery service(s) in that CDN - expected: error, actual: nil", *server.HostName)
+ }
+ server.CDNID = &oldCDNID
+ server.ProfileID = &oldProfile
+
params = url.Values{}
params.Add("name", moveToCacheGroup)
cgs, _, err := TOSession.GetCacheGroupsByQueryParamsWithHdr(params, nil)
@@ -548,10 +577,11 @@ func GetTestServersQueryParameters(t *testing.T) {
params.Set("dsId", strconv.Itoa(*ds.ID))
expectedHostnames := map[string]bool{
- "edge1-cdn1-cg3": false,
- "edge2-cdn1-cg3": false,
- "atlanta-mid-16": false,
- "edgeInCachegroup3": false,
+ "edge1-cdn1-cg3": false,
+ "edge2-cdn1-cg3": false,
+ "atlanta-mid-16": false,
+ "edgeInCachegroup3": false,
+ "midInSecondaryCachegroupInCDN1": false,
}
response, _, err := TOSession.GetServersWithHdr(¶ms, nil)
if err != nil {
@@ -562,7 +592,7 @@ func GetTestServersQueryParameters(t *testing.T) {
}
for _, server := range response.Response {
if _, exists := expectedHostnames[*server.HostName]; !exists {
- t.Fatalf("expected hostnames %v, actual %s actual: ", expectedHostnames, *server.HostName)
+ t.Fatalf("expected hostnames %v, actual %s", expectedHostnames, *server.HostName)
}
expectedHostnames[*server.HostName] = true
}
@@ -600,14 +630,15 @@ func GetTestServersQueryParameters(t *testing.T) {
params.Del("dsId")
params.Add("topology", topology)
expectedHostnames = map[string]bool{
- originHostname: false,
- "edge1-cdn1-cg3": false,
- "edge2-cdn1-cg3": false,
- "atlanta-mid-16": false,
- "atlanta-mid-17": false,
- "edgeInCachegroup3": false,
- "midInParentCachegroup": false,
- "midInSecondaryCachegroup": false,
+ originHostname: false,
+ "edge1-cdn1-cg3": false,
+ "edge2-cdn1-cg3": false,
+ "atlanta-mid-16": false,
+ "atlanta-mid-17": false,
+ "edgeInCachegroup3": false,
+ "midInParentCachegroup": false,
+ "midInSecondaryCachegroup": false,
+ "midInSecondaryCachegroupInCDN1": false,
}
response, _, err = TOSession.GetServersWithHdr(¶ms, nil)
if err != nil {
@@ -618,7 +649,7 @@ func GetTestServersQueryParameters(t *testing.T) {
}
for _, server := range response.Response {
if _, exists := expectedHostnames[*server.HostName]; !exists {
- t.Fatalf("expected hostnames %v, actual %s actual: ", expectedHostnames, *server.HostName)
+ t.Fatalf("expected hostnames %v, actual %s", expectedHostnames, *server.HostName)
}
expectedHostnames[*server.HostName] = true
}
diff --git a/traffic_ops/testing/api/v3/tc-fixtures.json b/traffic_ops/testing/api/v3/tc-fixtures.json
index 30c811f..796c630 100644
--- a/traffic_ops/testing/api/v3/tc-fixtures.json
+++ b/traffic_ops/testing/api/v3/tc-fixtures.json
@@ -222,6 +222,13 @@
"name": "dtrc3",
"shortName": "dtrc3",
"typeName": "EDGE_LOC"
+ },
+ {
+ "latitude": 0,
+ "longitude": 0,
+ "name": "cdn1-only",
+ "shortName": "cdn1-only",
+ "typeName": "EDGE_LOC"
}
],
"cdns": [
@@ -1066,6 +1073,198 @@
"type": "CLIENT_STEERING",
"xmlId": "ds-client-steering",
"anonymousBlockingEnabled": false
+ },
+ {
+ "active": true,
+ "cdnName": "cdn2",
+ "cacheurl": "",
+ "ccrDnsTtl": 3600,
+ "checkPath": "",
+ "consistentHashQueryParams": [],
+ "deepCachingType": "NEVER",
+ "displayName": "ds-forked-topology",
+ "dnsBypassCname": null,
+ "dnsBypassIp": "",
+ "dnsBypassIp6": "",
+ "dnsBypassTtl": 30,
+ "dscp": 40,
+ "edgeHeaderRewrite": null,
+ "fqPacingRate": 0,
+ "geoLimit": 0,
+ "geoLimitCountries": "",
+ "geoLimitRedirectURL": null,
+ "geoProvider": 0,
+ "globalMaxMbps": 0,
+ "globalMaxTps": 0,
+ "httpBypassFqdn": "",
+ "infoUrl": "TBD",
+ "initialDispersion": 1,
+ "ipv6RoutingEnabled": true,
+ "lastUpdated": "2018-04-06 16:48:51+00",
+ "logsEnabled": false,
+ "longDesc": "",
+ "longDesc1": "",
+ "longDesc2": "",
+ "matchList": [
+ {
+ "pattern": ".*\\.ds-forked-topology\\..*",
+ "setNumber": 0,
+ "type": "HOST_REGEXP"
+ }
+ ],
+ "maxDnsAnswers": 0,
+ "midHeaderRewrite": null,
+ "missLat": 41.881944,
+ "missLong": -87.627778,
+ "multiSiteOrigin": false,
+ "orgServerFqdn": "http://example.org",
+ "originShield": null,
+ "profileDescription": null,
+ "profileName": null,
+ "protocol": 0,
+ "qstringIgnore": 0,
+ "rangeRequestHandling": 0,
+ "regexRemap": null,
+ "regionalGeoBlocking": false,
+ "remapText": null,
+ "routingName": "cdn",
+ "signed": false,
+ "signingAlgorithm": null,
+ "sslKeyVersion": 0,
+ "tenant": "tenant1",
+ "tenantName": "tenant1",
+ "topology": "forked-topology",
+ "type": "HTTP",
+ "xmlId": "ds-forked-topology",
+ "anonymousBlockingEnabled": false
+ },
+ {
+ "active": true,
+ "cdnName": "cdn1",
+ "cacheurl": "",
+ "ccrDnsTtl": 3600,
+ "checkPath": "",
+ "consistentHashQueryParams": [],
+ "deepCachingType": "NEVER",
+ "displayName": "top-ds-in-cdn1",
+ "dnsBypassCname": null,
+ "dnsBypassIp": "",
+ "dnsBypassIp6": "",
+ "dnsBypassTtl": 30,
+ "dscp": 40,
+ "edgeHeaderRewrite": null,
+ "fqPacingRate": 0,
+ "geoLimit": 0,
+ "geoLimitCountries": "",
+ "geoLimitRedirectURL": null,
+ "geoProvider": 0,
+ "globalMaxMbps": 0,
+ "globalMaxTps": 0,
+ "httpBypassFqdn": "",
+ "infoUrl": "TBD",
+ "initialDispersion": 1,
+ "ipv6RoutingEnabled": true,
+ "lastUpdated": "2018-04-06 16:48:51+00",
+ "logsEnabled": false,
+ "longDesc": "",
+ "longDesc1": "",
+ "longDesc2": "",
+ "matchList": [
+ {
+ "pattern": ".*\\.top-ds-in-cdn1\\..*",
+ "setNumber": 0,
+ "type": "HOST_REGEXP"
+ }
+ ],
+ "maxDnsAnswers": 0,
+ "midHeaderRewrite": null,
+ "missLat": 41.881944,
+ "missLong": -87.627778,
+ "multiSiteOrigin": false,
+ "orgServerFqdn": "http://example.org",
+ "originShield": null,
+ "profileDescription": null,
+ "profileName": null,
+ "protocol": 0,
+ "qstringIgnore": 0,
+ "rangeRequestHandling": 0,
+ "regexRemap": null,
+ "regionalGeoBlocking": false,
+ "remapText": null,
+ "routingName": "cdn",
+ "signed": false,
+ "signingAlgorithm": null,
+ "sslKeyVersion": 0,
+ "tenant": "tenant1",
+ "tenantName": "tenant1",
+ "topology": "top-used-by-cdn1-and-cdn2",
+ "type": "HTTP",
+ "xmlId": "top-ds-in-cdn1",
+ "anonymousBlockingEnabled": false
+ },
+ {
+ "active": true,
+ "cdnName": "cdn2",
+ "cacheurl": "",
+ "ccrDnsTtl": 3600,
+ "checkPath": "",
+ "consistentHashQueryParams": [],
+ "deepCachingType": "NEVER",
+ "displayName": "top-ds-in-cdn2",
+ "dnsBypassCname": null,
+ "dnsBypassIp": "",
+ "dnsBypassIp6": "",
+ "dnsBypassTtl": 30,
+ "dscp": 40,
+ "edgeHeaderRewrite": null,
+ "fqPacingRate": 0,
+ "geoLimit": 0,
+ "geoLimitCountries": "",
+ "geoLimitRedirectURL": null,
+ "geoProvider": 0,
+ "globalMaxMbps": 0,
+ "globalMaxTps": 0,
+ "httpBypassFqdn": "",
+ "infoUrl": "TBD",
+ "initialDispersion": 1,
+ "ipv6RoutingEnabled": true,
+ "lastUpdated": "2018-04-06 16:48:51+00",
+ "logsEnabled": false,
+ "longDesc": "",
+ "longDesc1": "",
+ "longDesc2": "",
+ "matchList": [
+ {
+ "pattern": ".*\\.top-ds-in-cdn2\\..*",
+ "setNumber": 0,
+ "type": "HOST_REGEXP"
+ }
+ ],
+ "maxDnsAnswers": 0,
+ "midHeaderRewrite": null,
+ "missLat": 41.881944,
+ "missLong": -87.627778,
+ "multiSiteOrigin": false,
+ "orgServerFqdn": "http://example.org",
+ "originShield": null,
+ "profileDescription": null,
+ "profileName": null,
+ "protocol": 0,
+ "qstringIgnore": 0,
+ "rangeRequestHandling": 0,
+ "regexRemap": null,
+ "regionalGeoBlocking": false,
+ "remapText": null,
+ "routingName": "cdn",
+ "signed": false,
+ "signingAlgorithm": null,
+ "sslKeyVersion": 0,
+ "tenant": "tenant1",
+ "tenantName": "tenant1",
+ "topology": "top-used-by-cdn1-and-cdn2",
+ "type": "HTTP",
+ "xmlId": "top-ds-in-cdn2",
+ "anonymousBlockingEnabled": false
}
],
"deliveryServicesRegexes": [
@@ -3426,6 +3625,90 @@
"updPending": false
},
{
+ "cachegroup": "secondaryCachegroup",
+ "cdnName": "cdn1",
+ "domainName": "kabletown.net",
+ "hostName": "midInSecondaryCachegroupInCDN1",
+ "httpsPort": 443,
+ "interfaces": [
+ {
+ "ipAddresses": [
+ {
+ "address": "2001:db8:deed:beef::12/64",
+ "gateway": "2001:db8:deed:beef::1",
+ "serviceAddress": false
+ },
+ {
+ "address": "192.0.7.15/24",
+ "gateway": "192.0.7.1",
+ "serviceAddress": true
+ }
+ ],
+ "monitor": true,
+ "mtu": 9000,
+ "name": "bond0"
+ }
+ ],
+ "physLocation": "Denver",
+ "profile": "MID1",
+ "rack": "RR 119.02",
+ "revalPending": false,
+ "status": "REPORTED",
+ "tcpPort": 80,
+ "type": "MID",
+ "updPending": false
+ },
+ {
+ "cachegroup": "cdn1-only",
+ "cdnName": "cdn1",
+ "domainName": "foo.kabletown.net",
+ "guid": null,
+ "hostName": "edge-in-cdn1-only",
+ "httpsPort": 443,
+ "iloIpAddress": "",
+ "iloIpGateway": "",
+ "iloIpNetmask": "",
+ "iloPassword": "",
+ "iloUsername": "",
+ "interfaces": [
+ {
+ "ipAddresses": [
+ {
+ "address": "192.0.2.88/24",
+ "gateway": "192.0.2.1",
+ "serviceAddress": true
+ },
+ {
+ "address": "2001:db8:f33d:beef::2/64",
+ "gateway": "2001:db8:f33d:beef::1",
+ "serviceAddress": false
+ }
+ ],
+ "maxBandwidth": null,
+ "monitor": true,
+ "mtu": 9000,
+ "name": "bond0"
+ }
+ ],
+ "lastUpdated": "2018-03-28T17:30:00.220351+00:00",
+ "mgmtIpAddress": "",
+ "mgmtIpGateway": "",
+ "mgmtIpNetmask": "",
+ "offlineReason": null,
+ "physLocation": "Denver",
+ "profile": "EDGE1",
+ "rack": "RR 119.02",
+ "revalPending": false,
+ "routerHostName": "",
+ "routerPortName": "",
+ "status": "REPORTED",
+ "tcpPort": 80,
+ "type": "EDGE",
+ "updPending": false,
+ "xmppId": "",
+ "xmppPasswd": ""
+ },
+ {
"cachegroup": "fallback1",
"cdnName": "cdn2",
"domainName": "kabletown.net",
@@ -3630,6 +3913,40 @@
"updPending": false
},
{
+ "cachegroup": "topology-mid-cg-01",
+ "cdnName": "cdn1",
+ "domainName": "kabletown.net",
+ "hostName": "midInTopologyMidCg01InCDN1",
+ "httpsPort": 443,
+ "interfaces": [
+ {
+ "ipAddresses": [
+ {
+ "address": "2001:db8:de4d:beef::12/64",
+ "gateway": "2001:db8:de4d:beef::1",
+ "serviceAddress": false
+ },
+ {
+ "address": "192.0.12.21/24",
+ "gateway": "192.0.12.1",
+ "serviceAddress": true
+ }
+ ],
+ "monitor": true,
+ "mtu": 9000,
+ "name": "bond0"
+ }
+ ],
+ "physLocation": "Denver",
+ "profile": "MID1",
+ "rack": "RR 119.02",
+ "revalPending": false,
+ "status": "REPORTED",
+ "tcpPort": 80,
+ "type": "MID",
+ "updPending": false
+ },
+ {
"cachegroup": "topology-mid-cg-02",
"cdnName": "cdn2",
"domainName": "kabletown.net",
@@ -4230,6 +4547,24 @@
"parents": [1]
}
]
+ },
+ {
+ "name": "top-used-by-cdn1-and-cdn2",
+ "description": "a topology",
+ "nodes": [
+ {
+ "cachegroup": "dtrc1",
+ "parents": []
+ },
+ {
+ "cachegroup": "dtrc2",
+ "parents": [0]
+ },
+ {
+ "cachegroup": "dtrc3",
+ "parents": [0]
+ }
+ ]
}
],
"types": [
diff --git a/traffic_ops/testing/api/v3/topologies_test.go b/traffic_ops/testing/api/v3/topologies_test.go
index 2b43a31..4350c8e 100644
--- a/traffic_ops/testing/api/v3/topologies_test.go
+++ b/traffic_ops/testing/api/v3/topologies_test.go
@@ -21,7 +21,9 @@ package v3
import (
"fmt"
+ "net/url"
"reflect"
+ "strconv"
"strings"
"testing"
@@ -164,14 +166,6 @@ func updateSingleTopology(topology tc.Topology) error {
}
func UpdateTestTopologies(t *testing.T) {
- firstTopName := testData.Topologies[0].Name
- for _, top := range testData.Topologies {
- top.Name = firstTopName
- if err := updateSingleTopology(top); err != nil {
- t.Fatalf(err.Error())
- }
- }
- // Revert test topologies
for _, topology := range testData.Topologies {
if err := updateSingleTopology(topology); err != nil {
t.Fatalf(err.Error())
@@ -188,6 +182,62 @@ func UpdateTestTopologies(t *testing.T) {
if err == nil {
t.Errorf("making invalid update to topology - expected: error, actual: nil")
}
+
+ // attempt to add a cachegroup that only has caches in one CDN while the topology is assigned to DSes from multiple CDNs
+ top, _, err = TOSession.GetTopologyWithHdr("top-used-by-cdn1-and-cdn2", nil)
+ if err != nil {
+ t.Fatalf("cannot GET topology: %v", err)
+ }
+ params := url.Values{}
+ params.Add("topology", "top-used-by-cdn1-and-cdn2")
+ dses, _, err := TOSession.GetDeliveryServicesV30WithHdr(nil, params)
+ if err != nil {
+ t.Fatalf("cannot GET delivery services: %v", err)
+ }
+ if len(dses) < 2 {
+ t.Fatalf("expected at least 2 delivery services assigned to topology top-used-by-cdn1-and-cdn2, actual: %d", len(dses))
+ }
+ foundCDN1 := false
+ foundCDN2 := false
+ for _, ds := range dses {
+ if *ds.CDNName == "cdn1" {
+ foundCDN1 = true
+ } else if *ds.CDNName == "cdn2" {
+ foundCDN2 = true
+ }
+ }
+ if !foundCDN1 || !foundCDN2 {
+ t.Fatalf("expected delivery services assigned to topology top-used-by-cdn1-and-cdn2 to be assigned to cdn1 and cdn2")
+ }
+ cgs, _, err := TOSession.GetCacheGroupNullableByNameWithHdr("cdn1-only", nil)
+ if err != nil {
+ t.Fatalf("unable to GET cachegroup by name: %v", err)
+ }
+ if len(cgs) != 1 {
+ t.Fatalf("expected: to get 1 cachegroup named 'cdn1-only', actual: got %d", len(cgs))
+ }
+ params = url.Values{}
+ params.Add("cachegroup", strconv.Itoa(*cgs[0].ID))
+ servers, _, err := TOSession.GetServersWithHdr(¶ms, nil)
+ if err != nil {
+ t.Fatalf("unable to GET servers by cachegroup: %v", err)
+ }
+ for _, s := range servers.Response {
+ if *s.Cachegroup != "cdn1-only" {
+ t.Fatalf("GET servers by cachegroup 'cdn1-only' - expected: only servers in cachegroup 'cdn1-only', actual: got server in %s", *s.Cachegroup)
+ }
+ if *s.CDNName != "cdn1" {
+ t.Fatalf("expected: servers in cachegroup 'cdn1-only' to only be in cdn1, actual: servers in cdn %s", *s.CDNName)
+ }
+ }
+ top.Nodes = append(top.Nodes, tc.TopologyNode{
+ Cachegroup: "cdn1-only",
+ Parents: []int{0},
+ })
+ _, _, err = TOSession.UpdateTopology(top.Name, *top)
+ if err == nil {
+ t.Errorf("making invalid update to topology (cachegroup contains only servers from cdn1 while the topology is assigned to delivery services in cdn1 and cdn2) - expected: error, actual: nil")
+ }
}
func DeleteTestTopologies(t *testing.T) {
diff --git a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
index 03ac0a8..53ce367 100644
--- a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
+++ b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
@@ -881,6 +881,51 @@ WHERE
return dses, nil
}
+// GetDeliveryServiceCDNsByTopology returns a slice of CDN IDs for all delivery services
+// assigned to the given topology.
+func GetDeliveryServiceCDNsByTopology(tx *sql.Tx, topology string) ([]int, error) {
+ q := `
+SELECT
+ COALESCE(ARRAY_AGG(DISTINCT d.cdn_id), '{}'::BIGINT[])
+FROM
+ deliveryservice d
+WHERE
+ d.topology = $1
+`
+ cdnIDs := []int64{}
+ if err := tx.QueryRow(q, topology).Scan(pq.Array(&cdnIDs)); err != nil {
+ return nil, fmt.Errorf("in GetDeliveryServiceCDNsByTopology: querying deliveryservices by topology '%s': %v", topology, err)
+ }
+ res := make([]int, len(cdnIDs))
+ for i, id := range cdnIDs {
+ res[i] = int(id)
+ }
+ return res, nil
+}
+
+// CheckCachegroupHasTopologyBasedDeliveryServicesOnCDN returns true if the given cachegroup is assigned to
+// any topologies with delivery services assigned on the given CDN.
+func CachegroupHasTopologyBasedDeliveryServicesOnCDN(tx *sql.Tx, cachegroupID int, CDNID int) (bool, error) {
+ q := `
+SELECT EXISTS(
+ SELECT
+ 1
+ FROM cachegroup c
+ JOIN topology_cachegroup tc on c.name = tc.cachegroup
+ JOIN topology t ON tc.topology = t.name
+ JOIN deliveryservice d on t.name = d.topology
+ WHERE
+ c.id = $1
+ AND d.cdn_id = $2
+)
+`
+ res := false
+ if err := tx.QueryRow(q, cachegroupID, CDNID).Scan(&res); err != nil {
+ return false, fmt.Errorf("in CachegroupHasTopologyBasedDeliveryServicesOnCDN: %v", err)
+ }
+ return res, nil
+}
+
// GetFederationIDForUserIDByXMLID retrieves the ID of the Federation assigned to the user defined by
// userID on the Delivery Service identified by xmlid. If no such federation exists, the boolean
// returned will be 'false', while the error indicates unexpected errors that occurred when querying.
diff --git a/traffic_ops/traffic_ops_golang/server/servers.go b/traffic_ops/traffic_ops_golang/server/servers.go
index 8a0a2b1..369dee9 100644
--- a/traffic_ops/traffic_ops_golang/server/servers.go
+++ b/traffic_ops/traffic_ops_golang/server/servers.go
@@ -26,7 +26,6 @@ import (
"encoding/json"
"errors"
"fmt"
- "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/topology"
"net"
"net/http"
"strconv"
@@ -43,6 +42,7 @@ import (
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/deliveryservice"
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/routing/middleware"
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/tenant"
+ "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/topology"
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/util/ims"
validation "github.com/go-ozzo/ozzo-validation"
@@ -1194,14 +1194,15 @@ func Update(w http.ResponseWriter, r *http.Request) {
api.HandleErr(w, r, tx, http.StatusNotFound, errors.New("the server doesn't exist, cannot update"), nil)
return
}
+ origServer := origSer[0]
originalXMPPID := ""
originalStatusID := 0
changeXMPPID := false
- if origSer[0].XMPPID != nil {
- originalXMPPID = *origSer[0].XMPPID
+ if origServer.XMPPID != nil {
+ originalXMPPID = *origServer.XMPPID
}
- if origSer[0].Status != nil {
- originalStatusID = *origSer[0].StatusID
+ if origServer.Status != nil {
+ originalStatusID = *origServer.StatusID
}
var server tc.ServerNullableV2
@@ -1220,7 +1221,7 @@ func Update(w http.ResponseWriter, r *http.Request) {
if newServer.StatusID != nil && *newServer.StatusID != originalStatusID {
newServer.StatusLastUpdated = ¤tTime
} else {
- newServer.StatusLastUpdated = origSer[0].StatusLastUpdated
+ newServer.StatusLastUpdated = origServer.StatusLastUpdated
}
serviceInterface, err := validateV3(&newServer, tx)
if err != nil {
@@ -1228,15 +1229,6 @@ func Update(w http.ResponseWriter, r *http.Request) {
return
}
- cacheGroupIds := []int{*origSer[0].CachegroupID}
- serverIds := []int{*origSer[0].ID}
- if *origSer[0].CachegroupID != *newServer.CachegroupID {
- if err = topology.CheckForEmptyCacheGroups(inf.Tx, cacheGroupIds, true, serverIds); err != nil {
- api.HandleErr(w, r, tx, http.StatusBadRequest, errors.New("server is the last one in its cachegroup, which is used by a topology, so it cannot be moved to another cachegroup: "+err.Error()), nil)
- return
- }
- }
-
server, err = newServer.ToServerV2()
if err != nil {
api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, fmt.Errorf("converting v3 server to v2 for update: %v", err))
@@ -1291,6 +1283,24 @@ func Update(w http.ResponseWriter, r *http.Request) {
}
}
+ if *origServer.CachegroupID != *server.CachegroupID || *origServer.CDNID != *server.CDNID {
+ hasDSOnCDN, err := dbhelpers.CachegroupHasTopologyBasedDeliveryServicesOnCDN(inf.Tx.Tx, *origServer.CachegroupID, *origServer.CDNID)
+ if err != nil {
+ api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, err)
+ return
+ }
+ CDNIDs := []int{}
+ if hasDSOnCDN {
+ CDNIDs = append(CDNIDs, *origServer.CDNID)
+ }
+ cacheGroupIds := []int{*origServer.CachegroupID}
+ serverIds := []int{*origServer.ID}
+ if err = topology.CheckForEmptyCacheGroups(inf.Tx, cacheGroupIds, CDNIDs, true, serverIds); err != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, errors.New("server is the last one in its cachegroup, which is used by a topology, so it cannot be moved to another cachegroup: "+err.Error()), nil)
+ return
+ }
+ }
+
server.ID = new(int)
*server.ID = inf.IntParams["id"]
@@ -1624,13 +1634,21 @@ func Delete(w http.ResponseWriter, r *http.Request) {
api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, fmt.Errorf("there are somehow two servers with id %d - cannot delete", id))
return
}
- if version.Major >= 3 {
- cacheGroupIds := []int{*servers[0].CachegroupID}
- serverIds := []int{*servers[0].ID}
- if err := topology.CheckForEmptyCacheGroups(inf.Tx, cacheGroupIds, true, serverIds); err != nil {
- api.HandleErr(w, r, tx, http.StatusBadRequest, errors.New("server is the last one in its cachegroup, which is used by a topology: "+err.Error()), nil)
- return
- }
+ server := servers[0]
+ cacheGroupIds := []int{*server.CachegroupID}
+ serverIds := []int{*server.ID}
+ hasDSOnCDN, err := dbhelpers.CachegroupHasTopologyBasedDeliveryServicesOnCDN(inf.Tx.Tx, *server.CachegroupID, *server.CDNID)
+ if err != nil {
+ api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, err)
+ return
+ }
+ CDNIDs := []int{}
+ if hasDSOnCDN {
+ CDNIDs = append(CDNIDs, *server.CDNID)
+ }
+ if err := topology.CheckForEmptyCacheGroups(inf.Tx, cacheGroupIds, CDNIDs, true, serverIds); err != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, errors.New("server is the last one in its cachegroup, which is used by a topology: "+err.Error()), nil)
+ return
}
userErr, sysErr, errCode = deleteInterfaces(id, tx)
@@ -1652,8 +1670,6 @@ func Delete(w http.ResponseWriter, r *http.Request) {
return
}
- server := servers[0]
-
if inf.Version.Major >= 3 {
api.WriteRespAlertObj(w, r, tc.SuccessLevel, "Server deleted", server)
} else {
diff --git a/traffic_ops/traffic_ops_golang/topology/topologies.go b/traffic_ops/traffic_ops_golang/topology/topologies.go
index bf77b01..57a9745 100644
--- a/traffic_ops/traffic_ops_golang/topology/topologies.go
+++ b/traffic_ops/traffic_ops_golang/topology/topologies.go
@@ -142,7 +142,12 @@ func (topology *TOTopology) Validate() error {
for index, cacheGroup := range cacheGroups {
cacheGroupIds[index] = *cacheGroup.ID
}
- rules["empty cachegroups"] = CheckForEmptyCacheGroups(topology.ReqInfo.Tx, cacheGroupIds, false, nil)
+ dsCDNs, err := dbhelpers.GetDeliveryServiceCDNsByTopology(topology.ReqInfo.Tx.Tx, topology.Name)
+ if err != nil {
+ log.Errorf("validating topology: %v", err)
+ return errors.New("unable to validate topology")
+ }
+ rules["empty cachegroups"] = CheckForEmptyCacheGroups(topology.ReqInfo.Tx, cacheGroupIds, dsCDNs, false, nil)
rules["required capabilities"] = topology.validateDSRequiredCapabilities()
/* Only perform further checks if everything so far is valid */
@@ -159,7 +164,10 @@ func (topology *TOTopology) Validate() error {
return util.JoinErrs(tovalidate.ToErrors(rules))
}
-func CheckForEmptyCacheGroups(tx *sqlx.Tx, cacheGroupIds []int, cachegroupsInTopology bool, excludeServerIds []int) error {
+// CheckForEmptyCacheGroups checks if the cachegroups are empty (altogether) or empty in any of the given CDN IDs.
+// If cachegroupsInTopology is true, it will only check cachegroups that are used in a topology. Any server IDs in
+// excludeServerIds will not be counted.
+func CheckForEmptyCacheGroups(tx *sqlx.Tx, cacheGroupIds []int, CDNIDs []int, cachegroupsInTopology bool, excludeServerIds []int) error {
if excludeServerIds == nil {
excludeServerIds = []int{}
}
@@ -180,14 +188,16 @@ func CheckForEmptyCacheGroups(tx *sqlx.Tx, cacheGroupIds []int, cachegroupsInTop
}
var (
- serverCount int
- cacheGroup string
- cacheGroups []string
- topologies []string
+ serverCountByCDN int
+ cacheGroup string
+ cdnID *int
)
+ cgServerCountsByCDN := make(map[int]map[string]int)
+ cgServerCounts := make(map[string]int)
+ topologySetByCachegroup := make(map[string]map[string]struct{})
defer log.Close(rows, "unable to close DB connection when checking for cachegroups with no servers")
for rows.Next() {
- var scanTo = []interface{}{&cacheGroup, &serverCount}
+ var scanTo = []interface{}{&cacheGroup, &cdnID, &serverCountByCDN}
var topologiesForRow []string
if cachegroupsInTopology {
scanTo = append(scanTo, pq.Array(&topologiesForRow))
@@ -196,23 +206,70 @@ func CheckForEmptyCacheGroups(tx *sqlx.Tx, cacheGroupIds []int, cachegroupsInTop
log.Errorf(systemError, err.Error())
return baseError
}
- if serverCount != 0 {
- break
+ if cdnID != nil {
+ if _, ok := cgServerCountsByCDN[*cdnID]; !ok {
+ cgServerCountsByCDN[*cdnID] = make(map[string]int)
+ }
+ cgServerCountsByCDN[*cdnID][cacheGroup] = serverCountByCDN
}
- cacheGroups = append(cacheGroups, cacheGroup)
+ cgServerCounts[cacheGroup] += serverCountByCDN
+
if cachegroupsInTopology {
- topologies = append(topologies, topologiesForRow...)
+ if _, ok := topologySetByCachegroup[cacheGroup]; !ok {
+ topologySetByCachegroup[cacheGroup] = make(map[string]struct{})
+ }
+ for _, topology := range topologiesForRow {
+ topologySetByCachegroup[cacheGroup][topology] = struct{}{}
+ }
+ }
+ }
+ topologiesByCachegroup := make(map[string][]string, len(topologySetByCachegroup))
+ for cg, topologySet := range topologySetByCachegroup {
+ for topology := range topologySet {
+ topologiesByCachegroup[cg] = append(topologiesByCachegroup[cg], topology)
+ }
+ }
+ emptyCachegroups := []string{}
+ for cg, count := range cgServerCounts {
+ if count == 0 {
+ messageEntry := cg
+ if cachegroupsInTopology {
+ messageEntry += " (in topologies: " + strings.Join(topologiesByCachegroup[cg], ", ") + ")"
+ }
+ emptyCachegroups = append(emptyCachegroups, messageEntry)
}
}
- if len(cacheGroups) > 0 {
- errMessage := "cachegroups with no servers in them: " + strings.Join(cacheGroups, ", ")
- if cachegroupsInTopology {
- errMessage += " in topologies: " + strings.Join(topologies, ", ")
+ if len(emptyCachegroups) > 0 {
+ errMessage := "cachegroups with no servers in them: " + strings.Join(emptyCachegroups, ", ")
+ return errors.New(errMessage)
+ }
+
+ errMessage := []string{}
+ for _, cdnID := range CDNIDs {
+ if _, ok := cgServerCountsByCDN[cdnID]; !ok {
+ return fmt.Errorf("topology is assigned to delivery service on CDN %d, but that CDN has no servers", cdnID)
+ }
+ emptyCachegroupsByCDN := []string{}
+ for cg, serverCount := range cgServerCountsByCDN[cdnID] {
+ if serverCount == 0 {
+ emptyCachegroupsByCDN = append(emptyCachegroupsByCDN, cg)
+ }
+ }
+ // check that this CDN has a count for all given cachegroups
+ for cg := range cgServerCounts {
+ if _, ok := cgServerCountsByCDN[cdnID][cg]; !ok {
+ emptyCachegroupsByCDN = append(emptyCachegroupsByCDN, cg)
+ }
+ }
+ if len(emptyCachegroupsByCDN) > 0 {
+ errMessage = append(errMessage, fmt.Sprintf("topology is assigned to delivery service(s) on CDN %d, but the following cachegroups have no servers in CDN %d: %s", cdnID, cdnID, strings.Join(emptyCachegroupsByCDN, ", ")))
}
- err = errors.New(errMessage)
}
- return err
+ if len(errMessage) > 0 {
+ return errors.New(strings.Join(errMessage, "; "))
+ }
+ return nil
}
func (topology *TOTopology) nodesInOtherTopologies() ([]tc.TopologyNode, map[string][]string, error) {
@@ -769,6 +826,7 @@ func selectEmptyCacheGroupsQuery(cachegroupsInTopology bool) string {
query := fmt.Sprintf(`
SELECT
c."name",
+ s.cdn_id,
COUNT(*) FILTER (
WHERE s.id IS NOT NULL
AND NOT(s."id" = ANY(CAST(:exclude_server_ids AS INT[])))
@@ -777,8 +835,7 @@ func selectEmptyCacheGroupsQuery(cachegroupsInTopology bool) string {
%s
LEFT JOIN "server" s ON c.id = s.cachegroup
WHERE c."id" = ANY(CAST(:cachegroup_ids AS BIGINT[]))
- GROUP BY c."name"
- ORDER BY server_count
+ GROUP BY c."name", s.cdn_id
`, topologyNames, joinTopologyCachegroups)
return query
}