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
Add support for rolling update of stateful Elastigroups #572
Conversation
The respawn command detects stateful Elastigroup and recycles stateful instances one at a time, instead of using the deploy API (that only works for the groups without stateful integrations).
No new tests were broken by this PR 😂 |
Add --batch-per-subnet for respawn of stateful Elastigroups
@a1exsh do you still want this change merged? If so please rebase the branch. The unit tests are fixed on master. |
@jmcs yes, we use the patched version and it saves us a lot of hassle. Thanks for fixing the tests! :) |
Here is an overview of what got changed by this pull request: Issues
======
- Added 11
See the complete overview on Codacy |
if batch_size is not None: | ||
raise Exception("Batch size is not supported when respawning stateful ElastiGroups") | ||
|
||
respawn_stateful_elastigroup(elastigroup_id, stack_name, region, batch_per_subnet, stateful_instances, spotinst_account) |
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.
Codacy found an issue: Line too long (128/100)
|
||
if not stateful_elastigroup_ready(stateful_instances): | ||
raise Exception( | ||
"Stateful ElastiGroup {} is not ready: some instances are not in the ACTIVE state".format(elastigroup_id) |
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.
Codacy found an issue: Line too long (117/100)
break | ||
|
||
|
||
def recycle_stateful_elastigroup_instance(elastigroup_id: str, instance: dict, spotinst_account): |
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.
Codacy found an issue: Missing function docstring
|
||
if batch_per_subnet: | ||
instances_by_subnet = stateful_elastigroup_instances_by_subnet(region, stateful_instances) | ||
for subnet, subnet_instances in sorted(instances_by_subnet.items(), key=lambda item: item[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.
Codacy found an issue: Line too long (102/100)
wait_for_stateful_elastigroup(elastigroup_id, spotinst_account) | ||
|
||
|
||
def stateful_elastigroup_instances_by_subnet(region: str, stateful_instances: list): |
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.
Codacy found an issue: Missing function docstring
@click.option( | ||
"--batch-per-subnet", | ||
is_flag=True, | ||
help="Recycle ElastiGroup instances in batches per subnet. Valid only for stateful ElastiGroups.", |
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.
Codacy found an issue: Line too long (102/100)
respawn_stateful_elastigroup(elastigroup_id, stack_name, region, batch_per_subnet, stateful_instances, spotinst_account) | ||
else: | ||
if batch_per_subnet: | ||
raise Exception("Batch per subnet is not supported when respawning stateless ElastiGroups") |
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.
Codacy found an issue: Line too long (103/100)
|
||
|
||
def respawn_stateful_elastigroup( | ||
elastigroup_id: str, stack_name: str, region: str, batch_per_subnet: bool, stateful_instances: list, spotinst_account |
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.
Codacy found an issue: Line too long (121/100)
return instances_by_subnet | ||
|
||
|
||
def stateful_elastigroup_ready(stateful_instances: list): |
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.
Codacy found an issue: Missing function docstring
@@ -1954,7 +1959,7 @@ def respawn_instances(stack_ref, inplace, force, batch_size_percentage, region): | |||
) | |||
elif group["type"] == ELASTIGROUP_RESOURCE_TYPE: | |||
respawn.respawn_elastigroup( | |||
group["resource_id"], group["stack_name"], region, batch_size_percentage | |||
group["resource_id"], group["stack_name"], region, batch_size_percentage, batch_per_subnet |
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.
Codacy found an issue: Line too long (106/100)
while True: | ||
time.sleep(WAIT_FOR_ELASTIGROUP_SEC) | ||
act.progress() | ||
stateful_instances = elastigroup_api.get_stateful_instances(elastigroup_id, spotinst_account) |
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.
Codacy found an issue: Line too long (105/100)
👍 |
1 similar comment
👍 |
The respawn command detects stateful Elastigroup and recycles stateful
instances one at a time, instead of using the deploy API (that only works for
the groups without stateful integrations).
Optional flag
--batch-per-subnet
may be used to recycle all instance of one AZ/subnet at a time.