Add a new config-deployment parameter

config/core/configmaps/deployment.yaml

@@ -43,3 +43,7 @@ data:
 
     # List of repositories for which tag to digest resolving should be skipped
     registriesSkippingTagResolving: "ko.local,dev.local"
+
+    # ProgressDeadline is the duration we wait for the deployment to
+    # be ready before considering it failed.
+    progressDeadline: "120s"

pkg/deployment/config.go

@@ -18,7 +18,9 @@ package deployment
 
 import (
 	"errors"
+	"fmt"
 	"strings"
+	"time"
 
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
@@ -29,23 +31,43 @@ const (
 	ConfigName = "config-deployment"
 
 	// QueueSidecarImageKey is the config map key for queue sidecar image
-	QueueSidecarImageKey           = "queueSidecarImage"
-	registriesSkippingTagResolving = "registriesSkippingTagResolving"
+	QueueSidecarImageKey = "queueSidecarImage"
+
+	// ProgressDeadlineDefault is the default value for the config's
+	// ProgressDeadlineSeconds.
+	ProgressDeadlineDefault = 120 * time.Second
+
+	registriesSkippingTagResolvingKey = "registriesSkippingTagResolving"
+	progressDeadlineKey               = "progressDeadline"
 )
 
+func defaultConfig() *Config {
+	return &Config{
+		ProgressDeadline:               ProgressDeadlineDefault,
+		RegistriesSkippingTagResolving: sets.NewString("ko.local", "dev.local"),
+	}
+}
+
 // NewConfigFromMap creates a DeploymentConfig from the supplied Map
 func NewConfigFromMap(configMap map[string]string) (*Config, error) {
-	nc := &Config{}
+	nc := defaultConfig()
 	qsideCarImage, ok := configMap[QueueSidecarImageKey]
 	if !ok {
 		return nil, errors.New("queue sidecar image is missing")
 	}
 	nc.QueueSidecarImage = qsideCarImage
 
-	if registries, ok := configMap[registriesSkippingTagResolving]; !ok {
-		// It is ok if registries are missing.
-		nc.RegistriesSkippingTagResolving = sets.NewString("ko.local", "dev.local")
-	} else {
+	if pd, ok := configMap[progressDeadlineKey]; ok {
+		v, err := time.ParseDuration(pd)
+		if err != nil {
+			return nil, fmt.Errorf("error parsing %s=%s as duration, %w", progressDeadlineKey, pd, err)
+		} else if v <= 0 {
+			return nil, fmt.Errorf("%s cannot be non-positive duration, was %v", progressDeadlineKey, v)
+		}
+		nc.ProgressDeadline = v
+	}
+
+	if registries, ok := configMap[registriesSkippingTagResolvingKey]; ok {
 		nc.RegistriesSkippingTagResolving = sets.NewString(strings.Split(registries, ",")...)
 	}
 	return nc, nil
@@ -64,4 +86,8 @@ type Config struct {
 
 	// Repositories for which tag to digest resolving should be skipped
 	RegistriesSkippingTagResolving sets.String
+
+	// ProgressDeadline is the time in seconds we wait for the deployment to
+	// be ready before considering it failed.
+	ProgressDeadline time.Duration
 }

pkg/deployment/config_test.go

@@ -18,92 +18,123 @@ package deployment
 
 import (
 	"testing"
+	"time"
 
 	"github.com/google/go-cmp/cmp"
+
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
-	"knative.dev/pkg/system"
 
 	. "knative.dev/pkg/configmap/testing"
+	"knative.dev/pkg/system"
 	_ "knative.dev/pkg/system/testing"
 )
 
-var noSidecarImage = ""
+const noSidecarImage = ""
 
 func TestControllerConfigurationFromFile(t *testing.T) {
 	cm, example := ConfigMapsFromTestFile(t, ConfigName, QueueSidecarImageKey)
 
 	if _, err := NewConfigFromConfigMap(cm); err != nil {
-		t.Errorf("NewConfigFromConfigMap(actual) = %v", err)
+		t.Error("NewConfigFromConfigMap(actual) =", err)
 	}
 
-	if _, err := NewConfigFromConfigMap(example); err != nil {
-		t.Errorf("NewConfigFromConfigMap(example) = %v", err)
+	if got, err := NewConfigFromConfigMap(example); err != nil {
+		t.Error("NewConfigFromConfigMap(example) =", err)
+	} else {
+		want := defaultConfig()
+		// We require QSI to be explicitly set. So do it here.
+		want.QueueSidecarImage = "ko://knative.dev/serving/cmd/queue"
+		if !cmp.Equal(got, want) {
+			t.Error("Example stanza does not match default, diff(-want,+got):", cmp.Diff(want, got))
+		}
 	}
 }
 
 func TestControllerConfiguration(t *testing.T) {
 	configTests := []struct {
-		name           string
-		wantErr        bool
-		wantController interface{}
-		config         *corev1.ConfigMap
+		name       string
+		wantErr    bool
+		wantConfig *Config
+		data       map[string]string
 	}{{
-		name:    "controller configuration with bad registries",
-		wantErr: false,
-		wantController: &Config{
+		name: "controller configuration with bad registries",
+		wantConfig: &Config{
 			RegistriesSkippingTagResolving: sets.NewString("ko.local", ""),
 			QueueSidecarImage:              noSidecarImage,
+			ProgressDeadline:               ProgressDeadlineDefault,
+		},
+		data: map[string]string{
+			QueueSidecarImageKey:              noSidecarImage,
+			registriesSkippingTagResolvingKey: "ko.local,,",
+		},
+	}, {
+		name: "controller configuration good progress deadline",
+		wantConfig: &Config{
+			RegistriesSkippingTagResolving: sets.NewString("ko.local", "dev.local"),
+			QueueSidecarImage:              noSidecarImage,
+			ProgressDeadline:               444 * time.Second,
+		},
+		data: map[string]string{
+			QueueSidecarImageKey: noSidecarImage,
+			progressDeadlineKey:  "444s",
 		},
-		config: &corev1.ConfigMap{
-			ObjectMeta: metav1.ObjectMeta{
-				Namespace: system.Namespace(),
-				Name:      ConfigName,
-			},
-			Data: map[string]string{
-				QueueSidecarImageKey:           noSidecarImage,
-				registriesSkippingTagResolving: "ko.local,,",
-			},
-		}}, {
-		name:    "controller configuration with registries",
-		wantErr: false,
-		wantController: &Config{
+	}, {
+		name: "controller configuration with registries",
+		wantConfig: &Config{
 			RegistriesSkippingTagResolving: sets.NewString("ko.local", "ko.dev"),
 			QueueSidecarImage:              noSidecarImage,
+			ProgressDeadline:               ProgressDeadlineDefault,
+		},
+		data: map[string]string{
+			QueueSidecarImageKey:              noSidecarImage,
+			registriesSkippingTagResolvingKey: "ko.local,ko.dev",
+		},
+	}, {
+		name:    "controller with no side car image",
+		wantErr: true,
+		data:    map[string]string{},
+	}, {
+		name:    "controller configuration invalid progress deadline",
+		wantErr: true,
+		data: map[string]string{
+			QueueSidecarImageKey: noSidecarImage,
+			progressDeadlineKey:  "not-a-number",
 		},
-		config: &corev1.ConfigMap{
-			ObjectMeta: metav1.ObjectMeta{
-				Namespace: system.Namespace(),
-				Name:      ConfigName,
-			},
-			Data: map[string]string{
-				QueueSidecarImageKey:           noSidecarImage,
-				registriesSkippingTagResolving: "ko.local,ko.dev",
-			},
+	}, {
+		name:    "controller configuration invalid progress deadline II",
+		wantErr: true,
+		data: map[string]string{
+			QueueSidecarImageKey: noSidecarImage,
+			progressDeadlineKey:  "-21",
 		},
 	}, {
-		name:           "controller with no side car image",
-		wantErr:        true,
-		wantController: (*Config)(nil),
-		config: &corev1.ConfigMap{
-			ObjectMeta: metav1.ObjectMeta{
-				Namespace: system.Namespace(),
-				Name:      ConfigName,
-			},
-			Data: map[string]string{},
+		name:    "controller configuration invalid progress deadline III",
+		wantErr: true,
+		data: map[string]string{
+			QueueSidecarImageKey: noSidecarImage,
+			progressDeadlineKey:  "0",
 		},
 	}}
 
 	for _, tt := range configTests {
-		actualController, err := NewConfigFromConfigMap(tt.config)
+		t.Run(tt.name, func(t *testing.T) {
+			gotConfig, err := NewConfigFromConfigMap(&corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: system.Namespace(),
+					Name:      ConfigName,
+				},
+				Data: tt.data,
+			})
 
-		if (err != nil) != tt.wantErr {
-			t.Fatalf("Test: %q; NewConfigFromConfigMap() error = %v, WantErr %v", tt.name, err, tt.wantErr)
-		}
+			if (err != nil) != tt.wantErr {
+				t.Fatalf("NewConfigFromConfigMap() error = %v, want: %v", err, tt.wantErr)
+			}
 
-		if diff := cmp.Diff(actualController, tt.wantController); diff != "" {
-			t.Fatalf("Test: %q; want %v, but got %v", tt.name, tt.wantController, actualController)
-		}
+			if got, want := gotConfig, tt.wantConfig; !cmp.Equal(got, want) {
+				t.Error("Config mismatch, diff(-want,+got):", cmp.Diff(want, got))
+			}
+		})
 	}
 }

pkg/reconciler/autoscaling/kpa/scaler.go

@@ -32,10 +32,10 @@ import (
 	pav1alpha1 "knative.dev/serving/pkg/apis/autoscaling/v1alpha1"
 	"knative.dev/serving/pkg/apis/networking"
 	nv1a1 "knative.dev/serving/pkg/apis/networking/v1alpha1"
+	"knative.dev/serving/pkg/deployment"
 	"knative.dev/serving/pkg/network"
 	"knative.dev/serving/pkg/reconciler/autoscaling/config"
 	aresources "knative.dev/serving/pkg/reconciler/autoscaling/resources"
-	rresources "knative.dev/serving/pkg/reconciler/revision/resources"
 	"knative.dev/serving/pkg/resources"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -61,7 +61,7 @@ const (
 	// race the Revision reconciler and scale down the pods before it can actually surface the pod errors.
 	// We should instead do pod failure diagnostics here immediately before scaling down the Deployment.
 	activationTimeoutBuffer = 10 * time.Second
-	activationTimeout       = time.Duration(rresources.ProgressDeadlineSeconds)*time.Second + activationTimeoutBuffer
+	activationTimeout       = deployment.ProgressDeadlineDefault + activationTimeoutBuffer
 )
 
 var probeOptions = []interface{}{

pkg/reconciler/revision/resources/constants.go

@@ -30,10 +30,6 @@ const (
 
 	// AppLabelKey is the label defining the application's name.
 	AppLabelKey = "app"
-
-	// ProgressDeadlineSeconds is the time in seconds we wait for the deployment to
-	// be ready before considering it failed.
-	ProgressDeadlineSeconds = int32(120)
 )
 
 var (

pkg/reconciler/revision/resources/deploy.go

@@ -260,7 +260,7 @@ func MakeDeployment(rev *v1.Revision,
 		Spec: appsv1.DeploymentSpec{
 			Replicas:                ptr.Int32(1),
 			Selector:                makeSelector(rev),
-			ProgressDeadlineSeconds: ptr.Int32(ProgressDeadlineSeconds),
+			ProgressDeadlineSeconds: ptr.Int32(int32(deployment.ProgressDeadlineDefault.Seconds())),
 			Template: corev1.PodTemplateSpec{
 				ObjectMeta: metav1.ObjectMeta{
 					Labels:      makeLabels(rev),

pkg/reconciler/revision/resources/deploy_test.go

@@ -37,6 +37,7 @@ import (
 	"knative.dev/serving/pkg/apis/serving"
 	v1 "knative.dev/serving/pkg/apis/serving/v1"
 	autoscalerconfig "knative.dev/serving/pkg/autoscaler/config"
+	"knative.dev/serving/pkg/deployment"
 	"knative.dev/serving/pkg/network"
 )
 
@@ -203,7 +204,7 @@ var (
 					serving.RevisionUID: "1234",
 				},
 			},
-			ProgressDeadlineSeconds: ptr.Int32(ProgressDeadlineSeconds),
+			ProgressDeadlineSeconds: ptr.Int32(int32(deployment.ProgressDeadlineDefault.Seconds())),
 			Template: corev1.PodTemplateSpec{
 				ObjectMeta: metav1.ObjectMeta{
 					Labels: map[string]string{