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.

Sign up for GitHub

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

Jump to bottom

osrdyne: support keda autoscaling for k8s driver #8741

Merged
merged 2 commits into from
Oct 11, 2024
Merged

osrdyne: support keda autoscaling for k8s driver #8741

merged 2 commits into from
Oct 11, 2024

Conversation

ElysaSrc
Copy link
Member

@ElysaSrc ElysaSrc commented Sep 4, 2024 •
edited
Loading

Closes #8408

@codecov-commenter
Copy link

codecov-commenter commented Sep 4, 2024 •
edited
Loading

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 0% with 149 lines in your changes missing coverage. Please review.

Project coverage is 38.99%. Comparing base (416ae4a) to head (633007c).
Report is 28 commits behind head on dev.

Files with missing lines Patch % Lines
osrdyne/src/drivers/kubernetes.rs 0.00% 124 Missing ⚠️
...re/src/main/java/fr/sncf/osrd/cli/WorkerCommand.kt 0.00% 9 Missing ⚠️
osrdyne/src/pool.rs 0.00% 9 Missing ⚠️
osrdyne/src/drivers/kubernetes/keda.rs 0.00% 4 Missing ⚠️
osrdyne/src/drivers/docker.rs 0.00% 1 Missing ⚠️
osrdyne/src/drivers/noop.rs 0.00% 1 Missing ⚠️
osrdyne/src/drivers/process_compose.rs 0.00% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files 
@@             Coverage Diff              @@
##                dev    #8741      +/-   ##
============================================
+ Coverage     38.58%   38.99%   +0.40%     
  Complexity     2244     2244              
============================================
  Files          1285     1289       +4     
  Lines         97813    97295     -518     
  Branches       3255     3277      +22     
============================================
+ Hits          37742    37939     +197     
+ Misses        58141    57419     -722     
- Partials       1930     1937       +7
Flag Coverage Δ
core 74.83% <0.00%> (+0.04%) ⬆️
editoast 72.38% <ø> (+0.14%) ⬆️
front 10.13% <ø> (-0.10%) ⬇️
gateway 2.19% <ø> (-0.04%) ⬇️
osrdyne 3.52% <0.00%> (+1.08%) ⬆️
railjson_generator 87.49% <ø> (ø)
tests 86.71% <ø> (ø)

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.

@ElysaSrc ElysaSrc force-pushed the ev/osrdyne-support-keda branch 3 times, most recently from 5b7900e to b16a0e6 Compare October 2, 2024 14:46
@ElysaSrc ElysaSrc marked this pull request as ready for review October 2, 2024 14:49
@ElysaSrc ElysaSrc requested a review from a team as a code owner October 2, 2024 14:49
@ElysaSrc ElysaSrc force-pushed the ev/osrdyne-support-keda branch 2 times, most recently from 6c442b7 to 6deb3db Compare October 2, 2024 14:57
@Khoyo Khoyo requested a review from a team as a code owner October 2, 2024 15:03
@Khoyo Khoyo requested review from Khoyo and Erashin and removed request for Khoyo October 2, 2024 15:03
@Khoyo Khoyo added the area:core Work on Core Service label Oct 2, 2024
@github-actions github-actions bot removed the area:core Work on Core Service label Oct 2, 2024
eckter
eckter approved these changes Oct 3, 2024
View reviewed changes
Copy link
Contributor

@eckter eckter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM for core

Not tested, but I trust you on this

@ElysaSrc ElysaSrc force-pushed the ev/osrdyne-support-keda branch 6 times, most recently from 5f771d6 to 25366e9 Compare October 11, 2024 07:17
bloussou
bloussou reviewed Oct 11, 2024
View reviewed changes
Copy link
Contributor

@bloussou bloussou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't wait to test this !!

osrdyne/src/drivers/kubernetes.rs Outdated Show resolved Hide resolved
@ElysaSrc ElysaSrc force-pushed the ev/osrdyne-support-keda branch from 25366e9 to de65af0 Compare October 11, 2024 07:54
flomonster
flomonster approved these changes Oct 11, 2024
View reviewed changes
osrdyne/src/drivers/kubernetes.rs Outdated Show resolved Hide resolved
osrdyne/src/drivers/kubernetes.rs Show resolved Hide resolved
woshilapin
woshilapin approved these changes Oct 11, 2024
View reviewed changes
Copy link
Contributor

@woshilapin woshilapin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Only minor comments.

osrdyne/src/drivers/kubernetes.rs Show resolved Hide resolved
osrdyne/src/drivers/kubernetes.rs Outdated Show resolved Hide resolved
@ElysaSrc ElysaSrc force-pushed the ev/osrdyne-support-keda branch from de65af0 to a331efb Compare October 11, 2024 08:09
@ElysaSrc ElysaSrc requested a review from bloussou October 11, 2024 08:51
@Khoyo @ElysaSrc
core: add the posibility to use hostname as WORKEY_ID
633007c 
This is useful when using keda or other autoscalers, as they won't
support assigning WORKER_IDs.

Signed-off-by: Younes Khoudli <[email protected]>
@ElysaSrc ElysaSrc force-pushed the ev/osrdyne-support-keda branch from a331efb to 633007c Compare October 11, 2024 08:53
bloussou
bloussou approved these changes Oct 11, 2024
View reviewed changes
Copy link
Contributor

@bloussou bloussou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's go !

Khoyo
Khoyo approved these changes Oct 11, 2024
View reviewed changes
Copy link
Contributor

@Khoyo Khoyo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

The amount of inline hasmap/btreemap construction we are starting to do makes me wonder if we should use maplit

@ElysaSrc ElysaSrc added this pull request to the merge queue Oct 11, 2024
@ElysaSrc
Copy link
Member Author

maplit

Will do in another PR later in time

Merged via the queue into dev with commit 2b23321 Oct 11, 2024
24 checks passed
@ElysaSrc ElysaSrc deleted the ev/osrdyne-support-keda branch October 11, 2024 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bloussou bloussou bloussou approved these changes

@flomonster flomonster flomonster approved these changes

@eckter eckter eckter approved these changes

@woshilapin woshilapin woshilapin approved these changes

@Khoyo Khoyo Khoyo approved these changes

@Erashin Erashin Awaiting requested review from Erashin

Assignees
No one assigned
Labels
area:osrdyne
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

osrdyne: Horizontal scaling for one infrastructure
7 participants
@ElysaSrc @codecov-commenter @Khoyo @woshilapin @eckter @flomonster @bloussou