Skip to content

Commit

Permalink
revision request start timeout should default to revision timeout sec…
Browse files Browse the repository at this point in the history
…ond (#13256)
  • Loading branch information
dprotaso authored Aug 24, 2022
1 parent d79b8bb commit 9402a71
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 2 deletions.
4 changes: 3 additions & 1 deletion config/core/configmaps/defaults.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ metadata:
app.kubernetes.io/component: controller
app.kubernetes.io/version: devel
annotations:
knative.dev/example-checksum: "4b3e0057"
knative.dev/example-checksum: "e7973912"
data:
_example: |
################################
Expand Down Expand Up @@ -57,6 +57,8 @@ data:
# revision-response-start-timeout-seconds contains the default number of
# seconds a request will be allowed to stay open while waiting to
# receive any bytes from the user's application, if none is specified.
#
# This defaults to 'revision-timeout-seconds'
revision-response-start-timeout-seconds: "300"
# revision-idle-timeout-seconds contains the default number of
Expand Down
10 changes: 9 additions & 1 deletion pkg/apis/config/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@ func NewDefaultsConfigFromMap(data map[string]string) (*Defaults, error) {

cm.AsInt64("revision-timeout-seconds", &nc.RevisionTimeoutSeconds),
cm.AsInt64("max-revision-timeout-seconds", &nc.MaxRevisionTimeoutSeconds),
cm.AsInt64("revision-response-start-timeout-seconds", &nc.RevisionRequestStartTimeoutSeconds),
cm.AsInt64("revision-idle-timeout-seconds", &nc.RevisionIdleTimeoutSeconds),

cm.AsInt64("container-concurrency", &nc.ContainerConcurrency),
Expand All @@ -137,6 +136,15 @@ func NewDefaultsConfigFromMap(data map[string]string) (*Defaults, error) {
return nil, err
}

// We default this to what the user has specified
nc.RevisionRequestStartTimeoutSeconds = nc.RevisionTimeoutSeconds

if err := cm.Parse(data,
cm.AsInt64("revision-response-start-timeout-seconds", &nc.RevisionRequestStartTimeoutSeconds),
); err != nil {
return nil, err
}

if nc.RevisionTimeoutSeconds > nc.MaxRevisionTimeoutSeconds {
return nil, fmt.Errorf("revision-timeout-seconds (%d) cannot be greater than max-revision-timeout-seconds (%d)", nc.RevisionTimeoutSeconds, nc.MaxRevisionTimeoutSeconds)
}
Expand Down
16 changes: 16 additions & 0 deletions pkg/apis/config/defaults_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,22 @@ func TestDefaultsConfiguration(t *testing.T) {
"container-name-template": "{{.Name}}",
"init-container-name-template": "my-template",
},
}, {
name: "revision response timeout defaults to revision-timeout-seconds",
wantErr: false,
data: map[string]string{
"revision-timeout-seconds": "200",
},
wantDefaults: &Defaults{
RevisionTimeoutSeconds: 200,
RevisionRequestStartTimeoutSeconds: 200,
MaxRevisionTimeoutSeconds: DefaultMaxRevisionTimeoutSeconds,
ContainerConcurrencyMaxLimit: DefaultMaxRevisionContainerConcurrency,
AllowContainerConcurrencyZero: DefaultAllowContainerConcurrencyZero,
EnableServiceLinks: ptr.Bool(false),
InitContainerNameTemplate: DefaultInitContainerNameTemplate,
UserContainerNameTemplate: DefaultUserContainerNameTemplate,
},
}}

for _, tt := range configTests {
Expand Down

0 comments on commit 9402a71

Please sign in to comment.