From 92f4aef64aee555be03972ed4cc9e44d66b7b51d Mon Sep 17 00:00:00 2001
From: Stavros Kontopoulos <st.kontopoulos@gmail.com>
Date: Tue, 7 Nov 2023 15:32:06 +0200
Subject: [PATCH] respect revision timeout defaults in cm

---
 pkg/apis/serving/v1/revision_defaults.go      | 16 +++++++++
 pkg/apis/serving/v1/revision_defaults_test.go | 34 +++++++++++++++++++
 2 files changed, 50 insertions(+)

diff --git a/pkg/apis/serving/v1/revision_defaults.go b/pkg/apis/serving/v1/revision_defaults.go
index 2b3f5f2f29da..9c0e4e892ba4 100644
--- a/pkg/apis/serving/v1/revision_defaults.go
+++ b/pkg/apis/serving/v1/revision_defaults.go
@@ -53,6 +53,22 @@ func (rs *RevisionSpec) SetDefaults(ctx context.Context) {
 		rs.TimeoutSeconds = ptr.Int64(cfg.Defaults.RevisionTimeoutSeconds)
 	}
 
+	// Default IdleTimeoutSeconds only in case we have a non-zero default and the latter is not larger than the revision timeout.
+	// A zero default or a zero value set from the user or a nil value skips timer setup at the QP side.
+	if rs.IdleTimeoutSeconds == nil {
+		if cfg.Defaults.RevisionIdleTimeoutSeconds < *rs.TimeoutSeconds && cfg.Defaults.RevisionIdleTimeoutSeconds != 0 {
+			rs.IdleTimeoutSeconds = ptr.Int64(cfg.Defaults.RevisionIdleTimeoutSeconds)
+		}
+	}
+
+	// Default ResponseStartTimeoutSeconds only in case we have a non-zero default and the latter is not larger than the revision timeout.
+	// A zero default or a zero value set from the user or a nil value skips timer setup at the QP side.
+	if rs.ResponseStartTimeoutSeconds == nil {
+		if cfg.Defaults.RevisionRequestStartTimeoutSeconds < *rs.TimeoutSeconds && cfg.Defaults.RevisionRequestStartTimeoutSeconds != 0 {
+			rs.ResponseStartTimeoutSeconds = ptr.Int64(cfg.Defaults.RevisionRequestStartTimeoutSeconds)
+		}
+	}
+
 	// Default ContainerConcurrency based on our configmap.
 	if rs.ContainerConcurrency == nil {
 		rs.ContainerConcurrency = ptr.Int64(cfg.Defaults.ContainerConcurrency)
diff --git a/pkg/apis/serving/v1/revision_defaults_test.go b/pkg/apis/serving/v1/revision_defaults_test.go
index 0fe5e65079b7..31aaf22ded07 100644
--- a/pkg/apis/serving/v1/revision_defaults_test.go
+++ b/pkg/apis/serving/v1/revision_defaults_test.go
@@ -95,6 +95,40 @@ func TestRevisionDefaulting(t *testing.T) {
 				},
 			},
 		},
+	}, {
+		name: "all revision timeouts set",
+		in:   &Revision{Spec: RevisionSpec{PodSpec: corev1.PodSpec{Containers: []corev1.Container{{}}}}},
+		wc: func(ctx context.Context) context.Context {
+			s := config.NewStore(logger)
+			s.OnConfigChanged(&corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: autoscalerconfig.ConfigName}})
+			s.OnConfigChanged(&corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: config.FeaturesConfigName}})
+			s.OnConfigChanged(&corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: config.DefaultsConfigName,
+				},
+				Data: map[string]string{
+					"revision-timeout-seconds":                "423",
+					"revision-idle-timeout-seconds":           "100",
+					"revision-response-start-timeout-seconds": "50",
+				},
+			})
+			return s.ToContext(ctx)
+		},
+		want: &Revision{
+			Spec: RevisionSpec{
+				ContainerConcurrency:        ptr.Int64(0),
+				TimeoutSeconds:              ptr.Int64(423),
+				ResponseStartTimeoutSeconds: ptr.Int64(50),
+				IdleTimeoutSeconds:          ptr.Int64(100),
+				PodSpec: corev1.PodSpec{
+					Containers: []corev1.Container{{
+						Name:           config.DefaultUserContainerName,
+						Resources:      defaultResources,
+						ReadinessProbe: defaultProbe,
+					}},
+				},
+			},
+		},
 	}, {
 		name: "with context, in create, expect ESL set",
 		in:   &Revision{Spec: RevisionSpec{PodSpec: corev1.PodSpec{Containers: []corev1.Container{{}}}}},