Repository: flant/shell-operator (go)

Issue:
Panic on retry
It is probably related to the new `exponential backoff` feature.

The shell-operator pod crashes with panic after some retry attempts. The logs are from an addon-operator, but hook retries cause the same panic.
```
{"binding":"ReloadAllModules","event.type":"OperatorStartup","level":"error","module":"istio","module.state":"failed","msg":"ModuleRun failed. Requeue task to retry after delay. Failed count is 36. Error: helm upgrade failed: exit status 1:\n Error: UPGRADE FAILED: rendered manifests contain a resource that already exists. Unable to continue with update: Secret \"registry\" in namespace \"sample-myrev\" exists and cannot be imported into the current release: invalid ownership metadata; label validation error: missing key \"app.kubernetes.io/managed-by\": must be set to \"Helm\"; annotation validation error: missing key \"meta.helm.sh/release-name\": must be set to \"istio\"; annotation validation error: missing key \"meta.helm.sh/release-namespace\": must be set to \"system\"","operator.component":"taskRunner","queue":"main","task.id":"60561c82-ad02-4543-a8bd-045fb5a81566","time":"2020-12-05T05:59:49Z"}
panic: non-positive interval for NewTicker

goroutine 145 [running]:
time.NewTicker(0x8000000145214600, 0x10c0761fb00)
	/usr/lib/go/src/time/tick.go:23 +0x147
github.com/flant/shell-operator/pkg/task/queue.(*TaskQueue).waitForTask(0xc000becb40, 0x8000000145214600, 0x2654ae2, 0x21)
	/usr/local/go/pkg/mod/github.com/flant/shell-operator@v1.0.0-beta.13/pkg/task/queue/task_queue.go:427 +0x247
github.com/flant/shell-operator/pkg/task/queue.(*TaskQueue).Start.func1(0xc000becb40)
	/usr/local/go/pkg/mod/github.com/flant/shell-operator@v1.0.0-beta.13/pkg/task/queue/task_queue.go:325 +0x166
created by github.com/flant/shell-operator/pkg/task/queue.(*TaskQueue).Start
	/usr/local/go/pkg/mod/github.com/flant/shell-operator@v1.0.0-beta.13/pkg/task/queue/task_queue.go:320 +0x4f
```

Pull request:
fix: prevent overflow in exponential backoff calculation
Fix #218 

Patch:
diff --git a/pkg/utils/exponential_backoff/delay.go b/pkg/utils/exponential_backoff/delay.go
index 3d78959..cc4cb37 100644
--- a/pkg/utils/exponential_backoff/delay.go
+++ b/pkg/utils/exponential_backoff/delay.go
@@ -7,9 +7,12 @@ import (
 )
 
 const MaxExponentialBackoffDelay = time.Duration(32 * time.Second)
-const ExponentialDelayFactor float64 = 2.0 // Each delay is twice longer.
+const ExponentialDelayFactor float64 = 2.0 // Each delta delay is twice bigger.
 const ExponentialDelayRandomMs = 1000      // Each delay has random additional milliseconds.
 
+// Count of exponential calculations before return max delay to prevent overflow with big numbers.
+var ExponentialCalculationsCount = int(math.Log(MaxExponentialBackoffDelay.Seconds()) / math.Log(ExponentialDelayFactor))
+
 // CalculateDelay returns delay distributed from initialDelay to default maxDelay (32s)
 //
 // Example:
@@ -25,13 +28,30 @@ func CalculateDelay(initialDelay time.Duration, retryCount int) time.Duration {
 	return CalculateDelayWithMax(initialDelay, MaxExponentialBackoffDelay, retryCount)
 }
 
+// CalculateDelayWithMax returns delay distributed from initialDelay to maxDelay based on retryCount number.
+//
+// Delay for retry number 0 is an initialDelay.
+//
+// Calculation of exponential delays starts from retry number 1.
+//
+// After ExponentialCalculationsCount rounds of calculations, maxDelay is returned.
 func CalculateDelayWithMax(initialDelay time.Duration, maxDelay time.Duration, retryCount int) time.Duration {
-	if retryCount == 0 {
+	var delayNs int64
+	switch {
+	case retryCount == 0:
 		return initialDelay
+	case retryCount <= ExponentialCalculationsCount:
+		// Calculate exponential delta for delay.
+		delayNs = int64(float64(time.Second) * math.Pow(ExponentialDelayFactor, float64(retryCount-1)))
+	default:
+		// No calculation, return maxDelay.
+		delayNs = maxDelay.Nanoseconds()
 	}
-	delayNs := int64(float64(time.Second) * math.Pow(ExponentialDelayFactor, float64(retryCount-1)))
-	rndDelayMs := rand.Intn(ExponentialDelayRandomMs)
-	delay := initialDelay + time.Duration(delayNs) + time.Duration(rndDelayMs)*time.Millisecond
+
+	// Random addition to delay.
+	rndDelayNs := rand.Int63n(ExponentialDelayRandomMs) * int64(time.Millisecond)
+
+	delay := initialDelay + time.Duration(delayNs) + time.Duration(rndDelayNs)
 	delay = delay.Truncate(100 * time.Millisecond)
 	if delay.Nanoseconds() > maxDelay.Nanoseconds() {
 		return maxDelay


Test patch:
diff --git a/pkg/utils/exponential_backoff/delay_test.go b/pkg/utils/exponential_backoff/delay_test.go
index bc0d56d..464a0f1 100644
--- a/pkg/utils/exponential_backoff/delay_test.go
+++ b/pkg/utils/exponential_backoff/delay_test.go
@@ -7,12 +7,29 @@ import (
 )
 
 func Test_Delay(t *testing.T) {
-	t.SkipNow()
+	var initialSeconds int64 = 5
+	initialDelay := time.Duration(5) * time.Second
 	rand.Seed(time.Now().UnixNano())
-	for i := 0; i < 16; i++ {
-		t.Logf("Delay for %02d retry: %s\n",
-			i,
-			CalculateDelay(5*time.Second, i),
-		)
+	for i := 0; i < 64; i++ {
+		delay := CalculateDelay(initialDelay, i)
+		seconds := int64(delay.Seconds())
+
+		switch {
+		case i == 0:
+			if seconds != initialSeconds {
+				t.Fatalf("delay for retry %d should be %d seconds, actual delay is %s", i, initialSeconds, delay)
+			}
+		case i <= ExponentialCalculationsCount:
+			if seconds > int64(MaxExponentialBackoffDelay.Seconds()) || seconds < 0 {
+				t.Fatalf("delay for retry %d should be greater than 0 and less than MaxExponentialBackoffDelay=%d seconds, actual delay is %s", i, int64(MaxExponentialBackoffDelay.Seconds()), delay)
+			}
+		case i > ExponentialCalculationsCount:
+			if seconds != int64(MaxExponentialBackoffDelay.Seconds()) {
+				t.Fatalf("delay for retry %d should be MaxExponentialBackoffDelay=%d seconds, actual delay is %s", i, int64(MaxExponentialBackoffDelay.Seconds()), delay)
+			}
+		}
+
+		// debug messages
+		//t.Logf("Delay for %02d retry: %s\n", i, delay)
 	}
 }