GT-341 Fix invalid Timeout calculation in case of ActionList (#1243)

Author: jwierzbo

CHANGELOG.md

@@ -14,6 +14,7 @@
 - (Maintenance) Add & Enable YAML Linter
 - (Feature) Optional ResignLeadership Action
 - (Feature) Improve CRD Management and deprecate CRD Chart
+- (Bugfix) Fix invalid Timeout calculation in case of ActionList
 
 ## [1.2.24](https://github.com/arangodb/kube-arangodb/tree/1.2.24) (2023-01-25)
 - (Bugfix) Fix deployment creation on ARM64

pkg/deployment/reconcile/plan_executor.go

@@ -396,18 +396,30 @@ func (d *Reconciler) executeAction(ctx context.Context, planAction api.Action, a
 		log.Warn("Action aborted. Removing the entire plan")
 		d.context.CreateEvent(k8sutil.NewPlanAbortedEvent(d.context.GetAPIObject(), string(planAction.Type), planAction.MemberID, planAction.Group.AsRole()))
 		return false, true, false, false, nil
-	} else if !timeout.Infinite() {
-		if time.Now().After(planAction.CreationTime.Add(timeout.Duration)) {
-			log.Warn("Action not finished in time. Removing the entire plan")
-			d.context.CreateEvent(k8sutil.NewPlanTimeoutEvent(d.context.GetAPIObject(), string(planAction.Type), planAction.MemberID, planAction.Group.AsRole()))
-			return false, true, false, false, nil
-		}
+	} else if isActionTimeout(timeout, planAction) {
+		log.Warn("Action not finished in time. Removing the entire plan")
+		d.context.CreateEvent(k8sutil.NewPlanTimeoutEvent(d.context.GetAPIObject(), string(planAction.Type), planAction.MemberID, planAction.Group.AsRole()))
+		return false, true, false, false, nil
+
 	}
 
 	// Timeout not yet expired, come back soon
 	return false, false, true, false, nil
 }
 
+func isActionTimeout(timeout api.Timeout, planAction api.Action) bool {
+	if planAction.StartTime == nil {
+		return false
+	}
+	if planAction.StartTime.IsZero() {
+		return false
+	}
+	if timeout.Infinite() {
+		return false
+	}
+	return time.Since(planAction.StartTime.Time) > timeout.Duration
+}
+
 func (d *Reconciler) executeActionCheckProgress(ctx context.Context, action Action) (ready bool, abort bool, retErr error) {
 	retErr = panics.RecoverWithSection("ActionProgress", func() (err error) {
 		ready, abort, err = action.CheckProgress(ctx)

pkg/deployment/reconcile/plan_executor_test.go

@@ -0,0 +1,76 @@
+//
+// DISCLAIMER
+//
+// Copyright 2016-2023 ArangoDB GmbH, Cologne, Germany
+//
+// Licensed 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.
+//
+// Copyright holder is ArangoDB GmbH, Cologne, Germany
+//
+
+package reconcile
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+	meta "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1"
+)
+
+func TestIsActionTimeout(t *testing.T) {
+	type testCase struct {
+		timeout        api.Timeout
+		action         api.Action
+		expectedResult bool
+	}
+
+	timeFiveMinutesAgo := meta.Time{
+		Time: time.Now().Add(-time.Hour),
+	}
+
+	testCases := map[string]testCase{
+		"nil start time": {
+			timeout:        api.Timeout{},
+			action:         api.Action{},
+			expectedResult: false,
+		},
+		"infinite timeout": {
+			timeout:        api.NewTimeout(0),
+			action:         api.Action{},
+			expectedResult: false,
+		},
+		"timeouted case": {
+			timeout: api.NewTimeout(time.Minute),
+			action: api.Action{
+				StartTime: &timeFiveMinutesAgo,
+			},
+			expectedResult: true,
+		},
+		"still in progress case": {
+			timeout: api.NewTimeout(time.Minute * 10),
+			action: api.Action{
+				StartTime: &timeFiveMinutesAgo,
+			},
+			expectedResult: true,
+		},
+	}
+
+	for n, c := range testCases {
+		t.Run(n, func(t *testing.T) {
+			require.Equal(t, c.expectedResult, isActionTimeout(c.timeout, c.action))
+		})
+	}
+}