723998 - Set up a puppet manifest for autoland

Status: RESOLVED INVALID

(Release Engineering :: General, defect)

Description

[Marc Jessome[:mjessome]]

Attachment: Autoland manifest v1

Set up a puppet manifest to set up a staging machine and a production machine.

Comment 1

[[e-mail] (:bhearsum)]

Comment on attachment 594223 [details] [diff] [review]
Autoland manifest v1

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

::: buildmaster-production.pp
@@ +298,5 @@
>      include releng::master
>      include signingserver
>  }
> +
> +node "autoland-staging02" inherits "masternode" {

Does this node need rabbitmq set up on it, too?

@@ +299,5 @@
>      include signingserver
>  }
> +
> +node "autoland-staging02" inherits "masternode" {
> +    $codeTag = "staging"

This belongs as an argument rather than being completely defined on the node. I think you should move the autoland::instance out of the autoland class and put it directly here instead. Then you can pass codeTag and staging_flag as arguments to it rather than them being global to the node. The way you have it now it would be impossible to have multiple instances on a single node if we wanted to for some reason.

::: modules/autoland/manifests/init.pp
@@ +1,1 @@
> +class signingserver {

Need to fix the name of this class.

@@ +34,5 @@
> +    autoland::instance {
> +        "/home/autoland/autoland-env":
> +            code_tag       => $codeTag,
> +            user           => "autoland",
> +            require        => File["/home/autoland"];

I think this should required User["autoland"] rather than just the home directory.

::: modules/autoland/manifests/instance.pp
@@ +83,5 @@
> +            onlyif => "/bin/sh -c '! $basedir/bin/python -c \"import coverage\"'";
> +        "$basedir-twisted":
> +            command => "$basedir/bin/pip install --no-deps --no-index --find-links=${package_dir_http} Twisted==11.0.0",
> +            require => Exec["$basedir-virtualenv"],
> +            onlyif => "/bin/sh -c '! $basedir/bin/python -c \"import twisted\"'";

Some of these are packages we don't have on the Puppet server yet. Let me know if you need a hand getting them in the right place, I'm not sure if you have permission to.

::: modules/autoland/templates/config.ini.erb
@@ +18,5 @@
> +username=<%=autoland_bz_user%>
> +password=<%=autoland_bz_password%>
> +attachment_url=<%=attachment_url%>
> +api_url=<%=api_url%>
> +url=https://bugzilla.mozilla.org/show_bug.cgi?id=

Is this URL really the same for staging and production?

::: secrets.pp.template
@@ +89,5 @@
> +    $autoland_ss_user = 'user'
> +    $autoland_ss_password = 'pass'
> +    $autoland_hg_user = 'user'
> +    $autoland_ldap_bind_dn = 'bind_dn'
> +    $autoland_ldap_password = 'pass'

I don't think either of these LDAP things are autoland specific, please drop the autoland_ from them.

Comment 2

[Marc Jessome[:mjessome]]

(In reply to Ben Hearsum [:bhearsum] from comment #1)
> Comment on attachment 594223 [details] [diff] [review]
> Autoland manifest v1
> 
> Review of attachment 594223 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: buildmaster-production.pp
> @@ +298,5 @@
> >      include releng::master
> >      include signingserver
> >  }
> > +
> > +node "autoland-staging02" inherits "masternode" {
> 
> Does this node need rabbitmq set up on it, too?
That is a good question. My thought was that we could run it on the production mq, under a different exchange, or different queue names, however I was afraid that this might make a mess of things. I'll look into it.

> ::: secrets.pp.template
> @@ +89,5 @@
> > +    $autoland_ss_user = 'user'
> > +    $autoland_ss_password = 'pass'
> > +    $autoland_hg_user = 'user'
> > +    $autoland_ldap_bind_dn = 'bind_dn'
> > +    $autoland_ldap_password = 'pass'
> 
> I don't think either of these LDAP things are autoland specific, please drop
> the autoland_ from them.
The ldap bind_dn is actually specific to the account "[e-mail]". Should I keep that autoland specific, or not?

Everything else is being fixed as suggested. Thanks!

Comment 3

[[e-mail] (:bhearsum)]

(In reply to Marc Jessome[:mjessome] from comment #2)
> (In reply to Ben Hearsum [:bhearsum] from comment #1)
> > Comment on attachment 594223 [details] [diff] [review]
> > Autoland manifest v1
> > 
> > Review of attachment 594223 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > ::: buildmaster-production.pp
> > @@ +298,5 @@
> > >      include releng::master
> > >      include signingserver
> > >  }
> > > +
> > > +node "autoland-staging02" inherits "masternode" {
> > 
> > Does this node need rabbitmq set up on it, too?
> That is a good question. My thought was that we could run it on the
> production mq, under a different exchange, or different queue names, however
> I was afraid that this might make a mess of things. I'll look into it.

I don't have any opinion on this, just wanted to ask the question! If you guys think it should run on an existing rabbit, great!

> > ::: secrets.pp.template
> > @@ +89,5 @@
> > > +    $autoland_ss_user = 'user'
> > > +    $autoland_ss_password = 'pass'
> > > +    $autoland_hg_user = 'user'
> > > +    $autoland_ldap_bind_dn = 'bind_dn'
> > > +    $autoland_ldap_password = 'pass'
> > 
> > I don't think either of these LDAP things are autoland specific, please drop
> > the autoland_ from them.
> The ldap bind_dn is actually specific to the account
> "[e-mail]". Should I keep that autoland specific, or not?
> 
> Everything else is being fixed as suggested. Thanks!

Oh, OK. I didn't realize it was indeed autoland specific. Carry on!

Comment 4

[Marc Jessome[:mjessome]]

Attachment: Autoland manifest v2

Hopefully I understood what you wanted here, that's why I'm marking it for feedback.

Of course, this doesn't cover your earlier comment on python packages on the server.

Comment 5

[[e-mail] (:bhearsum)]

Comment on attachment 595157 [details] [diff] [review]
Autoland manifest v2

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

This looks great! As long as it tests out ok, and the tools repo stuff gets changed, it's ready to go IMO.

::: modules/autoland/manifests/init.pp
@@ +1,1 @@
> +class autoland::dep {

You should rename this file to dep.pp. Puppet does some automagic importing based on class name/filename matching.

Comment 6

[Marc Jessome[:mjessome]]

Attachment: Autoland manifest v3

Renamed to dep.pp -- Hopefully ready to give this a spin?

Comment 7

[Rail Aliiev [:rail]]

Assigning to Marc since he's been working on it.

Comment 8

[[e-mail] (:bhearsum)]

Comment on attachment 595833 [details] [diff] [review]
Autoland manifest v3

This looks fine, but I don't want to r+ without it being tested, to make sure it doesn't land prematurely.

Comment 9

[[e-mail] (:bhearsum)]

Marc, I set-up Puppet environment on master-puppet1 with this patch applied. Let's chat sometime today and I'll give you a tutorial on how to test.

Comment 10

[Marc Jessome[:mjessome]]

Attachment: Autoland manifest v4

This patch is what I've puppetized autoland-staging02 with.

Comment 11

[[e-mail] (:bhearsum)]

Comment on attachment 605064 [details] [diff] [review]
Autoland manifest v4

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

::: modules/autoland/manifests/instance.pp
@@ +33,5 @@
> +        #"$basedir-clone-tools":
> +        #    creates => "$basedir/tools",
> +        #    command => "/usr/bin/hg clone -r $code_tag $tools_repo tools",
> +        #    cwd => "$basedir",
> +        #    require => [Exec["$basedir-virtualenv"], Package["git"], Package["mercurial"];

I don't think this will end up depending on git...

@@ +38,5 @@
> +        "$basedir-clone-tools":
> +            creates => "$basedir/tools",
> +            command => "/usr/bin/git clone -b $code_tag $tools_repo tools",
> +            cwd => "$basedir",
> +            require => [Exec["$basedir-virtualenv"], Package["git"], Package["mercurial"]];

And this doesn't depend on Mercurial.

@@ +50,5 @@
> +            onlyif => "/bin/sh -c '! $basedir/bin/python -c \"import argparse\"'";
> +        "$basedir-mercurial":
> +            command => "$basedir/bin/pip install --no-deps --no-index --find-links=${package_dir_http} mercurial==1.9.1",
> +            require => [Exec["$basedir-virtualenv"], Package["mercurial"]],
> +            onlyif => "/bin/sh -c '! $basedir/bin/python -c \"import mercurial\"'";

Why are you installing Mercurial in the virtualenv if you're already depending on it? You should be doing one or the other.

@@ +82,5 @@
> +            onlyif => "/bin/sh -c '! $basedir/bin/python -c \"import MySQLdb\"'";
> +        #"$basedir-supervisor":
> +        #    command => "$basedir/bin/pip install --no-deps --no-index --find-links=${package_dir_http} MySQL-python==1.2.3",
> +        #    require => Exec["$basedir-virtualenv"],
> +        #    onlyif => "/bin/sh -c '! $basedir/bin/python -c \"import supervisor\"'";

Do you need supervisor or not?

Comment 12

[Marc Jessome[:mjessome]]

(In reply to Ben Hearsum [:bhearsum] from comment #11)
> Comment on attachment 605064 [details] [diff] [review]
> Autoland manifest v4
> 
> Review of attachment 605064 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: modules/autoland/manifests/instance.pp
> @@ +33,5 @@
> > +        #"$basedir-clone-tools":
> > +        #    creates => "$basedir/tools",
> > +        #    command => "/usr/bin/hg clone -r $code_tag $tools_repo tools",
> > +        #    cwd => "$basedir",
> > +        #    require => [Exec["$basedir-virtualenv"], Package["git"], Package["mercurial"];
> 
> I don't think this will end up depending on git...
correct, thanks.
> 
> @@ +38,5 @@
> > +        "$basedir-clone-tools":
> > +            creates => "$basedir/tools",
> > +            command => "/usr/bin/git clone -b $code_tag $tools_repo tools",
> > +            cwd => "$basedir",
> > +            require => [Exec["$basedir-virtualenv"], Package["git"], Package["mercurial"]];
> 
> And this doesn't depend on Mercurial.
> 
> @@ +50,5 @@
> > +            onlyif => "/bin/sh -c '! $basedir/bin/python -c \"import argparse\"'";
> > +        "$basedir-mercurial":
> > +            command => "$basedir/bin/pip install --no-deps --no-index --find-links=${package_dir_http} mercurial==1.9.1",
> > +            require => [Exec["$basedir-virtualenv"], Package["mercurial"]],
> > +            onlyif => "/bin/sh -c '! $basedir/bin/python -c \"import mercurial\"'";
> 
> Why are you installing Mercurial in the virtualenv if you're already
> depending on it? You should be doing one or the other.
Since the VM's are set up with python 2.4, I didn't think that a base mercurial install would come bundled with and install the python modules for 2.6. I'm not sure if this is right, now that you mention it.
> 
> @@ +82,5 @@
> > +            onlyif => "/bin/sh -c '! $basedir/bin/python -c \"import MySQLdb\"'";
> > +        #"$basedir-supervisor":
> > +        #    command => "$basedir/bin/pip install --no-deps --no-index --find-links=${package_dir_http} MySQL-python==1.2.3",
> > +        #    require => Exec["$basedir-virtualenv"],
> > +        #    onlyif => "/bin/sh -c '! $basedir/bin/python -c \"import supervisor\"'";
> 
> Do you need supervisor or not?
Supervisor will be required, but I haven't written the configuration for it yet. This will be done once we have autoland-staging01 re-imaged.

Comment 13

[Marc Jessome[:mjessome]]

Attachment: autoland manifest v5

Comment 14

[[e-mail] (:bhearsum)]

Comment on attachment 606199 [details] [diff] [review]
autoland manifest v5

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

Other than the questions about api-dev and the global hgrc, this looks okay.

::: buildmaster-production.pp
@@ +334,5 @@
> +        "/home/autoland/autoland-env":
> +            code_tag       => "production",
> +            user           => "autoland",
> +            attachment_url => "https://bugzilla.mozilla.org/attachment.cgi?id=",
> +            api_url        => "https://api-dev.bugzilla.mozilla.org/latest/",

Do you really want to point production at api-dev?

::: modules/autoland/manifests/instance.pp
@@ +37,5 @@
> +        #"$basedir-clone-tools":
> +        #    creates => "$basedir/tools",
> +        #    command => "/usr/bin/hg clone -r $code_tag $tools_repo tools",
> +        #    cwd => "$basedir",
> +        #    require => [Exec["$basedir-virtualenv"], Package["mercurial"]];

You can't depend on Package["mercurial"] anymore now that you've dropped it from dep.pp.

@@ +102,5 @@
> +        "/home/autoland/.ssh":
> +            ensure => directory;
> +        "/home/autoland/.ssh/id_rsa.pub":
> +            source => "puppet:///modules/autoland/id_rsa.pub";
> +        # when running in supervisor, a global hgrc is required

This doesn't sound right if the process is running as a normal user....maybe this was fallout from when you were running as root?

Comment 15

[Marc Jessome[:mjessome]]

(In reply to Ben Hearsum [:bhearsum] from comment #14)
> Comment on attachment 606199 [details] [diff] [review]
> autoland manifest v5
> 
> Review of attachment 606199 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Other than the questions about api-dev and the global hgrc, this looks okay.
> 
> ::: buildmaster-production.pp
> @@ +334,5 @@
> > +        "/home/autoland/autoland-env":
> > +            code_tag       => "production",
> > +            user           => "autoland",
> > +            attachment_url => "https://bugzilla.mozilla.org/attachment.cgi?id=",
> > +            api_url        => "https://api-dev.bugzilla.mozilla.org/latest/",
> 
> Do you really want to point production at api-dev?
According to https://wiki.mozilla.org/Bugzilla:REST_API, api-dev is the main access point.

> 
> ::: modules/autoland/manifests/instance.pp
> @@ +37,5 @@
> > +        #"$basedir-clone-tools":
> > +        #    creates => "$basedir/tools",
> > +        #    command => "/usr/bin/hg clone -r $code_tag $tools_repo tools",
> > +        #    cwd => "$basedir",
> > +        #    require => [Exec["$basedir-virtualenv"], Package["mercurial"]];
> 
> You can't depend on Package["mercurial"] anymore now that you've dropped it
> from dep.pp.
Package["mercurial"] is defined in releng::master which is now included in buildmaster-production.pp. Is it pointless to depend on it here?

> 
> @@ +102,5 @@
> > +        "/home/autoland/.ssh":
> > +            ensure => directory;
> > +        "/home/autoland/.ssh/id_rsa.pub":
> > +            source => "puppet:///modules/autoland/id_rsa.pub";
> > +        # when running in supervisor, a global hgrc is required
> 
> This doesn't sound right if the process is running as a normal user....maybe
> this was fallout from when you were running as root?
I'll look into this one more when I get the chance.

Comment 16

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 606199 [details] [diff] [review]
autoland manifest v5

Looks great, awesome work on this.

Comment 17

[Marc Jessome[:mjessome]]

Since this is going to be obsoleted by the IT puppet manifests on our new VMs, I'm going to mark this invalid. Thanks for the help, Ben.