rawlinp commented on a change in pull request #4724:
URL: https://github.com/apache/trafficcontrol/pull/4724#discussion_r434826150
##########
File path: docs/source/api/v3/cdns_name_snapshot.rst
##########
@@ -119,6 +120,7 @@ Response Structure
:contentServers: An object containing keys which are the (short) hostnames of
the :term:`Edge-Tier cache servers` in the CDN; the values corresponding to
those keys are routing information for said servers
:cacheGroup: A string that is the :ref:`cache-group-name` of the
:term:`Cache Group` to which the server belongs
+ :capabilities: An array of this Cache Group's :term:`Server
Capabilities`. If the Cache Group has no Server Capabilities, this field is
omitted.
Review comment:
I think you mean "Server" instead of "Cache Group" here
##########
File path: docs/source/api/v3/cdns_name_snapshot_new.rst
##########
@@ -118,6 +119,7 @@ Response Structure
:contentServers: An object containing keys which are the (short) hostnames of
the :term:`Edge-Tier cache servers` in the CDN; the values corresponding to
those keys are routing information for said servers
:cacheGroup: A string that is the :ref:`cache-group-name` of the
:term:`Cache Group` to which the server belongs
+ :capabilities: An array of this Cache Group's :term:`Server
Capabilities`. If the Cache Group has no Server Capabilities, this field is
omitted.
Review comment:
ditto previous comment
##########
File path: docs/source/api/v3/cdns_name_snapshot.rst
##########
@@ -359,184 +370,204 @@ Response Structure
"dnssec.enabled": "false",
"domain_name": "mycdn.ciab.test",
"federationmapping.polling.interval": "60000",
- "federationmapping.polling.url":
"https://${toHostname}/api/3.0/federations";,
+ "federationmapping.polling.url":
"https://${toHostname}/api/2.0/federations/all";,
"geolocation.polling.interval": "86400000",
"geolocation.polling.url":
"https://trafficops.infra.ciab.test:443/GeoLite2-City.mmdb.gz";,
"keystore.maintenance.interval": "300",
"neustar.polling.interval": "86400000",
"neustar.polling.url":
"https://trafficops.infra.ciab.test:443/neustar.tar.gz";,
"soa": {
- "admin": "twelve_monkeys",
- "expire": "604800",
- "minimum": "30",
- "refresh": "28800",
- "retry": "7200"
+ "admin": "twelve_monkeys",
+ "expire": "604800",
+ "minimum": "30",
+ "refresh": "28800",
+ "retry": "7200"
},
"steeringmapping.polling.interval": "60000",
"ttls": {
- "A": "3600",
- "AAAA": "3600",
- "DNSKEY": "30",
- "DS": "30",
- "NS": "3600",
- "SOA": "86400"
+ "A": "3600",
+ "AAAA": "3600",
+ "DNSKEY": "30",
+ "DS": "30",
+ "NS": "3600",
+ "SOA": "86400"
},
"zonemanager.cache.maintenance.interval": "300",
"zonemanager.threadpool.scale": "0.50"
- },
- "contentServers": {
+ },
+ "contentRouters": {
+ "trafficrouter": {
+ "api.port": "3333",
+ "fqdn": "trafficrouter.infra.ciab.test",
+ "httpsPort": 443,
+ "ip": "172.26.0.15",
+ "ip6": "",
+ "location": "CDN_in_a_Box_Edge",
+ "port": 80,
+ "profile": "CCR_CIAB",
+ "secure.api.port": "3443",
+ "status": "ONLINE"
+ }
+ },
+ "contentServers": {
"edge": {
- "cacheGroup": "CDN_in_a_Box_Edge",
- "fqdn": "edge.infra.ciab.test",
- "hashCount": 999,
- "hashId": "edge",
- "httpsPort": 443,
- "interfaceName": "eth0",
- "ip": "172.16.239.100",
- "ip6": "fc01:9400:1000:8::100",
- "locationId": "CDN_in_a_Box_Edge",
- "port": 80,
- "profile": "ATS_EDGE_TIER_CACHE",
- "status": "REPORTED",
- "type": "EDGE",
- "deliveryServices": {
- "demo1": [
- "edge.demo1.mycdn.ciab.test"
- ]
- },
- "routingDisabled": 0
+ "cacheGroup": "CDN_in_a_Box_Edge",
+ "capabilities": [
+ "heat-vision"
Review comment:
😆 🔥
##########
File path: traffic_ops/traffic_ops_golang/crconfig/topologies.go
##########
@@ -0,0 +1,66 @@
+package crconfig
+
+import (
+ "database/sql"
+ "errors"
+ "fmt"
+ "github.com/apache/trafficcontrol/lib/go-tc"
+ "github.com/lib/pq"
+)
+
+/*
Review comment:
nit: the apache header is pretty consistently placed between the package
declaration and the imports
##########
File path:
traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/cache/Cache.java
##########
@@ -18,11 +18,7 @@
import java.net.Inet6Address;
import java.net.InetAddress;
import java.net.UnknownHostException;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
Review comment:
nit: I think intellij probably did this for you, but I think we pretty
consistently try to use explicit imports of only what we use
##########
File path:
traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/config/ConfigHandler.java
##########
@@ -18,19 +18,11 @@
import java.io.IOException;
import java.net.UnknownHostException;
import java.net.URL;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Date;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-import java.util.TreeSet;
-import java.util.Iterator;
+import java.util.*;
Review comment:
nit: I think intellij probably did this for you, but I think we pretty
consistently try to use explicit imports of only what we use
##########
File path:
traffic_router/core/src/test/java/com/comcast/cdn/traffic_control/traffic_router/core/config/ConfigHandlerTest.java
##########
@@ -15,12 +15,11 @@
package com.comcast.cdn.traffic_control.traffic_router.core.config;
-import java.util.Arrays;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.Map;
-import java.util.Set;
+import java.util.*;
Review comment:
see previous nits
##########
File path:
traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/ds/DeliveryService.java
##########
@@ -590,6 +610,14 @@ public String getRoutingName() {
return routingName;
}
+ public String getTopology() {
+ return topology;
+ }
+
+ public boolean hasRequiredCapabilities(final Set<String> capabilities) {
+ return this.requiredCapabilities.containsAll(capabilities);
Review comment:
I think this logic might be backwards. The _server_ needs to have _all_
the capabilities that the DS requires. So I think this should be
`capabilities.containsAll(requiredCapabilities)`, and consider renaming
`capabilities` to `serverCapabilities` to make it more clear.
##########
File path: traffic_ops/traffic_ops_golang/crconfig/topologies.go
##########
@@ -0,0 +1,66 @@
+package crconfig
+
+import (
+ "database/sql"
+ "errors"
+ "fmt"
+ "github.com/apache/trafficcontrol/lib/go-tc"
+ "github.com/lib/pq"
+)
+
+/*
+ * 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.
+ */
+
+func makeTopologies(tx *sql.Tx) (map[string]tc.CRConfigTopology, error) {
+ query := `
+SELECT
+ t.name,
+ (SELECT ARRAY_AGG(tc.cachegroup ORDER BY tc.cachegroup)
+ FROM topology_cachegroup tc
+ JOIN cachegroup c ON c.name = tc.cachegroup
+ JOIN type ON type.id = c.type
+ WHERE t.name = tc.topology
+ AND type.name = $1
+ ) AS nodes
+FROM topology t
+ORDER BY t.name
+`
+ var rows *sql.Rows
+ var err error
+ if rows, err = tx.Query(query, tc.CacheGroupEdgeTypeName); err != nil {
+ return nil, errors.New("querying topologies: " + err.Error())
+ }
+
+ topologies := map[string]tc.CRConfigTopology{}
+ for rows.Next() {
+ topology := tc.CRConfigTopology{}
+ var name string
+ if err = rows.Scan(
+ &name,
+ pq.Array(&topology.Nodes),
+ ); err != nil {
+ return nil, errors.New("scanning topology: " +
err.Error())
+ }
+ topologies[name] = topology
+ }
+ if err = rows.Close(); err != nil {
Review comment:
Closing the rows should be up on L49 in a `defer` statement (maybe using
`log.Close` too as you always suggest 😃 )
----------------------------------------------------------------
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:
[email protected]
