Skip to content

Conversation

wild-endeavor
Copy link
Contributor

@wild-endeavor wild-endeavor commented Jul 2, 2025

Why are the changes needed?

This is a user request to let the CPU limit differ from the CPU request. Note this change is only for CPU. Please see this blog for additional background.

Please keep in mind also that Flyte "limits" are not limits in the K8s sense - they're limits on requests. Flyte then proceeds to set Pod limits equal to requests. The reason Flyte does this is because of how K8s QoS works. Limits that are higher than requests result in a lower class leading to potentially higher eviction.

If turned on, tasks that do not request a CPU limit will now not get a CPU limit. Please be aware that K8s will reject scheduling any such pod if there's a resource quota that specifies a CPU limit in the pod's namespace.

What changes were proposed in this pull request?

  • Add a flag in container_helper.go to allow for limit to differ.
  • Add two flags that gate this feature. This was done because of the decrease in QoS that this results in, and the potential for K8s scheduling errors.

Alternatives considered

  • Task level flag, injected into TaskMetadata. This was deemed too intrusive of a change for a feature that may be hard to use at this time. Would be a substantially larger change.

How was this patch tested?

Tested on internal clusters.

Setup process

Both feature flags need to be turned on to enable this behavior.
For Admin:

task_resources:
  allow-cpu-limit-to-float-from-request: true

For propeller

k8s:
  allow-cpu-limit-to-float-from-request: true

Screenshots

Check all the applicable boxes

  • I updated the documentation accordingly.
  • All new and existing tests passed.
  • All commits are signed-off.

Related PRs

Docs link

Summary by Bito

This pull request introduces a feature in Flyte that allows CPU limits to differ from CPU requests, enhancing resource management flexibility. It includes new configuration flags, updates to the execution manager, and comprehensive tests to ensure the new functionality works correctly without disrupting existing behavior.

Signed-off-by: Yee Hing Tong <[email protected]>
@flyte-bot
Copy link
Collaborator

Bito Automatic Review Skipped - Draft PR

Bito didn't auto-review because this pull request is in draft status.
No action is needed if you didn't intend for the agent to review it. Otherwise, to manually trigger a review, type /review in a comment and save.
You can change draft PR review settings here, or contact your Bito workspace admin at [email protected].

Copy link

codecov bot commented Jul 2, 2025

Codecov Report

Attention: Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.

Project coverage is 58.68%. Comparing base (e38f32c) to head (8006672).

Files with missing lines Patch % Lines
flyteadmin/pkg/runtime/task_resource_provider.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6519   +/-   ##
=======================================
  Coverage   58.67%   58.68%           
=======================================
  Files         938      938           
  Lines       71466    71477   +11     
=======================================
+ Hits        41936    41946   +10     
- Misses      26344    26345    +1     
  Partials     3186     3186           
Flag Coverage Δ
unittests-datacatalog 59.03% <ø> (ø)
unittests-flyteadmin 56.22% <75.00%> (+0.02%) ⬆️
unittests-flytecopilot 39.56% <ø> (ø)
unittests-flytectl 64.72% <ø> (-0.06%) ⬇️
unittests-flyteidl 76.12% <ø> (ø)
unittests-flyteplugins 61.18% <100.00%> (+0.01%) ⬆️
unittests-flytepropeller 54.83% <ø> (ø)
unittests-flytestdlib 64.02% <ø> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@wild-endeavor wild-endeavor added added Merged changes that add new functionality changed For changes in existing functionality and removed changed For changes in existing functionality labels Jul 3, 2025
@wild-endeavor wild-endeavor marked this pull request as ready for review July 3, 2025 06:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

added Merged changes that add new functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants