Fix t3c timeouts and retries

CHANGELOG.md

@@ -7,6 +7,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 ### Changed
 - Traffic Portal now obscures sensitive text in Delivery Service "Raw Remap" fields, private SSL keys, "Header Rewrite" rules, and ILO interface passwords by default.
 
+### Fixed
+- [#7007](https://github.com/apache/trafficcontrol/issues/7007) Fixed t3c timeout and retries not working.
+
 ## [7.0.0] - 2022-07-19
 ### Added
 - [Traffic Portal] Added Layered Profile feature to /servers/

cache-config/t3c-apply/config/config.go

@@ -83,7 +83,7 @@ type Cfg struct {
 	SkipOSCheck         bool
 	UseStrategies       t3cutil.UseStrategiesFlag
 	TOInsecure          bool
-	TOTimeoutMS         time.Duration
+	TOTimeout           time.Duration
 	TOUser              string
 	TOPass              string
 	TOURL               string
@@ -413,7 +413,6 @@ If any of the related flags are also set, they override the mode's default behav
 	skipOsCheck := *skipOSCheckPtr
 	useStrategies := t3cutil.UseStrategiesFlag(*useStrategiesPtr)
 	toInsecure := *toInsecurePtr
-	toTimeoutMS := time.Millisecond * time.Duration(*toTimeoutMSPtr)
 	toURL := *toURLPtr
 	toUser := *toUserPtr
 	toPass := *toPassPtr
@@ -511,7 +510,7 @@ If any of the related flags are also set, they override the mode's default behav
 		SkipOSCheck:                 skipOsCheck,
 		UseStrategies:               useStrategies,
 		TOInsecure:                  toInsecure,
-		TOTimeoutMS:                 toTimeoutMS,
+		TOTimeout:                   time.Millisecond * time.Duration(*toTimeoutMSPtr),
 		TOUser:                      toUser,
 		TOPass:                      toPass,
 		TOURL:                       toURL,
@@ -616,7 +615,7 @@ func printConfig(cfg Cfg) {
 	log.Debugf("ReverseProxyDisable: %t\n", cfg.ReverseProxyDisable)
 	log.Debugf("SkipOSCheck: %t\n", cfg.SkipOSCheck)
 	log.Debugf("TOInsecure: %t\n", cfg.TOInsecure)
-	log.Debugf("TOTimeoutMS: %d\n", cfg.TOTimeoutMS)
+	log.Debugf("TOTimeout: %v\n", cfg.TOTimeout)
 	log.Debugf("TOUser: %s\n", cfg.TOUser)
 	log.Debugf("TOPass: Pass len: '%d'\n", len(cfg.TOPass))
 	log.Debugf("TOURL: %s\n", cfg.TOURL)

cache-config/t3c-apply/torequest/cmd.go

@@ -234,7 +234,7 @@ func getPackages(cfg config.Cfg) ([]Package, error) {
 func sendUpdate(cfg config.Cfg, configApplyTime, revalApplyTime *time.Time, configApplyBool, revalApplyBool *bool) error {
 	args := []string{
 		`update`,
-		"--traffic-ops-timeout-milliseconds=" + strconv.FormatInt(int64(cfg.TOTimeoutMS), 10),
+		"--traffic-ops-timeout-milliseconds=" + strconv.FormatInt(int64(cfg.TOTimeout/time.Millisecond), 10),
 		"--traffic-ops-user=" + cfg.TOUser,
 		"--traffic-ops-password=" + cfg.TOPass,
 		"--traffic-ops-url=" + cfg.TOURL,
@@ -463,7 +463,7 @@ func request(cfg config.Cfg, command string) ([]byte, error) {
 	args := []string{
 		`request`,
 		"--traffic-ops-insecure=" + strconv.FormatBool(cfg.TOInsecure),
-		"--traffic-ops-timeout-milliseconds=" + strconv.FormatInt(int64(cfg.TOTimeoutMS), 10),
+		"--traffic-ops-timeout-milliseconds=" + strconv.FormatInt(int64(cfg.TOTimeout/time.Millisecond), 10),
 		"--cache-host-name=" + cfg.CacheHostName,
 		`--get-data=` + command,
 	}
@@ -521,7 +521,7 @@ func requestConfig(cfg config.Cfg) ([]byte, error) {
 	args := []string{
 		"request",
 		"--traffic-ops-insecure=" + strconv.FormatBool(cfg.TOInsecure),
-		"--traffic-ops-timeout-milliseconds=" + strconv.FormatInt(int64(cfg.TOTimeoutMS), 10),
+		"--traffic-ops-timeout-milliseconds=" + strconv.FormatInt(int64(cfg.TOTimeout/time.Millisecond), 10),
 		"--cache-host-name=" + cfg.CacheHostName,
 		`--get-data=config`,
 	}

cache-config/t3c-apply/torequest/torequest_test.go

@@ -21,6 +21,7 @@ package torequest
 
 import (
 	"testing"
+	"time"
 
 	"github.com/apache/trafficcontrol/cache-config/t3c-apply/config"
 	"github.com/apache/trafficcontrol/cache-config/t3cutil"
@@ -37,7 +38,7 @@ var testCfg config.Cfg = config.Cfg{
 	ReverseProxyDisable: false,
 	Files:               t3cutil.ApplyFilesFlagReval,
 	SkipOSCheck:         false,
-	TOTimeoutMS:         1000,
+	TOTimeout:           time.Millisecond * 1000,
 	TOUser:              "mickey",
 	TOPass:              "mouse",
 	TOURL:               "http://mouse.com",

cache-config/t3c-request/config/config.go

@@ -111,7 +111,6 @@ func InitConfig(appVersion string, gitRevision string) (Cfg, error) {
 	}
 
 	dispersion := time.Second * time.Duration(*dispersionPtr)
-	toTimeoutMS := time.Millisecond * time.Duration(*toTimeoutMSPtr)
 	toURL := *toURLPtr
 	toUser := *toUserPtr
 	toPass := *toPassPtr
@@ -156,7 +155,7 @@ func InitConfig(appVersion string, gitRevision string) (Cfg, error) {
 			CacheHostName:  cacheHostName,
 			GetData:        *getDataPtr,
 			TOInsecure:     *toInsecurePtr,
-			TOTimeoutMS:    toTimeoutMS,
+			TOTimeout:      time.Millisecond * time.Duration(*toTimeoutMSPtr),
 			TOUser:         toUser,
 			TOPass:         toPass,
 			TOURL:          toURLParsed,
@@ -193,7 +192,7 @@ func (cfg Cfg) PrintConfig() {
 	log.Debugf("LoginDispersion : %s\n", cfg.LoginDispersion)
 	log.Debugf("CacheHostName: %s\n", cfg.CacheHostName)
 	log.Debugf("TOInsecure: %v\n", cfg.TOInsecure)
-	log.Debugf("TOTimeoutMS: %s\n", cfg.TOTimeoutMS)
+	log.Debugf("TOTimeout: %v\n", cfg.TOTimeout)
 	log.Debugf("TOUser: %s\n", cfg.TOUser)
 	log.Debugf("TOPass: xxxxxx\n")
 	log.Debugf("TOURL: %s\n", cfg.TOURL)

cache-config/t3c-request/t3c-request.go

@@ -38,6 +38,12 @@ var Version = "0.4"
 // This is overwritten by the build with the current project version.
 var GitRevision = "nogit"
 
+// NumTrafficOpsRetries is the number of times to exponentially backoff and retry Traffic Ops failures.
+//
+// The exponential backoff is 2^retry seconds, so
+// 6 retries backs off up to about 2 minutes with the last try waiting about a minute.
+const NumTrafficOpsRetries = 6 // TODO make configurable
+
 func main() {
 	cfg, err := config.InitConfig(Version, GitRevision)
 	if err != nil {
@@ -52,13 +58,14 @@ func main() {
 		cfg.TOUser,
 		cfg.TOPass,
 		cfg.TOInsecure,
-		cfg.TOTimeoutMS,
+		cfg.TOTimeout,
 		cfg.UserAgent(),
 	)
 	if err != nil {
 		log.Errorf("%s\n", err)
 		os.Exit(2)
 	}
+	cfg.TCCfg.TOClient.NumRetries = NumTrafficOpsRetries
 	if cfg.TCCfg.TOClient.FellBack() {
 		log.Warnln("Traffic Ops does not support the latest version supported by this app! Falling back to previous major Traffic Ops API version!")
 	}

cache-config/t3c-update/config/config.go

@@ -176,7 +176,6 @@ func InitConfig(appVersion string, gitRevision string) (Cfg, error) {
 	}
 
 	dispersion := time.Second * time.Duration(*dispersionPtr)
-	toTimeoutMS := time.Millisecond * time.Duration(*toTimeoutMSPtr)
 	toURL := *toURLPtr
 	toUser := *toUserPtr
 	toPass := *toPassPtr
@@ -225,7 +224,7 @@ func InitConfig(appVersion string, gitRevision string) (Cfg, error) {
 			CacheHostName: cacheHostName,
 			GetData:       "update-status",
 			TOInsecure:    *toInsecurePtr,
-			TOTimeoutMS:   toTimeoutMS,
+			TOTimeout:     time.Millisecond * time.Duration(*toTimeoutMSPtr),
 			TOUser:        toUser,
 			TOPass:        toPass,
 			TOURL:         toURLParsed,

cache-config/t3c-update/t3c-update.go

@@ -40,6 +40,12 @@ var Version = "0.4"
 // This is overwritten by the build with the current project version.
 var GitRevision = "nogit"
 
+// NumTrafficOpsRetries is the number of times to exponentially backoff and retry Traffic Ops failures.
+//
+// The exponential backoff is 2^retry seconds, so
+// 6 retries backs off up to about 2 minutes with the last try waiting about a minute.
+const NumTrafficOpsRetries = 6 // TODO make configurable
+
 func main() {
 	cfg, err := config.InitConfig(Version, GitRevision)
 	if err != nil {
@@ -54,13 +60,14 @@ func main() {
 		cfg.TOUser,
 		cfg.TOPass,
 		cfg.TOInsecure,
-		cfg.TOTimeoutMS,
+		cfg.TOTimeout,
 		cfg.UserAgent(),
 	)
 	if err != nil {
 		log.Errorf("%s\n", err)
 		os.Exit(2)
 	}
+	cfg.TCCfg.TOClient.NumRetries = NumTrafficOpsRetries
 	if cfg.TCCfg.TOClient.FellBack() {
 		log.Warnln("Traffic Ops does not support the latest version supported by this app! Falling back to previous major Traffic Ops API version!")
 	}

cache-config/t3cutil/getdata.go

@@ -39,7 +39,7 @@ type TCCfg struct {
 	GetData       string
 	TOClient      *toreq.TOClient
 	TOInsecure    bool
-	TOTimeoutMS   time.Duration
+	TOTimeout     time.Duration
 	TOPass        string
 	TOUser        string
 	TOURL         *url.URL

cache-config/t3cutil/getdatacfg.go

@@ -185,6 +185,7 @@ func MakeReqMetaData(respHdr http.Header) ReqMetaData {
 //
 // The revalOnly arg is whether to only get data necessary to revalidate, versus all data necessary to generate cache config.
 func GetConfigData(toClient *toreq.TOClient, disableProxy bool, cacheHostName string, revalOnly bool, oldCfg *ConfigData, version string) (*ConfigData, error) {
+	log.Infof("GetConfigData cl.NumRetries: %+v cl.Client.Timeout %+v\n", toClient.NumRetries, toClient.Timeout())
 	start := time.Now()
 	defer func() { log.Infof("GetTOData took %v\n", time.Since(start)) }()
 

cache-config/t3cutil/toreq/client.go

@@ -66,6 +66,13 @@ type TOClient struct {
 	NumRetries int
 }
 
+func (cl *TOClient) Timeout() time.Duration {
+	if cl.c != nil {
+		return cl.c.Client.Timeout
+	}
+	return cl.old.Timeout()
+}
+
 func (cl *TOClient) URL() string {
 	if cl.c == nil {
 		return cl.old.URL()
@@ -90,6 +97,7 @@ func (cl *TOClient) HTTPClient() *http.Client {
 
 // New logs into Traffic Ops, returning the TOClient which contains the logged-in client.
 func New(url *url.URL, user string, pass string, insecure bool, timeout time.Duration, userAgent string) (*TOClient, error) {
+	log.Infof("toreq.New timeout %+v\n", timeout)
 	log.Infoln("URL: '" + url.String() + "' User: '" + user + "' Pass len: '" + strconv.Itoa(len(pass)) + "'")
 
 	cookiePath := torequtil.CookieCachePath(user)

cache-config/t3cutil/toreq/toreqold/client.go

@@ -57,6 +57,10 @@ func (cl *TOClient) APIVersion() string {
 	return cl.c.APIVersion()
 }
 
+func (cl *TOClient) Timeout() time.Duration {
+	return cl.c.Client.Timeout
+}
+
 // New logs into Traffic Ops, returning the TOClient which contains the logged-in client.
 func New(url *url.URL, user string, pass string, insecure bool, timeout time.Duration, userAgent string) (*TOClient, error) {
 	log.Infoln("URL: '" + url.String() + "' User: '" + user + "' Pass len: '" + strconv.Itoa(len(pass)) + "'")