Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add feature flag treat-pod-as-always-schedulable #15397

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading