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/06/04 23:33:02 UTC

[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #4724: Supporting routing for topology-based delivery services

zrhoffman commented on a change in pull request #4724:
URL: https://github.com/apache/trafficcontrol/pull/4724#discussion_r435607518



##########
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:
       Logic flipped in 01f871c17

##########
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:
       Using explicit imports in 9e680217f

##########
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:
       Using explicit imports in 9e680217f

##########
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:
       Using explicit imports in 9e680217f (and increased Intellij's threshold for using wildcard imports to 9999 classes, because -1 just always uses it)

##########
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:
       Aw you got me. Deferring in 51fc49d66

##########
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:
       Apache header moved in 242a9fd66

##########
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:
       Corrected to Cache Server in 64507498a

##########
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:
       ![](https://i.imgur.com/XpJTyM0.png) <sup>:fire: :fire:</sup>

##########
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:
       Corrected to Cache Server in 64507498a




----------------------------------------------------------------
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