-
Notifications
You must be signed in to change notification settings - Fork 67
Sync scale from zero part 2 #230
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
model-engine/model_engine_server/infra/gateways/resources/k8s_endpoint_resource_delegate.py
Outdated
Show resolved
Hide resolved
| if image_params.requirements_folder: | ||
| folders_to_include.append(image_params.requirements_folder) | ||
|
|
||
| dockerfile_root_folder = image_params.dockerfile.split("/")[0] |
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.
todo split out into own pr
|
|
||
| @staticmethod | ||
| async def _create_keda_scaled_object(scaled_object: Dict[str, Any], name: str) -> None: | ||
| # TODO test |
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.
seems to work e2e
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.
at least create works, update idk
model-engine/model_engine_server/infra/gateways/resources/k8s_endpoint_resource_delegate.py
Outdated
Show resolved
Hide resolved
model-engine/model_engine_server/infra/gateways/resources/k8s_endpoint_resource_delegate.py
Outdated
Show resolved
Hide resolved
| name=name, | ||
| ) | ||
| new_scaled_object = deep_update(existing_scaled_object, scaled_object) | ||
| await custom_objects_api.replace_namespaced_custom_object( |
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.
Should we do replace? Or patch? I just remember that replacing caused rolling restarts to not work, which doesn't apply here, but if it's all the same, maybe just patch?
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 saw that other resources using the custom_objects_api apparently needed to use replace, so am doing that here as well. Separately there's an option restoreToOriginalReplicaCount here that is default false, and which I haven't set, so I think what should happen is that as the ScaledObject gets deleted the deployment doesn't get affected, so we shouldn't get downtime here at least.
| target: | ||
| type: Value | ||
| averageValue: ${CONCURRENCY} | ||
| keda-scaled-object.yaml: |- |
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.
where is this file used currently?
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.
tbh not sure, maybe it's used for the integration tests in circleci?
in any case if we're not creating keda scaled objects in minikube I think we should be fine and won't break tests, but once we do we might have to helm install keda into minikube or something (or support some more options so keda doesn't become a hard dependency for self hosting)
-Test create, get, delete
-Create and get and delete work
-Autoscaling does seem to work
-also add in some code for allowing artifactlike endpoints to be able to build
-todo docker image fine tuning entrypoint probably needs high-priority turned on