Rail Aliiev [:rail] Assignee
	Description • 11 years ago

We should move everything related to AWS from cruncher and host it on a separate VM, probably in AWS.

	Dustin J. Mitchell [:dustin] (he/him)
	Comment 2 • 11 years ago

Comment on attachment 8340495 [details] [diff] [review]
initial skeleton

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

Looks like a good start!

Do you want to create a new users::buildduty instead of using users::builder?

::: manifests/moco-nodes.pp
@@ +682,5 @@
>  }
> +
> +node "aws-manager.b.m.o" {
> +    include toplevel::server
> +    include aws_manager

When I initially suggested just toplevel::server, it was with the thought that the rest would be configured by hand.  It's definitely better to do that config with puppet, but in that case this should be via a new toplevel class (toplevel::server::aws_manager maybe?)

::: modules/aws_manager/manifests/cron.pp
@@ +6,5 @@
> +    include users::builder
> +    aws_manager::crontask {
> +        "aws_watch_pending.py":
> +            ensure         => present,
> +            $minute        => '*/5',

no $ here :)

::: modules/config/manifests/base.pp
@@ +241,5 @@
>  
>      # "yes" to install 'em, "no" to not do so.  See bug 913011
>      $install_avds = "no"
>  
> +    # AWS management

extra newline after this

	Rail Aliiev [:rail] Assignee
	Comment 3 • 10 years ago

This worked fine. I tested only one script and it worked. I'll still need to add 1 cronjob as a followup for this.

	Nick Thomas [:nthomas] (UTC+12)
	Comment 4 • 10 years ago

Log rotation would be great to have, eg aws_watch_pending.log goes back to Dec 20 last year, 520MB in size. Keeping a month or so should be plenty.

	Rail Aliiev [:rail] Assignee
	Comment 5 • 10 years ago

Adding logrotate would be great, yeah. TODO :)

	Dustin J. Mitchell [:dustin] (he/him)
	Comment 6 • 10 years ago

Comment on attachment 8388912 [details] [diff] [review]
aws_cruncher-puppet-7.diff

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

Looks great!  Don't forget docs for the new module and its secrets.

	Rail Aliiev [:rail] Assignee
	Comment 7 • 10 years ago

refresh + instance2ami.py cron job

I would like to start the migration tomorrow.

	Rail Aliiev [:rail] Assignee
	Comment 8 • 10 years ago

Comment on attachment 8395943 [details] [diff] [review]
aws_cruncher-puppet-1.diff

remote:   https://hg.mozilla.org/build/puppet/rev/0b59ef9160bb
remote:   https://hg.mozilla.org/build/puppet/rev/d6a893cb8693

	Rail Aliiev [:rail] Assignee
	Comment 10 • 10 years ago

Comment on attachment 8396396 [details] [diff] [review]
pip.diff

http://hg.mozilla.org/build/puppet/rev/cf82372894fc
http://hg.mozilla.org/build/puppet/rev/8163e90e9a47

	Rail Aliiev [:rail] Assignee
	Comment 12 • 10 years ago

s/us-west-1//

	Rail Aliiev [:rail] Assignee
	Comment 13 • 10 years ago

Comment on attachment 8396425 [details] [diff] [review]
watch_pending.diff

https://hg.mozilla.org/build/puppet/rev/75a296390c91
https://hg.mozilla.org/build/puppet/rev/3e4bfe3baf64

	Rail Aliiev [:rail] Assignee
	Comment 14 • 10 years ago

* Better logging is on the list of the things to improve. Let's mimic cruncher's approach for now.
* missing \n

	Rail Aliiev [:rail] Assignee
	Comment 15 • 10 years ago

Comment on attachment 8396463 [details] [diff] [review]
tuneup.diff

remote:   https://hg.mozilla.org/build/puppet/rev/5eed9b758caa
remote:   https://hg.mozilla.org/build/puppet/rev/666168a8763c

	Rail Aliiev [:rail] Assignee
	Comment 16 • 10 years ago

We want moar spam!

	Rail Aliiev [:rail] Assignee
	Comment 17 • 10 years ago

Comment on attachment 8396531 [details] [diff] [review]
combine_stderr_stdout.diff

remote:   https://hg.mozilla.org/build/puppet/rev/5c4001c44f24
remote:   https://hg.mozilla.org/build/puppet/rev/fa83c3960848

	Rail Aliiev [:rail] Assignee
	Comment 18 • 10 years ago

another followup

	Dustin J. Mitchell [:dustin] (he/him)
	Comment 19 • 10 years ago

Comment on attachment 8396548 [details] [diff] [review]
combine_stderr_stdout2.diff

standing review from me for this sort of thing :)

	Rail Aliiev [:rail] Assignee
	Comment 20 • 10 years ago

Comment on attachment 8396548 [details] [diff] [review]
combine_stderr_stdout2.diff

remote:   https://hg.mozilla.org/build/puppet/rev/e7d42d43948d
remote:   https://hg.mozilla.org/build/puppet/rev/1988626f6a6c

	Rail Aliiev [:rail] Assignee
	Comment 21 • 10 years ago

More or less everything went smoothly. The only thing I forgot to address is servo related tasks.

We have only 2 cron jobs for servo, they use the same scripts but use different parameters and files. I added $script argument to allow reuse the same script name.

The only part that makes me uncomfortable to request a review is having some hard coded values.

* passwords-servo.json.erb uses "servobld". This can be moved to the config, but I'm not quite sure what this indirection would help us with.
* Similar with $servo_passwords

As possible work around I can add something like ::config::servo_is_cool (of course!) and make the manifest entries conditional on that.

	Dustin J. Mitchell [:dustin] (he/him)
	Comment 22 • 10 years ago

Comment on attachment 8396649 [details] [diff] [review]
servo-puppet.diff

I think this is OK as-is, since it's an odd case of moco stuff managing servo infra.  It would be more natural, I think, for this whole module to be instantiated separately in a dedicated servo instance, and do the right thing there -- but that's a lot more abstraction, plus an additional always-on instance.

	Rail Aliiev [:rail] Assignee
	Comment 23 • 10 years ago

The same patch + /etc/invtool.conf
A --noop puppet output:

Notice: /Stage[main]/Aws_manager::Cron/Aws_manager::Crontask[aws_watch_pending_servo]/File[/etc/cron.d/aws_manager-aws_watch_pending_servo.cron]/ensure: current_value absent, should be file (noop)
Notice: /Stage[main]/Aws_manager::Install/File[/etc/invtool.conf]/ensure: current_value absent, should be file (noop)
Notice: /Stage[main]/Aws_manager::Cron/Aws_manager::Crontask[aws_stop_idle_servo]/File[/etc/cron.d/aws_manager-aws_stop_idle_servo.cron]/ensure: current_value absent, should be file (noop)
Notice: /Stage[main]/Aws_manager::Secrets/File[/builds/aws_manager/secrets/passwords-servo.json]/ensure: current_value absent, should be file (noop)
Notice: /Stage[main]/Aws_manager::Secrets/File[/builds/aws_manager/secrets/aws-secrets-servo.json]/ensure: current_value absent, should be file (noop)
Notice: /Stage[main]/Aws_manager::Cron/Aws_manager::Crontask[aws_stop_idle_servo]/File[/builds/aws_manager/bin/aws_manager-aws_stop_idle_servo.sh]/ensure: current_value absent, should be file (noop)
Notice: Class[Aws_manager::Install]: Would have triggered 'refresh' from 1 events
Notice: Aws_manager::Crontask[aws_stop_idle_servo]: Would have triggered 'refresh' from 2 events
Notice: /Stage[main]/Aws_manager::Cron/Aws_manager::Crontask[aws_watch_pending_servo]/File[/builds/aws_manager/bin/aws_manager-aws_watch_pending_servo.sh]/ensure: current_value absent, should be file (noop)
Notice: Aws_manager::Crontask[aws_watch_pending_servo]: Would have triggered 'refresh' from 2 events

	Dustin J. Mitchell [:dustin] (he/him)
	Comment 24 • 10 years ago

Comment on attachment 8397071 [details] [diff] [review]
servo-puppet-1.diff

*STAMP*

	Rail Aliiev [:rail] Assignee
	Comment 25 • 10 years ago

Comment on attachment 8397071 [details] [diff] [review]
servo-puppet-1.diff

remote:   https://hg.mozilla.org/build/puppet/rev/088f3b94d616
remote:   https://hg.mozilla.org/build/puppet/rev/bd5cabaf5484

	Rail Aliiev [:rail] Assignee
	Comment 26 • 10 years ago

The docs are live: https://wiki.mozilla.org/ReleaseEngineering/PuppetAgain/Modules/aws_manager

	Rail Aliiev [:rail] Assignee
	Comment 28 • 10 years ago

Comment on attachment 8401836 [details] [diff] [review]
try.diff

remote:   https://hg.mozilla.org/build/puppet/rev/2a7b716e480c
remote:   https://hg.mozilla.org/build/puppet/rev/564a3d8f25d9