-
Notifications
You must be signed in to change notification settings - Fork 32
✨Autoscaling: allow to set EC2 type specific docker node labels (⚠️ DevOPS) #8635
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
base: master
Are you sure you want to change the base?
✨Autoscaling: allow to set EC2 type specific docker node labels (⚠️ DevOPS) #8635
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #8635 +/- ##
==========================================
+ Coverage 87.55% 89.26% +1.71%
==========================================
Files 2009 1349 -660
Lines 79039 56662 -22377
Branches 1382 227 -1155
==========================================
- Hits 69200 50578 -18622
+ Misses 9444 6021 -3423
+ Partials 395 63 -332
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
2dbe9f8 to
5a41131
Compare
7685bb6 to
476780c
Compare
476780c to
8d4f264
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 adds support for EC2 instance type-specific Docker node labels in the autoscaling system. Previously, all EC2 instances received the same Docker node labels regardless of their type, which caused issues when non-GPU machines incorrectly received GPU labels.
Key changes:
- Extended
EC2InstanceBootSpecificmodel to includecustom_node_labelsfield for per-instance-type label configuration - Changed
EC2_INSTANCES_ALLOWED_TYPESand related settings from plain types toJson[T]types for proper JSON parsing - Refactored tag key constants to use
TypeAdaptervalidation and defined them as module-level constants - Updated tests to verify custom node labels are correctly applied to EC2 instances
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/aws-library/src/aws_library/ec2/_models.py | Added custom_node_labels field to EC2InstanceBootSpecific model; migrated from TypeAlias to Python 3.13 type syntax |
| services/autoscaling/src/simcore_service_autoscaling/core/settings.py | Changed EC2 settings fields to use Json[T] type annotations for proper JSON parsing |
| services/autoscaling/src/simcore_service_autoscaling/utils/utils_docker.py | Updated get_new_node_docker_tags to merge custom node labels from EC2 instance type configuration |
| services/autoscaling/src/simcore_service_autoscaling/utils/utils_ec2.py | Refactored tag key constants to use TypeAdapter validation |
| services/autoscaling/src/simcore_service_autoscaling/modules/cluster_scaling/_provider_computational.py | Updated computational provider to include custom node labels |
| services/autoscaling/src/simcore_service_autoscaling/modules/cluster_scaling/_auto_scaling_core.py | Changed to use get_application_settings helper function |
| services/autoscaling/tests/unit/test_utils_docker.py | Added test coverage for custom node labels functionality |
| services/autoscaling/tests/unit/test_modules_cluster_scaling_dynamic.py | Updated test expectations to include custom node labels |
| services/autoscaling/tests/unit/test_modules_cluster_scaling_computational.py | Updated test expectations to include custom node labels |
| services/autoscaling/tests/unit/test_core_settings.py | Added validation test for invalid custom node labels; corrected expected error input type |
| services/clusters-keeper/src/simcore_service_clusters_keeper/utils/ec2.py | Refactored tag key constants and improved type validation using TypeAdapter |
| services/clusters-keeper/src/simcore_service_clusters_keeper/modules/clusters_management_core.py | Updated to use TypeAdapter for tag value validation |
| services/clusters-keeper/src/simcore_service_clusters_keeper/modules/clusters.py | Updated to use TypeAdapter for tag value validation |
| .pre-commit-config.yaml | Upgraded pyupgrade from v3.21.1 to v3.21.2 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
GitHK
left a comment
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.
👍
mrnicegyu11
left a comment
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.
Very nice thanks a lot
pcrespov
left a comment
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.
thx!



What do these changes do?
Currently all the newly created nodes via autoscaling get the same docker node labels (such as gpu, dynamic-sidecar). This can create problems when non-gpu machines are created but they still receive these labels.
This PR adds the option to define per EC2 type custom node labels.
For example that means that t3.medium machines will NOT have anymore the
gpulabel set.That requires a change in how the EC2_INSTANCES_ALLOWED_TYPES environment variable is created.
Related issue/s
How to test
Dev-ops