Closed Bug 834076 Opened 11 years ago Closed 11 years ago

Create a tool to start/stop/reboot/disable/enable AWS slaves

Categories

(Infrastructure & Operations Graveyard :: CIDuty, task)

x86_64
Linux
task
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: rail, Assigned: rail)

Details

(Whiteboard: [aws])

Attachments

(1 file, 1 obsolete file)

Attached patch aws_manage_instances.py (obsolete) — Splinter Review
$ python aws_manage_instances.py --help
usage: aws_manage_instances.py [-h] -k SECRETS [--disable | --enable] [-n]
                               [-q]
                               {stop,start,restart,status} host [host ...]

positional arguments:
  {stop,start,restart,status}
                        action to be performed
  host                  hosts to be processed

optional arguments:
  -h, --help            show this help message and exit
  -k SECRETS, --secrets SECRETS
                        file where secrets can be found
  --disable             Set moz-state tag to a value which prevents automatic
                        start
  --enable              Set moz-state tag to a value which enables automatic
                        start
  -n, --dry-run         Dry run mode
  -q, --quiet           Supress logging messages
Attachment #705652 - Flags: review?(catlee)
Comment on attachment 705652 [details] [diff] [review]
aws_manage_instances.py

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

looks great! just a few fixes before this is ready.

::: aws/aws_manage_instances.py
@@ +5,5 @@
> +import logging
> +from time import gmtime, strftime
> +from boto.ec2 import connect_to_region
> +
> +logging.basicConfig(format="%(asctime)s - %(levelname)s - %(message)s")

move this into __main__ - it makes this module more importable

@@ +43,5 @@
> +    instance_id = i.id
> +    name = i.tags.get('Name', '')
> +    ip = i.private_ip_address
> +    state = i.state
> +    print name, instance_id, ip, state

can this print out the tags too? or at least moz-state? this tool should be able to print out whether a slave is enabled or disabled.

@@ +49,5 @@
> +
> +if __name__ == '__main__':
> +
> +    parser = argparse.ArgumentParser()
> +    parser.add_argument("-k", "--secrets", required=True,

can this be made optional? and then we can set $AWS_CREDENTIAL_FILE in buildduty's .bashrc.

@@ +50,5 @@
> +if __name__ == '__main__':
> +
> +    parser = argparse.ArgumentParser()
> +    parser.add_argument("-k", "--secrets", required=True,
> +                        type=argparse.FileType('r'),

O_O

argparse is awesome!

@@ +63,5 @@
> +    )
> +    state.add_argument(
> +        "--enable", action="store_true",
> +        help="Set moz-state tag to a value which enables automatic start"
> +    )

After reading though this, I think enable/disable should be actions. In particular, "aws_manage_instances.py --disable status bld-linux-ec2-123" doesn't make sense.

@@ +79,5 @@
> +        log.setLevel(logging.DEBUG)
> +    else:
> +        log.setLevel(logging.ERROR)
> +
> +    for region in REGIONS:

it would be great to be able to override this on the cmdline. not a blocker for here though.

@@ +96,5 @@
> +                log.warn("Skipping (terminated?) %s (%s)..." % (name,
> +                                                                instance_id))
> +                continue
> +            if name in args.hosts:
> +                log.info("Found  %s (%s)..." % (name, instance_id))

extra space here for some reason? (after "Found")

@@ +107,5 @@
> +                        "%Y-%m-%d %H:%M:%S +0000", gmtime())
> +                if moz_state:
> +                    log.info("Changing state to: %s" % moz_state)
> +                    if args.dry_run:
> +                        log.info("Dry run mode, not applying...")

can you log what the tag would have been set to?
Attachment #705652 - Flags: review?(catlee) → review-
(In reply to Chris AtLee [:catlee] from comment #1)
> Comment on attachment 705652 [details] [diff] [review]
> > +logging.basicConfig(format="%(asctime)s - %(levelname)s - %(message)s")
> 
> move this into __main__ - it makes this module more importable

Done.

> @@ +43,5 @@
> > +    instance_id = i.id
> > +    name = i.tags.get('Name', '')
> > +    ip = i.private_ip_address
> > +    state = i.state
> > +    print name, instance_id, ip, state
> 
> can this print out the tags too? or at least moz-state? this tool should be
> able to print out whether a slave is enabled or disabled.

Sure. I added some formatting as well.

> > +    parser.add_argument("-k", "--secrets", required=True,
> 
> can this be made optional? and then we can set $AWS_CREDENTIAL_FILE in
> buildduty's .bashrc.

Make sense. Done
 
> argparse is awesome!

You see! :)

> After reading though this, I think enable/disable should be actions. In
> particular, "aws_manage_instances.py --disable status bld-linux-ec2-123"
> doesn't make sense.

Yeah... I implemented those as actions as well

> > +    for region in REGIONS:
> 
> it would be great to be able to override this on the cmdline. not a blocker
> for here though.

Sure. Done.
 
> > +                log.info("Found  %s (%s)..." % (name, instance_id))
> 
> extra space here for some reason? (after "Found")

No reason. Nuked.
 
> @@ +107,5 @@
> > +                        "%Y-%m-%d %H:%M:%S +0000", gmtime())
> > +                if moz_state:
> > +                    log.info("Changing state to: %s" % moz_state)
> > +                    if args.dry_run:
> > +                        log.info("Dry run mode, not applying...")
> 
> can you log what the tag would have been set to?

Actually, I do, 2 lines above. It's moved to disable() function.

interdiff: https://gist.github.com/4617736
Attachment #705652 - Attachment is obsolete: true
Attachment #705729 - Flags: review?(catlee)
Comment on attachment 705729 [details] [diff] [review]
aws_manage_instances.py

   _~  _~
   __|=|_|=|__
   \ o.o.o.oY/
    \_______/
  ~~~~~~~~~~~~~~
Attachment #705729 - Flags: review?(catlee) → review+
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
Product: Release Engineering → Infrastructure & Operations
Product: Infrastructure & Operations → Infrastructure & Operations Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: