aws_create_instance should not be allowed to run forever

NEW
Assigned to

Status

Release Engineering
General
3 years ago
20 days ago

People

(Reporter: jlund, Assigned: jlund)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
AFAIK we use aws_create_instance exclusively for creating loans. This script has a couple 'while True' loops that will run indefinitely by sleeping for 10s every time it encounters an error.

We should be able to set a maximum amount of attempts on this script.
(Assignee)

Comment 1

3 years ago
Created attachment 8447780 [details] [diff] [review]
140629_bug_1031953_create_instance_max_attempts-cloud-tools.patch

callek, you probably have the most context here as rail/catlee are both out till later this week.

now that we are using this script in automation, we shouldn't let it be able to run forever. In addition to this patch, should include a 'timeout' option from slaveapi side of things, but I'm having trouble implementing a guaranteed timeout against our current gevent and gevent_subprocess versions.

Regardless, this patch makes sense as it at least allows us to override the times where we purposely let the script run forever.
Attachment #8447780 - Flags: review?(bugspam.Callek)
Comment on attachment 8447780 [details] [diff] [review]
140629_bug_1031953_create_instance_max_attempts-cloud-tools.patch

Review of attachment 8447780 [details] [diff] [review]:
-----------------------------------------------------------------

handing off to rail
Attachment #8447780 - Flags: review?(bugspam.Callek) → review?(rail)
Comment on attachment 8447780 [details] [diff] [review]
140629_bug_1031953_create_instance_max_attempts-cloud-tools.patch

Review of attachment 8447780 [details] [diff] [review]:
-----------------------------------------------------------------

::: scripts/aws_create_instance.py
@@ +294,5 @@
> +        attempt += 1
> +        if max_attempts:
> +            keep_going = max_attempts >= attempt
> +        else:
> +            keep_going = True

I think, the else block is redundant. Also, you can move "attempt += 1" inside the if block.

@@ +331,5 @@
> +        attempt += 1
> +        if max_attempts:
> +            keep_going = max_attempts >= attempt
> +        else:
> +            keep_going = True

the same here.
Attachment #8447780 - Flags: review?(rail) → review+
(Assignee)

Comment 4

3 years ago
Comment on attachment 8447780 [details] [diff] [review]
140629_bug_1031953_create_instance_max_attempts-cloud-tools.patch

doh! I blame lack of sleep

pushed with r? feedback: https://hg.mozilla.org/build/cloud-tools/rev/98e5a28bc987
Attachment #8447780 - Flags: checked-in+
(Assignee)

Updated

2 years ago
Blocks: 1154548
Component: Tools → General
Product: Release Engineering → Release Engineering
You need to log in before you can comment on or make changes to this bug.