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

TAS: reduce friction by defaulting the PodSet annotations #3754

Open
3 tasks
mimowo opened this issue Dec 6, 2024 · 8 comments · May be fixed by #4519
Open
3 tasks

TAS: reduce friction by defaulting the PodSet annotations #3754

mimowo opened this issue Dec 6, 2024 · 8 comments · May be fixed by #4519
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@mimowo
Copy link
Contributor

mimowo commented Dec 6, 2024

What would you like to be added:

Allow scheduling workloads without TAS annotations (kueue.x-k8s.io/podset-required-topology or kueue.x-k8s.io/podset-preferred-topology) on every PodSet.

The idea is to default the annotations, but we need to decide where is the configuration.

Why is this needed:

Currently, users of TAS need to set annotations on every PodTemplate. While this gives control, it also creates friction and room for error, because users may forget to set the annotation. Also, most users want to use the "preferred" mode, and they don't need to control the topology level.

Completion requirements:

This enhancement requires the following artifacts:

  • Design doc (KEP update)
  • API change
  • Docs update

The artifacts should be linked in subsequent comments.

@mimowo mimowo added the kind/feature Categorizes issue or PR as related to a new feature. label Dec 6, 2024
@mimowo
Copy link
Contributor Author

mimowo commented Dec 6, 2024

cc @mwielgus @mwysokin @tenzen-y

@mimowo mimowo changed the title TAS: reduce friction for setting the PodSet annotations TAS: reduce friction by defaulting the PodSet annotations Dec 6, 2024
@mwysokin
Copy link
Contributor

Maybe ResourceFlavor? Since it's usually related to the machine type and it can benefit only some of the machine types? Maybe the default could be kubernetes.io/hostname since it seems to give the best compact placement?

@tenzen-y
Copy link
Member

Allow scheduling workloads without TAS annotations (kueue.x-k8s.io/podset-required-topology or kueue.x-k8s.io/podset-preferred-topology) on every PodSet.

This sounds like making TAS a default scheduling policy.
IIUC, TAS currently tries to pack Pods into the same and high-used nodes as much as possible (mostAllocated).
However, the policy sometimes does not meet inference and other workloads.

What about adding validations? I mean, if Jobs have TAS annotations in .metadata.annotations, we reject the Job and return appropriate places where they should put TAS annotations.

@mimowo
Copy link
Contributor Author

mimowo commented Mar 5, 2025

One lightweight idea is to make TAS default when all RFs in a given CQ have topologyName. Then, for every resource flavor the default is the lowest level - which would be kubernetes.io/hostname effectively as proposed in #3754 (comment).

The benefit of this approach is that it allows users to migrate CQ-by-CQ to TAS, and does not require setting the annotations by the user.

This sounds like making TAS a default scheduling policy.

Not really, we would still require a deliberate admin action to configure the ResourceFlavors to have topologyName. Otherwise, default Kueue is used.

IIUC, TAS currently tries to pack Pods into the same and high-used nodes as much as possible (mostAllocated).
However, the policy sometimes does not meet inference and other workloads.

That is true, but I would prefer to think about this as a separate improvement to TAS. For example, we may introduce another PodSet annotation. We have "required", and "preferred", maybe we also need "spread" or "relax", etc.

@mimowo
Copy link
Contributor Author

mimowo commented Mar 6, 2025

/assign

@tenzen-y
Copy link
Member

tenzen-y commented Mar 6, 2025

One lightweight idea is to make TAS default when all RFs in a given CQ have topologyName. Then, for every resource flavor the default is the lowest level - which would be kubernetes.io/hostname effectively as proposed in #3754 (comment).

The benefit of this approach is that it allows users to migrate CQ-by-CQ to TAS, and does not require setting the annotations by the user.

Basically, sgtm.
One question is which enforcement level (required vs preferred) should Kueue automatically insert?

@mimowo
Copy link
Contributor Author

mimowo commented Mar 6, 2025

One question is which enforcement level (required vs preferred) should Kueue automatically insert?

Preferred - this is our go to annotation recommended to users, for most workloads. "required" could make some workloads unschedulable. Still, if some workload is ultra sensitive on networking the user can set the "required" annotation and it will have precedence over the default (preferred).

@tenzen-y
Copy link
Member

tenzen-y commented Mar 6, 2025

One question is which enforcement level (required vs preferred) should Kueue automatically insert?

Preferred - this is our go to annotation recommended to users, for most workloads. "required" could make some workloads unschedulable. Still, if some workload is ultra sensitive on networking the user can set the "required" annotation and it will have precedence over the default (preferred).

That sounds reasonable.

@mimowo mimowo linked a pull request Mar 7, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants