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.

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

♻️Autoscaling: set default termination time of EC2 instance to 1 minute #3970

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class EC2InstancesSettings(BaseCustomSettings):
)

EC2_INSTANCES_TIME_BEFORE_TERMINATION: datetime.timedelta = Field(
default=datetime.timedelta(minutes=55),
default=datetime.timedelta(minutes=1),
description="Time after which an EC2 instance may be terminated (repeat every hour, min 0, max 59 minutes)",
)

Expand Down
27 changes: 20 additions & 7 deletions services/autoscaling/tests/unit/test_dynamic_scaling_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,19 @@ def mock_machines_buffer(monkeypatch: pytest.MonkeyPatch) -> Iterator[int]:
yield num_machines_in_buffer


@pytest.fixture
def with_valid_time_before_termination(
monkeypatch: pytest.MonkeyPatch,
) -> datetime.timedelta:
time = "00:11:00"
monkeypatch.setenv("EC2_INSTANCES_TIME_BEFORE_TERMINATION", time)
sanderegg marked this conversation as resolved.
Show resolved Hide resolved
return parse_obj_as(datetime.timedelta, time)


@pytest.fixture
async def drained_host_node(
host_node: Node, async_docker_client: aiodocker.Docker
) -> AsyncIterator[Node]:

assert host_node.ID
assert host_node.Version
assert host_node.Version.Index
Expand Down Expand Up @@ -759,6 +767,7 @@ async def test__find_terminateable_nodes_with_no_hosts(

async def test__find_terminateable_nodes_with_drained_host(
minimal_configuration: None,
with_valid_time_before_termination: datetime.timedelta,
initialized_app: FastAPI,
cluster: Callable[..., Cluster],
drained_host_node: Node,
Expand Down Expand Up @@ -870,13 +879,12 @@ def create_associated_instance(
app_settings: ApplicationSettings,
faker: Faker,
) -> Callable[[Node, bool], AssociatedInstance]:
assert app_settings.AUTOSCALING_EC2_INSTANCES
assert (
app_settings.AUTOSCALING_EC2_INSTANCES.EC2_INSTANCES_TIME_BEFORE_TERMINATION
> datetime.timedelta(seconds=10)
), "this tests relies on the fact that the time before termination is above 10 seconds"

def _creator(node: Node, terminateable_time: bool) -> AssociatedInstance:
assert app_settings.AUTOSCALING_EC2_INSTANCES
assert (
app_settings.AUTOSCALING_EC2_INSTANCES.EC2_INSTANCES_TIME_BEFORE_TERMINATION
> datetime.timedelta(seconds=10)
), "this tests relies on the fact that the time before termination is above 10 seconds"
assert app_settings.AUTOSCALING_EC2_INSTANCES
seconds_delta = (
-datetime.timedelta(seconds=10)
Expand All @@ -901,6 +909,7 @@ def _creator(node: Node, terminateable_time: bool) -> AssociatedInstance:

async def test__try_scale_down_cluster_with_no_nodes(
minimal_configuration: None,
with_valid_time_before_termination: datetime.timedelta,
initialized_app: FastAPI,
cluster: Callable[..., Cluster],
mock_remove_nodes: mock.Mock,
Expand All @@ -920,6 +929,7 @@ async def test__try_scale_down_cluster_with_no_nodes(

async def test__try_scale_down_cluster(
minimal_configuration: None,
with_valid_time_before_termination: datetime.timedelta,
initialized_app: FastAPI,
cluster: Callable[..., Cluster],
host_node: Node,
Expand Down Expand Up @@ -953,6 +963,7 @@ async def test__try_scale_down_cluster(

async def test__activate_drained_nodes_with_no_tasks(
minimal_configuration: None,
with_valid_time_before_termination: datetime.timedelta,
initialized_app: FastAPI,
host_node: Node,
drained_host_node: Node,
Expand Down Expand Up @@ -985,6 +996,7 @@ async def test__activate_drained_nodes_with_no_tasks(

async def test__activate_drained_nodes_with_no_drained_nodes(
minimal_configuration: None,
with_valid_time_before_termination: datetime.timedelta,
autoscaling_docker: AutoscalingDocker,
initialized_app: FastAPI,
host_node: Node,
Expand Down Expand Up @@ -1029,6 +1041,7 @@ async def test__activate_drained_nodes_with_no_drained_nodes(

async def test__activate_drained_nodes_with_drained_node(
minimal_configuration: None,
with_valid_time_before_termination: datetime.timedelta,
autoscaling_docker: AutoscalingDocker,
initialized_app: FastAPI,
drained_host_node: Node,
Expand Down