-
Notifications
You must be signed in to change notification settings - Fork 1k
Enable multiple components estimating #6857
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
Enable multiple components estimating #6857
Conversation
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6857 +/- ##
==========================================
+ Coverage 46.24% 46.28% +0.03%
==========================================
Files 692 693 +1
Lines 47194 47237 +43
==========================================
+ Hits 21826 21864 +38
- Misses 23715 23721 +6
+ Partials 1653 1652 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
/retest |
a640c75 to
45aa42b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces support for multiple component estimation in the scheduler by adding logic to handle workloads with multiple pod templates that need to be scheduled to a single cluster. The feature is gated behind MultiplePodTemplatesScheduling and uses a new MaxAvailableComponentSets estimator API.
Key Changes:
- Added conditional logic to use component set estimation for multi-template workloads
- Implemented validation to determine when multi-template scheduling applies
- Added comprehensive test coverage for the new estimation logic
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| pkg/scheduler/core/util.go | Integrates multi-template estimation into the main replica calculation flow with feature gate check |
| pkg/scheduler/core/estimation.go | Implements core logic for multi-template scheduling validation and available set calculation |
| pkg/scheduler/core/estimation_test.go | Provides comprehensive test coverage for multi-template scheduling applicability and calculation scenarios |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
45aa42b to
b9c71cb
Compare
|
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces the capability for multiple component estimation in the scheduler, which is a great feature. The changes are well-structured, with new logic encapsulated in estimation.go and corresponding tests.
I have a few suggestions to improve the implementation:
- The applicability check for multi-template scheduling seems to have a logic error regarding the number of components, and a test case has a misleading name.
- There's an opportunity to optimize a loop in
calculateMultiTemplateAvailableSetsfor better performance. - The legacy replica calculation logic, which is now in an
elseblock, has a potential bug related to cluster ordering that should be addressed for robustness.
b9c71cb to
c779158
Compare
2f3fc59 to
3c4af68
Compare
Signed-off-by: RainbowMango <[email protected]>
3c4af68 to
39bfa3c
Compare
|
After a basic manual test, it shows that the current patch can call the new estimator and propagate the workloads correctly. Following tests run with Volcano Job, as we just implemented the default interpreter recently. 1: Install Volcano Job CRD kubectl apply -f https://raw.githubusercontent.com/volcano-sh/volcano/refs/heads/master/installer/helm/chart/volcano/crd/bases/batch.volcano.sh_jobs.yaml2: Create a PropagationPolicy for Volcano Job: apiVersion: policy.karmada.io/v1alpha1
kind: PropagationPolicy
metadata:
name: foo
spec:
resourceSelectors:
- apiVersion: batch.volcano.sh/v1alpha1
kind: Job
name: dk-job
placement:
clusterAffinity:
clusterNames:
- member1
- member2
replicaScheduling:
replicaDivisionPreference: Aggregated # declares that want to aggregate the replicas
replicaSchedulingType: Divided
spreadConstraints: # but restrict to use only 1 cluster
- spreadByField: cluster
minGroups: 1
maxGroups: 13: Create a Volcano Job: apiVersion: batch.volcano.sh/v1alpha1
kind: Job
metadata:
name: dk-job
spec:
maxRetry: 3
minAvailable: 3
plugins:
env: []
ssh: []
svc:
- --disable-network-policy=true
queue: default
schedulerName: volcano
tasks:
- minAvailable: 1
name: job-nginx1
replicas: 1
template:
metadata:
name: nginx1
spec:
containers:
- args:
- sleep 10
command:
- bash
- -c
image: nginx:latest
imagePullPolicy: IfNotPresent
name: nginx
resources:
requests:
cpu: 100m
nodeSelector:
kubernetes.io/os: linux
restartPolicy: OnFailure
- minAvailable: 2
name: job-nginx2
replicas: 3
template:
metadata:
name: nginx2
spec:
containers:
- args:
- sleep 30
command:
- bash
- -c
image: nginx:latest
imagePullPolicy: IfNotPresent
name: nginx
resources:
requests:
cpu: 100m
nodeSelector:
kubernetes.io/os: linux
restartPolicy: OnFailure4. Check propagation state: -bash-5.0# karmadactl get jobs.batch.volcano.sh --operation-scope=all
NAME CLUSTER STATUS MINAVAILABLE RUNNINGS AGE ADOPTION
dk-job Karmada 34m -
dk-job member1 34m YIt shows that the job now has been scheduled to 5. Check the schedule result from RsourceBinding: apiVersion: work.karmada.io/v1alpha2
kind: ResourceBinding
metadata:
name: dk-job-job
spec:
clusters:
- name: member1 # without replicas assigned
components:
- name: job-nginx1
replicaRequirements:
nodeClaim:
nodeSelector:
kubernetes.io/os: linux
resourceRequest:
cpu: 100m
replicas: 1
- name: job-nginx2
replicaRequirements:
nodeClaim:
nodeSelector:
kubernetes.io/os: linux
resourceRequest:
cpu: 100m
replicas: 2
conflictResolution: Abort
placement:
clusterAffinity:
clusterNames:
- member1
- member2
replicaScheduling:
replicaDivisionPreference: Aggregated
replicaSchedulingType: Divided
spreadConstraints:
- maxGroups: 1
minGroups: 1
spreadByField: cluster
resource:
apiVersion: batch.volcano.sh/v1alpha1
kind: Job
name: dk-job
namespace: default
resourceVersion: "2405"
uid: 56ef1978-a1fc-44fa-a9ad-084a988d2f2b
schedulerName: default-scheduler
status:
aggregatedStatus:
- applied: true
clusterName: member1
health: Unhealthy
status: {}
conditions:
- lastTransitionTime: "2025-10-27T06:55:11Z"
message: Binding has been scheduled successfully.
reason: Success
status: "True"
type: Scheduled
- lastTransitionTime: "2025-10-27T06:55:11Z"
message: All works have been successfully applied
reason: FullyAppliedSuccess
status: "True"
type: FullyApplied
lastScheduledTime: "2025-10-27T06:55:11Z"
schedulerObservedGeneration: 26: Check scheduler log: It shows that the general estimator helped calculate available replicas( |
|
@mszacillo @zhzhuang-zju @seanlaii |
|
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind feature
What this PR does / why we need it:
This PR introduces support for multiple component estimation in the scheduler by adding logic to handle workloads with multiple pod templates that need to be scheduled to a single cluster. The feature is gated behind
MultiplePodTemplatesSchedulingand uses a newMaxAvailableComponentSetsestimator API.Which issue(s) this PR fixes:
Part of #6734
Special notes for your reviewer:
See test report below: #6857 (comment).
Currently, since replicas are not set for multi-template workloads, the system does not perform actual replica distribution. As a result, when observing the ResourceBinding, one cannot see the replica count per cluster, which makes debugging and validation difficult. This behavior needs further improvement, as the current logic for replica allocation relies solely on checking whether replicas > 0—a condition that is very fragile and insufficient for multi-template scenarios.
Additionally, we should add comprehensive end-to-end (E2E) tests for multi-template workloads to ensure correctness and robustness.
Does this PR introduce a user-facing change?: