Fix atstccfg Delivery Service Server caching. (#4387) (#4389)

* Fix atstccfg deliveryserviceserver caching

Fixses an inconsistent ORT bug with files, noticably parent.config,
not having all lines. Bug was filtering before saving the cache file.

DSS gets everything, and then filters. It was filtering before saving
the cache file, causing subsequent ORT calls under the cache age which
had more DSSes to be missing them.

This fixes it to filter after saving/loading the cache file, instead
of before.

* Add ORT use_cache arg, default true.

Adds a use_cache argument to ORT (default true) to allow callers
to tell atstccfg to not use the cache.

We've had several cache bugs in the past, and caching is hard. The
atstccfg app already has a --no-cache arg, this just adds it to ORT,
which allows users to disable the cache if there's an issue, without
having to modify ORT, just their cron.

(cherry picked from commit 9c65e69)

Co-authored-by: Robert Butts <rob05c@users.noreply.github.com>

Author: rawlinp

traffic_ops/ort/atstccfg/toreq/toreq.go

@@ -242,45 +242,44 @@ func GetDeliveryServiceServers(cfg config.TCCfg, dsIDs []int, serverIDs []int) (
 		if err != nil {
 			return errors.New("getting delivery service servers from Traffic Ops '" + MaybeIPStr(reqInf) + "': " + err.Error())
 		}
+		dss := obj.(*[]tc.DeliveryServiceServer)
+		*dss = toDSS.Response
+		return nil
+	})
+	if err != nil {
+		return nil, errors.New("getting delivery service servers: " + err.Error())
+	}
 
-		serverIDsMap := map[int]struct{}{}
-		for _, id := range serverIDs {
-			serverIDsMap[id] = struct{}{}
-		}
+	serverIDsMap := map[int]struct{}{}
+	for _, id := range serverIDs {
+		serverIDsMap[id] = struct{}{}
+	}
+	dsIDsMap := map[int]struct{}{}
+	for _, id := range dsIDs {
+		dsIDsMap[id] = struct{}{}
+	}
 
-		dsIDsMap := map[int]struct{}{}
-		for _, id := range dsIDs {
-			dsIDsMap[id] = struct{}{}
+	// Older TO's may ignore the server ID list, so we need to filter them out manually to be sure.
+	// Also, if DeliveryServiceServersAlwaysGetAll, we need to filter here anyway.
+	filteredDSServers := []tc.DeliveryServiceServer{}
+	for _, dsServer := range dsServers {
+		if dsServer.Server == nil || dsServer.DeliveryService == nil {
+			continue // TODO warn? error?
 		}
-
-		// Older TO's may ignore the server ID list, so we need to filter them out manually to be sure.
-		filteredDSServers := []tc.DeliveryServiceServer{}
-		for _, dsServer := range toDSS.Response {
-			if dsServer.Server == nil || dsServer.DeliveryService == nil {
-				continue // TODO warn? error?
-			}
-			if len(serverIDsMap) > 0 {
-				if _, ok := serverIDsMap[*dsServer.Server]; !ok {
-					continue
-				}
+		if len(serverIDsMap) > 0 {
+			if _, ok := serverIDsMap[*dsServer.Server]; !ok {
+				continue
 			}
-			if len(dsIDsMap) > 0 {
-				if _, ok := dsIDsMap[*dsServer.DeliveryService]; !ok {
-					continue
-				}
+		}
+		if len(dsIDsMap) > 0 {
+			if _, ok := dsIDsMap[*dsServer.DeliveryService]; !ok {
+				continue
 			}
-			filteredDSServers = append(filteredDSServers, dsServer)
 		}
-
-		dss := obj.(*[]tc.DeliveryServiceServer)
-		*dss = filteredDSServers
-		return nil
-	})
-	if err != nil {
-		return nil, errors.New("getting delivery service servers: " + err.Error())
+		filteredDSServers = append(filteredDSServers, dsServer)
 	}
 
-	return dsServers, nil
+	return filteredDSServers, nil
 }
 
 func GetServerProfileParameters(cfg config.TCCfg, profileName string) ([]tc.Parameter, error) {

traffic_ops/ort/traffic_ops_ort.pl

@@ -45,6 +45,7 @@
 my $override_hostname_short = '';
 my $override_hostname_full = '';
 my $override_domainname = '';
+my $use_cache = 1;
 
 GetOptions( "dispersion=i"       => \$dispersion, # dispersion (in seconds)
             "retries=i"          => \$retries,
@@ -55,6 +56,7 @@
             "override_hostname_short=s" => \$override_hostname_short,
             "override_hostname_full=s" => \$override_hostname_full,
             "override_domainname=s" => \$override_domainname,
+            "use_cache=i" => \$use_cache,
           );
 
 if ( $#ARGV < 1 ) {
@@ -356,6 +358,7 @@ sub usage {
 	print "\t   override_hostname_short=<text> => override the short hostname of the OS for config generation. Default = ''.\n";
 	print "\t   override_hostname_full=<text>  => override the full hostname of the OS for config generation. Default = ''.\n";
 	print "\t   override_domainname=<text>     => override the domainname of the OS for config generation. Default = ''.\n";
+	print "\t   use_cache=<0|1>                => whether to use cached Traffic Ops data for config generation. Default = 1, use cache.\n";
 	print "====-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-====\n";
 	exit 1;
 }
@@ -1505,7 +1508,12 @@ sub lwp_get {
 
 	my ( $TO_USER, $TO_PASS ) = split( /:/, $TM_LOGIN );
 
-	$response_content = `$atstccfg_cmd --traffic-ops-user='$TO_USER' --traffic-ops-password='$TO_PASS' --traffic-ops-url='$request' --log-location-error=stderr --log-location-warning=stderr --log-location-info=null 2>$atstccfg_log_path`;
+	my $no_cache_arg = '';
+	if ( $use_cache == 0 ) {
+		$no_cache_arg = '--no-cache';
+	}
+
+	$response_content = `$atstccfg_cmd $no_cache_arg --traffic-ops-user='$TO_USER' --traffic-ops-password='$TO_PASS' --traffic-ops-url='$request' --log-location-error=stderr --log-location-warning=stderr --log-location-info=null 2>$atstccfg_log_path`;
 
 	my $atstccfg_exit_code = $?;
 	$atstccfg_exit_code = atstccfg_code_to_http_code($atstccfg_exit_code);