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/03 19:08:34 UTC

[trafficcontrol] branch 5.0.x updated (02612bb -> 0271e73)

This is an automated email from the ASF dual-hosted git repository.

ocket8888 pushed a change to branch 5.0.x
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git.


    from 02612bb  Fix CIAB mid enrollment (#5227)
     new 0de25ad  TP: converts delivery service requests table to ag-grid (#5183)
     new a5bc4ff  Check cache availability by IP version instead of filtering later (#5186)
     new 3efcb6f  Migration fixes (#5228)
     new 0271e73  TP: allows topology-based delivery services to be assigned to ORG servers (#5236)

The 4 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../actions/todb-tests/Dockerfile                  |   9 +-
 .../actions/{todb-init => todb-tests}/README.md    |  20 +-
 .github/actions/{go-fmt => todb-tests}/action.yml  |   4 +-
 .github/actions/todb-tests/entrypoint.sh           |  66 +++++
 .../{go.fmt.yml => traffic.ops.database.yml}       |  23 +-
 CHANGELOG.md                                       |   7 +
 PULL_REQUEST_TEMPLATE.md                           |   1 -
 docs/source/api/v2/deliveryservice_requests.rst    |   3 +
 docs/source/api/v3/deliveryservice_requests.rst    |   3 +
 .../2020072700000000_remove_redundancy.sql         |   6 +-
 ...081108261100_add_server_ip_profile_trigger.sql} |   0
 .../deliveryservice/request/requests.go            |   1 +
 .../modules/form/topology/form.topology.tpl.html   |   4 +-
 .../app/src/common/modules/table/_table.scss       |  21 ++
 .../TableDeliveryServiceRequestsController.js      | 322 ++++++++++++++++++---
 .../table.deliveryServiceRequests.tpl.html         | 102 ++++---
 .../modules/table/jobs/TableJobsController.js      |   2 -
 .../TableAssignDeliveryServicesController.js       |  16 +-
 .../table.assignDeliveryServices.tpl.html          |   2 +-
 .../private/deliveryServiceRequests/list/index.js  |   5 +-
 .../api/controllers/CoverageZoneController.java    |   6 +-
 .../controllers/DeepCoverageZoneController.java    |   4 +-
 .../traffic_router/core/dns/ZoneManager.java       |   3 +-
 .../traffic_router/core/edge/Node.java             |  74 ++---
 .../traffic_router/core/router/TrafficRouter.java  | 153 +++++-----
 .../traffic_router/core/dns/ZoneManagerTest.java   |   5 +-
 .../traffic_router/core/external/SteeringTest.java |   4 +-
 .../traffic_router/core/loc/CoverageZoneTest.java  |  11 +-
 .../core/router/DNSRoutingMissesTest.java          |   7 +-
 .../core/router/TrafficRouterTest.java             |  69 ++++-
 30 files changed, 658 insertions(+), 295 deletions(-)
 copy traffic_portal/Gemfile => .github/actions/todb-tests/Dockerfile (89%)
 copy .github/actions/{todb-init => todb-tests}/README.md (65%)
 copy .github/actions/{go-fmt => todb-tests}/action.yml (90%)
 create mode 100755 .github/actions/todb-tests/entrypoint.sh
 copy .github/workflows/{go.fmt.yml => traffic.ops.database.yml} (78%)
 rename traffic_ops/app/db/migrations/{20200811082611_add_server_ip_profile_trigger.sql => 2020081108261100_add_server_ip_profile_trigger.sql} (100%)


[trafficcontrol] 02/04: Check cache availability by IP version instead of filtering later (#5186)

Posted by oc...@apache.org.
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 a5bc4ff25c1777f3314fbf08eaefffa5039a3510
Author: Rawlin Peters <ra...@apache.org>
AuthorDate: Mon Nov 2 15:02:32 2020 -0700

    Check cache availability by IP version instead of filtering later (#5186)
    
    * Check cache availability by IP version instead of filtering later
    
    If the returned caches turn out to all be unavailable for the client's
    IP version, we don't want to 503. Instead, we want to follow the regular
    cache availability logic (falling back to closest available location,
    etc).
    
    * Add changelog
    
    * De-duplicate line in test
    
    (cherry picked from commit d0c9699e586d9c4fcf3d94407d9028aebc8766df)
---
 CHANGELOG.md                                       |   2 +
 .../api/controllers/CoverageZoneController.java    |   6 +-
 .../controllers/DeepCoverageZoneController.java    |   4 +-
 .../traffic_router/core/dns/ZoneManager.java       |   3 +-
 .../traffic_router/core/edge/Node.java             |  74 +++-------
 .../traffic_router/core/router/TrafficRouter.java  | 153 ++++++++++-----------
 .../traffic_router/core/dns/ZoneManagerTest.java   |   5 +-
 .../traffic_router/core/external/SteeringTest.java |   4 +-
 .../traffic_router/core/loc/CoverageZoneTest.java  |  11 +-
 .../core/router/DNSRoutingMissesTest.java          |   7 +-
 .../core/router/TrafficRouterTest.java             |  69 ++++++++--
 11 files changed, 173 insertions(+), 165 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a968e10..6087eb8 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -117,6 +117,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - Fixed an issue with Traffic Router failing to authenticate if secrets are changed
 - Fixed validation error message for Traffic Ops `POST /api/x/profileparameters` route
 - Fixed #5216 - Removed duplicate button to link delivery service to server [Related Github issue](https://github.com/apache/trafficcontrol/issues/5216)
+- Fixed an issue where Traffic Router would erroneously return 503s or NXDOMAINs if the caches in a cachegroup were all unavailable for a client's requested IP version, rather than selecting caches from the next closest available cachegroup.
 
 ### Changed
 - Changed some Traffic Ops Go Client methods to use `DeliveryServiceNullable` inputs and outputs.
@@ -476,6 +477,7 @@ will be returned indicating that overlap exists.
 ### Changed
 - Reformatted this CHANGELOG file to the keep-a-changelog format
 
+[unreleased]: https://github.com/apache/trafficcontrol/compare/RELEASE-5.0.0...HEAD
 [5.0.0]: https://github.com/apache/trafficcontrol/compare/RELEASE-4.1.0...RELEASE-5.0.0
 [4.1.0]: https://github.com/apache/trafficcontrol/compare/RELEASE-4.0.0...RELEASE-4.1.0
 [4.0.0]: https://github.com/apache/trafficcontrol/compare/RELEASE-3.0.0...RELEASE-4.0.0
diff --git a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/api/controllers/CoverageZoneController.java b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/api/controllers/CoverageZoneController.java
index 918158f..53e60bf 100644
--- a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/api/controllers/CoverageZoneController.java
+++ b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/api/controllers/CoverageZoneController.java
@@ -17,6 +17,7 @@ package com.comcast.cdn.traffic_control.traffic_router.api.controllers;
 
 import com.comcast.cdn.traffic_control.traffic_router.core.edge.Cache;
 import com.comcast.cdn.traffic_control.traffic_router.core.edge.CacheLocation;
+import com.comcast.cdn.traffic_control.traffic_router.core.edge.Node.IPVersions;
 import com.comcast.cdn.traffic_control.traffic_router.core.router.TrafficRouterManager;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.http.HttpStatus;
@@ -38,7 +39,8 @@ public class CoverageZoneController {
 	public @ResponseBody
 	ResponseEntity<CacheLocation> getCacheLocationForIp(@RequestParam(name = "ip") final String ip,
 	                                    @RequestParam(name = "deliveryServiceId") final String deliveryServiceId) {
-		final CacheLocation cacheLocation = trafficRouterManager.getTrafficRouter().getCoverageZoneCacheLocation(ip, deliveryServiceId);
+		final IPVersions requestVersion = ip.contains(":") ? IPVersions.IPV6ONLY : IPVersions.IPV4ONLY;
+		final CacheLocation cacheLocation = trafficRouterManager.getTrafficRouter().getCoverageZoneCacheLocation(ip, deliveryServiceId, requestVersion);
 		if (cacheLocation == null) {
 			return ResponseEntity.status(HttpStatus.NOT_FOUND).body(null);
 		}
@@ -50,7 +52,7 @@ public class CoverageZoneController {
 	public @ResponseBody
 	ResponseEntity<List<Cache>> getCachesForDeliveryService(@RequestParam(name = "deliveryServiceId") final String deliveryServiceId,
 	                                                        @RequestParam(name = "cacheLocationId") final String cacheLocationId) {
-		final List<Cache> caches = trafficRouterManager.getTrafficRouter().selectCachesByCZ(deliveryServiceId, cacheLocationId, null);
+		final List<Cache> caches = trafficRouterManager.getTrafficRouter().selectCachesByCZ(deliveryServiceId, cacheLocationId, null, IPVersions.ANY);
 		if (caches == null || caches.isEmpty()) {
 			return ResponseEntity.status(HttpStatus.NOT_FOUND).body(null);
 		}
diff --git a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/api/controllers/DeepCoverageZoneController.java b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/api/controllers/DeepCoverageZoneController.java
index c45f119..71f9fa4 100644
--- a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/api/controllers/DeepCoverageZoneController.java
+++ b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/api/controllers/DeepCoverageZoneController.java
@@ -16,6 +16,7 @@
 package com.comcast.cdn.traffic_control.traffic_router.api.controllers;
 
 import com.comcast.cdn.traffic_control.traffic_router.core.edge.CacheLocation;
+import com.comcast.cdn.traffic_control.traffic_router.core.edge.Node.IPVersions;
 import com.comcast.cdn.traffic_control.traffic_router.core.router.TrafficRouterManager;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.http.HttpStatus;
@@ -35,7 +36,8 @@ public class DeepCoverageZoneController {
     public @ResponseBody
     ResponseEntity<CacheLocation> getCacheLocationForIp(@RequestParam(name = "ip") final String ip,
                                                         @RequestParam(name = "deliveryServiceId") final String deliveryServiceId) {
-        final CacheLocation cacheLocation = trafficRouterManager.getTrafficRouter().getCoverageZoneCacheLocation(ip, deliveryServiceId, true, null);
+        final IPVersions requestVersion = ip.contains(":") ? IPVersions.IPV6ONLY : IPVersions.IPV4ONLY;
+        final CacheLocation cacheLocation = trafficRouterManager.getTrafficRouter().getCoverageZoneCacheLocation(ip, deliveryServiceId, true, null, requestVersion);
         if (cacheLocation == null) {
             return ResponseEntity.status(HttpStatus.NOT_FOUND).body(null);
         }
diff --git a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/dns/ZoneManager.java b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/dns/ZoneManager.java
index fdbcf53..64ee8b8 100644
--- a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/dns/ZoneManager.java
+++ b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/dns/ZoneManager.java
@@ -46,6 +46,7 @@ import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
 
+import com.comcast.cdn.traffic_control.traffic_router.core.edge.Node.IPVersions;
 import com.comcast.cdn.traffic_control.traffic_router.core.util.JsonUtils;
 import com.comcast.cdn.traffic_control.traffic_router.core.util.JsonUtilsException;
 import com.fasterxml.jackson.databind.JsonNode;
@@ -589,7 +590,7 @@ public class ZoneManager extends Resolver {
 		request.setHostname(edgeName.toString(true)); // Name.toString(true) - omit the trailing dot
 
 		for (final CacheLocation cacheLocation : data.getCacheLocations()) {
-			final List<Cache> caches = tr.selectCachesByCZ(ds, cacheLocation);
+			final List<Cache> caches = tr.selectCachesByCZ(ds, cacheLocation, IPVersions.ANY);
 
 			if (caches == null) {
 				continue;
diff --git a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/edge/Node.java b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/edge/Node.java
index a50cb99..0c92dab 100644
--- a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/edge/Node.java
+++ b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/edge/Node.java
@@ -37,14 +37,18 @@ public class Node extends DefaultHashable {
 	private static final Logger LOGGER = Logger.getLogger(Node.class);
 	private static final int REPLICAS = 1000;
 
-	public enum IpVersions {
-		IPV4ONLY, IPV6ONLY, BOTH
+	public enum IPVersions {
+		IPV4ONLY, IPV6ONLY, ANY
 	}
-	private IpVersions ipAvailableVersions;
 	protected final String id;
 	private String fqdn;
 	private List<InetRecord> ipAddresses;
-	private List<InetRecord> unavailableIpAddresses;
+	private InetAddress ip4;
+	private InetAddress ip6;
+	private boolean isAvailable = false;
+	private boolean ipv4Available = true;
+	private boolean ipv6Available = true;
+	private boolean hasAuthority = false;
 	private int port;
 	private final Map<String, Cache.DeliveryServiceReference> deliveryServices = new HashMap<>();
 	private final Set<String> capabilities = new HashSet<>();
@@ -158,8 +162,6 @@ public class Node extends DefaultHashable {
 		return "Node [id=" + id + "] ";
 	}
 
-	boolean isAvailable = false;
-	boolean hasAuthority = false;
 	public void setIsAvailable(final boolean isAvailable) {
 		this.hasAuthority = true;
 		this.isAvailable = isAvailable;
@@ -170,11 +172,18 @@ public class Node extends DefaultHashable {
 	public boolean isAvailable() {
 		return isAvailable;
 	}
-	InetAddress ip4;
-	InetAddress ip6;
+	public boolean isAvailable(final IPVersions requestVersion) {
+	    switch (requestVersion) {
+			case IPV4ONLY:
+			    return isAvailable && ipv4Available;
+			case IPV6ONLY:
+			    return isAvailable && ipv6Available;
+			default:
+			    return isAvailable;
+		}
+	}
 	public void setIpAddress(final String ip, final String ip6, final long ttl) throws UnknownHostException {
 		this.ipAddresses = new ArrayList<InetRecord>();
-		this.unavailableIpAddresses = new ArrayList<InetRecord>();
 
 		if (ip != null && !ip.isEmpty()) {
 			this.ip4 = InetAddress.getByName(ip);
@@ -200,8 +209,6 @@ public class Node extends DefaultHashable {
 
 	@SuppressWarnings({"PMD.CyclomaticComplexity", "PMD.NPathComplexity"})
 	public void setState(final JsonNode state) {
-		final boolean ipv4Available;
-		final boolean ipv6Available;
 		if (state == null) {
 			LOGGER.warn("got null health state for " + fqdn + ". Setting it to unavailable!");
 			isAvailable = false;
@@ -212,44 +219,7 @@ public class Node extends DefaultHashable {
 			ipv4Available = JsonUtils.optBoolean(state, "ipv4Available", true);
 			ipv6Available = JsonUtils.optBoolean(state, "ipv6Available", true);
 		}
-
-		final List<InetRecord> newlyAvailable = new ArrayList<>();
-		final List<InetRecord> newlyUnavailable = new ArrayList<>();
-
-		if (ipv4Available && !ipv6Available) {
-			this.ipAvailableVersions = IpVersions.IPV4ONLY;
-		} else if (ipv6Available && !ipv4Available) {
-			this.ipAvailableVersions = IpVersions.IPV6ONLY;
-		} else {
-			this.ipAvailableVersions = IpVersions.BOTH;
-		}
-
-		for (final InetRecord record : ipAddresses) {
-			if (record.getAddress().equals(ip4) && !ipv4Available) {
-				newlyUnavailable.add(record);
-			}
-			if (record.getAddress().equals(ip6) && !ipv6Available) {
-				newlyUnavailable.add(record);
-			}
-		}
-
-		for (final InetRecord record : unavailableIpAddresses) {
-			if (record.getAddress().equals(ip4) && ipv4Available) {
-				newlyAvailable.add(record);
-			}
-			if (record.getAddress().equals(ip6) && ipv6Available) {
-				newlyAvailable.add(record);
-			}
-		}
-
-		ipAddresses.addAll(newlyAvailable);
-		ipAddresses.removeAll(newlyUnavailable);
-		unavailableIpAddresses.addAll(newlyUnavailable);
-		unavailableIpAddresses.removeAll(newlyAvailable);
-
 		this.setIsAvailable(isAvailable);
-
-
 	}
 
 	public int getHttpsPort() {
@@ -259,12 +229,4 @@ public class Node extends DefaultHashable {
 	public void setHttpsPort(final int httpsPort) {
 		this.httpsPort = httpsPort;
 	}
-
-	public IpVersions getIpAvailableVersions() {
-		return ipAvailableVersions;
-	}
-
-	public void setIpAvailableVersions(final IpVersions ipAvailableVersions) {
-		this.ipAvailableVersions = ipAvailableVersions;
-	}
 }
diff --git a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/router/TrafficRouter.java b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/router/TrafficRouter.java
index d5ee972..67cb266 100644
--- a/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/router/TrafficRouter.java
+++ b/traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/router/TrafficRouter.java
@@ -31,6 +31,7 @@ import com.comcast.cdn.traffic_control.traffic_router.core.edge.CacheRegister;
 import com.comcast.cdn.traffic_control.traffic_router.core.edge.InetRecord;
 import com.comcast.cdn.traffic_control.traffic_router.core.edge.Location;
 import com.comcast.cdn.traffic_control.traffic_router.core.edge.Node;
+import com.comcast.cdn.traffic_control.traffic_router.core.edge.Node.IPVersions;
 import com.comcast.cdn.traffic_control.traffic_router.core.edge.TrafficRouterLocation;
 import com.comcast.cdn.traffic_control.traffic_router.core.hash.ConsistentHasher;
 import com.comcast.cdn.traffic_control.traffic_router.core.loc.AnonymousIp;
@@ -194,7 +195,7 @@ public class TrafficRouter {
 	 *            the DeliveryService to check
 	 * @return collection of supported caches
 	 */
-	public List<Cache> getSupportingCaches(final List<Cache> caches, final DeliveryService ds) {
+	public List<Cache> getSupportingCaches(final List<Cache> caches, final DeliveryService ds, final IPVersions requestVersion) {
 		final List<Cache> supportingCaches = new ArrayList<Cache>();
 
 		for (final Cache cache : caches) {
@@ -202,7 +203,7 @@ public class TrafficRouter {
 				continue;
 			}
 
-			if (cache.hasAuthority() ? cache.isAvailable() : true) {
+			if (!cache.hasAuthority() || (cache.isAvailable(requestVersion))) {
 				supportingCaches.add(cache);
 			}
 		}
@@ -324,7 +325,7 @@ public class TrafficRouter {
 	 * only "Maxmind" and "Neustar" are supported)
 	 * @param deliveryServiceId Currently only used for logging error information, should be an
 	 * identifier for a Delivery Service
-	 * @return A {@link #GeolocationService} that can be used to geo-locate clients <em>or</em>
+	 * @return A {@link GeolocationService} that can be used to geo-locate clients <em>or</em>
 	 * {@code null} if an error occurs.
 	 */
 	private GeolocationService getGeolocationService(final String geolocationProvider, final String deliveryServiceId) {
@@ -379,7 +380,7 @@ public class TrafficRouter {
 	}
 
 	/**
-	 * Gets a {@link #List} of {@link Cache}s that are capabable of serving a given Delivery Service.
+	 * Gets a {@link List} of {@link Cache}s that are capabable of serving a given Delivery Service.
 	 * <p>
 	 * The caches chosen are from the closest, non-empty, cache location to the client's physical
 	 * location, up to the Location Limit ({@link DeliveryService#getLocationLimit()}) of the
@@ -387,11 +388,11 @@ public class TrafficRouter {
 	 * </p>
 	 * @param ds The Delivery Service being served.
 	 * @param clientLocation The physical location of the requesting client.
-	 * @param track The {@link #Track} object on which a result location shall be set, should one be found
-	 * @return A {@link #List} of {@link Cache}s that should be used to service a request should such a collection be found, or
+	 * @param track The {@link Track} object on which a result location shall be set, should one be found
+	 * @return A {@link List} of {@link Cache}s that should be used to service a request should such a collection be found, or
 	 * {@code null} if the no applicable {@link Cache}s could be found.
 	 */
-	public List<Cache> getCachesByGeo(final DeliveryService ds, final Geolocation clientLocation, final Track track) throws GeolocationException {
+	public List<Cache> getCachesByGeo(final DeliveryService ds, final Geolocation clientLocation, final Track track, final IPVersions requestVersion) throws GeolocationException {
 		int locationsTested = 0;
 
 		final int locationLimit = ds.getLocationLimit();
@@ -401,7 +402,7 @@ public class TrafficRouter {
 		final List<CacheLocation> cacheLocations = (List<CacheLocation>) orderLocations(cacheLocations1, clientLocation);
 
 		for (final CacheLocation location : cacheLocations) {
-			final List<Cache> caches = selectCaches(location, ds);
+			final List<Cache> caches = selectCaches(location, ds, requestVersion);
 			if (caches != null) {
 				track.setResultLocation(location.getGeolocation());
 				if (track.getResultLocation().equals(GEO_ZERO_ZERO)) {
@@ -427,7 +428,7 @@ public class TrafficRouter {
 	 * </p>
 	 * @param request The HTTP request made by the client.
 	 * @param ds The Delivery Service being served.
-	 * @param track The {@link #Track} object that tracks how requests are served
+	 * @param track The {@link Track} object that tracks how requests are served
 	 */
 	protected List<Cache> selectCaches(final HTTPRequest request, final DeliveryService ds, final Track track) throws GeolocationException {
 		return selectCaches(request, ds, track, true);
@@ -437,7 +438,7 @@ public class TrafficRouter {
 	 * Selects {@link Cache}s to serve a request for a Delivery Service.
 	 * @param request The HTTP request made by the client.
 	 * @param ds The Delivery Service being served.
-	 * @param track The {@link #Track} object that tracks how requests are served
+	 * @param track The {@link Track} object that tracks how requests are served
 	 * @param enableDeep Sets whether or not "Deep Caching" may be used.
 	 */
 	@SuppressWarnings("PMD.CyclomaticComplexity")
@@ -445,23 +446,24 @@ public class TrafficRouter {
 		CacheLocation cacheLocation;
 		ResultType result = ResultType.CZ;
 		final boolean useDeep = enableDeep && (ds.getDeepCache() == DeliveryService.DeepCachingType.ALWAYS);
+		final IPVersions requestVersion = request.getClientIP().contains(":") ? IPVersions.IPV6ONLY : IPVersions.IPV4ONLY;
 
 		if (useDeep) {
 			// Deep caching is enabled. See if there are deep caches available
-			cacheLocation = getDeepCoverageZoneCacheLocation(request.getClientIP(), ds);
+			cacheLocation = getDeepCoverageZoneCacheLocation(request.getClientIP(), ds, requestVersion);
 			if (cacheLocation != null && cacheLocation.getCaches().size() != 0) {
 				// Found deep caches for this client, and there are caches that might be available there.
 				result = ResultType.DEEP_CZ;
 			} else {
 				// No deep caches for this client, would have used them if there were any. Fallback to regular CZ
-				cacheLocation = getCoverageZoneCacheLocation(request.getClientIP(), ds);
+				cacheLocation = getCoverageZoneCacheLocation(request.getClientIP(), ds, requestVersion);
 			}
 		} else {
 			// Deep caching not enabled for this Delivery Service; use the regular CZ
-			cacheLocation = getCoverageZoneCacheLocation(request.getClientIP(), ds, useDeep, track);
+			cacheLocation = getCoverageZoneCacheLocation(request.getClientIP(), ds, false, track, requestVersion);
 		}
 
-		List<Cache>caches = selectCachesByCZ(ds, cacheLocation, track, result);
+		List<Cache>caches = selectCachesByCZ(ds, cacheLocation, track, result, requestVersion);
 
 		if (caches != null) {
 			return caches;
@@ -470,14 +472,14 @@ public class TrafficRouter {
 		if (ds.isCoverageZoneOnly()) {
 			if (ds.getGeoRedirectUrl() != null) {
 				//use the NGB redirect
-				caches = enforceGeoRedirect(track, ds, request.getClientIP(), null);
+				caches = enforceGeoRedirect(track, ds, request.getClientIP(), null, requestVersion);
 			} else {
 				track.setResult(ResultType.MISS);
 				track.setResultDetails(ResultDetails.DS_CZ_ONLY);
 			}
 		} else if (track.continueGeo) {
 			// continue Geo can be disabled when backup group is used -- ended up an empty cache list if reach here
-			caches = selectCachesByGeo(request.getClientIP(), ds, cacheLocation, track);
+			caches = selectCachesByGeo(request.getClientIP(), ds, cacheLocation, track, requestVersion);
 		}
 
 		return caches;
@@ -500,9 +502,9 @@ public class TrafficRouter {
 	 * @param deliveryService The Delivery Service being served.
 	 * @param cacheLocation A selected {@link CacheLocation} from which {@link Cache}s will be
 	 * extracted based on the client's location.
-	 * @param track The {@link #Track} object that tracks how requests are served
+	 * @param track The {@link Track} object that tracks how requests are served
 	 */
-	public List<Cache> selectCachesByGeo(final String clientIp, final DeliveryService deliveryService, final CacheLocation cacheLocation, final Track track) throws GeolocationException {
+	public List<Cache> selectCachesByGeo(final String clientIp, final DeliveryService deliveryService, final CacheLocation cacheLocation, final Track track, final IPVersions requestVersion) throws GeolocationException {
 		Geolocation clientLocation = null;
 
 		try {
@@ -517,7 +519,7 @@ public class TrafficRouter {
 				LOGGER.debug(String
 						.format("client is blocked by geolimit, use the NGB redirect url: %s",
 								deliveryService.getGeoRedirectUrl()));
-				return enforceGeoRedirect(track, deliveryService, clientIp, track.getClientGeolocation());
+				return enforceGeoRedirect(track, deliveryService, clientIp, track.getClientGeolocation(), requestVersion);
 			} else {
 				track.setResultDetails(ResultDetails.DS_CLIENT_GEO_UNSUPPORTED);
 				return null;
@@ -534,7 +536,7 @@ public class TrafficRouter {
 			}
 		}
 
-		final List<Cache> caches = getCachesByGeo(deliveryService, clientLocation, track);
+		final List<Cache> caches = getCachesByGeo(deliveryService, clientLocation, track, requestVersion);
 
 		if (caches == null || caches.isEmpty()) {
 			track.setResultDetails(ResultDetails.GEO_NO_CACHE_FOUND);
@@ -737,8 +739,9 @@ public class TrafficRouter {
 			return result;
 		}
 
-		final CacheLocation cacheLocation = getCoverageZoneCacheLocation(request.getClientIP(), ds, false, track);
-		List<Cache> caches = selectCachesByCZ(ds, cacheLocation, track);
+		final IPVersions requestVersion = request.getQueryType() == Type.AAAA ? IPVersions.IPV6ONLY : IPVersions.IPV4ONLY;
+		final CacheLocation cacheLocation = getCoverageZoneCacheLocation(request.getClientIP(), ds, false, track, requestVersion);
+		List<Cache> caches = selectCachesByCZ(ds, cacheLocation, track, requestVersion);
 
 		if (caches != null) {
 			track.setResult(ResultType.CZ);
@@ -769,7 +772,7 @@ public class TrafficRouter {
 		}
 
 		if (track.continueGeo) {
-			caches = selectCachesByGeo(request.getClientIP(), ds, cacheLocation, track);
+			caches = selectCachesByGeo(request.getClientIP(), ds, cacheLocation, track, requestVersion);
 		}
 
 		if (caches != null) {
@@ -934,15 +937,15 @@ public class TrafficRouter {
 	 * Selects caches to service requests for a Delivery Service from a cache location based on
 	 * Coverage Zone configuration.
 	 * <p>
-	 * This is equivalent to calling {@link #selectCachesByCZ(DeliveryService, CacheLocation, Track)}
+	 * This is equivalent to calling {@link #selectCachesByCZ(DeliveryService, CacheLocation, Track, IPVersions)}
 	 * with a 'null' "track" argument.
 	 * </p>
 	 * @param ds The Delivery Service being served.
 	 * @param cacheLocation The location from which caches will be selected.
 	 * @return All of the caches in the given location capable of serving ds.
 	 */
-	public List<Cache> selectCachesByCZ(final DeliveryService ds, final CacheLocation cacheLocation) {
-		return selectCachesByCZ(ds, cacheLocation, null);
+	public List<Cache> selectCachesByCZ(final DeliveryService ds, final CacheLocation cacheLocation, final IPVersions requestVersion) {
+		return selectCachesByCZ(ds, cacheLocation, null, requestVersion);
 	}
 
 	/**
@@ -954,23 +957,23 @@ public class TrafficRouter {
 	 * @return All of the caches in the given location capable of serving the identified Delivery
 	 * Service.
 	 */
-	public List<Cache> selectCachesByCZ(final String deliveryServiceId, final String cacheLocationId, final Track track) {
-		return selectCachesByCZ(cacheRegister.getDeliveryService(deliveryServiceId), cacheRegister.getCacheLocation(cacheLocationId), track);
+	public List<Cache> selectCachesByCZ(final String deliveryServiceId, final String cacheLocationId, final Track track, final IPVersions requestVersion) {
+		return selectCachesByCZ(cacheRegister.getDeliveryService(deliveryServiceId), cacheRegister.getCacheLocation(cacheLocationId), track, requestVersion);
 	}
 
 	/**
 	 * Selects caches to service requests for a Delivery Service from a cache location based on
 	 * Coverage Zone Configuration.
 	 * <p>
-	 * This is equivalent to calling {@link #selectCachesByCZ(DeliveryService, CacheLocation, Track, ResultType)}
-	 * with the "result" argument set to {@link #ResultType.CZ}.
+	 * This is equivalent to calling {@link #selectCachesByCZ(DeliveryService, CacheLocation, Track, ResultType, IPVersions)}
+	 * with the "result" argument set to {@link ResultType#CZ}.
 	 * </p>
 	 * @param ds The Delivery Service being served.
 	 * @param cacheLocation The location from which caches will be selected
 	 * @return All of the caches in the given location capable of serving ds.
 	 */
-	private List<Cache> selectCachesByCZ(final DeliveryService ds, final CacheLocation cacheLocation, final Track track) {
-		return selectCachesByCZ(ds, cacheLocation, track, ResultType.CZ); // ResultType.CZ was the original default before DDC
+	private List<Cache> selectCachesByCZ(final DeliveryService ds, final CacheLocation cacheLocation, final Track track, final IPVersions requestVersion) {
+		return selectCachesByCZ(ds, cacheLocation, track, ResultType.CZ, requestVersion); // ResultType.CZ was the original default before DDC
 	}
 
 	/**
@@ -980,7 +983,7 @@ public class TrafficRouter {
 	 * Obviously, at this point, the location from which to select caches must already be known.
 	 * So it's totally possible that that decision wasn't made based on Coverage Zones at all,
 	 * that's just the default routing result chosen by a common caller of this method
-	 * ({@link #selectCachesByCZ(DeliveryService, CacheLocation, Track)}).
+	 * ({@link #selectCachesByCZ(DeliveryService, CacheLocation, Track, IPVersions)}).
 	 * </p>
 	 * @param ds The Delivery Service being served.
 	 * @param cacheLocation The location from which caches will be selected.
@@ -988,12 +991,12 @@ public class TrafficRouter {
 	 * This is used for tracking routing results.
 	 * @return All of the caches in the given location capable of serving ds.
 	 */
-	private List<Cache> selectCachesByCZ(final DeliveryService ds, final CacheLocation cacheLocation, final Track track, final ResultType result) {
+	private List<Cache> selectCachesByCZ(final DeliveryService ds, final CacheLocation cacheLocation, final Track track, final ResultType result, final IPVersions requestVersion) {
 		if (cacheLocation == null || ds == null || !ds.isLocationAvailable(cacheLocation)) {
 			return null;
 		}
 
-		final List<Cache> caches = selectCaches(cacheLocation, ds);
+		final List<Cache> caches = selectCaches(cacheLocation, ds, requestVersion);
 
 		if (caches != null && track != null) {
 			track.setResult(result);
@@ -1043,9 +1046,6 @@ public class TrafficRouter {
 			final DeliveryService ds = steeringResult.getDeliveryService();
 			List<Cache> caches = selectCaches(request, ds, track);
 
-			if (caches != null) {
-				caches = editCacheListForIpVersion(!request.getClientIP().contains(":"), caches);
-			}
 			// child Delivery Services can use their query parameters
 			final String pathToHash = steeringHash + ds.extractSignificantQueryParams(request);
 
@@ -1193,11 +1193,7 @@ public class TrafficRouter {
 
 		routeResult.setDeliveryService(deliveryService);
 
-		List<Cache> caches = selectCaches(request, deliveryService, track);
-		if (caches != null) {
-			caches = editCacheListForIpVersion(!request.getClientIP().contains(":"), caches);
-		}
-
+		final List<Cache> caches = selectCaches(request, deliveryService, track);
 		if (caches == null || caches.isEmpty()) {
 			if (track.getResult() == ResultType.GEO_REDIRECT) {
 				routeResult.setUrl(new URL(deliveryService.getGeoRedirectUrl()));
@@ -1235,18 +1231,6 @@ public class TrafficRouter {
 		return routeResult;
 	}
 
-	private List<Cache> editCacheListForIpVersion(final boolean requestIsIpv4, final List<Cache> caches) {
-		final List<Cache> removeCaches = new ArrayList<>();
-		for (final Cache cache : caches) {
-			if ((!requestIsIpv4 && cache.getIpAvailableVersions() == Cache.IpVersions.IPV4ONLY) ||
-					requestIsIpv4 && cache.getIpAvailableVersions() == Cache.IpVersions.IPV6ONLY) {
-				removeCaches.add(cache);
-			}
-		}
-		caches.removeAll(removeCaches);
-		return caches;
-	}
-
 	/**
 	 * Gets all the possible steering results for a request to a Delivery Service.
 	 * @param request The client HTTP request.
@@ -1361,12 +1345,12 @@ public class TrafficRouter {
 		return null;
 	}
 
-	public CacheLocation getCoverageZoneCacheLocation(final String ip, final String deliveryServiceId) {
-		return getCoverageZoneCacheLocation(ip, deliveryServiceId, false, null); // default is not deep
+	public CacheLocation getCoverageZoneCacheLocation(final String ip, final String deliveryServiceId, final IPVersions requestVersion) {
+		return getCoverageZoneCacheLocation(ip, deliveryServiceId, false, null, requestVersion); // default is not deep
 	}
 
 	@SuppressWarnings({"PMD.CyclomaticComplexity", "PMD.NPathComplexity"})
-	public CacheLocation getCoverageZoneCacheLocation(final String ip, final String deliveryServiceId, final boolean useDeep, final Track track) {
+	public CacheLocation getCoverageZoneCacheLocation(final String ip, final String deliveryServiceId, final boolean useDeep, final Track track, final IPVersions requestVersion) {
 		final NetworkNode networkNode = useDeep ? getDeepNetworkNode(ip) : getNetworkNode(ip);
 		final LocalizationMethod localizationMethod = useDeep ? LocalizationMethod.DEEP_CZ : LocalizationMethod.CZ;
 
@@ -1386,7 +1370,7 @@ public class TrafficRouter {
 			return null;
 		}
 
-		if (cacheLocation != null && !getSupportingCaches(cacheLocation.getCaches(), deliveryService).isEmpty()) {
+		if (cacheLocation != null && !getSupportingCaches(cacheLocation.getCaches(), deliveryService, requestVersion).isEmpty()) {
 			return cacheLocation;
 		}
 
@@ -1406,7 +1390,7 @@ public class TrafficRouter {
 			return null;
 		}
 
-		if (cacheLocation != null && !getSupportingCaches(cacheLocation.getCaches(), deliveryService).isEmpty()) {
+		if (cacheLocation != null && !getSupportingCaches(cacheLocation.getCaches(), deliveryService, requestVersion).isEmpty()) {
 			// lazy loading in case a CacheLocation has not yet been associated with this NetworkNode
 			networkNode.setLocation(cacheLocation);
 			return cacheLocation;
@@ -1418,7 +1402,7 @@ public class TrafficRouter {
 				if (bkCacheLocation != null && !bkCacheLocation.isEnabledFor(localizationMethod)) {
 					continue;
 				}
-				if (bkCacheLocation != null && !getSupportingCaches(bkCacheLocation.getCaches(), deliveryService).isEmpty()) {
+				if (bkCacheLocation != null && !getSupportingCaches(bkCacheLocation.getCaches(), deliveryService, requestVersion).isEmpty()) {
 					LOGGER.debug("Got backup CZ cache group " + bkCacheLocation.getId() + " for " + ip + ", ds " + deliveryServiceId);
 					if (track != null) {
 						track.setFromBackupCzGroup(true);
@@ -1439,7 +1423,7 @@ public class TrafficRouter {
 		// Check whether the CZF entry has a geolocation and use it if so.
 		List<CacheLocation> availableLocations = cacheRegister.filterAvailableCacheLocations(deliveryServiceId);
 		availableLocations = filterEnabledLocations(availableLocations, localizationMethod);
-		final CacheLocation closestCacheLocation = getClosestCacheLocation(availableLocations, networkNode.getGeolocation(), cacheRegister.getDeliveryService(deliveryServiceId));
+		final CacheLocation closestCacheLocation = getClosestCacheLocation(availableLocations, networkNode.getGeolocation(), cacheRegister.getDeliveryService(deliveryServiceId), requestVersion);
 
 		if (closestCacheLocation != null) {
 			LOGGER.debug("Got closest CZ cache group " + closestCacheLocation.getId() + " for " + ip + ", ds " + deliveryServiceId);
@@ -1456,16 +1440,16 @@ public class TrafficRouter {
 				.collect(Collectors.toList());
 	}
 
-	public CacheLocation getDeepCoverageZoneCacheLocation(final String ip, final DeliveryService deliveryService) {
-		return getCoverageZoneCacheLocation(ip, deliveryService, true, null);
+	public CacheLocation getDeepCoverageZoneCacheLocation(final String ip, final DeliveryService deliveryService, final IPVersions requestVersion) {
+		return getCoverageZoneCacheLocation(ip, deliveryService, true, null, requestVersion);
 	}
 
-	public CacheLocation getCoverageZoneCacheLocation(final String ip, final DeliveryService deliveryService, final boolean useDeep, final Track track) {
-		return getCoverageZoneCacheLocation(ip, deliveryService.getId(), useDeep, track);
+	public CacheLocation getCoverageZoneCacheLocation(final String ip, final DeliveryService deliveryService, final boolean useDeep, final Track track, final IPVersions requestVersion) {
+		return getCoverageZoneCacheLocation(ip, deliveryService.getId(), useDeep, track, requestVersion);
 	}
 
-	public CacheLocation getCoverageZoneCacheLocation(final String ip, final DeliveryService deliveryService) {
-		return getCoverageZoneCacheLocation(ip, deliveryService.getId());
+	public CacheLocation getCoverageZoneCacheLocation(final String ip, final DeliveryService deliveryService, final IPVersions requestVersion) {
+		return getCoverageZoneCacheLocation(ip, deliveryService.getId(), requestVersion);
 	}
 
 	/**
@@ -1516,8 +1500,9 @@ public class TrafficRouter {
 			return null;
 		}
 
-		final CacheLocation coverageZoneCacheLocation = getCoverageZoneCacheLocation(ip, deliveryService, useDeep, null);
-		final List<Cache> caches = selectCachesByCZ(deliveryService, coverageZoneCacheLocation);
+		final IPVersions requestVersion = ip.contains(":") ? IPVersions.IPV6ONLY : IPVersions.IPV4ONLY;
+		final CacheLocation coverageZoneCacheLocation = getCoverageZoneCacheLocation(ip, deliveryService, useDeep, null, requestVersion);
+		final List<Cache> caches = selectCachesByCZ(deliveryService, coverageZoneCacheLocation, requestVersion);
 
 		if (caches == null || caches.isEmpty()) {
 			return null;
@@ -1558,15 +1543,16 @@ public class TrafficRouter {
 			return null;
 		}
 
+		final IPVersions requestVersion = ip.contains(":") ? IPVersions.IPV6ONLY : IPVersions.IPV4ONLY;
 		List<Cache> caches = null;
 		if (deliveryService.isCoverageZoneOnly() && deliveryService.getGeoRedirectUrl() != null) {
 			//use the NGB redirect
-			caches = enforceGeoRedirect(StatTracker.getTrack(), deliveryService, ip, null);
+			caches = enforceGeoRedirect(StatTracker.getTrack(), deliveryService, ip, null, requestVersion);
 		} else {
-			final CacheLocation cacheLocation = getCoverageZoneCacheLocation(ip, deliveryServiceId);
+			final CacheLocation cacheLocation = getCoverageZoneCacheLocation(ip, deliveryServiceId, requestVersion);
 
 			try {
-				caches = selectCachesByGeo(ip, deliveryService, cacheLocation, StatTracker.getTrack());
+				caches = selectCachesByGeo(ip, deliveryService, cacheLocation, StatTracker.getTrack(), requestVersion);
 			} catch (GeolocationException e) {
 				LOGGER.warn("Failed gettting list of caches by geolocation for ip " + ip + " delivery service id '" + deliveryServiceId + "'");
 			}
@@ -1712,7 +1698,7 @@ public class TrafficRouter {
 	 * File given a clients IP and request.
 	 * @param ip The client's IP address
 	 * @param deliveryServiceId The "xml_id" of a Delivery Service being routed
-	 * @param requestPath The client's HTTP request
+	 * @param request The client's HTTP request
 	 * @return A cache object chosen to serve the client's request
 	 */
 	public Cache consistentHashSteeringForCoverageZone(final String ip, final String deliveryServiceId, final HTTPRequest request) {
@@ -1722,8 +1708,9 @@ public class TrafficRouter {
 			return null;
 		}
 
-		final CacheLocation coverageZoneCacheLocation = getCoverageZoneCacheLocation(ip, deliveryService, false, null);
-		final List<Cache> caches = selectCachesByCZ(deliveryService, coverageZoneCacheLocation);
+		final IPVersions requestVersion = ip.contains(":") ? IPVersions.IPV6ONLY : IPVersions.IPV4ONLY;
+		final CacheLocation coverageZoneCacheLocation = getCoverageZoneCacheLocation(ip, deliveryService, false, null, requestVersion);
+		final List<Cache> caches = selectCachesByCZ(deliveryService, coverageZoneCacheLocation, requestVersion);
 
 		if (caches == null || caches.isEmpty()) {
 			return null;
@@ -1763,7 +1750,7 @@ public class TrafficRouter {
 	 * Chooses a target Delivery Service of a given Delivery Service to service a given request and
 	 * {@link #XTC_STEERING_OPTION} value.
 	 *
-	 * @param deliveryServiceId The "xml_id" of the Delivery Service being requested
+	 * @param deliveryService The DeliveryService being requested
 	 * @param request The client's HTTP request
 	 * @param xtcSteeringOption The value of the client's {@link #XTC_STEERING_OPTION} HTTP Header.
 	 * @return The chosen target Delivery Service, or null if one could not be determined.
@@ -1822,7 +1809,7 @@ public class TrafficRouter {
 		return locations;
 	}
 
-	private CacheLocation getClosestCacheLocation(final List<CacheLocation> cacheLocations, final Geolocation clientLocation, final DeliveryService deliveryService) {
+	private CacheLocation getClosestCacheLocation(final List<CacheLocation> cacheLocations, final Geolocation clientLocation, final DeliveryService deliveryService, final IPVersions requestVersion) {
 		if (clientLocation == null) {
 			return null;
 		}
@@ -1830,7 +1817,7 @@ public class TrafficRouter {
 	    final List<CacheLocation> orderedLocations = (List<CacheLocation>) orderLocations(cacheLocations, clientLocation);
 
 		for (final CacheLocation cacheLocation : orderedLocations) {
-			if (!getSupportingCaches(cacheLocation.getCaches(), deliveryService).isEmpty()) {
+			if (!getSupportingCaches(cacheLocation.getCaches(), deliveryService, requestVersion).isEmpty()) {
 				return cacheLocation;
 			}
 		}
@@ -1847,12 +1834,12 @@ public class TrafficRouter {
 	 *            the delivery service for the request
 	 * @return the selected cache or null if none can be found
 	 */
-	private List<Cache> selectCaches(final CacheLocation location, final DeliveryService ds) {
+	private List<Cache> selectCaches(final CacheLocation location, final DeliveryService ds, final IPVersions requestVersion) {
 		if (LOGGER.isDebugEnabled()) {
 			LOGGER.debug("Trying location: " + location.getId());
 		}
 
-		final List<Cache> caches = getSupportingCaches(location.getCaches(), ds);
+		final List<Cache> caches = getSupportingCaches(location.getCaches(), ds, requestVersion);
 		if (caches.isEmpty()) {
 			if (LOGGER.isDebugEnabled()) {
 				LOGGER.debug("No online, supporting caches were found at location: "
@@ -1902,7 +1889,7 @@ public class TrafficRouter {
 		return dnssecZoneDiffingEnabled;
 	}
 
-	private List<Cache> enforceGeoRedirect(final Track track, final DeliveryService ds, final String clientIp, final Geolocation queriedClientLocation) {
+	private List<Cache> enforceGeoRedirect(final Track track, final DeliveryService ds, final String clientIp, final Geolocation queriedClientLocation, final IPVersions requestVersion) {
 		final String urlType = ds.getGeoRedirectUrlType();
 		track.setResult(ResultType.GEO_REDIRECT);
 
@@ -1944,7 +1931,7 @@ public class TrafficRouter {
 		List<Cache> caches = null;
 
 		try {
-			caches = getCachesByGeo(ds, clientLocation, track);
+			caches = getCachesByGeo(ds, clientLocation, track, requestVersion);
 		} catch (GeolocationException e) {
 			LOGGER.error("Failed getting caches by geolocation " + e.getMessage());
 		}
diff --git a/traffic_router/core/src/test/java/com/comcast/cdn/traffic_control/traffic_router/core/dns/ZoneManagerTest.java b/traffic_router/core/src/test/java/com/comcast/cdn/traffic_control/traffic_router/core/dns/ZoneManagerTest.java
index fbaa618..a82025b 100644
--- a/traffic_router/core/src/test/java/com/comcast/cdn/traffic_control/traffic_router/core/dns/ZoneManagerTest.java
+++ b/traffic_router/core/src/test/java/com/comcast/cdn/traffic_control/traffic_router/core/dns/ZoneManagerTest.java
@@ -51,6 +51,7 @@ import com.comcast.cdn.traffic_control.traffic_router.core.TestBase;
 import com.comcast.cdn.traffic_control.traffic_router.core.edge.Cache;
 import com.comcast.cdn.traffic_control.traffic_router.core.edge.CacheLocation;
 import com.comcast.cdn.traffic_control.traffic_router.core.edge.CacheRegister;
+import com.comcast.cdn.traffic_control.traffic_router.core.edge.Node.IPVersions;
 import com.comcast.cdn.traffic_control.traffic_router.core.ds.DeliveryService;
 import com.comcast.cdn.traffic_control.traffic_router.core.router.TrafficRouter;
 import com.comcast.cdn.traffic_control.traffic_router.core.router.TrafficRouterManager;
@@ -108,8 +109,8 @@ public class ZoneManagerTest {
 			final Name edgeName = new Name(ds.getRoutingName() + "." + domain + ".");
 
 			for (InetAddress source : netMap.values()) {
-				final CacheLocation location = trafficRouter.getCoverageZoneCacheLocation(source.getHostAddress(), ds);
-				final List<Cache> caches = trafficRouter.selectCachesByCZ(ds, location);
+				final CacheLocation location = trafficRouter.getCoverageZoneCacheLocation(source.getHostAddress(), ds, IPVersions.IPV4ONLY);
+				final List<Cache> caches = trafficRouter.selectCachesByCZ(ds, location, IPVersions.IPV4ONLY);
 
 				if (caches == null) {
 					continue;
diff --git a/traffic_router/core/src/test/java/com/comcast/cdn/traffic_control/traffic_router/core/external/SteeringTest.java b/traffic_router/core/src/test/java/com/comcast/cdn/traffic_control/traffic_router/core/external/SteeringTest.java
index a72f18d..e365ef8 100644
--- a/traffic_router/core/src/test/java/com/comcast/cdn/traffic_control/traffic_router/core/external/SteeringTest.java
+++ b/traffic_router/core/src/test/java/com/comcast/cdn/traffic_control/traffic_router/core/external/SteeringTest.java
@@ -326,8 +326,8 @@ public class SteeringTest {
 		HttpPost httpPost = new HttpPost("http://localhost:" + testHttpPort + "/steering");
 		httpClient.execute(httpPost).close();
 
-		// steering is checked every 15 seconds by default.
-		Thread.sleep(30 * 1000);
+		// a polling interval of 60 seconds is common
+		Thread.sleep(90 * 1000);
 
 		Map<String, List<String>> rehashedPaths = new HashMap<>();
 		rehashedPaths.put(smallerTarget, new ArrayList<String>());
diff --git a/traffic_router/core/src/test/java/com/comcast/cdn/traffic_control/traffic_router/core/loc/CoverageZoneTest.java b/traffic_router/core/src/test/java/com/comcast/cdn/traffic_control/traffic_router/core/loc/CoverageZoneTest.java
index 9be34b9..4d90f67 100644
--- a/traffic_router/core/src/test/java/com/comcast/cdn/traffic_control/traffic_router/core/loc/CoverageZoneTest.java
+++ b/traffic_router/core/src/test/java/com/comcast/cdn/traffic_control/traffic_router/core/loc/CoverageZoneTest.java
@@ -19,6 +19,7 @@ import com.comcast.cdn.traffic_control.traffic_router.core.edge.Cache;
 import com.comcast.cdn.traffic_control.traffic_router.core.edge.CacheLocation;
 import com.comcast.cdn.traffic_control.traffic_router.core.edge.CacheLocation.LocalizationMethod;
 import com.comcast.cdn.traffic_control.traffic_router.core.edge.CacheRegister;
+import com.comcast.cdn.traffic_control.traffic_router.core.edge.Node.IPVersions;
 import com.comcast.cdn.traffic_control.traffic_router.core.ds.DeliveryService;
 import com.comcast.cdn.traffic_control.traffic_router.core.router.TrafficRouter;
 import com.comcast.cdn.traffic_control.traffic_router.geolocation.Geolocation;
@@ -100,19 +101,19 @@ public class CoverageZoneTest {
 
 		trafficRouter = PowerMockito.mock(TrafficRouter.class);
 		Whitebox.setInternalState(trafficRouter, "cacheRegister", cacheRegister);
-		when(trafficRouter.getCoverageZoneCacheLocation("12.23.34.45", "delivery-service-1")).thenCallRealMethod();
-		when(trafficRouter.getCoverageZoneCacheLocation("12.23.34.45", "delivery-service-1", false, null)).thenCallRealMethod();
+		when(trafficRouter.getCoverageZoneCacheLocation("12.23.34.45", "delivery-service-1", IPVersions.IPV4ONLY)).thenCallRealMethod();
+		when(trafficRouter.getCoverageZoneCacheLocation("12.23.34.45", "delivery-service-1", false, null, IPVersions.IPV4ONLY)).thenCallRealMethod();
 		when(trafficRouter.getCacheRegister()).thenReturn(cacheRegister);
 		when(trafficRouter.orderLocations(anyListOf(CacheLocation.class),any(Geolocation.class))).thenCallRealMethod();
-		when(trafficRouter.getSupportingCaches(anyListOf(Cache.class), eq(deliveryService))).thenCallRealMethod();
+		when(trafficRouter.getSupportingCaches(anyListOf(Cache.class), eq(deliveryService), any(IPVersions.class))).thenCallRealMethod();
 		when(trafficRouter.filterEnabledLocations(anyListOf(CacheLocation.class), any(CacheLocation.LocalizationMethod.class))).thenCallRealMethod();
 		PowerMockito.when(trafficRouter, "getNetworkNode", "12.23.34.45").thenReturn(eastNetworkNode);
-		PowerMockito.when(trafficRouter, "getClosestCacheLocation", anyListOf(CacheLocation.class), any(CacheLocation.class), any(DeliveryService.class)).thenCallRealMethod();
+		PowerMockito.when(trafficRouter, "getClosestCacheLocation", anyListOf(CacheLocation.class), any(CacheLocation.class), any(DeliveryService.class), any(IPVersions.class)).thenCallRealMethod();
 	}
 
 	@Test
 	public void trafficRouterReturnsNearestCacheGroupForDeliveryService() throws Exception {
-		CacheLocation cacheLocation = trafficRouter.getCoverageZoneCacheLocation("12.23.34.45", "delivery-service-1");
+		CacheLocation cacheLocation = trafficRouter.getCoverageZoneCacheLocation("12.23.34.45", "delivery-service-1", IPVersions.IPV4ONLY);
 		assertThat(cacheLocation.getId(), equalTo("west-cache-group"));
 		// NOTE: far-east-cache-group is actually closer to the client but isn't enabled for CZ-localization and must be filtered out
 	}
diff --git a/traffic_router/core/src/test/java/com/comcast/cdn/traffic_control/traffic_router/core/router/DNSRoutingMissesTest.java b/traffic_router/core/src/test/java/com/comcast/cdn/traffic_control/traffic_router/core/router/DNSRoutingMissesTest.java
index 34ba482..9361630 100644
--- a/traffic_router/core/src/test/java/com/comcast/cdn/traffic_control/traffic_router/core/router/DNSRoutingMissesTest.java
+++ b/traffic_router/core/src/test/java/com/comcast/cdn/traffic_control/traffic_router/core/router/DNSRoutingMissesTest.java
@@ -17,6 +17,7 @@ package com.comcast.cdn.traffic_control.traffic_router.core.router;
 
 import com.comcast.cdn.traffic_control.traffic_router.core.edge.CacheLocation;
 import com.comcast.cdn.traffic_control.traffic_router.core.edge.CacheRegister;
+import com.comcast.cdn.traffic_control.traffic_router.core.edge.Node.IPVersions;
 import com.comcast.cdn.traffic_control.traffic_router.core.ds.DeliveryService;
 import com.comcast.cdn.traffic_control.traffic_router.core.loc.FederationRegistry;
 import com.comcast.cdn.traffic_control.traffic_router.core.request.DNSRequest;
@@ -63,7 +64,7 @@ public class DNSRoutingMissesTest {
         trafficRouter = mock(TrafficRouter.class);
         when(trafficRouter.getCacheRegister()).thenReturn(mock(CacheRegister.class));
         Whitebox.setInternalState(trafficRouter, "federationRegistry", federationRegistry);
-        when(trafficRouter.selectCachesByGeo(anyString(), any(DeliveryService.class), any(CacheLocation.class), any(Track.class))).thenCallRealMethod();
+        when(trafficRouter.selectCachesByGeo(anyString(), any(DeliveryService.class), any(CacheLocation.class), any(Track.class), any(IPVersions.class))).thenCallRealMethod();
 
         track = spy(StatTracker.getTrack());
         doCallRealMethod().when(trafficRouter).route(request, track);
@@ -156,7 +157,7 @@ public class DNSRoutingMissesTest {
 
     @Test
     public void itSetsDetailsWhenCacheNotFoundByGeolocation() throws Exception {
-        doCallRealMethod().when(trafficRouter).selectCachesByGeo(anyString(), any(DeliveryService.class), any(CacheLocation.class), any(Track.class));
+        doCallRealMethod().when(trafficRouter).selectCachesByGeo(anyString(), any(DeliveryService.class), any(CacheLocation.class), any(Track.class), any(IPVersions.class));
         CacheLocation cacheLocation = mock(CacheLocation.class);
         CacheRegister cacheRegister = mock(CacheRegister.class);
 
@@ -168,7 +169,7 @@ public class DNSRoutingMissesTest {
         when(deliveryService.isDns()).thenReturn(true);
 
         doReturn(deliveryService).when(trafficRouter).selectDeliveryService(request);
-        doReturn(cacheLocation).when(trafficRouter).getCoverageZoneCacheLocation("192.168.34.56", deliveryService);
+        doReturn(cacheLocation).when(trafficRouter).getCoverageZoneCacheLocation("192.168.34.56", deliveryService, IPVersions.IPV4ONLY);
         doReturn(cacheRegister).when(trafficRouter).getCacheRegister();
 
         trafficRouter.route(request, track);
diff --git a/traffic_router/core/src/test/java/com/comcast/cdn/traffic_control/traffic_router/core/router/TrafficRouterTest.java b/traffic_router/core/src/test/java/com/comcast/cdn/traffic_control/traffic_router/core/router/TrafficRouterTest.java
index 52286d8..77815c6 100644
--- a/traffic_router/core/src/test/java/com/comcast/cdn/traffic_control/traffic_router/core/router/TrafficRouterTest.java
+++ b/traffic_router/core/src/test/java/com/comcast/cdn/traffic_control/traffic_router/core/router/TrafficRouterTest.java
@@ -20,6 +20,7 @@ import com.comcast.cdn.traffic_control.traffic_router.core.edge.CacheLocation;
 import com.comcast.cdn.traffic_control.traffic_router.core.edge.CacheLocation.LocalizationMethod;
 import com.comcast.cdn.traffic_control.traffic_router.core.edge.CacheRegister;
 import com.comcast.cdn.traffic_control.traffic_router.core.edge.InetRecord;
+import com.comcast.cdn.traffic_control.traffic_router.core.edge.Node.IPVersions;
 import com.comcast.cdn.traffic_control.traffic_router.core.ds.DeliveryService;
 import com.comcast.cdn.traffic_control.traffic_router.core.ds.Dispersion;
 import com.comcast.cdn.traffic_control.traffic_router.core.ds.SteeringRegistry;
@@ -50,7 +51,7 @@ import static org.hamcrest.Matchers.equalTo;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyBoolean;
 import static org.mockito.Matchers.anyString;
-import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.doCallRealMethod;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
@@ -144,9 +145,9 @@ public class TrafficRouterTest {
         List<Cache> caches = new ArrayList<Cache>();
         caches.add(cache);
         when(trafficRouter.selectCaches(any(HTTPRequest.class), any(DeliveryService.class), any(Track.class))).thenReturn(caches);
-        when(trafficRouter.selectCachesByGeo(anyString(), any(DeliveryService.class), any(CacheLocation.class), any(Track.class))).thenCallRealMethod();
+        when(trafficRouter.selectCachesByGeo(anyString(), any(DeliveryService.class), any(CacheLocation.class), any(Track.class), any(IPVersions.class))).thenCallRealMethod();
         when(trafficRouter.getClientLocation(anyString(), any(DeliveryService.class), any(CacheLocation.class), any(Track.class))).thenReturn(new Geolocation(40, -100));
-        when(trafficRouter.getCachesByGeo(any(DeliveryService.class), any(Geolocation.class), any(Track.class))).thenCallRealMethod();
+        when(trafficRouter.getCachesByGeo(any(DeliveryService.class), any(Geolocation.class), any(Track.class), any(IPVersions.class))).thenCallRealMethod();
         when(trafficRouter.getCacheRegister()).thenReturn(cacheRegister);
         when(trafficRouter.orderLocations(any(List.class), any(Geolocation.class))).thenCallRealMethod();
 
@@ -156,6 +157,54 @@ public class TrafficRouterTest {
     }
 
     @Test
+    public void itFiltersByIPAvailability() throws Exception {
+
+        DeliveryService ds = mock(DeliveryService.class);
+
+        Cache cacheIPv4 = mock(Cache.class);
+        when(cacheIPv4.hasDeliveryService(anyString())).thenReturn(true);
+        when(cacheIPv4.hasAuthority()).thenReturn(true);
+        when(cacheIPv4.isAvailable(any(IPVersions.class))).thenCallRealMethod();
+        doCallRealMethod().when(cacheIPv4).setIsAvailable(anyBoolean());
+        setInternalState(cacheIPv4, "ipv4Available", true);
+        setInternalState(cacheIPv4, "ipv6Available", false);
+        cacheIPv4.setIsAvailable(true);
+        when(cacheIPv4.getId()).thenReturn("cache IPv4");
+
+        Cache cacheIPv6 = mock(Cache.class);
+        when(cacheIPv6.hasDeliveryService(anyString())).thenReturn(true);
+        when(cacheIPv6.hasAuthority()).thenReturn(true);
+        when(cacheIPv6.isAvailable(any(IPVersions.class))).thenCallRealMethod();
+        doCallRealMethod().when(cacheIPv6).setIsAvailable(anyBoolean());
+        setInternalState(cacheIPv6, "ipv4Available", false);
+        setInternalState(cacheIPv6, "ipv6Available", true);
+        cacheIPv6.setIsAvailable(true);
+        when(cacheIPv6.getId()).thenReturn("cache IPv6");
+
+        List<Cache> caches = new ArrayList<Cache>();
+        caches.add(cacheIPv4);
+        caches.add(cacheIPv6);
+
+        when(trafficRouter.getSupportingCaches(any(List.class), any(DeliveryService.class), any(IPVersions.class))).thenCallRealMethod();
+
+        List<Cache> supportingIPv4Caches = trafficRouter.getSupportingCaches(caches, ds, IPVersions.IPV4ONLY);
+        assertThat(supportingIPv4Caches.size(), equalTo(1));
+        assertThat(supportingIPv4Caches.get(0).getId(), equalTo("cache IPv4"));
+
+        List<Cache> supportingIPv6Caches = trafficRouter.getSupportingCaches(caches, ds, IPVersions.IPV6ONLY);
+        assertThat(supportingIPv6Caches.size(), equalTo(1));
+        assertThat(supportingIPv6Caches.get(0).getId(), equalTo("cache IPv6"));
+
+        List<Cache> supportingEitherCaches = trafficRouter.getSupportingCaches(caches, ds, IPVersions.ANY);
+        assertThat(supportingEitherCaches.size(), equalTo(2));
+
+        cacheIPv6.setIsAvailable(false);
+        List<Cache> supportingAvailableCaches = trafficRouter.getSupportingCaches(caches, ds, IPVersions.ANY);
+        assertThat(supportingAvailableCaches.size(), equalTo(1));
+        assertThat(supportingAvailableCaches.get(0).getId(), equalTo("cache IPv4"));
+    }
+
+    @Test
     public void itChecksDefaultLocation() throws Exception {
         String ip = "1.2.3.4";
         Track track = new Track();
@@ -172,11 +221,11 @@ public class TrafficRouterTest {
         List<Cache> list = new ArrayList<>();
         list.add(cache);
         when(deliveryService.getMissLocation()).thenReturn(defaultUSLocation);
-        when(trafficRouter.getCachesByGeo(deliveryService, deliveryService.getMissLocation(), track)).thenReturn(list);
-        when(trafficRouter.selectCachesByGeo(ip, deliveryService, null, track)).thenCallRealMethod();
+        when(trafficRouter.getCachesByGeo(deliveryService, deliveryService.getMissLocation(), track, IPVersions.IPV4ONLY)).thenReturn(list);
+        when(trafficRouter.selectCachesByGeo(ip, deliveryService, null, track, IPVersions.IPV4ONLY)).thenCallRealMethod();
         when(trafficRouter.isValidMissLocation(deliveryService)).thenCallRealMethod();
-        List<Cache> result = trafficRouter.selectCachesByGeo(ip, deliveryService, null, track);
-        verify(trafficRouter).getCachesByGeo(deliveryService, deliveryService.getMissLocation(), track);
+        List<Cache> result = trafficRouter.selectCachesByGeo(ip, deliveryService, null, track, IPVersions.IPV4ONLY);
+        verify(trafficRouter).getCachesByGeo(deliveryService, deliveryService.getMissLocation(), track, IPVersions.IPV4ONLY);
         assertThat(result.size(), equalTo(1));
         assertThat(result.get(0), equalTo(cache));
         assertThat(track.getResult(), equalTo(Track.ResultType.GEO_DS));
@@ -215,15 +264,15 @@ public class TrafficRouterTest {
 
         when(trafficRouter.selectCaches(any(HTTPRequest.class), any(DeliveryService.class), any(Track.class))).thenCallRealMethod();
         when(trafficRouter.selectCaches(any(HTTPRequest.class), any(DeliveryService.class), any(Track.class), anyBoolean())).thenCallRealMethod();
-        when(trafficRouter.selectCachesByGeo(anyString(), any(DeliveryService.class), any(CacheLocation.class), any(Track.class))).thenCallRealMethod();
+        when(trafficRouter.selectCachesByGeo(anyString(), any(DeliveryService.class), any(CacheLocation.class), any(Track.class), any(IPVersions.class))).thenCallRealMethod();
 
         Geolocation clientLocation = new Geolocation(40, -100);
         when(trafficRouter.getClientLocation(anyString(), any(DeliveryService.class), any(CacheLocation.class), any(Track.class))).thenReturn(clientLocation);
 
-        when(trafficRouter.getCachesByGeo(any(DeliveryService.class), any(Geolocation.class), any(Track.class))).thenCallRealMethod();
+        when(trafficRouter.getCachesByGeo(any(DeliveryService.class), any(Geolocation.class), any(Track.class), any(IPVersions.class))).thenCallRealMethod();
         when(trafficRouter.filterEnabledLocations(any(List.class), any(LocalizationMethod.class))).thenCallRealMethod();
         when(trafficRouter.orderLocations(any(List.class), any(Geolocation.class))).thenCallRealMethod();
-        when(trafficRouter.getSupportingCaches(any(List.class), any(DeliveryService.class))).thenCallRealMethod();
+        when(trafficRouter.getSupportingCaches(any(List.class), any(DeliveryService.class), any(IPVersions.class))).thenCallRealMethod();
 
         HTTPRequest httpRequest = new HTTPRequest();
         httpRequest.setClientIP("192.168.10.11");


[trafficcontrol] 04/04: TP: allows topology-based delivery services to be assigned to ORG servers (#5236)

Posted by oc...@apache.org.
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 0271e734e82da916b08d9cca24fe85e361ad2f63
Author: Jeremy Mitchell <mi...@users.noreply.github.com>
AuthorDate: Tue Nov 3 08:46:18 2020 -0700

    TP: allows topology-based delivery services to be assigned to ORG servers (#5236)
    
    * allows topology-based delivery services to be assigned to ORG servers
    
    * adds changelog.md entry
    
    * syncs topology name validation in UI w/ api validation
    
    * another changelog.md update
    
    (cherry picked from commit 82f7549057a4a71a4f7435253c6d368078c1664e)
---
 CHANGELOG.md                                             |  2 ++
 .../common/modules/form/topology/form.topology.tpl.html  |  4 ++--
 .../TableAssignDeliveryServicesController.js             | 16 ++++++++++------
 .../table.assignDeliveryServices.tpl.html                |  2 +-
 4 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1b29c24..24e1829 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -120,6 +120,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - Fixed an issue where Traffic Router would erroneously return 503s or NXDOMAINs if the caches in a cachegroup were all unavailable for a client's requested IP version, rather than selecting caches from the next closest available cachegroup.
 - Fixed an issue where downgrading the database would fail while having server interfaces with null gateways, MTU, and/or netmasks.
 - Fixed an issue where partial upgrades of the database would occasionally fail to apply 2020081108261100_add_server_ip_profile_trigger.
+- Fixed #5197 - Allows users to assign topology-based DS to ORG servers [Related Github issue](https://github.com/apache/trafficcontrol/issues/5197)
+- Fixed #5161 - Fixes topology name character validation [Related Github issue](https://github.com/apache/trafficcontrol/issues/5161)
 
 ### Changed
 - Changed some Traffic Ops Go Client methods to use `DeliveryServiceNullable` inputs and outputs.
diff --git a/traffic_portal/app/src/common/modules/form/topology/form.topology.tpl.html b/traffic_portal/app/src/common/modules/form/topology/form.topology.tpl.html
index 0225b8d..df8822a 100644
--- a/traffic_portal/app/src/common/modules/form/topology/form.topology.tpl.html
+++ b/traffic_portal/app/src/common/modules/form/topology/form.topology.tpl.html
@@ -58,9 +58,9 @@ under the License.
             <div class="form-group" ng-class="{'has-error': hasError(topologyForm.name), 'has-feedback': hasError(topologyForm.name)}">
                 <label class="control-label col-md-2 col-sm-2 col-xs-12">Name *</label>
                 <div class="col-md-10 col-sm-10 col-xs-12">
-                    <input name="name" type="text" class="form-control" ng-model="topology.name" ng-disabled="!settings.isNew" pattern="[A-Za-z0-9]([A-Za-z\-0-9]*[A-Za-z0-9])?" required autofocus>
+                    <input name="name" type="text" class="form-control" ng-model="topology.name" ng-disabled="!settings.isNew" pattern="^[a-zA-Z0-9\-_]+$" required autofocus>
                     <small class="input-error" ng-show="hasPropertyError(topologyForm.name, 'required')">Required</small>
-                    <small class="input-error" ng-show="hasPropertyError(topologyForm.name, 'pattern')">No special characters, periods, underscores, or spaces and cannot begin or end with a hyphen</small>
+                    <small class="input-error" ng-show="hasPropertyError(topologyForm.name, 'pattern')">Alphanumeric, dash, or underscore characters only</small>
                     <span ng-show="hasError(topologyForm.name)" class="form-control-feedback"><i class="fa fa-times"></i></span>
                 </div>
             </div>
diff --git a/traffic_portal/app/src/common/modules/table/serverDeliveryServices/TableAssignDeliveryServicesController.js b/traffic_portal/app/src/common/modules/table/serverDeliveryServices/TableAssignDeliveryServicesController.js
index ede43ac..a659d75 100644
--- a/traffic_portal/app/src/common/modules/table/serverDeliveryServices/TableAssignDeliveryServicesController.js
+++ b/traffic_portal/app/src/common/modules/table/serverDeliveryServices/TableAssignDeliveryServicesController.js
@@ -17,7 +17,7 @@
  * under the License.
  */
 
-var TableAssignDeliveryServicesController = function(server, deliveryServices, assignedDeliveryServices, $scope, $uibModalInstance) {
+var TableAssignDeliveryServicesController = function(server, deliveryServices, assignedDeliveryServices, $scope, $uibModalInstance, serverUtils) {
 
 	var selectedDeliveryServices = [];
 
@@ -35,7 +35,7 @@ var TableAssignDeliveryServicesController = function(server, deliveryServices, a
 				return parseInt($(this).attr('id'));
 			}).get();
 		$scope.selectedDeliveryServices = _.map(deliveryServices, function(ds) {
-			if (ds.topology) {
+			if (ds.topology && $scope.isCache(server)) {
 				return ds;
 			}
 			if (visibleDSIds.includes(ds.id)) {
@@ -53,6 +53,8 @@ var TableAssignDeliveryServicesController = function(server, deliveryServices, a
 
 	$scope.server = server;
 
+	$scope.isCache = serverUtils.isCache;
+
 	$scope.selectedDeliveryServices = _.map(deliveryServices, function(ds) {
 		var isAssigned = _.find(assignedDeliveryServices, function(assignedDS) { return assignedDS.id == ds.id });
 		if (isAssigned) {
@@ -62,10 +64,12 @@ var TableAssignDeliveryServicesController = function(server, deliveryServices, a
 	});
 
 	$scope.toggleRow = function(ds) {
-		if (!ds.topology) {
-			ds.selected = !ds.selected;
-			$scope.onChange();
+		// a ds w/ a topology has no use being assigned to cache servers
+		if (ds.topology && $scope.isCache(server)) {
+			return;
 		}
+		ds.selected = !ds.selected;
+		$scope.onChange();
 	};
 
 	$scope.selectAll = function($event) {
@@ -110,5 +114,5 @@ var TableAssignDeliveryServicesController = function(server, deliveryServices, a
 
 };
 
-TableAssignDeliveryServicesController.$inject = ['server', 'deliveryServices', 'assignedDeliveryServices', '$scope', '$uibModalInstance'];
+TableAssignDeliveryServicesController.$inject = ['server', 'deliveryServices', 'assignedDeliveryServices', '$scope', '$uibModalInstance', 'serverUtils'];
 module.exports = TableAssignDeliveryServicesController;
diff --git a/traffic_portal/app/src/common/modules/table/serverDeliveryServices/table.assignDeliveryServices.tpl.html b/traffic_portal/app/src/common/modules/table/serverDeliveryServices/table.assignDeliveryServices.tpl.html
index 698e0e8..7ba0b48 100644
--- a/traffic_portal/app/src/common/modules/table/serverDeliveryServices/table.assignDeliveryServices.tpl.html
+++ b/traffic_portal/app/src/common/modules/table/serverDeliveryServices/table.assignDeliveryServices.tpl.html
@@ -33,7 +33,7 @@ under the License.
         </thead>
         <tbody>
         <tr id="{{::ds.id}}" class="ds-row" ng-class="::{'active': ds.selected}" ng-repeat="ds in ::selectedDeliveryServices" ng-click="toggleRow(ds)">
-            <td><input type="checkbox" ng-model="ds.selected" ng-disabled="ds.topology"></td>
+            <td><input type="checkbox" ng-model="ds.selected" ng-disabled="ds.topology && isCache(server)"></td>
             <td data-search="^{{::ds.xmlId}}$">{{::ds.xmlId}}</td>
             <td data-search="^{{::ds.displayName}}$">{{::ds.displayName}}</td>
             <td data-search="^{{::ds.type}}$">{{::ds.type}}</td>


[trafficcontrol] 03/04: Migration fixes (#5228)

Posted by oc...@apache.org.
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 3efcb6f5bbfd2d1e907152387900e524c96307cb
Author: ocket8888 <oc...@apache.org>
AuthorDate: Mon Nov 2 15:05:48 2020 -0700

    Migration fixes (#5228)
    
    * Fix bad migration file name
    
    * Add COALESCEs for newly-nullable columns downgrading to non-nullable
    
    * Added todb tests action
    
    * Added database tests workflow
    
    * Remove now-unnecessary PR checklist item
    
    * Add CHANGELOG entry
    
    * Fix bad tests
    
    (cherry picked from commit 2da6408444144f301f3bc955bd25ebc11c29fd84)
---
 .github/actions/todb-tests/Dockerfile              | 24 ++++++++
 .github/actions/todb-tests/README.md               | 38 +++++++++++++
 .github/actions/todb-tests/action.yml              | 22 ++++++++
 .github/actions/todb-tests/entrypoint.sh           | 66 ++++++++++++++++++++++
 .github/workflows/traffic.ops.database.yml         | 37 ++++++++++++
 CHANGELOG.md                                       |  2 +
 PULL_REQUEST_TEMPLATE.md                           |  1 -
 .../2020072700000000_remove_redundancy.sql         |  6 +-
 ...081108261100_add_server_ip_profile_trigger.sql} |  0
 9 files changed, 192 insertions(+), 4 deletions(-)

diff --git a/.github/actions/todb-tests/Dockerfile b/.github/actions/todb-tests/Dockerfile
new file mode 100644
index 0000000..132a4ae
--- /dev/null
+++ b/.github/actions/todb-tests/Dockerfile
@@ -0,0 +1,24 @@
+# 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.
+
+FROM alpine:3.12
+
+RUN apk add --no-cache bash
+
+COPY entrypoint.sh /
+
+ENTRYPOINT /entrypoint.sh
diff --git a/.github/actions/todb-tests/README.md b/.github/actions/todb-tests/README.md
new file mode 100644
index 0000000..fd3ce70
--- /dev/null
+++ b/.github/actions/todb-tests/README.md
@@ -0,0 +1,38 @@
+<!--
+  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.
+-->
+
+# todb-tests Docker action
+This action runs Traffic Ops database tests.
+
+## Outputs
+
+### `exit-code`
+0 if the tests passed, 1 otherwise.
+
+## Example usage
+```yaml
+jobs:
+  tests:
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: Checkout
+        uses: actions/checkout@master
+      - name: ./.github/actions/todb-tests
+```
diff --git a/.github/actions/todb-tests/action.yml b/.github/actions/todb-tests/action.yml
new file mode 100644
index 0000000..aeb78e9
--- /dev/null
+++ b/.github/actions/todb-tests/action.yml
@@ -0,0 +1,22 @@
+# 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.
+
+name: 'todb-tests'
+description: 'Runs any and all tests for the Traffic Ops database'
+runs:
+  using: 'docker'
+  image: 'Dockerfile'
diff --git a/.github/actions/todb-tests/entrypoint.sh b/.github/actions/todb-tests/entrypoint.sh
new file mode 100755
index 0000000..08c83e6
--- /dev/null
+++ b/.github/actions/todb-tests/entrypoint.sh
@@ -0,0 +1,66 @@
+#!/bin/bash
+# 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.
+
+set -e
+
+cd traffic_ops/app/db/migrations;
+
+# Ensure proper order
+SORTED="$(mktemp)";
+SORTEDDASHN="$(mktemp)";
+
+ls | sort > "$SORTED";
+ls | sort -n > "$SORTEDDASHN";
+
+CODE=0;
+
+if [ ! -z "$(diff $SORTED $SORTEDDASHN)" ]; then
+	echo "ERROR: expected sort -n and sort to give the same migration order:" >&2;
+	diff "$SORTED" "$SORTEDDASHN" >&2;
+	CODE=1;
+fi
+
+rm "$SORTED" "$SORTEDDASHN";
+
+# No two migrations may share a timestamp
+ls | cut -d _ -f 1 | uniq -d | while read -r file; do
+	echo "ERROR: more than one file uses timestamp $file - timestamps must be unique" >&2;
+	CODE=1;
+done
+
+# Files must be named like {{timestamp}}_{{migration name}}.sql
+for file in "$(ls)"; do
+	if ! [[ "$file" =~ [0-9]+_[^\.]+\.sql ]]; then
+		echo "ERROR: traffic_ops/app/db/migrations/$file: wrong filename, must match \d+_[^\\.]+\\.sql" >&2;
+		CODE=1;
+	fi
+done
+
+set +e;
+# All new migrations must use 16-digit timestamps.
+VIOLATING_FILES="$(ls | sort | cut -d _ -f 1 | sed -n -e '/2020061622101648/,$p' | tr '[:space:]' '\n' | grep -vE '^[0-9]{16}$')";
+set -e;
+
+if [[ ! -z "$VIOLATING_FILES" ]]; then
+	for file in "$VIOLATING_FILES"; do
+		echo "ERROR: traffic_ops/app/db/migrations/$file(name).sql: wrong filename, all migrations after 2020-06-16 must use 16-digit timestamps in their filenames" >&2;
+	done
+	CODE=1;
+fi
+
+exit "$CODE";
diff --git a/.github/workflows/traffic.ops.database.yml b/.github/workflows/traffic.ops.database.yml
new file mode 100644
index 0000000..26987a3
--- /dev/null
+++ b/.github/workflows/traffic.ops.database.yml
@@ -0,0 +1,37 @@
+# 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.
+
+name: Traffic Ops Database Tests
+
+on:
+  create:
+  push:
+    paths:
+      - traffic_ops/app/db/**
+  pull_request:
+    types: [opened, reopened, edited, synchronize]
+    paths:
+      - traffic_ops/app/db/**
+
+jobs:
+  documentation:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@master
+      - name: Rut Traffic Ops Database Tests
+        uses: ./.github/actions/todb-tests
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6087eb8..1b29c24 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -118,6 +118,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - Fixed validation error message for Traffic Ops `POST /api/x/profileparameters` route
 - Fixed #5216 - Removed duplicate button to link delivery service to server [Related Github issue](https://github.com/apache/trafficcontrol/issues/5216)
 - Fixed an issue where Traffic Router would erroneously return 503s or NXDOMAINs if the caches in a cachegroup were all unavailable for a client's requested IP version, rather than selecting caches from the next closest available cachegroup.
+- Fixed an issue where downgrading the database would fail while having server interfaces with null gateways, MTU, and/or netmasks.
+- Fixed an issue where partial upgrades of the database would occasionally fail to apply 2020081108261100_add_server_ip_profile_trigger.
 
 ### Changed
 - Changed some Traffic Ops Go Client methods to use `DeliveryServiceNullable` inputs and outputs.
diff --git a/PULL_REQUEST_TEMPLATE.md b/PULL_REQUEST_TEMPLATE.md
index cb6578d..db0fd0e 100644
--- a/PULL_REQUEST_TEMPLATE.md
+++ b/PULL_REQUEST_TEMPLATE.md
@@ -83,7 +83,6 @@ e.g.
 - [ ] This PR includes documentation OR I have explained why documentation is unnecessary
 - [ ] This PR includes an update to CHANGELOG.md OR such an update is not necessary
 - [ ] This PR includes any and all required license headers
-- [ ] This PR ensures that database migration sequence is correct OR this PR does not include a database migration
 - [ ] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the Apache Software Foundation's security guidelines](https://www.apache.org/security/) for details)
 
 
diff --git a/traffic_ops/app/db/migrations/2020072700000000_remove_redundancy.sql b/traffic_ops/app/db/migrations/2020072700000000_remove_redundancy.sql
index 66b078d..bc39d63 100644
--- a/traffic_ops/app/db/migrations/2020072700000000_remove_redundancy.sql
+++ b/traffic_ops/app/db/migrations/2020072700000000_remove_redundancy.sql
@@ -44,11 +44,11 @@ ALTER TABLE server ADD CONSTRAINT need_gateway_if_ip CHECK (ip_address IS NULL O
 ALTER TABLE server ADD CONSTRAINT need_netmask_if_ip CHECK (ip_address IS NULL OR ip_address = '' OR ip_netmask IS NOT NULL);
 
 UPDATE server SET ip_address = host(ip_address.address),
-  ip_netmask = host(netmask(ip_address.address)),
-  ip_gateway = host(ip_address.gateway),
+  ip_netmask = COALESCE(host(netmask(ip_address.address)), ''),
+  ip_gateway = COALESCE(host(ip_address.gateway), ''),
   ip_address_is_service = ip_address.service_address,
   interface_name = ip_address.interface,
-  interface_mtu = interface.mtu
+  interface_mtu = COALESCE(interface.mtu, 0)
   FROM ip_address
   JOIN interface ON ip_address.interface = interface.name
   WHERE server.id = ip_address.server
diff --git a/traffic_ops/app/db/migrations/20200811082611_add_server_ip_profile_trigger.sql b/traffic_ops/app/db/migrations/2020081108261100_add_server_ip_profile_trigger.sql
similarity index 100%
rename from traffic_ops/app/db/migrations/20200811082611_add_server_ip_profile_trigger.sql
rename to traffic_ops/app/db/migrations/2020081108261100_add_server_ip_profile_trigger.sql


[trafficcontrol] 01/04: TP: converts delivery service requests table to ag-grid (#5183)

Posted by oc...@apache.org.
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 0de25ad49cd840fbcb68225119795bb1f5ce55c2
Author: Jeremy Mitchell <mi...@users.noreply.github.com>
AuthorDate: Mon Nov 2 13:53:28 2020 -0700

    TP: converts delivery service requests table to ag-grid (#5183)
    
    * converts delivery service requests table to ag-grid
    
    * adds back refresh
    
    * rearranging the context menu
    
    * removes strikethru from completed dsrs
    
    * changing colors
    
    * removes shadowing
    
    * removes column filter if column is hidden
    
    * adds changelog.md entry
    
    * handles error scenario when ds update fails
    
    * reverts bug fix
    
    * removes double click but still allows cell selection
    
    * adds documentation for new query param - createdAt
    
    * fixes docs for rfc reference
    
    * adds lastUpdated column and as default, hides lastUpdated and lastEditedBy columns
    
    (cherry picked from commit 9258b7708d7e722c0b611e5c921153e9db5b274f)
---
 CHANGELOG.md                                       |   1 +
 docs/source/api/v2/deliveryservice_requests.rst    |   3 +
 docs/source/api/v3/deliveryservice_requests.rst    |   3 +
 .../deliveryservice/request/requests.go            |   1 +
 .../app/src/common/modules/table/_table.scss       |  21 ++
 .../TableDeliveryServiceRequestsController.js      | 322 ++++++++++++++++++---
 .../table.deliveryServiceRequests.tpl.html         | 102 ++++---
 .../modules/table/jobs/TableJobsController.js      |   2 -
 .../private/deliveryServiceRequests/list/index.js  |   5 +-
 9 files changed, 377 insertions(+), 83 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index fee821f..a968e10 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -144,6 +144,7 @@ will be returned indicating that overlap exists.
 - Changed Spring Framework Java dependency to 4.2.5.
 - Changed certificate loading code in Traffic Router to use Bouncy Castle instead of deprecated Sun libraries.
 - Changed deprecated AsyncHttpClient Java dependency to use new active mirror and updated to version 2.12.1.
+- Changed Traffic Portal to use the more performant and powerful ag-grid for the delivery service request (DSR) table.
 
 ### Deprecated
 - Deprecated the non-nullable `DeliveryService` Go struct and other structs that use it. `DeliveryServiceNullable` structs should be used instead.
diff --git a/docs/source/api/v2/deliveryservice_requests.rst b/docs/source/api/v2/deliveryservice_requests.rst
index 862eed6..36ef8c1 100644
--- a/docs/source/api/v2/deliveryservice_requests.rst
+++ b/docs/source/api/v2/deliveryservice_requests.rst
@@ -49,6 +49,9 @@ Request Structure
 	| changeType| no       | Filter for :ref:`ds_requests` of the change type specified.                              |
 	|           |          | Can be ``create``, ``update``, or ``delete``.                                            |
 	+-----------+----------+------------------------------------------------------------------------------------------+
+	| createdAt | no       | Filter for :ref:`ds_requests` created on a certain date/time.                            |
+	|           |          | Value must be :rfc:`3339` compliant. Eg. 2019-09-19T19:35:38.828535Z                     |
+	+-----------+----------+------------------------------------------------------------------------------------------+
 	| id        | no       | Filter for the :ref:`Delivery Service Request <ds_requests>` identified by this          |
 	|           |          | integral, unique identifier.                                                             |
 	+-----------+----------+------------------------------------------------------------------------------------------+
diff --git a/docs/source/api/v3/deliveryservice_requests.rst b/docs/source/api/v3/deliveryservice_requests.rst
index d4a0c81..33093a0 100644
--- a/docs/source/api/v3/deliveryservice_requests.rst
+++ b/docs/source/api/v3/deliveryservice_requests.rst
@@ -49,6 +49,9 @@ Request Structure
 	| changeType| no       | Filter for :ref:`ds_requests` of the change type specified.                              |
 	|           |          | Can be ``create``, ``update``, or ``delete``.                                            |
 	+-----------+----------+------------------------------------------------------------------------------------------+
+	| createdAt | no       | Filter for :ref:`ds_requests` created on a certain date/time.                            |
+	|           |          | Value must be :rfc:`3339` compliant. Eg. 2019-09-19T19:35:38.828535Z                     |
+	+-----------+----------+------------------------------------------------------------------------------------------+
 	| id        | no       | Filter for the :ref:`Delivery Service Request <ds_requests>` identified by this          |
 	|           |          | integral, unique identifier.                                                             |
 	+-----------+----------+------------------------------------------------------------------------------------------+
diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/request/requests.go b/traffic_ops/traffic_ops_golang/deliveryservice/request/requests.go
index dd7a61c..56d44bf 100644
--- a/traffic_ops/traffic_ops_golang/deliveryservice/request/requests.go
+++ b/traffic_ops/traffic_ops_golang/deliveryservice/request/requests.go
@@ -92,6 +92,7 @@ func (req *TODeliveryServiceRequest) Read(h http.Header, useIMS bool) ([]interfa
 		"author":     dbhelpers.WhereColumnInfo{Column: "a.username"},
 		"authorId":   dbhelpers.WhereColumnInfo{Column: "r.author_id", Checker: api.IsInt},
 		"changeType": dbhelpers.WhereColumnInfo{Column: "r.change_type"},
+		"createdAt":  dbhelpers.WhereColumnInfo{Column: "r.created_at"},
 		"id":         dbhelpers.WhereColumnInfo{Column: "r.id", Checker: api.IsInt},
 		"status":     dbhelpers.WhereColumnInfo{Column: "r.status"},
 		"xmlId":      dbhelpers.WhereColumnInfo{Column: "r.deliveryservice->>'xmlId'"},
diff --git a/traffic_portal/app/src/common/modules/table/_table.scss b/traffic_portal/app/src/common/modules/table/_table.scss
index 5a0b225..71664d5 100644
--- a/traffic_portal/app/src/common/modules/table/_table.scss
+++ b/traffic_portal/app/src/common/modules/table/_table.scss
@@ -220,3 +220,24 @@ div.dropdown button.menu-item-button {
     }
   }
 }
+
+.ds-requests-table {
+  .ag-row.draft-request {
+    background-color: #e2e3e5;
+  }
+  .ag-row.submitted-request {
+    background-color: #dff0d8;
+  }
+  .ag-row.pending-request {
+    background-color: #fff3cd;
+  }
+  .ag-row.completed-request {
+    background-color: #fcfcfc;
+  }
+  .ag-row.rejected-request {
+    background-color: #f8d7da;
+    .ag-cell {
+      text-decoration: line-through;
+    }
+  }
+}
diff --git a/traffic_portal/app/src/common/modules/table/deliveryServiceRequests/TableDeliveryServiceRequestsController.js b/traffic_portal/app/src/common/modules/table/deliveryServiceRequests/TableDeliveryServiceRequestsController.js
index 5132370..b10f3fb 100644
--- a/traffic_portal/app/src/common/modules/table/deliveryServiceRequests/TableDeliveryServiceRequestsController.js
+++ b/traffic_portal/app/src/common/modules/table/deliveryServiceRequests/TableDeliveryServiceRequestsController.js
@@ -17,7 +17,70 @@
  * under the License.
  */
 
-var TableDeliveryServicesRequestsController = function (dsRequests, $scope, $state, $uibModal, $anchorScroll, $q, $location, dateUtils, locationUtils, typeService, deliveryServiceService, deliveryServiceRequestService, messageModel, propertiesModel, userModel) {
+var TableDeliveryServicesRequestsController = function (tableName, dsRequests, $scope, $state, $uibModal, $anchorScroll, $q, $location, $document, dateUtils, locationUtils, typeService, deliveryServiceService, deliveryServiceRequestService, messageModel, propertiesModel, userModel) {
+
+	/**
+	 * Gets value to display a default tooltip.
+	 */
+	function defaultTooltip(params) {
+		return params.value;
+	}
+
+	/**
+	 * Formats the contents of a 'createdAt' and 'lastUpdated' column cell as "relative to now".
+	 */
+	function dateCellFormatter(params) {
+		return params.value ? dateUtils.getRelativeTime(params.value) : params.value;
+	}
+
+	columns = [
+		{
+			headerName: "Delivery Service",
+			field: "deliveryService.xmlId",
+			hide: false
+		},
+		{
+			headerName: "Type",
+			field: "changeType",
+			hide: false
+		},
+		{
+			headerName: "Status",
+			field: "status",
+			hide: false
+		},
+		{
+			headerName: "Author",
+			field: "author",
+			hide: false
+		},
+		{
+			headerName: "Assignee",
+			field: "assignee",
+			hide: false
+		},
+		{
+			headerName: "Last Edited By",
+			field: "lastEditedBy",
+			hide: true
+		},
+		{
+			headerName: "Last Updated",
+			field: "lastUpdated",
+			hide: true,
+			filter: "agDateColumnFilter",
+			tooltip: dateCellFormatter,
+			valueFormatter: dateCellFormatter
+		},
+		{
+			headerName: "Created",
+			field: "createdAt",
+			hide: false,
+			filter: "agDateColumnFilter",
+			tooltip: dateCellFormatter,
+			valueFormatter: dateCellFormatter
+		}
+	];
 
 	var createComment = function (request, placeholder) {
 		var params = {
@@ -52,32 +115,166 @@ var TableDeliveryServicesRequestsController = function (dsRequests, $scope, $sta
 	$scope.PENDING = 3;
 	$scope.COMPLETE = 4;
 
-	$scope.dsRequests = dsRequests;
+	/** All of the ds requests - createdAt and lastUpdated fields converted to actual Date */
+	$scope.dsRequests = dsRequests.map(
+		function(x) {
+			x.createdAt = x.createdAt ? new Date(x.createdAt.replace("+00", "Z")) : x.createdAt;
+			x.lastUpdated = x.lastUpdated ? new Date(x.lastUpdated.replace("+00", "Z")) : x.lastUpdated;
+		});
 
-	$scope.getRelativeTime = dateUtils.getRelativeTime;
+	$scope.quickSearch = '';
 
-	$scope.refresh = function () {
-		$state.reload(); // reloads all the resolves for the view
+	$scope.pageSize = 100;
+
+	$scope.mouseDownSelectionText = "";
+
+	/** Options, configuration, data and callbacks for the ag-grid table. */
+	$scope.gridOptions = {
+		onCellMouseDown: function() {
+			$scope.mouseDownSelectionText = window.getSelection().toString();
+		},
+		onRowClicked: function(params) {
+			const selection = window.getSelection().toString();
+			if(selection === "" || selection === $scope.mouseDownSelectionText) {
+				let path = '/delivery-service-requests/' + params.data.id + '?type=';
+				typeService.getType(params.data.deliveryService.typeId)
+					.then(function (result) {
+						path += result.name;
+						locationUtils.navigateToPath(path);
+					});
+				$scope.$apply();
+			}
+			$scope.mouseDownSelectionText = "";
+		},
+		columnDefs: columns,
+		enableCellTextSelection: true,
+		suppressMenuHide: true,
+		multiSortKey: 'ctrl',
+		alwaysShowVerticalScroll: true,
+		defaultColDef: {
+			filter: true,
+			sortable: true,
+			resizable: true,
+			tooltip: defaultTooltip
+		},
+		rowClassRules: {
+			'draft-request': function(params) {
+				return params.data.status === 'draft';
+			},
+			'submitted-request': function(params) {
+				return params.data.status === 'submitted';
+			},
+			'pending-request': function(params) {
+				return params.data.status === 'pending';
+			},
+			'completed-request': function(params) {
+				return params.data.status === 'complete';
+			},
+			'rejected-request': function(params) {
+				return params.data.status === 'rejected';
+			},
+		},
+		rowData: dsRequests,
+		pagination: true,
+		paginationPageSize: $scope.pageSize,
+		rowBuffer: 0,
+		onColumnResized: function(params) {
+			localStorage.setItem(tableName + "_table_columns", JSON.stringify($scope.gridOptions.columnApi.getColumnState()));
+		},
+		tooltipShowDelay: 500,
+		allowContextMenuWithControlKey: true,
+		preventDefaultOnContextMenu: true,
+		onCellContextMenu: function(params) {
+			$scope.showMenu = true;
+			$scope.menuStyle.left = String(params.event.clientX) + "px";
+			$scope.menuStyle.top = String(params.event.clientY) + "px";
+			$scope.menuStyle.bottom = "unset";
+			$scope.menuStyle.right = "unset";
+			$scope.$apply();
+			const boundingRect = document.getElementById("context-menu").getBoundingClientRect();
+
+			if (boundingRect.bottom > window.innerHeight){
+				$scope.menuStyle.bottom = String(window.innerHeight - params.event.clientY) + "px";
+				$scope.menuStyle.top = "unset";
+			}
+			if (boundingRect.right > window.innerWidth) {
+				$scope.menuStyle.right = String(window.innerWidth - params.event.clientX) + "px";
+				$scope.menuStyle.left = "unset";
+			}
+			$scope.request = params.data;
+			$scope.$apply();
+		},
+		onColumnVisible: function(params) {
+			if (params.visible){
+				return;
+			}
+			for (let column of params.columns) {
+				if (column.filterActive) {
+					const filterModel = $scope.gridOptions.api.getFilterModel();
+					if (column.colId in filterModel) {
+						delete filterModel[column.colId];
+						$scope.gridOptions.api.setFilterModel(filterModel);
+					}
+				}
+			}
+		},
+		colResizeDefault: "shift"
+	};
+
+	/** This is used to position the context menu under the cursor. */
+	$scope.menuStyle = {
+		left: 0,
+		top: 0,
+	};
+
+	/** Toggles the visibility of a column that has the ID provided as 'col'. */
+	$scope.toggleVisibility = function(col) {
+		const visible = $scope.gridOptions.columnApi.getColumn(col).isVisible();
+		$scope.gridOptions.columnApi.setColumnVisible(col, !visible);
+	};
+
+	/** Downloads the table as a CSV */
+	$scope.exportCSV = function() {
+		const params = {
+			allColumns: true,
+			fileName: "delivery_service_requests.csv",
+		};
+		$scope.gridOptions.api.exportDataAsCsv(params);
+	}
+
+	$scope.onQuickSearchChanged = function() {
+		$scope.gridOptions.api.setQuickFilter($scope.quickSearch);
+		localStorage.setItem(tableName + "_quick_search", $scope.quickSearch);
+	};
+
+	$scope.onPageSizeChanged = function() {
+		const value = Number($scope.pageSize);
+		$scope.gridOptions.api.paginationSetPageSize(value);
+		localStorage.setItem(tableName + "_page_size", value);
+	};
+
+	$scope.clearTableFilters = function() {
+		// clear the quick search
+		$scope.quickSearch = '';
+		$scope.onQuickSearchChanged();
+		// clear any column filters
+		$scope.gridOptions.api.setFilterModel(null);
 	};
 
 	$scope.fulfillable = function (request) {
-		return request.status == 'submitted';
+		return request && request.status == 'submitted';
 	};
 
 	$scope.rejectable = function (request) {
-		return request.status == 'submitted';
+		return request && request.status == 'submitted';
 	};
 
 	$scope.completeable = function (request) {
-		return request.status == 'pending';
+		return request && request.status == 'pending';
 	};
 
 	$scope.open = function (request) {
-		return (request.status == 'draft' || request.status == 'submitted');
-	};
-
-	$scope.closed = function (request) {
-		return (request.status == 'rejected' || request.status == 'complete');
+		return request && (request.status == 'draft' || request.status == 'submitted');
 	};
 
 	$scope.assignRequest = function (request, assign, $event) {
@@ -247,15 +444,6 @@ var TableDeliveryServicesRequestsController = function (dsRequests, $scope, $sta
 		});
 	};
 
-	$scope.editDeliveryServiceRequest = function (request) {
-		var path = '/delivery-service-requests/' + request.id + '?type=';
-		typeService.getType(request.deliveryService.typeId)
-			.then(function (result) {
-				path += result.name;
-				locationUtils.navigateToPath(path);
-			});
-	};
-
 	$scope.fulfillRequest = function (request, $event) {
 		$event.stopPropagation(); // this kills the click event so it doesn't trigger anything else
 		var path = '/delivery-service-requests/' + request.id + '?type=';
@@ -266,32 +454,88 @@ var TableDeliveryServicesRequestsController = function (dsRequests, $scope, $sta
 			});
 	};
 
+	$scope.refresh = function () {
+		$state.reload(); // reloads all the resolves for the view
+	};
+
+	/**** Initialization code, including loading user columns from localstorage ****/
 	angular.element(document).ready(function () {
-		var dsRequestsTable = $('#dsRequestsTable').dataTable({
-			"paging": false,
-			"dom": '<"filter-checkbox">frtip',
-			"columnDefs": [
-				{'orderable': false, 'targets': 7}
-			],
-			"aaSorting": []
+		try {
+			// need to create the show/hide column checkboxes and bind to the current visibility
+			const colstates = JSON.parse(localStorage.getItem(tableName + "_table_columns"));
+			if (colstates) {
+				if (!$scope.gridOptions.columnApi.setColumnState(colstates)) {
+					console.error("Failed to load stored column state: one or more columns not found");
+				}
+			} else {
+				$scope.gridOptions.api.sizeColumnsToFit();
+			}
+		} catch (e) {
+			console.error("Failure to retrieve required column info from localStorage (key=" + tableName + "_table_columns):", e);
+		}
+
+		try {
+			const filterState = JSON.parse(localStorage.getItem(tableName + "_table_filters")) || {};
+			$scope.gridOptions.api.setFilterModel(filterState);
+		} catch (e) {
+			console.error("Failure to load stored filter state:", e);
+		}
+
+		$scope.gridOptions.api.addEventListener("filterChanged", function() {
+			localStorage.setItem(tableName + "_table_filters", JSON.stringify($scope.gridOptions.api.getFilterModel()));
 		});
-		$('div.filter-checkbox').html('<input id="showClosed" type="checkbox"><label for="showClosed">&nbsp;&nbsp;Show Closed</label>');
 
-		// only show "open" ds requests on render
-		dsRequestsTable.fnFilter('draft|submitted|pending', 2, true, false);
+		try {
+			const sortState = JSON.parse(localStorage.getItem(tableName + "_table_sort"));
+			$scope.gridOptions.api.setSortModel(sortState);
+		} catch (e) {
+			console.error("Failure to load stored sort state:", e);
+		}
+
+		try {
+			$scope.quickSearch = localStorage.getItem(tableName + "_quick_search");
+			$scope.gridOptions.api.setQuickFilter($scope.quickSearch);
+		} catch (e) {
+			console.error("Failure to load stored quick search:", e);
+		}
 
-		$('#showClosed').click(function () {
-			var checked = $('#showClosed').is(':checked');
-			if (checked) {
-				dsRequestsTable.fnFilter('', 2, true, false);
-			} else {
-				dsRequestsTable.fnFilter('draft|submitted|pending', 2, true, false);
+		try {
+			const ps = localStorage.getItem(tableName + "_page_size");
+			if (ps && ps > 0) {
+				$scope.pageSize = Number(ps);
+				$scope.gridOptions.api.paginationSetPageSize($scope.pageSize);
 			}
+		} catch (e) {
+			console.error("Failure to load stored page size:", e);
+		}
+
+		$scope.gridOptions.api.addEventListener("sortChanged", function() {
+			localStorage.setItem(tableName + "_table_sort", JSON.stringify($scope.gridOptions.api.getSortModel()));
+		});
+
+		$scope.gridOptions.api.addEventListener("columnMoved", function() {
+			localStorage.setItem(tableName + "_table_columns", JSON.stringify($scope.gridOptions.columnApi.getColumnState()));
 		});
 
+		$scope.gridOptions.api.addEventListener("columnVisible", function() {
+			$scope.gridOptions.api.sizeColumnsToFit();
+			try {
+				colStates = $scope.gridOptions.columnApi.getColumnState();
+				localStorage.setItem(tableName + "_table_columns", JSON.stringify(colStates));
+			} catch (e) {
+				console.error("Failed to store column defs to local storage:", e);
+			}
+		});
+
+		// clicks outside the context menu will hide it
+		$document.bind("click", function(e) {
+			$scope.showMenu = false;
+			e.stopPropagation();
+			$scope.$apply();
+		});
 	});
 
 };
 
-TableDeliveryServicesRequestsController.$inject = ['dsRequests', '$scope', '$state', '$uibModal', '$anchorScroll', '$q', '$location', 'dateUtils', 'locationUtils', 'typeService', 'deliveryServiceService', 'deliveryServiceRequestService', 'messageModel', 'propertiesModel', 'userModel'];
+TableDeliveryServicesRequestsController.$inject = ['tableName', 'dsRequests', '$scope', '$state', '$uibModal', '$anchorScroll', '$q', '$location', '$document', 'dateUtils', 'locationUtils', 'typeService', 'deliveryServiceService', 'deliveryServiceRequestService', 'messageModel', 'propertiesModel', 'userModel'];
 module.exports = TableDeliveryServicesRequestsController;
diff --git a/traffic_portal/app/src/common/modules/table/deliveryServiceRequests/table.deliveryServiceRequests.tpl.html b/traffic_portal/app/src/common/modules/table/deliveryServiceRequests/table.deliveryServiceRequests.tpl.html
index 493c6bb..35429c5 100644
--- a/traffic_portal/app/src/common/modules/table/deliveryServiceRequests/table.deliveryServiceRequests.tpl.html
+++ b/traffic_portal/app/src/common/modules/table/deliveryServiceRequests/table.deliveryServiceRequests.tpl.html
@@ -22,51 +22,71 @@ under the License.
         <ol class="breadcrumb pull-left">
             <li class="active">Delivery Service Requests</li>
         </ol>
-        <div class="pull-right" role="group">
-            <button class="btn btn-default" title="Refresh" ng-click="refresh()"><i class="fa fa-refresh"></i></button>
+        <div class="pull-right">
+            <div class="form-inline" role="search">
+                <input id="quickSearch" name="quickSearch" type="search" class="form-control text-input" placeholder="Quick search..." ng-model="quickSearch" ng-change="onQuickSearchChanged()" aria-label="Search"/>
+                <div class="input-group text-input">
+                    <span class="input-group-addon">
+                        <label for="pageSize">Page size</label>
+                    </span>
+                    <input id="pageSize" name="pageSize" type="number" class="form-control" placeholder="100" ng-model="pageSize" ng-change="onPageSizeChanged()" />
+                </div>
+                <div id="toggleColumns" class="btn-group" role="group" title="Select Table Columns" uib-dropdown is-open="columnSettings.isopen">
+                    <button type="button" class="btn btn-default dropdown-toggle" uib-dropdown-toggle aria-haspopup="true" aria-expanded="false">
+                        <i class="fa fa-columns"></i>&nbsp;
+                        <span class="caret"></span>
+                    </button>
+                    <menu ng-click="$event.stopPropagation()" class="column-settings dropdown-menu-right dropdown-menu" uib-dropdown-menu>
+                        <li role="menuitem" ng-repeat="c in gridOptions.columnApi.getAllColumns() | orderBy:'colDef.headerName'">
+                            <div class="checkbox">
+                                <label><input type="checkbox" ng-checked="c.isVisible()" ng-click="toggleVisibility(c.colId)">{{::c.colDef.headerName}}</label>
+                            </div>
+                        </li>
+                    </menu>
+                </div>
+                <div class="btn-group" role="group" uib-dropdown is-open="more.isopen">
+                    <button name="moreBtn" type="button" class="btn btn-default dropdown-toggle" uib-dropdown-toggle aria-haspopup="true" aria-expanded="false">
+                        More&nbsp;
+                        <span class="caret"></span>
+                    </button>
+                    <ul class="dropdown-menu-right dropdown-menu" uib-dropdown-menu>
+                        <li role="menuitem"><button class="menu-item-button" type="button" ng-click="clearTableFilters()">Clear Table Filters</button></li>
+                        <li role="menuitem"><button class="menu-item-button" type="button" ng-click="exportCSV()">Export CSV</button></li>
+                    </ul>
+                </div>
+            </div>
         </div>
         <div class="clearfix"></div>
     </div>
     <div class="x_content">
-        <br>
-        <table id="dsRequestsTable" class="table responsive-utilities jambo_table">
-            <thead>
-            <tr class="headings">
-                <th>Delivery Service</th>
-                <th>Type</th>
-                <th>Status</th>
-                <th>Author</th>
-                <th>Assignee</th>
-                <th>Last Edited By</th>
-                <th>Created</th>
-                <th style="text-align: right;">Actions</th>
-            </tr>
-            </thead>
-            <tbody>
-            <tr ng-click="editDeliveryServiceRequest(request)" ng-repeat="request in ::dsRequests" ng-class="::{'active': closed(request)}">
-                <td name="xmlId" data-search="^{{::request.deliveryService.xmlId}}$">{{::request.deliveryService.xmlId}}</td>
-                <td data-search="^{{::request.changeType}}$">{{::request.changeType}}</td>
-                <td data-search="^{{::request.status}}$">
-                    <span ng-if="!open(request)">{{::request.status}}</span>
-                    <a ng-if="open(request)" class="link" title="Change Status" ng-click="editStatus(request, $event)">{{::request.status}}</a>
-                </td>
-                <td data-search="^{{::request.author}}$">{{::request.author}}</td>
-                <td data-search="^{{::request.assignee}}$">
-                    <span ng-show="!open(request)">{{::request.assignee}}</span>
-                    <a ng-show="open(request) && !request.assignee" class="link" title="Assign Yourself" ng-click="assignRequest(request, true, $event)">No one - assign yourself</a>
-                    <a ng-show="open(request) && request.assignee" class="link" title="Unassign" ng-click="assignRequest(request, false, $event)">{{::request.assignee}} - unassign</a>
-                </td>
-                <td data-search="^{{::request.lastEditedBy}}$">{{::request.lastEditedBy}}</td>
-                <td title="{{request.createdAt}} (UTC)" data-search="^{{::getRelativeTime(request.createdAt)}}$" data-order="{{::request.createdAt}}">{{::getRelativeTime(request.createdAt)}}</td>
-                <td style="text-align: right;">
-                    <span ng-if="fulfillable(request)"><a class="link action-link" title="Fulfill Request" ng-click="fulfillRequest(request, $event)">fulfill</a> | </span>
-                    <span ng-if="rejectable(request)"><a class="link action-link" title="Reject Request" ng-click="rejectRequest(request, $event)">reject</a> | </span>
-                    <span ng-if="completeable(request)"><a class="link action-link" title="Complete Request" ng-click="completeRequest(request, $event)">complete</a></span>
-                    <span ng-if="open(request)"><a class="link action-link" title="Delete Request" ng-click="deleteRequest(request, $event)">delete</a></span>
-                </td>
-            </tr>
-            </tbody>
-        </table>
+        <div style="height: 740px;" ag-grid="gridOptions" class="ds-requests-table ag-theme-alpine"></div>
     </div>
 </div>
 
+<menu id="context-menu" class="dropdown-menu" ng-style="menuStyle" type="contextmenu" ng-show="showMenu">
+    <ul>
+        <li role="menuitem">
+            <button type="button" ng-click="editStatus(request, $event)" ng-disabled="!open(request)">Edit Status</button>
+        </li>
+        <hr class="divider"/>
+        <li role="menuitem" ng-show="!request.assignee">
+            <button type="button" ng-click="assignRequest(request, true, $event)" ng-disabled="!open(request)">Assign Request</button>
+        </li>
+        <li role="menuitem" ng-show="request.assignee">
+            <button type="button" ng-click="assignRequest(request, false, $event)" ng-disabled="!open(request)">Unassign Request</button>
+        </li>
+        <hr class="divider"/>
+        <li role="menuitem">
+            <button type="button" ng-click="fulfillRequest(request, $event)" ng-disabled="!fulfillable(request)">Fulfill Request</button>
+        </li>
+        <li role="menuitem" ng-show="rejectable(request)">
+            <button type="button" ng-click="rejectRequest(request, $event)">Reject Request</button>
+        </li>
+        <li role="menuitem" ng-show="completeable(request)">
+            <button type="button" ng-click="completeRequest(request, $event)">Complete Request</button>
+        </li>
+        <li role="menuitem">
+            <button type="button" ng-click="deleteRequest(request, $event)" ng-disabled="!open(request)">Delete Request</button>
+        </li>
+    </ul>
+</menu>
diff --git a/traffic_portal/app/src/common/modules/table/jobs/TableJobsController.js b/traffic_portal/app/src/common/modules/table/jobs/TableJobsController.js
index 133905b..e8a8b41 100644
--- a/traffic_portal/app/src/common/modules/table/jobs/TableJobsController.js
+++ b/traffic_portal/app/src/common/modules/table/jobs/TableJobsController.js
@@ -80,8 +80,6 @@ var TableJobsController = function(tableName, jobs, $document, $scope, $state, $
 			x.expires = new Date(x.startTime.getTime() + ttl*3600*1000);
 		});
 
-	$scope.jobs = jobs;
-
 	$scope.quickSearch = '';
 
 	$scope.pageSize = 100;
diff --git a/traffic_portal/app/src/modules/private/deliveryServiceRequests/list/index.js b/traffic_portal/app/src/modules/private/deliveryServiceRequests/list/index.js
index d86c7c9..1dc1004 100644
--- a/traffic_portal/app/src/modules/private/deliveryServiceRequests/list/index.js
+++ b/traffic_portal/app/src/modules/private/deliveryServiceRequests/list/index.js
@@ -27,8 +27,11 @@ module.exports = angular.module('trafficPortal.private.deliveryServiceRequests.l
 						templateUrl: 'common/modules/table/deliveryServiceRequests/table.deliveryServiceRequests.tpl.html',
 						controller: 'TableDeliveryServiceRequestsController',
 						resolve: {
+							tableName: function() {
+								return 'ds-requests';
+							},
 							dsRequests: function(deliveryServiceRequestService) {
-								return deliveryServiceRequestService.getDeliveryServiceRequests();
+								return deliveryServiceRequestService.getDeliveryServiceRequests({ orderby: 'createdAt', sortOrder: 'desc' });
 							}
 						}
 					}