diff --git a/apis/kueue/v1alpha1/tas_types.go b/apis/kueue/v1alpha1/tas_types.go index acb6ae2bdc..be022487c8 100644 --- a/apis/kueue/v1alpha1/tas_types.go +++ b/apis/kueue/v1alpha1/tas_types.go @@ -52,6 +52,11 @@ const ( // of the PodSet of the admitted Workload corresponding to the PodTemplate. // The label is set when starting the Job, and removed on stopping the Job. PodSetLabel = "kueue.x-k8s.io/podset" + + // TASLabel is a label set on the Job's PodTemplate to indicate that the + // PodSet is admitted using TopologyAwareScheduling, and all Pods created + // from the Job's PodTemplate also have the label. + TASLabel = "kueue.x-k8s.io/tas" ) // TopologySpec defines the desired state of Topology diff --git a/keps/2724-topology-aware-schedling/README.md b/keps/2724-topology-aware-schedling/README.md index 7aef7c1a6e..dafe5d9300 100644 --- a/keps/2724-topology-aware-schedling/README.md +++ b/keps/2724-topology-aware-schedling/README.md @@ -46,6 +46,7 @@ - [Workload API alternatives](#workload-api-alternatives) - [Drop the topologyAssignment.levels field](#drop-the-topologyassignmentlevels-field) - [Rename the topologyAssignment.domains.values field as levelValues](#rename-the-topologyassignmentdomainsvalues-field-as-levelvalues) + - [Drop dedicated TAS label](#drop-dedicated-tas-label) ## Summary @@ -564,6 +565,11 @@ const ( // of the PodSet of the admitted Workload corresponding to the PodTemplate. // The label is set when starting the Job, and removed on stopping the Job. PodSetLabel = "kueue.x-k8s.io/podset" + + // TASLabel is a label set on the Job's PodTemplate to indicate that the + // PodSet is admitted using TopologyAwareScheduling, and all Pods created + // from the Job's PodTemplate also have the label. + TASLabel = "kueue.x-k8s.io/tas" ) ``` @@ -686,6 +692,7 @@ The new validations which are for MVP, but likely will be relaxed in the future: ReplicatedJob level (see [Story 2](#story-2)) - re-evaluate the need to support for "preferred/required" preferences at the Workload level (see [Story 3](#story-3)) +- re-evaluate the need for the `kueue.x-k8s.io/tas` #### Stable @@ -864,3 +871,18 @@ it specifies values for the keys in the `levels` field. * the field is on Workload API, which is generally not a user-facing API in Kueue. Workload objects are meant for the internal mechanics of Kueue. * the intention of the field can be expressed in a comment. + +### Drop dedicated TAS label + +We could reduce the API surface by dropping the TAS label. The label is going +to be used in two places: +1. in TopologyUngater to quickly skip non-TAS pods in the events handler +2. when accounting for usage from Pods to quickly list only non-TAS pods + +However, it was pointed out in discussions that the use case could probably be +fulfilled with a dedicated indexed virtual field. + +**Reasons for discarding/deferring** + +Increased code complexity which could defer the 0.9 release for the Alpha +version. We will re-evaluate the need for the label before the Beta release. \ No newline at end of file diff --git a/pkg/podset/podset.go b/pkg/podset/podset.go index 3ec6c4b56b..2cb267a1aa 100644 --- a/pkg/podset/podset.go +++ b/pkg/podset/podset.go @@ -63,6 +63,7 @@ func FromAssignment(ctx context.Context, client client.Client, assignment *kueue Annotations: make(map[string]string), } if features.Enabled(features.TopologyAwareScheduling) && assignment.TopologyAssignment != nil { + info.Labels[kueuealpha.TASLabel] = "true" info.SchedulingGates = append(info.SchedulingGates, corev1.PodSchedulingGate{ Name: kueuealpha.TopologySchedulingGate, }) diff --git a/pkg/podset/podset_test.go b/pkg/podset/podset_test.go index 970368051a..eb3a27d335 100644 --- a/pkg/podset/podset_test.go +++ b/pkg/podset/podset_test.go @@ -188,6 +188,9 @@ func TestFromAssignment(t *testing.T) { wantInfo: PodSetInfo{ Name: "name", Count: 4, + Labels: map[string]string{ + kueuealpha.TASLabel: "true", + }, NodeSelector: map[string]string{ "f1l1": "f1v1", "f1l2": "f1v2", @@ -428,6 +431,25 @@ func TestMergeRestore(t *testing.T) { Obj(), wantRestoreChanges: true, }, + "podset with tas label; empty info": { + podSet: utiltesting.MakePodSet("", 1). + Labels(map[string]string{kueuealpha.TASLabel: "true"}). + Obj(), + wantPodSet: utiltesting.MakePodSet("", 1). + Labels(map[string]string{kueuealpha.TASLabel: "true"}). + Obj(), + }, + "podset with tas label; info re-adds the same": { + podSet: utiltesting.MakePodSet("", 1). + Labels(map[string]string{kueuealpha.TASLabel: "true"}). + Obj(), + info: PodSetInfo{ + Labels: map[string]string{kueuealpha.TASLabel: "true"}, + }, + wantPodSet: utiltesting.MakePodSet("", 1). + Labels(map[string]string{kueuealpha.TASLabel: "true"}). + Obj(), + }, } for name, tc := range cases {