From 28c7a9d18cfc9fd2cdeea361560681a9a650830a Mon Sep 17 00:00:00 2001
From: Rohit Aggarwal <rohit.aggarwal@broadcom.com>
Date: Thu, 1 Aug 2024 15:34:03 +0530
Subject: [PATCH] Adding timeout in waitRules

Signed-off-by: Rohit Aggarwal <rohit.aggarwal@broadcom.com>
---
 pkg/kapp/config/config.go                     |  1 +
 .../resourcesmisc/custom_waiting_resource.go  | 39 +++++++++++++++
 test/e2e/wait_timeout_test.go                 | 47 +++++++++++++++++++
 3 files changed, 87 insertions(+)

diff --git a/pkg/kapp/config/config.go b/pkg/kapp/config/config.go
index 014d2fa9d..06efb8616 100644
--- a/pkg/kapp/config/config.go
+++ b/pkg/kapp/config/config.go
@@ -56,6 +56,7 @@ type WaitRuleConditionMatcher struct {
 	Success                    bool
 	SupportsObservedGeneration bool
 	UnblockChanges             bool
+	Timeout                    string
 }
 
 type WaitRuleYtt struct {
diff --git a/pkg/kapp/resourcesmisc/custom_waiting_resource.go b/pkg/kapp/resourcesmisc/custom_waiting_resource.go
index 473d5c292..d1a3ef3f2 100644
--- a/pkg/kapp/resourcesmisc/custom_waiting_resource.go
+++ b/pkg/kapp/resourcesmisc/custom_waiting_resource.go
@@ -5,12 +5,16 @@ package resourcesmisc
 
 import (
 	"fmt"
+	"sync"
+	"time"
 
 	ctlconf "carvel.dev/kapp/pkg/kapp/config"
 	ctlres "carvel.dev/kapp/pkg/kapp/resources"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+var timeoutMap sync.Map
+
 type CustomWaitingResource struct {
 	resource ctlres.Resource
 	waitRule ctlconf.WaitRule
@@ -81,12 +85,28 @@ func (s CustomWaitingResource) IsDoneApplying() DoneApplyState {
 	hasConditionWaitingForGeneration := false
 	// Check on failure conditions first
 	for _, condMatcher := range s.waitRule.ConditionMatchers {
+		// Check whether timeout has occured
+		var isTimeOutConditionPresent bool
+
 		for _, cond := range obj.Status.Conditions {
 			if cond.Type == condMatcher.Type && cond.Status == condMatcher.Status {
 				if condMatcher.SupportsObservedGeneration && obj.Metadata.Generation != cond.ObservedGeneration {
 					hasConditionWaitingForGeneration = true
 					continue
 				}
+
+				if condMatcher.Timeout != "" {
+					isTimeOutConditionPresent = true
+					if s.hasTimeoutOccurred(condMatcher.Timeout, fmt.Sprintf("%s.%s", s.resource.Namespace(), s.resource.Name())) {
+						return DoneApplyState{Done: true, Successful: false, Message: fmt.Sprintf(
+							"Encountered failure condition %s == %s: %s (message: %s) continuously for %s duration",
+							cond.Type, condMatcher.Status, cond.Reason, cond.Message, condMatcher.Timeout)}
+					}
+					return DoneApplyState{Done: false, Message: fmt.Sprintf(
+						"%s: %s (message: %s)",
+						cond.Type, cond.Reason, cond.Message)}
+				}
+
 				if condMatcher.Failure {
 					return DoneApplyState{Done: true, Successful: false, Message: fmt.Sprintf(
 						"Encountered failure condition %s == %s: %s (message: %s)",
@@ -94,6 +114,12 @@ func (s CustomWaitingResource) IsDoneApplying() DoneApplyState {
 				}
 			}
 		}
+
+		// Reset the timer in case timeout condition flipped from being present to not present in the Cluster resource status
+		if !isTimeOutConditionPresent {
+			timeoutMap.Delete(fmt.Sprintf("%s.%s", s.resource.Namespace(), s.resource.Name()))
+			continue
+		}
 	}
 
 	unblockChangeMsg := ""
@@ -132,3 +158,16 @@ func (s CustomWaitingResource) IsDoneApplying() DoneApplyState {
 
 	return DoneApplyState{Done: false, Message: "No failing or successful conditions found"}
 }
+
+func (s CustomWaitingResource) hasTimeoutOccurred(timeout string, key string) bool {
+	expiryTime, found := timeoutMap.Load(key)
+	if found {
+		return time.Now().Sub(expiryTime.(time.Time)) > 0
+	}
+	dur, err := time.ParseDuration(timeout)
+	if err != nil {
+		dur = 15 * time.Minute
+	}
+	timeoutMap.Store(key, time.Now().Add(dur))
+	return false
+}
diff --git a/test/e2e/wait_timeout_test.go b/test/e2e/wait_timeout_test.go
index f77fa4608..29e4ac5e0 100644
--- a/test/e2e/wait_timeout_test.go
+++ b/test/e2e/wait_timeout_test.go
@@ -4,6 +4,7 @@
 package e2e
 
 import (
+	"fmt"
 	"strings"
 	"testing"
 
@@ -32,6 +33,33 @@ func TestWaitTimeout(t *testing.T) {
        restartPolicy: Never
 `
 
+	yaml2 := `
+apiVersion: v1
+kind: Pod
+metadata:
+  name: nginx
+spec:
+  containers:
+  - name: nginx
+    image: %s
+    ports:
+    - containerPort: 80
+---
+apiVersion: kapp.k14s.io/v1alpha1
+kind: Config
+waitRules:
+- supportsObservedGeneration: true
+  conditionMatchers:
+  - type: ContainersReady
+    status: "False"
+    timeout: 50s
+  - type: Ready
+    status: "True"
+    success: true
+  resourceMatchers:
+  - apiVersionKindMatcher: {apiVersion: v1, kind: Pod}
+`
+
 	name := "test-wait-timeout"
 	cleanUp := func() {
 		kapp.Run([]string{"delete", "-a", name})
@@ -67,4 +95,23 @@ func TestWaitTimeout(t *testing.T) {
 
 		require.NoErrorf(t, err, "Expected to be successful without resource timeout")
 	})
+
+	cleanUp()
+
+	logger.Section("Deploy timeout after staying in a condition for certain time", func() {
+		_, err := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--json"},
+			RunOpts{IntoNs: true, AllowError: true, StdinReader: strings.NewReader(fmt.Sprintf(yaml2, "nginx:200"))})
+
+		require.Error(t, err)
+		require.Contains(t, err.Error(), "message: containers with unready status: [nginx]) continuously for 50s duration")
+	})
+
+	cleanUp()
+
+	logger.Section("Deploy should be successful", func() {
+		_, err := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--json"},
+			RunOpts{IntoNs: true, AllowError: true, StdinReader: strings.NewReader(fmt.Sprintf(yaml2, "nginx"))})
+
+		require.NoError(t, err)
+	})
 }