Skip to content

Commit

Permalink
Add feature flag treat-pod-as-always-schedulable
Browse files Browse the repository at this point in the history
The feature flag allows to declare that Pods in the system will eventually all get scheduled and Revisions should therefore not be marked unschedulable
  • Loading branch information
SaschaSchwarze0 committed Oct 17, 2024
1 parent 77c7e1d commit b7433df
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 6 deletions.
11 changes: 10 additions & 1 deletion config/core/configmaps/features.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: "9ff569ad"
knative.dev/example-checksum: "e2b4e75b"
data:
_example: |-
################################
Expand Down Expand Up @@ -234,3 +234,12 @@ data:
# Default queue proxy resource requests and limits to good values for most cases if set.
queueproxy.resource-defaults: "disabled"
# treat-pod-as-always-schedulable can be used to define that Pods in the system will always be
# scheduled, and a Revision should not be marked unschedulable.
# Setting this to `enabled` makes sense if you have cluster-autoscaling set up for you cluster
# where unschedulable Pods trigger the addition of a new Node and are therefore a short and
# transient state.
#
# See https://github.com/knative/serving/issues/14862
treat-pod-as-always-schedulable: "disabled"
5 changes: 4 additions & 1 deletion pkg/apis/config/features.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ func defaultFeaturesConfig() *Features {
SecurePodDefaults: Disabled,
TagHeaderBasedRouting: Disabled,
AutoDetectHTTP2: Disabled,
TreatPodAsAlwaysSchedulable: Disabled,
}
}

Expand Down Expand Up @@ -119,7 +120,8 @@ func NewFeaturesConfigFromMap(data map[string]string) (*Features, error) {
asFlag("tag-header-based-routing", &nc.TagHeaderBasedRouting),
asFlag("queueproxy.resource-defaults", &nc.QueueProxyResourceDefaults),
asFlag("queueproxy.mount-podinfo", &nc.QueueProxyMountPodInfo),
asFlag("autodetect-http2", &nc.AutoDetectHTTP2)); err != nil {
asFlag("autodetect-http2", &nc.AutoDetectHTTP2),
asFlag("treat-pod-as-always-schedulable", &nc.TreatPodAsAlwaysSchedulable)); err != nil {
return nil, err
}
return nc, nil
Expand Down Expand Up @@ -161,6 +163,7 @@ type Features struct {
SecurePodDefaults Flag
TagHeaderBasedRouting Flag
AutoDetectHTTP2 Flag
TreatPodAsAlwaysSchedulable Flag
}

// asFlag parses the value at key as a Flag into the target, if it exists.
Expand Down
29 changes: 29 additions & 0 deletions pkg/apis/config/features_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ func TestFeaturesConfiguration(t *testing.T) {
SecurePodDefaults: Enabled,
QueueProxyResourceDefaults: Enabled,
TagHeaderBasedRouting: Enabled,
TreatPodAsAlwaysSchedulable: Enabled,
}),
data: map[string]string{
"multi-container": "Enabled",
Expand All @@ -103,6 +104,7 @@ func TestFeaturesConfiguration(t *testing.T) {
"secure-pod-defaults": "Enabled",
"queueproxy.resource-defaults": "Enabled",
"tag-header-based-routing": "Enabled",
"treat-pod-as-always-schedulable": "Enabled",
},
}, {
name: "multi-container Allowed",
Expand Down Expand Up @@ -654,6 +656,33 @@ func TestFeaturesConfiguration(t *testing.T) {
data: map[string]string{
"kubernetes.podspec-hostnetwork": "Disabled",
},
}, {
name: "treat-pod-as-always-schedulable Allowed",
wantErr: false,
wantFeatures: defaultWith(&Features{
TreatPodAsAlwaysSchedulable: Allowed,
}),
data: map[string]string{
"treat-pod-as-always-schedulable": "Allowed",
},
}, {
name: "treat-pod-as-always-schedulable Enabled",
wantErr: false,
wantFeatures: defaultWith(&Features{
TreatPodAsAlwaysSchedulable: Enabled,
}),
data: map[string]string{
"treat-pod-as-always-schedulable": "Enabled",
},
}, {
name: "treat-pod-as-always-schedulable Disabled",
wantErr: false,
wantFeatures: defaultWith(&Features{
TreatPodAsAlwaysSchedulable: Disabled,
}),
data: map[string]string{
"treat-pod-as-always-schedulable": "Disabled",
},
}}

for _, tt := range configTests {
Expand Down
12 changes: 8 additions & 4 deletions pkg/reconciler/revision/reconcile_resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
"knative.dev/pkg/kmp"
"knative.dev/pkg/logging"
"knative.dev/pkg/logging/logkey"
apicfg "knative.dev/serving/pkg/apis/config"
v1 "knative.dev/serving/pkg/apis/serving/v1"
"knative.dev/serving/pkg/networking"
"knative.dev/serving/pkg/reconciler/revision/config"
Expand Down Expand Up @@ -87,10 +88,13 @@ func (c *Reconciler) reconcileDeployment(ctx context.Context, rev *v1.Revision)

// Update the revision status if pod cannot be scheduled (possibly resource constraints)
// If pod cannot be scheduled then we expect the container status to be empty.
for _, cond := range pod.Status.Conditions {
if cond.Type == corev1.PodScheduled && cond.Status == corev1.ConditionFalse {
rev.Status.MarkResourcesAvailableFalse(cond.Reason, cond.Message)
break
treatPodAsAlwaysSchedulable := config.FromContext(ctx).Features.TreatPodAsAlwaysSchedulable
if treatPodAsAlwaysSchedulable == apicfg.Disabled {
for _, cond := range pod.Status.Conditions {
if cond.Type == corev1.PodScheduled && cond.Status == corev1.ConditionFalse {
rev.Status.MarkResourcesAvailableFalse(cond.Reason, cond.Message)
break
}
}
}

Expand Down
3 changes: 3 additions & 0 deletions pkg/reconciler/revision/table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -615,6 +615,9 @@ func TestReconcile(t *testing.T) {
Object: pa("foo", "pod-schedule-error", WithReachabilityUnreachable),
}},
Key: "foo/pod-schedule-error",
Ctx: defaultconfig.ToContext(context.Background(), &defaultconfig.Config{Features: &defaultconfig.Features{
TreatPodAsAlwaysSchedulable: defaultconfig.Disabled,
}}),
}, {
Name: "ready steady state",
// Test the transition that Reconcile makes when Endpoints become ready on the
Expand Down

0 comments on commit b7433df

Please sign in to comment.