You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by ra...@apache.org on 2020/02/10 21:29:25 UTC

[trafficcontrol] branch 4.0.x updated: Fix atstccfg Delivery Service Server caching. (#4387) (#4389)

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

rawlin pushed a commit to branch 4.0.x
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git


The following commit(s) were added to refs/heads/4.0.x by this push:
     new 92a1428  Fix atstccfg Delivery Service Server caching. (#4387) (#4389)
92a1428 is described below

commit 92a14286c60e46da08d08c5dfa3114b31e44311a
Author: Rawlin Peters <ra...@apache.org>
AuthorDate: Mon Feb 10 14:29:17 2020 -0700

    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 9c65e69ac5429d533ff400e2694bce94a8d439b1)
    
    Co-authored-by: Robert Butts <ro...@users.noreply.github.com>
---
 traffic_ops/ort/atstccfg/toreq/toreq.go | 61 ++++++++++++++++-----------------
 traffic_ops/ort/traffic_ops_ort.pl      | 10 +++++-
 2 files changed, 39 insertions(+), 32 deletions(-)

diff --git a/traffic_ops/ort/atstccfg/toreq/toreq.go b/traffic_ops/ort/atstccfg/toreq/toreq.go
index c0fec11..eb9ecf0 100644
--- a/traffic_ops/ort/atstccfg/toreq/toreq.go
+++ b/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) {
diff --git a/traffic_ops/ort/traffic_ops_ort.pl b/traffic_ops/ort/traffic_ops_ort.pl
index ac56b5c..cb43da0 100755
--- a/traffic_ops/ort/traffic_ops_ort.pl
+++ b/traffic_ops/ort/traffic_ops_ort.pl
@@ -45,6 +45,7 @@ my $skip_os_check = 0;
 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 @@ GetOptions( "dispersion=i"       => \$dispersion, # dispersion (in seconds)
             "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);