Fix configuration timeout defaulting (#15617)

* Fix configuration defaulting * address comments, refactoring * refactor * lint * fixes * lint * address comments
knative · Jan 21, 2025 · 6265a8e · 6265a8e
1 parent 5842f16
commit 6265a8e
Show file tree

Hide file tree

Showing 3 changed files with 211 additions and 175 deletions.
diff --git a/pkg/apis/serving/v1/configuration_defaults_test.go b/pkg/apis/serving/v1/configuration_defaults_test.go
@@ -18,20 +18,25 @@ package v1
 
 import (
 	"context"
+	"strconv"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	"go.uber.org/zap"
 	authv1 "k8s.io/api/authentication/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"knative.dev/pkg/apis"
+	logtesting "knative.dev/pkg/logging/testing"
 	"knative.dev/pkg/ptr"
-
 	"knative.dev/serving/pkg/apis/config"
 	"knative.dev/serving/pkg/apis/serving"
+	cconfig "knative.dev/serving/pkg/reconciler/configuration/config"
 )
 
+const someTimeoutSeconds = 400
+
 func TestConfigurationDefaulting(t *testing.T) {
 	tests := []struct {
 		name string
@@ -156,6 +161,53 @@ func TestConfigurationDefaulting(t *testing.T) {
 				},
 			},
 		},
+	}, {
+		name: "run latest with identical timeout defaults",
+		in: &Configuration{
+			Spec: ConfigurationSpec{
+				Template: RevisionTemplateSpec{
+					Spec: RevisionSpec{
+						PodSpec: corev1.PodSpec{
+							EnableServiceLinks: ptr.Bool(true),
+							Containers: []corev1.Container{{
+								Image: "busybox",
+							}},
+						},
+						ContainerConcurrency: ptr.Int64(config.DefaultContainerConcurrency),
+					},
+				},
+			},
+		},
+		want: &Configuration{
+			Spec: ConfigurationSpec{
+				Template: RevisionTemplateSpec{
+					Spec: RevisionSpec{
+						PodSpec: corev1.PodSpec{
+							EnableServiceLinks: ptr.Bool(true),
+							Containers: []corev1.Container{{
+								Name:           config.DefaultUserContainerName,
+								Image:          "busybox",
+								Resources:      defaultResources,
+								ReadinessProbe: defaultProbe,
+							}},
+						},
+						TimeoutSeconds:       ptr.Int64(someTimeoutSeconds),
+						ContainerConcurrency: ptr.Int64(config.DefaultContainerConcurrency),
+					},
+				},
+			},
+		},
+		ctx: defaultConfigurationContextWithStore(logtesting.TestLogger(t), corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: config.FeaturesConfigName}},
+			corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: config.DefaultsConfigName,
+				},
+				Data: map[string]string{
+					"revision-timeout-seconds":                strconv.Itoa(someTimeoutSeconds),
+					"revision-response-start-timeout-seconds": strconv.Itoa(someTimeoutSeconds),
+					"revision-idle-timeout-seconds":           strconv.Itoa(someTimeoutSeconds),
+				},
+			})(context.Background()),
 	}}
 
 	for _, test := range tests {
@@ -328,3 +380,13 @@ func TestConfigurationUserInfo(t *testing.T) {
 		})
 	}
 }
+
+func defaultConfigurationContextWithStore(logger *zap.SugaredLogger, cms ...corev1.ConfigMap) func(ctx context.Context) context.Context {
+	return func(ctx context.Context) context.Context {
+		s := cconfig.NewStore(logger)
+		for _, cm := range cms {
+			s.OnConfigChanged(&cm)
+		}
+		return s.ToContext(ctx)
+	}
+}