diff --git a/pkg/apis/autoscaling/v1alpha1/pa_lifecycle.go b/pkg/apis/autoscaling/v1alpha1/pa_lifecycle.go index 9cff0ab9f547..ab39d5558c49 100644 --- a/pkg/apis/autoscaling/v1alpha1/pa_lifecycle.go +++ b/pkg/apis/autoscaling/v1alpha1/pa_lifecycle.go @@ -173,6 +173,11 @@ func (pa *PodAutoscaler) ProgressDeadline() (time.Duration, bool) { return pa.annotationDuration(serving.ProgressDeadlineAnnotation) } +// RevisionTimeout returns the revision timeout annotation value, or false if not present. +func (pa *PodAutoscaler) RevisionTimeout() (time.Duration, bool) { + return pa.annotationDuration(serving.RevisionTimeoutAnnotation) +} + // InitialScale returns the initial scale on the revision if present, or false if not present. func (pa *PodAutoscaler) InitialScale() (int32, bool) { // The value is validated in the webhook. @@ -187,6 +192,12 @@ func (pa *PodAutoscaler) IsReady() bool { pas.GetCondition(PodAutoscalerConditionReady).IsTrue() } +// CanFailActivationOnUnreachableRevision checks whether the pod autoscaler is Unreachable and has been activating +// for at least the specified idle period +func (pa *PodAutoscaler) CanFailActivationOnUnreachableRevision(now time.Time, idlePeriod time.Duration) bool { + return pa.Spec.Reachability == ReachabilityUnreachable && pa.Status.CanFailActivation(now, idlePeriod) +} + // IsActive returns true if the pod autoscaler has finished scaling. func (pas *PodAutoscalerStatus) IsActive() bool { return pas.GetCondition(PodAutoscalerConditionActive).IsTrue() diff --git a/pkg/apis/serving/register.go b/pkg/apis/serving/register.go index 7fc51964f1e7..f41a6ec065c5 100644 --- a/pkg/apis/serving/register.go +++ b/pkg/apis/serving/register.go @@ -144,6 +144,9 @@ const ( // ProgressDeadlineAnnotationKey is the label key for the per revision progress deadline to set for the deployment ProgressDeadlineAnnotationKey = GroupName + "/progress-deadline" + + // RevisionTimeoutAnnotationKey is the label key for the revision timeout to set for the pod autoscaler + RevisionTimeoutAnnotationKey = GroupName + "/revision-timeout" ) var ( @@ -202,4 +205,7 @@ var ( ProgressDeadlineAnnotation = kmap.KeyPriority{ ProgressDeadlineAnnotationKey, } + RevisionTimeoutAnnotation = kmap.KeyPriority{ + RevisionTimeoutAnnotationKey, + } ) diff --git a/pkg/autoscaler/scaling/autoscaler.go b/pkg/autoscaler/scaling/autoscaler.go index 01f815f5d942..73144843f3cc 100644 --- a/pkg/autoscaler/scaling/autoscaler.go +++ b/pkg/autoscaler/scaling/autoscaler.go @@ -169,7 +169,13 @@ func (a *autoscaler) Scale(logger *zap.SugaredLogger, now time.Time) ScaleResult } else { logger.Errorw("Failed to obtain metrics", zap.Error(err)) } - return invalidSR + if errors.Is(err, metrics.ErrNoData) && !a.panicTime.IsZero() && a.panicTime.Add(spec.StableWindow).Before(now) { + // We are not receiving data, but we panicked before, and the panic stable window has gone, we want to continue. + // We want to un-panic and scale down to 0 if the rest of the conditions allow it + logger.Debug("No data to scale on yet, and panicked before, but the stable window has gone, we want to continue") + } else { + return invalidSR + } } // Make sure we don't get stuck with the same number of pods, if the scale up rate diff --git a/pkg/reconciler/autoscaling/kpa/kpa_test.go b/pkg/reconciler/autoscaling/kpa/kpa_test.go index c45d96449c6c..814eac5727b9 100644 --- a/pkg/reconciler/autoscaling/kpa/kpa_test.go +++ b/pkg/reconciler/autoscaling/kpa/kpa_test.go @@ -101,6 +101,7 @@ const ( paStableWindow = 45 * time.Second progressDeadline = 121 * time.Second stableWindow = 5 * time.Minute + defaultRevisionTimeout = 45 ) func defaultConfigMapData() map[string]string { @@ -1782,7 +1783,9 @@ func newTestRevision(namespace, name string) *v1.Revision { serving.ConfigurationLabelKey: "test-service", }, }, - Spec: v1.RevisionSpec{}, + Spec: v1.RevisionSpec{ + TimeoutSeconds: ptr.Int64(defaultRevisionTimeout), + }, } } diff --git a/pkg/reconciler/autoscaling/kpa/scaler.go b/pkg/reconciler/autoscaling/kpa/scaler.go index 5cb95f2fc719..b9a788a91fcf 100644 --- a/pkg/reconciler/autoscaling/kpa/scaler.go +++ b/pkg/reconciler/autoscaling/kpa/scaler.go @@ -201,6 +201,9 @@ func (ks *scaler) handleScaleToZero(ctx context.Context, pa *autoscalingv1alpha1 if pa.Status.CanFailActivation(now, activationTimeout) { logger.Info("Activation has timed out after ", activationTimeout) return desiredScale, true + } else if revTimeout, ok := pa.RevisionTimeout(); ok && pa.CanFailActivationOnUnreachableRevision(now, revTimeout) { + logger.Info("Activation has timed out after revision-timeout ", revTimeout) + return desiredScale, true } ks.enqueueCB(pa, activationTimeout) return scaleUnknown, false diff --git a/pkg/reconciler/revision/resources/deploy_test.go b/pkg/reconciler/revision/resources/deploy_test.go index a668d5efe1d3..012153282b54 100644 --- a/pkg/reconciler/revision/resources/deploy_test.go +++ b/pkg/reconciler/revision/resources/deploy_test.go @@ -47,6 +47,11 @@ import ( . "knative.dev/serving/pkg/testing/v1" ) +const ( + defaultRevisionTimeout = 45 + defaultRevisionTimeoutStr = "45s" +) + var ( servingContainerName = "serving-container" sidecarContainerName = "sidecar-container-1" @@ -195,7 +200,7 @@ var ( } defaultPodSpec = &corev1.PodSpec{ - TerminationGracePeriodSeconds: refInt64(45), + TerminationGracePeriodSeconds: ptr.Int64(45), EnableServiceLinks: ptr.Bool(false), } @@ -273,15 +278,11 @@ func defaultRevision() *v1.Revision { UID: "1234", }, Spec: v1.RevisionSpec{ - TimeoutSeconds: ptr.Int64(45), + TimeoutSeconds: ptr.Int64(defaultRevisionTimeout), }, } } -func refInt64(num int64) *int64 { - return &num -} - type containerOption func(*corev1.Container) type podSpecOption func(*corev1.PodSpec) type deploymentOption func(*appsv1.Deployment) @@ -1425,7 +1426,10 @@ func TestMakeDeployment(t *testing.T) { }, { ImageDigest: "ubuntu@sha256:deadbffe", }})), - want: appsv1deployment(), + want: appsv1deployment(func(deploy *appsv1.Deployment) { + deploy.Annotations = map[string]string{serving.RevisionTimeoutAnnotationKey: defaultRevisionTimeoutStr} + deploy.Spec.Template.Annotations = map[string]string{serving.RevisionTimeoutAnnotationKey: defaultRevisionTimeoutStr} + }), }, { name: "with owner", rev: revision("bar", "foo", @@ -1439,7 +1443,10 @@ func TestMakeDeployment(t *testing.T) { WithContainerStatuses([]v1.ContainerStatus{{ ImageDigest: "busybox@sha256:deadbeef", }})), - want: appsv1deployment(), + want: appsv1deployment(func(deploy *appsv1.Deployment) { + deploy.Annotations = map[string]string{serving.RevisionTimeoutAnnotationKey: defaultRevisionTimeoutStr} + deploy.Spec.Template.Annotations = map[string]string{serving.RevisionTimeoutAnnotationKey: defaultRevisionTimeoutStr} + }), }, { name: "with sidecar annotation override", rev: revision("bar", "foo", @@ -1458,9 +1465,13 @@ func TestMakeDeployment(t *testing.T) { }), want: appsv1deployment(func(deploy *appsv1.Deployment) { deploy.Annotations = kmeta.UnionMaps(deploy.Annotations, - map[string]string{sidecarIstioInjectAnnotation: "false"}) + map[string]string{ + sidecarIstioInjectAnnotation: "false", + serving.RevisionTimeoutAnnotationKey: defaultRevisionTimeoutStr}) deploy.Spec.Template.Annotations = kmeta.UnionMaps(deploy.Spec.Template.Annotations, - map[string]string{sidecarIstioInjectAnnotation: "false"}) + map[string]string{ + sidecarIstioInjectAnnotation: "false", + serving.RevisionTimeoutAnnotationKey: defaultRevisionTimeoutStr}) }), }, { name: "with progress-deadline override", @@ -1478,6 +1489,8 @@ func TestMakeDeployment(t *testing.T) { }}), withoutLabels), want: appsv1deployment(func(deploy *appsv1.Deployment) { deploy.Spec.ProgressDeadlineSeconds = ptr.Int32(42) + deploy.Annotations = map[string]string{serving.RevisionTimeoutAnnotationKey: defaultRevisionTimeoutStr} + deploy.Spec.Template.Annotations = map[string]string{serving.RevisionTimeoutAnnotationKey: defaultRevisionTimeoutStr} }), }, { name: "with progress-deadline annotation", @@ -1493,8 +1506,13 @@ func TestMakeDeployment(t *testing.T) { }}), withoutLabels), want: appsv1deployment(func(deploy *appsv1.Deployment) { deploy.Spec.ProgressDeadlineSeconds = ptr.Int32(42) - deploy.Annotations = map[string]string{serving.ProgressDeadlineAnnotationKey: "42s"} - deploy.Spec.Template.Annotations = map[string]string{serving.ProgressDeadlineAnnotationKey: "42s"} + deploy.Annotations = map[string]string{ + serving.ProgressDeadlineAnnotationKey: "42s", + serving.RevisionTimeoutAnnotationKey: defaultRevisionTimeoutStr} + deploy.Spec.Template.Annotations = map[string]string{ + serving.ProgressDeadlineAnnotationKey: "42s", + serving.RevisionTimeoutAnnotationKey: defaultRevisionTimeoutStr} + }), }, { name: "with ProgressDeadline annotation and configmap override", @@ -1513,8 +1531,12 @@ func TestMakeDeployment(t *testing.T) { }}), withoutLabels), want: appsv1deployment(func(deploy *appsv1.Deployment) { deploy.Spec.ProgressDeadlineSeconds = ptr.Int32(42) - deploy.Annotations = map[string]string{serving.ProgressDeadlineAnnotationKey: "42s"} - deploy.Spec.Template.Annotations = map[string]string{serving.ProgressDeadlineAnnotationKey: "42s"} + deploy.Annotations = map[string]string{ + serving.ProgressDeadlineAnnotationKey: "42s", + serving.RevisionTimeoutAnnotationKey: defaultRevisionTimeoutStr} + deploy.Spec.Template.Annotations = map[string]string{ + serving.ProgressDeadlineAnnotationKey: "42s", + serving.RevisionTimeoutAnnotationKey: defaultRevisionTimeoutStr} }), }, { name: "cluster initial scale", @@ -1531,6 +1553,9 @@ func TestMakeDeployment(t *testing.T) { ), want: appsv1deployment(func(deploy *appsv1.Deployment) { deploy.Spec.Replicas = ptr.Int32(int32(10)) + deploy.Annotations = map[string]string{serving.RevisionTimeoutAnnotationKey: defaultRevisionTimeoutStr} + deploy.Spec.Template.Annotations = map[string]string{serving.RevisionTimeoutAnnotationKey: defaultRevisionTimeoutStr} + }), }, { name: "cluster initial scale override by revision initial scale", @@ -1550,8 +1575,12 @@ func TestMakeDeployment(t *testing.T) { ), want: appsv1deployment(func(deploy *appsv1.Deployment) { deploy.Spec.Replicas = ptr.Int32(int32(20)) - deploy.Spec.Template.Annotations = map[string]string{autoscaling.InitialScaleAnnotationKey: "20"} - deploy.Annotations = map[string]string{autoscaling.InitialScaleAnnotationKey: "20"} + deploy.Spec.Template.Annotations = map[string]string{ + autoscaling.InitialScaleAnnotationKey: "20", + serving.RevisionTimeoutAnnotationKey: defaultRevisionTimeoutStr} + deploy.Annotations = map[string]string{ + autoscaling.InitialScaleAnnotationKey: "20", + serving.RevisionTimeoutAnnotationKey: defaultRevisionTimeoutStr} }), }} diff --git a/pkg/reconciler/revision/resources/meta.go b/pkg/reconciler/revision/resources/meta.go index 42d035f7dc8e..da13824decd6 100644 --- a/pkg/reconciler/revision/resources/meta.go +++ b/pkg/reconciler/revision/resources/meta.go @@ -19,9 +19,11 @@ package resources import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" + "knative.dev/pkg/kmap" "knative.dev/pkg/kmeta" "knative.dev/serving/pkg/apis/serving" v1 "knative.dev/serving/pkg/apis/serving/v1" + "strconv" ) var ( @@ -56,7 +58,11 @@ func makeLabels(revision *v1.Revision) map[string]string { } func makeAnnotations(revision *v1.Revision) map[string]string { - return kmeta.FilterMap(revision.GetAnnotations(), excludeAnnotations.Has) + annotations := kmap.Filter(revision.GetAnnotations(), excludeAnnotations.Has) + if revision.Spec.TimeoutSeconds != nil { + annotations[serving.RevisionTimeoutAnnotation.Key()] = strconv.FormatInt(*revision.Spec.TimeoutSeconds, 10) + "s" + } + return annotations } // makeSelector constructs the Selector we will apply to K8s resources.