You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2020/11/18 01:01:54 UTC
[GitHub] [trafficcontrol] rawlinp opened a new pull request #5299: Add validation to topology updates and server updates/deletions
rawlinp opened a new pull request #5299:
URL: https://github.com/apache/trafficcontrol/pull/5299
## What does this PR (Pull Request) do?
Adds the following validation to topology updates and server updates/deletions:
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.
- [x] This PR fixes #5171
## Which Traffic Control components are affected by this PR?
- Traffic Ops
## What is the best way to verify this PR?
Review the added tests, ensure they pass.
## The following criteria are ALL met by this PR
- [x] This PR includes tests
- [x] validation, no docs necessary
- [x] This PR includes an update to CHANGELOG.md
- [x] This PR includes any and all required license headers
- [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the Apache Software Foundation's security guidelines](https://www.apache.org/security/) for details)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 merged pull request #5299: Add validation to topology updates and server updates/deletions
Posted by GitBox <gi...@apache.org>.
ocket8888 merged pull request #5299:
URL: https://github.com/apache/trafficcontrol/pull/5299
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #5299: Add validation to topology updates and server updates/deletions
Posted by GitBox <gi...@apache.org>.
zrhoffman commented on a change in pull request #5299:
URL: https://github.com/apache/trafficcontrol/pull/5299#discussion_r526421077
##########
File path: 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
Review comment:
The `1` is unnecessary, can be just `SELECT FROM`
##########
File path: 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) {
Review comment:
Godoc doesn't match function name
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org