-
Notifications
You must be signed in to change notification settings - Fork 38
feat: update scheduler for capacity-based property scheduling #1223
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
Conversation
d0ca169 to
0b39772
Compare
pkg/scheduler/framework/plugins/clusteraffinity/types_azure_test.go
Outdated
Show resolved
Hide resolved
pkg/scheduler/framework/plugins/clusteraffinity/types_azure_test.go
Outdated
Show resolved
Hide resolved
pkg/scheduler/framework/plugins/clusteraffinity/types_azure_test.go
Outdated
Show resolved
Hide resolved
pkg/utils/test_util.go
Outdated
| return nil, errors.New("test error: mapping does not exist") | ||
| } | ||
|
|
||
| // CreateMockAttributeBasedVMSizeRecommenderServer creates a mock HTTP server for testing AttributeBasedVMSizeRecommenderClient. |
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.
it should be under test/utils folder. This file should be deprecated and we cannot mix the test code and prod code.
https://github.com/Azure/fleet/blob/main/pkg/clients/azure/compute/vmsizerecommenderclient_test.go#L208 Can you also refactor this code too?
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.
I'll deprecate the file on kubefleet in a different PR. I will refactor the code in the vmsizerecommenderclient with my function here.
|
Have not reviewed the whole PR, will do after you backport the changes, thanks |
0274e87 to
6b5ad97
Compare
ff12634 to
45611b1
Compare
50af2ba to
e7a8f3f
Compare
Signed-off-by: Britania Rodriguez Reyes <britaniar@microsoft.com>
e7a8f3f to
ff749f2
Compare
Description of your changes
I have:
Updated the profile so scheduler can have the option to have a property checker (default is nil).
Update the scheduler plugin to include new property checker
Updated clusterRequirements to work similarly but with added property checker.
Update matches where properties are checked to include a check for capacity.
Run
make reviewableto ensure this PR is ready for review.How has this code been tested
Special notes for your reviewer
Currently has the bulk of the code to work so the tests are accurate. Will make smaller once the other two PRs are merged.