797946 - build puppetagain modules to deploy mobile-services system to support bmm

Status: RESOLVED FIXED

(Infrastructure & Operations :: RelOps: General, task)

Description

[Jake Watkins [:dividehex]]

Puppetagain modules need to be written to automate the deployment of mobile-services servers which will include BMM (Black Mobile Magic) aka the panda re-imaging system.

Comment 1

[Jake Watkins [:dividehex]]

Attachment: mobile-services puppetagain patch

Added toplevel::server::mobile-services and added mobile-services.build.scl1.mozilla.com node

Also added new fileserver.conf section for /data/bmm

Comment 2

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 668098 [details] [diff] [review]
mobile-services puppetagain patch

Looks good so far, even if it doesn't do much :)

Comment 3

[Jake Watkins [:dividehex]]

Attachment: bmm module

Comment 4

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 668968 [details] [diff] [review]
bmm module

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

This looks good, aside from the additional factoring described below.

::: modules/blackmobilemagic/manifests/config/httpd.pp
@@ +2,5 @@
> +    file {
> +        "/etc/httpd/conf.d/bmm_httpd.conf":
> +            ensure => present,
> +            source => "puppet:///modules/blackmobilemagic/bmm_httpd.conf",
> +            notify => Service["httpd"];

This should probably use the existing httpd module, which will need a small bit of tweaking to also work on CentOS (it's Darwin-only right now).

@@ +16,5 @@
> +        "/opt/bmm/www/squashfs/precise-panda-live-build.1.squashfs":
> +            ensure => file,
> +            source => "puppet:///bmm/private/squashfs/precise-panda-live-build.1.squashfs";
> +        "/opt/bmm/www/artifacts":
> +            ensure => directory;

Would it make sense to just synchronize this whole directory recursively?  Same for squashfs?

::: modules/blackmobilemagic/manifests/config/syslogd.pp
@@ +2,5 @@
> +    file {
> +        "/etc/rsyslog.conf":
> +            ensure => present,
> +            source => "puppet:///modules/blackmobilemagic/rsyslog.conf",
> +            notify => Service["rsyslog"];

I'm kinda on the fence about this one - it seems like rsyslog should have its own module, but it's *really* simple.

::: modules/blackmobilemagic/manifests/config/tftpd.pp
@@ +7,5 @@
> +        "/var/lib/tftpboot":
> +            ensure => directory;
> +        "/tftpboot":
> +            ensure => link,
> +            target => "/var/lib/tftpboot";

The above feels like it should be in a 'tfpd' module, like the httpd module.  The module probably doesn't need much more than this and the service { } in blackmobilemagic::service.

::: modules/blackmobilemagic/manifests/init.pp
@@ +1,2 @@
> +class blackmobilemagic {
> +    case $::operatingsystem {

Is this really necessary?  If all of the packages installed on, say, OS X, it would probably work, right?  Or, to look at it another way, if we *added* OS X support, it would be done by making each of the classes this one includes support OS X, rather than adding a new clause to this case statement.

Comment 5

[Jake Watkins [:dividehex]]

Attachment: bmm module

** Lots of refactoring
- moved rsyslog and tftpd to their own modules
- added run-parts style dir for rsyslog (/etc/rsyslog.d)
- moved OS checks to the package installation level
- httpd uses existing httpd module
- set artifacts and squashfs dirs to recurse

Comment 6

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 669868 [details] [diff] [review]
bmm module

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

All minor points.  Good to go with these fixes made!

::: modules/blackmobilemagic/manifests/config/httpd.pp
@@ +17,5 @@
> +        "/opt/bmm/www/squashfs":
> +            recurse => true,
> +            purge => true,
> +            source => [ "puppet:///bmm/squashfs", "puppet:///bmm/private/squashfs" ],
> +            sourceselect => all,

Very nice!

::: modules/httpd/manifests/config.pp
@@ +22,5 @@
> +            include httpd::settings
> +            if ($file != undef) and ($contents != undef) {
> +                file {
> +                    "$file" :
> +                        notify => Service['httpd'],

The notify is good, but I think the require needs to be here, too?  Otherwise it will try to install the files in /etc/httpd/conf.d before that directory is created

::: modules/packages/manifests/tftp-server.pp
@@ +3,5 @@
> +        CentOS: {
> +            package {
> +                "tftp-server":
> +                    ensure => latest,
> +                    require => Class["packages::xinetd"];

You'll need to include this as well as requiring it.

::: modules/rsyslog/manifests/settings.pp
@@ +1,2 @@
> +class rsyslog::settings {
> +    include ::shared

I think you want `include users::root` here, since that's the class containing the variables you reference..

Comment 7

[Dustin J. Mitchell [:dustin] (he/him)]

Hah, I didn't see that this was checked in.  A few minor points that I'll fix up and r? here:

err: /Stage[main]/Blackmobilemagic::Config::Tftpd/File[/var/lib/tftpboot/pxelinux.cfg]/ensure: change from absent to directory failed: Cannot create /var/lib/tftpboot/pxelinux.cfg; parent directory /var/lib/tftpboot does not exist
err: /Stage[main]/Blackmobilemagic::Config::Tftpd/File[/var/lib/tftpboot/panda-live]/ensure: change from absent to directory failed: Cannot create /var/lib/tftpboot/panda-live; parent directory /var/lib/tftpboot does not exist

and, classes with - in them cause problems, so we should rename the toplevel class.  I'll just call it 'bmm', since that's what it's running.

Comment 8

[Dustin J. Mitchell [:dustin] (he/him)]

The first were tricky.  Puppet does relative class imports, so

class blackmobilemagic::config::tftpd {
    include tftpd
...
}

just includes itself.  I changed that to include ::tftpd and it works.

Comment 9

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: bug797946.patch

Comment 10

[Dustin J. Mitchell [:dustin] (he/him)]

Oh, and as a reminder, the puppet changes need to get documented.  I have it in my TODO list so we wont' forget.

Comment 11

[Jake Watkins [:dividehex]]

Comment on attachment 670142 [details] [diff] [review]
bug797946.patch

looks good.

Comment 12

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 670142 [details] [diff] [review]
bug797946.patch

OK, all that's left is docs.

Comment 13

[Dustin J. Mitchell [:dustin] (he/him)]

I took care of the docs today.