820238 - PuppetAgain config for new Linux64 testing reference platform

Status: RESOLVED FIXED

(Infrastructure & Operations Graveyard :: CIDuty, task, P2)

Description

[Chris Cooper [:coop] (he/him)]

We're creating a new Linux64 testing reference platform that's going to run Ubuntu 12.04.1 LTS. 

We don't currently have anything in place for Ubuntu, so we'll need to bootstrap this platform. Rail is looking to setup a similar config in AWS, but he won't be concerned about graphics performance, only correctness. We may need extra packages/scripts to get all the talos suites running (graphics drivers, screensaver control, etc.)

Comment 1

[Rail Aliiev [:rail]]

I'll take this. I'll start with manifests for AWS, once it ready (or in parallel) we can adjust them for iX.

Comment 2

[Rail Aliiev [:rail]]

Attachment: Ubuntu 12.04 v0.1

* I ran this patch using puppet apply (puppet 2.7.11)
* I had to hack and replace puppet:/// with a local path for virtualenv
* so far it gets to the state with running VNC
* still may require some optimization
* needs to start buildbot (probably using the same approach we use for Fedora)

Comments and suggestions are welcome! :)

Comment 3

[Justin Wood (:Callek)]

Comment on attachment 696434 [details] [diff] [review]
Ubuntu 12.04 v0.1

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

Overall looks great, mostly nits/thought-provoking-questions.

::: manifests/nodes.pp
@@ +70,5 @@
>      $is_bmm_admin_host = $fqdn ? { /^mobile-imaging-001/ => true, default => false }
>      $mozpool_staging = false
>      include toplevel::server::mozpool
>  }
> +node /railz-ubuntu-\d+/ {

for sure we want a different node def when you deploy ;-)

::: modules/buildslave/manifests/install/version.pp
@@ +17,5 @@
>          "0.8.4-pre-moz2": {
>              include packages::mozilla::python27
> +            case $::operatingsystem {
> +                Ubuntu: {
> +                    $python = '/usr/bin/python'

special reason we don't want the /tools/ commonality here? I'd argue it does have benefits to have an untied-to-system python used here (easier upgrades, --in theory-- [since we can have side-by-side], etc.)

::: modules/buildslave/manifests/startup.pp
@@ +23,5 @@
>      # select an implementation class based on operating system
>      $startuptype = $operatingsystem ? {
>          CentOS      => "initd",
> +        Darwin      => "launchd",
> +        Ubuntu      => "desktop"

do we expect other OS's to want "desktop" style but divergent solutions for starting up the slave, if so perhaps ubuntu_desktop or something. But thats more a style nit and shouldn't block this overall project.

::: modules/ntp/manifests/atboot.pp
@@ +1,4 @@
>  class ntp::atboot {
>      include packages::ntp
>  
>      case $operatingsystem {

you changed this elsewhere to $::operatingsystem can we either not change that or change this while you're here (consistent style and all)

::: modules/packages/manifests/editors.pp
@@ +9,5 @@
>              }
>          }
> +        Ubuntu: {
> +            package {
> +                ["nano", "vim"]:

does "vim" == what CentOS defines as "vim-minimal" or is it more feature-rich? if has-more-features any special reason?

::: modules/packages/manifests/mozilla/python27.pp
@@ +14,5 @@
>          }
> +        Ubuntu: {
> +	    Anchor['packages::mozilla::python27::begin'] ->
> +            package {
> +                "python":

as mentioned elsewhere, any reason this isn't using mozilla's variant?

::: modules/packages/manifests/puppet.pp
@@ +18,5 @@
>          }
> +        Ubuntu: {
> +            package {
> +                "puppet":
> +                    ensure => "latest";

why not "puppet_rpm_version" here?

::: modules/puppet/manifests/atboot.pp
@@ +33,1 @@
>              # On CentOS, puppet runs from an initscript that blocks until the

If init script works the same way for ubuntu can you modify this comment to be accurate?

::: modules/python/manifests/virtualenv/settings.pp
@@ +4,5 @@
>      $misc_python_dir = "/tools/misc-python"
>  
>      # the puppet URL for the python/packages downloads
> +    # TODO: revert this
> +    $packages_dir_source = "/data/python/packages"

:-)

::: modules/screenresolution/manifests/talos.pp
@@ +20,5 @@
>                  }
>              }
>          }
> +        Ubuntu: {
> +            # Set by VNC server

in concept of keeping it in one place, [if possible] I'd favor having a file created here for ubuntu, that puppet uses to create the VNC setup script, or something similar. Such that we don't have to look/modify many places to change the resolution should we need to.

-- Can be followup

::: modules/sudoers/files/sudoers.Ubuntu
@@ +1,1 @@
> +## Sudoers allows particular users to run various commands as

You'll want to modify sudoers::settings::rebootpath for Ubuntu as well (e.g. http://mxr.mozilla.org/build/source/puppet/modules/sudoers/manifests/settings.pp )

::: modules/vnc/manifests/appearance.pp
@@ +3,5 @@
>  include dirs::usr::local::bin
>  include users::root
>  
> +case $::operatingsystem {
> +    Darwin: {

can we do a Ubuntu/CentOS/whatever case, and then a fail() case like elsewhere if we're casing out classes of OS's here?

::: modules/vnc/manifests/init.pp
@@ +49,5 @@
> +                    ensure  => file,
> +                    mode    => 0600,
> +                    owner   => $::users::builder::username,
> +                    group   => $::users::builder::group,
> +                    content => base64decode($::config::secrets::builder_pw_vnc_base64);

huh, does this really take a *bare* password, or are we base64encoding the vncpasswd result?

Comment 4

[Rail Aliiev [:rail]]

First of all, thanks for the early feedback!

(In reply to Justin Wood (:Callek) from comment #3)
> special reason we don't want the /tools/ commonality here? I'd argue it does
> have benefits to have an untied-to-system python used here (easier upgrades,
> --in theory-- [since we can have side-by-side], etc.)

I would prefer to avoid creating custom packages. Maintaining them aren't free. In the worst case scenario I'd better to have a symlink in /tools to system python.

> ::: modules/buildslave/manifests/startup.pp
> @@ +23,5 @@
> >      # select an implementation class based on operating system
> >      $startuptype = $operatingsystem ? {
> >          CentOS      => "initd",
> > +        Darwin      => "launchd",
> > +        Ubuntu      => "desktop"
> 
> do we expect other OS's to want "desktop" style but divergent solutions for
> starting up the slave, if so perhaps ubuntu_desktop or something. But thats
> more a style nit and shouldn't block this overall project.

I haven't touch this part yet. I hope there will be nothing Ubuntu specific, since Unity tries to follow XDG. We can use another name (xdg?) though.

 
> ::: modules/ntp/manifests/atboot.pp
> @@ +1,4 @@
> >  class ntp::atboot {
> >      include packages::ntp
> >  
> >      case $operatingsystem {
> 
> you changed this elsewhere to $::operatingsystem can we either not change
> that or change this while you're here (consistent style and all)

Yeah, it should be $::operatingsystem everywhere, because newer puppet complains about the other notation.

> ::: modules/packages/manifests/editors.pp
> @@ +9,5 @@
> >              }
> >          }
> > +        Ubuntu: {
> > +            package {
> > +                ["nano", "vim"]:
> 
> does "vim" == what CentOS defines as "vim-minimal" or is it more
> feature-rich? if has-more-features any special reason?

vim in Ubuntu is something like vim-enhanced in Fedora. It's not a big deal IMHO, since it's not used as a part of tests.

> ::: modules/packages/manifests/puppet.pp
> @@ +18,5 @@
> >          }
> > +        Ubuntu: {
> > +            package {
> > +                "puppet":
> > +                    ensure => "latest";
> 
> why not "puppet_rpm_version" here?

*rpm* version won't work here. Unless we have a custom deb repo for Ubuntu latest is the only available version. 
 
> ::: modules/puppet/manifests/atboot.pp
> @@ +33,1 @@
> >              # On CentOS, puppet runs from an initscript that blocks until the
> 
> If init script works the same way for ubuntu can you modify this comment to
> be accurate?

Probably I'll remove this customization in favor of desktop style init here.

> in concept of keeping it in one place, [if possible] I'd favor having a file
> created here for ubuntu, that puppet uses to create the VNC setup script, or
> something similar. Such that we don't have to look/modify many places to
> change the resolution should we need to.

I'll think about it. Having something on one place for all platforms may be useful.

> -- Can be followup
> 
> ::: modules/sudoers/files/sudoers.Ubuntu
> @@ +1,1 @@
> > +## Sudoers allows particular users to run various commands as
> 
> You'll want to modify sudoers::settings::rebootpath for Ubuntu as well (e.g.
> http://mxr.mozilla.org/build/source/puppet/modules/sudoers/manifests/
> settings.pp )

That was just a copy/paste to make puppet happy. I'll check the file in depth.
 
> ::: modules/vnc/manifests/appearance.pp
> @@ +3,5 @@
> >  include dirs::usr::local::bin
> >  include users::root
> >  
> > +case $::operatingsystem {
> > +    Darwin: {
> 
> can we do a Ubuntu/CentOS/whatever case, and then a fail() case like
> elsewhere if we're casing out classes of OS's here?

OK.

> ::: modules/vnc/manifests/init.pp
> @@ +49,5 @@
> > +                    ensure  => file,
> > +                    mode    => 0600,
> > +                    owner   => $::users::builder::username,
> > +                    group   => $::users::builder::group,
> > +                    content => base64decode($::config::secrets::builder_pw_vnc_base64);
> 
> huh, does this really take a *bare* password, or are we base64encoding the
> vncpasswd result?

The latter.

Comment 5

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

Comment on attachment 696434 [details] [diff] [review]
Ubuntu 12.04 v0.1

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

Apologies for a lot of "what Callek said" comments here, but .. what Callek said :)

In particular, keeping this implementation DRY is pretty important.  Related, making the resulting system look as much as possible like the other systems configured with puppetagain is pretty important, too, particularly around which versions of tools are installed, and how they're installed.

::: modules/buildslave/manifests/install/version.pp
@@ +17,5 @@
>          "0.8.4-pre-moz2": {
>              include packages::mozilla::python27
> +            case $::operatingsystem {
> +                Ubuntu: {
> +                    $python = '/usr/bin/python'

I'm with Callek on this one - we pretty much decided to deploy a custom python exactly so that we could have the same version everywhere, and a means to gracefully upgrade.  It's a bit of work to build the packages (and document the process repeatably), but not that bad.

At any rate, this shouldn't be special-casing on operating system.

::: modules/packages/manifests/mozilla/py27_mercurial.pp
@@ +21,5 @@
> +                "mercurial":
> +                    ensure => latest,
> +                    require => Class['packages::mozilla::python27'];
> +            } -> Anchor['packages::mozilla::py27_mercurial::end']
> +        }

Same here as for Python, above - I think we either need to build custom packages everywhere - as we already have for Darwin and CentOS - or nowhere, which would mean ripping this support out.  Half-and-half is just ugly and will lead to sadness.

::: modules/packages/manifests/puppet.pp
@@ +18,5 @@
>          }
> +        Ubuntu: {
> +            package {
> +                "puppet":
> +                    ensure => "latest";

Or more accurately, $puppet_version

::: modules/packages/manifests/tightvncserver.pp
@@ +1,1 @@
> +class packages::tightvncserver {

This should probably just be 'vnc' or 'vncserver', since tightvncserver isn't installed on Darwin.

Also, why not use x11vnc instead of tight?

::: modules/users/manifests/builder/autologin.pp
@@ +19,5 @@
>                      value => $::users::builder::username;
>              }
>          }
> +        Ubuntu: {
> +            # Managed by VNC server

Similar to what Callek said about screen resolution, it'd be great to have the autologin configured here.  I don't see how it's configured for tightvnc, anyway..

::: modules/vnc/manifests/appearance.pp
@@ +1,4 @@
>  class vnc::appearance {
>  
>  include dirs::usr::local::bin
>  include users::root

In keeping with "leave it cleaner than when you arrived", can you fix the indentation here?

::: modules/vnc/templates/tightvncserver.conf.erb
@@ +1,5 @@
> +start on runlevel [2345]
> +stop on runlevel [016]
> +
> +pre-start script
> +    su <%= scope.lookupvar('::config::builder_username') %> -c 'tightvncserver :0 -geometry 1600x1200 -nolisten tcp -localhost'

I don't know much about tightvnc, but it looks like this is expecting the VNC server to start an X session on :0, since none is running otherwise -- so really, cltbld never logs in, so much as tightvnc creates a session under cltbld's identity.  Does this session get linked properly with the video hardware?

Comment 6

[Rail Aliiev [:rail]]

(In reply to Dustin J. Mitchell [:dustin] from comment #5)
> ::: modules/packages/manifests/tightvncserver.pp
> @@ +1,1 @@
> > +class packages::tightvncserver {
> 
> This should probably just be 'vnc' or 'vncserver', since tightvncserver
> isn't installed on Darwin.
> 
> Also, why not use x11vnc instead of tight?

Could be... Initially I thought only about AWS based machines without any graphics card. Probably it's worth to spend some time and make it work using framebuffer or real hardware depending on hardware.
 
> ::: modules/users/manifests/builder/autologin.pp
> @@ +19,5 @@
> >                      value => $::users::builder::username;
> >              }
> >          }
> > +        Ubuntu: {
> > +            # Managed by VNC server
> 
> Similar to what Callek said about screen resolution, it'd be great to have
> the autologin configured here.  I don't see how it's configured for
> tightvnc, anyway..

OK. The current version reuses the same variables from screenresolution. tightvncserver creates ~/.vnc/startup which launches Xsession. There is no autologin from a DM, something like startx.
 
> ::: modules/vnc/manifests/appearance.pp
> @@ +1,4 @@
> >  class vnc::appearance {
> >  
> >  include dirs::usr::local::bin
> >  include users::root
> 
> In keeping with "leave it cleaner than when you arrived", can you fix the
> indentation here?

Already done :)

I'll rework the current patch and post another one once it's ready.

Comment 7

[Rail Aliiev [:rail]]

Attachment: Ubuntu 12.04 v0.2

This patch is more or less close to its final state, except a couple of changes blocked by custom packages we'll need (python, hg, etc).

I still need to read puppet-manifests to figure out what else should be tweaked (additional packages, configs, etc).

Feel free to add your comments while I'm off Mon-Wed :P

Comment 8

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

Comment on attachment 698010 [details] [diff] [review]
Ubuntu 12.04 v0.2

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

::: modules/packages/manifests/mozilla/py27_mercurial.pp
@@ +22,5 @@
> +            package {
> +                "mercurial":
> +                    ensure => latest;
> +            } -> Anchor['packages::mozilla::py27_mercurial::end']
> +        }

Is this going to get fixed in the final analysis, so that the path is always /tools/python27-mercurial/bin/hg?

::: modules/packages/manifests/puppet.pp
@@ +18,5 @@
>          }
> +        Ubuntu: {
> +            package {
> +                "puppet":
> +                    ensure => "latest";  # TODO: need to fix the version?

Yes, definitely :)

Comment 9

[Rail Aliiev [:rail]]

(In reply to Dustin J. Mitchell [:dustin] from comment #8)
> Is this going to get fixed in the final analysis, so that the path is always
> /tools/python27-mercurial/bin/hg?

Yes, I'm still going to poke around custom Ubuntu repos and the procedure to build and push packages.

Comment 10

[Rail Aliiev [:rail]]

I would like to discuss a couple of things here:

1) which version of python we should use for buildbot slave process?
2) which version of python we should use for unittests?
3) Can we use system python (2.7.3) for 1) and 2) ? On 
4) Do we really care about having the same path to Python executable under /tools?

Comment 11

[Chris AtLee [:catlee]]

(In reply to Rail Aliiev [:rail] from comment #10)
> I would like to discuss a couple of things here:
> 
> 1) which version of python we should use for buildbot slave process?

doesn't matter that much

> 2) which version of python we should use for unittests?

2.7.3 ideally

> 3) Can we use system python (2.7.3) for 1) and 2) ? On 

I believe so

> 4) Do we really care about having the same path to Python executable under
> /tools?

I don't think so

Comment 12

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

If we're not using the same path everywhere, that's fine - it's up to your automation to special-case all of the platforms.  But let's take out the custom packages on CentOS and OS X.  PuppetAgain aims to configure systems consistently, and either all-custom or none-custom are the consistent options.

Comment 13

[Justin Wood (:Callek)]

(In reply to Chris AtLee [:catlee] from comment #11)
> (In reply to Rail Aliiev [:rail] from comment #10)
> > I would like to discuss a couple of things here:
> > 
> > 1) which version of python we should use for buildbot slave process?
> 
> doesn't matter that much
> 
> > 2) which version of python we should use for unittests?
> 
> 2.7.3 ideally
> 

Actually I'd say ideally the *moz* version we do, because we also *patch* python

http://mxr.mozilla.org/build/source/puppet/modules/packages/manifests/mozilla/python27.spec#15

Unless you can articulate that the patches are bad/not needed, in which case we should normalize it on other platforms too.

Consistency here is going to save us in the long run, with a small amount of pain now. We got into the puppet-manifests mess [and buildbot-configs] by not being and staying consistent.

Comment 14

[Chris Cooper [:coop] (he/him)]

I agree with catlee in comment 11 on all points.

Comment 15

[Chris Cooper [:coop] (he/him)]

(In reply to Chris Cooper [:coop] from comment #14)
> I agree with catlee in comment 11 on all points.

To address https://bugzilla.mozilla.org/show_bug.cgi?id=602908#c34, let me clarify that I would like us specifically to use 2.7.3 here (as suggested).

Comment 16

[Rail Aliiev [:rail]]

Attachment: Ubuntu 12.04 v0.9

* ported manifests from puppet-manifests 
* adjusted node regexes
* added packages::aptrepo so we are ready for custom repos/packages
* fonts and fonts.conf (from Fedora)
* stopping some services on startup
* prepended ~/bin to PATH (from Fedora)

Regarding symlinks under /tools. ATM our testing platform (Fedora 12) has nothing under /tools. Python (2.5) lives in $HOME directory. No git installed. No custom mercurial version (system is 1.5.1 there). Even though python is installed by default, I added it as python2.7. Additionally I added python2.7-dev (ported from puppet-manifests) which I believe used by automation for virtualenv.

I tested this patch on an HP slave. 2 files had been changed: /home/cltbld/.bashrc and /etc/sysconfig/i18n (expected).

Comment 17

[Rail Aliiev [:rail]]

Attachment: Ubuntu 12.04 v0.91

I removed the part which changes PATH and added TZ settings for Ubuntu.

Comment 18

[Rail Aliiev [:rail]]

Attachment: configs

* Enable Ubuntu 64 on cedar
* run all mochitest tests even though chunk 1 will fail (there is no easy way to skip 1st chunk)
* run mochitest-a11y, mochitest-ipcplugins, crashtest, jsreftest
* will need update of secrets.pp

Comment 19

[Rail Aliiev [:rail]]

Attachment: TBPL changes

Comment 20

[Armen [:armenzg]]

Comment on attachment 702431 [details] [diff] [review]
configs

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

As long as it passes test-masters.sh I'm fine to land as is.

Comment 21

[Ed Morley [:emorley]]

Comment on attachment 702432 [details] [diff] [review]
TBPL changes

Is this going to eventually replace Fedora for Linux64?

Are we at any point going to have both listed on non-cedar? If so, I'd like to s/Linux64/Fedora64/ for extra clarity. If this is cedar-only for now, then I'm happy leaving it.

Either way, could we perhaps use "Ubuntu x64" or at a push "Ubuntu 12 x64" rather than the full version string? :-)

Comment 22

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

Comment on attachment 702338 [details] [diff] [review]
Ubuntu 12.04 v0.91

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

We haven't quite settled the Python-package question.  I'm worried that, when we add, say, Mercurial to the list of packages on Ubuntu, the desired version of Mercurial won't align with what's available in Precise, and we'll be in a fix.  I realize there's a desire to deploy this quickly, and that building packages isn't quick, and that existing automation (outside of puppet) doesn't expect Python to be available under /tools.  At a minimum, I'd like to make sure we have bugs on file to get this to a consistent state *after* it's deployed, but I think some small changes here can make that a lot easier.

In particular, I think it will make all of our lives easier down the road if the Python paths are consistent.  It removes the need to special-case the python path in python::virtualenv, at least, and also means that other scripts and tools can count on that path.  And it means that we make those changes now, rather than later, at which point we will be "fixing what ain't broken".

The "easy" way to get that win now is to symlink from /tools/python27/bin/python2.7 to /usr/bin/python.  Then, if we later decide to build a custom package, it just puts a real binary at that location.  No need to re-create virtualenvs, no need to update other special-cases for Python locations (e.g., in runslave.py).

This applies for Mercurial paths, too, although from what I see here those aren't actually used on testers.

I'm flagging Callek for review here on this issue, mostly because I know he's on-site with y'all and can work out the particulars.  I'll be content with whatever outcome is agreed on in Toronto.

As for the remainder, it looks great and there's a lot of nicely-designed stuff in here.  The only major issue is the Puppet version, but with that fixed I'm happy to see this landed.

::: modules/buildslave/manifests/startup/desktop.pp
@@ +5,5 @@
> +         "$::users::builder::home/.config/autostart"]:
> +            ensure => directory,
> +            owner  => $users::builder::username,
> +            group  => $users::builder::group;
> +        "$::users::builder::home/.config/autostart/gnome-terminal.desktop":

Minor, but these should probably have ${..}

::: modules/packages/manifests/fonts.pp
@@ +8,5 @@
> +                 "ttf-paktype", "fonts-stix", "fonts-unfonts-core",
> +                 "fonts-unfonts-extra"]:
> +                     ensure => latest;
> +            }
> +        }

This should have a `default: { fail(..) }`

::: modules/packages/manifests/mozilla/py27_mercurial.pp
@@ +17,5 @@
>              } -> Anchor['packages::mozilla::py27_mercurial::end']
>          }
> +        Ubuntu: {
> +            $mercurial = "/usr/bin/hg"
> +            Anchor['packages::mozilla::py27_mercurial::begin'] ->

This should just `include packages::mercurial`

::: modules/packages/manifests/puppet.pp
@@ +18,5 @@
>          }
> +        Ubuntu: {
> +            package {
> +                "puppet":
> +                    ensure => "latest";  # TODO: need to fix the version?

Yep!  This should match the app version used on the other OS's exactly (so $puppet_version, not $puppet_rpm_version).

::: modules/packages/manifests/setup.pp
@@ +70,5 @@
> +            # daily basis or when /etc/.repo-flag is bumped
> +            schedule {
> +                "daily":
> +                    period => daily;
> +            }

++

@@ +87,5 @@
> +                "/etc/.repo-flag":
> +                    content =>
> +                    "# see \$repoflag in modules/packages/manifests/setup.pp\n$repoflag\n",
> +                    notify => Exec['apt-get-update'];
> +                # Make sure that the main file is empty

This part - $repoflag and the file resource - should probably be factored out of the $::operatingsystem case

Comment 23

[Rail Aliiev [:rail]]

Attachment: Ubuntu 12.04 v1.0

* Added /tools symlinks 
* Addressed comments

Interdiff: https://gist.github.com/4551139

I'l file bugs for the following TODO items before we switch to this platform (we'll need to run run this platform in parallel with Fedora on one of the project branches):

* Build puppet deb package
* Document package building and adding packages to the repo
* Allow selectively assign slave platforms to unittest chunks
* Document ref platform
* (package and) deploy node.exe to ~/bin
* ...
* PROFIT!

Comment 24

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

Sweet, thanks!

Comment 25

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

Just to check, this isn't waiting on me for anything, right?

I'm working on Ubuntu kickstart support, but AIUI AWS provides you with adequate test systems in the meantime.

Comment 26

[Rail Aliiev [:rail]]

dustin, you're correct, there is nothing to be done from your side. Thanks a lot for the commetns and suggestions. I'm waiting for Callek's review which is expected tomorrow.

Comment 27

[Rail Aliiev [:rail]]

The final version of the patch will replace ":2" display with ":0" since it's used in buildbot-configs/mozilla-test.

Comment 28

[Rail Aliiev [:rail]]

Attachment: configs

It turns out that there is no easy way to disable talos tests without plumbing :/

* I had to move the new slave platform to PLATFORMS
* To skip talos tests I had to remove it from ALL_PLATFORMS, NO_WIN and NO_MAC
* some PEP8 clean up
* better output in __main__, the same format as in mozilla/config.py

builbotcustom and tools patches incoming.

Comment 29

[Rail Aliiev [:rail]]

Attachment: tools

* Added ubuntu64 to buildfarm/utils/run_jetpack.py
* Applied some PEP8 suggestions

Comment 30

[Rail Aliiev [:rail]]

Attachment: buildbotcustom

All 3 patches pass test-masters.sh, have sane diff of "python config.py"  and dump_masters.py output.

Comment 31

[Rail Aliiev [:rail]]

Comment on attachment 704615 [details] [diff] [review]
tools

http://hg.mozilla.org/build/tools/rev/e3d6b9ce60c8

Comment 32

[Rail Aliiev [:rail]]

Comment on attachment 704616 [details] [diff] [review]
buildbotcustom

http://hg.mozilla.org/build/buildbotcustom/rev/88d4e6cc2057

Comment 33

[Rail Aliiev [:rail]]

Attachment: TBPL changes

(In reply to Ed Morley [:edmorley UTC+0] from comment #21)
> Comment on attachment 702432 [details] [diff] [review]
> TBPL changes
> 
> Is this going to eventually replace Fedora for Linux64?

Yes. We are going to start Ubuntu in Amazon to run some unittests. Once we set up Ubuntu on real hardware we can replace Fedora slaves. No ETA for the latter yet.

> Are we at any point going to have both listed on non-cedar? If so, I'd like
> to s/Linux64/Fedora64/ for extra clarity. If this is cedar-only for now,
> then I'm happy leaving it.

It's cedar only for now, but we'll have both Fedora and Ubuntu running on most of the branches at some point. 

How about this patch?

Comment 34

[Ed Morley [:emorley]]

Comment on attachment 704630 [details] [diff] [review]
TBPL changes

Hmm not sure what we should do here - this patch puts tests and builds on separate lines since the builds contain "Linux" in their buildername, whereas the tests "Fedora" - which may be confusing. What platform do we build on - CentOS still, even for EC2?

I'd almost rather put the builds on the fedora line (even though it's not quite true) - like we do for the Windows line, where the builds were on win2k8 but the tests on winnt 6.1

Comment 35

[Rail Aliiev [:rail]]

Attachment: TBPL changes, s/linux/fedora/

(In reply to Ed Morley (Away 18th-20th Jan) [:edmorley UTC+0] from comment #34)
> Comment on attachment 704630 [details] [diff] [review]
> TBPL changes
> 
> Hmm not sure what we should do here - this patch puts tests and builds on
> separate lines since the builds contain "Linux" in their buildername,
> whereas the tests "Fedora" - which may be confusing. What platform do we
> build on - CentOS still, even for EC2?

Yes, CentOS.

> I'd almost rather put the builds on the fedora line (even though it's not
> quite true) - like we do for the Windows line, where the builds were on
> win2k8 but the tests on winnt 6.1

Something like this one?

Comment 36

[Rail Aliiev [:rail]]

FTR, slavealloc changes:

INSERT INTO distros (name) VALUES ('ubuntu64'); -- inserted with ID 25
INSERT INTO speeds (name) VALUES ('m1.medium'); -- inserted with ID 15

INSERT INTO pools (name) VALUES ('tests-aws-us-east-1-linux'); -- inserted with ID 47
INSERT INTO pools (name) VALUES ('tests-aws-us-west-2-linux'); -- inserted with ID 49

INSERT INTO slave_passwords (poolid, distroid, password) VALUES (47, NULL, pass);
INSERT INTO slave_passwords (poolid, distroid, password) VALUES (49, NULL, pass);

Comment 37

[Ed Morley [:emorley]]

Comment on attachment 704646 [details] [diff] [review]
TBPL changes, s/linux/fedora/

lgtm :-)

Comment 38

[Rail Aliiev [:rail]]

Comment on attachment 704646 [details] [diff] [review]
TBPL changes, s/linux/fedora/

http://hg.mozilla.org/webtools/tbpl/rev/183241a96fe5

Comment 39

[Rail Aliiev [:rail]]

Attachment: puppet-manifests

BuildSlaves-tests.py changes

Comment 40

[Justin Wood (:Callek)]

Comment on attachment 703013 [details] [diff] [review]
Ubuntu 12.04 v1.0

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

Not directly ubuntu-config that I love about this patch:
* moving specification of /tools/python and mercurial stuff to package variables referenced directly
* fixing $::operatingsystem to always ref the global

Mostly minor nits, and while I'm not a fan of some of these "# done with xvfb" lines, so long as we have the pointer I won't block on it.

At your discretion feel free to point an interdiff back at me

::: modules/buildslave/files/gnome-terminal.desktop
@@ +1,3 @@
> +[Desktop Entry]
> +Type=Application
> +Exec=gnome-terminal -x python /usr/local/bin/runslave.py

I'd be happier if we called the tools python here, and used this as a template, but it *does not* block landing this, and we can revisit later.

::: modules/disableservices/manifests/common.pp
@@ +18,5 @@
>          }
> +        Ubuntu: {
> +            service {
> +                ['acpid', 'avahi-daemon', 'anacron', 'apport', 'modemmanager',
> +                 'whoopsie', 'cups', 'bluetooth', 'lightdm', 'network-manager']:

no idea if more should be stopped or not, so I'm rubber-stamping this hunk

::: modules/disableservices/manifests/displaymanager.pp
@@ +1,5 @@
> +class disableservices::displaymanager {
> +    # This class disables unnecessary login manager start
> +
> +    case $::operatingsystem {
> +        Ubuntu: {

as always, nit: Darwin: { # No corresponding service } default: { fail... }

::: modules/nrpe/manifests/check/buildbot.pp
@@ +9,2 @@
>                  CentOS => "$plugins_dir/check_procs -w 1:1 -C twistd --argument-array=buildbot.tac",
>                  Darwin => "$plugins_dir/check_procs -w 1:1 -C python --argument-array=buildbot.tac",

discussed on IRC, Darwin is not actually used here, since there are no Darwin puppetAgain configs that instantiate this class, and this class is only used on non-test-machines.

So in a followup we can either rip it out until we have a Darwin builder, or leave it presuming it will be fine ;-)

::: modules/ntp/manifests/atboot.pp
@@ +10,5 @@
>              }
>          }
> +        Ubuntu: {
> +            service {
> +                "ntp":

discussed in IRC, we need a service that won't stay up after run when instantiating in "atboot" here. https://wiki.mozilla.org/ReleaseEngineering/PuppetAgain/Modules/ntp

::: modules/packages/manifests/aptrepo.pp
@@ +1,1 @@
> +define packages::aptrepo ($repo_name = $title, $url_path, $distribution, $components, $gpg_id='', $gpg_key='') {

I don't see gpg_id or gpg_key used anywhere yet, is this expected soon? Was this package/define copied from some other place, or what? (Trying to identify if we should rip out unused code, that *could* be broken until we need it, or if this is tested by a third party, or if we expect a change soon to make use)

@@ +17,5 @@
> +            owner => 0,
> +            group => 0,
> +            mode => 0644,
> +            notify => Exec['apt-get-update'],
> +            content => template("packages/sources.list.erb");

minor-nit: name this template somehow with ubuntu in name, so its more obvious when looking at templates dir, e.g. ubuntu-sources.list.erb

::: modules/packages/manifests/fonts.pp
@@ +5,5 @@
> +                ["fonts-kacst-one", "fonts-kacst", "fonts-liberation",
> +                 "ttf-indic-fonts-core", "ttf-kannada-fonts", "ttf-dejavu",
> +                 "ttf-oriya-fonts", "ttf-punjabi-fonts", "ttf-arphic-uming",
> +                 "ttf-paktype", "fonts-stix", "fonts-unfonts-core",
> +                 "fonts-unfonts-extra"]:

rs+ on font list ;-) I expect there is a good reason we need/want these specific ones.

::: modules/packages/manifests/puppet.pp
@@ +20,5 @@
> +        Ubuntu: {
> +            package {
> +                "puppet":
> +                    ensure => "latest";
> +                    # TODO: use `ensure => $puppet_deb_version;`

is this TODO fixable yet? If not what is it blocked on?

::: modules/packages/manifests/setup.pp
@@ +78,5 @@
> +                    schedule    => "daily";
> +                "apt-get-update":
> +                    command     => "/usr/bin/apt-get update",
> +                    refreshonly => true;
> +            }

to avoid affecting performance I'd rather a script run at startup that does all this if its been > a day since last purge/check. Before buildbot starts up.

Since we're not doing talos in this first iteration I am *ok* with that being a followup.

We can even have a "force update" type flag too for when we know we need/want to.

The idea is also to avoid bumping packages while stuff is running that uses them. Luckily short-term we have a static snapshot.

::: modules/toplevel/manifests/slave.pp
@@ +15,5 @@
>      # apply tweaks
>      include tweaks::dev-ptmx
>      include tweaks::rc_local
>      include disableservices::slave
> +    include tweaks::locale

extreme-minor-nit: move this line above disableservices::

::: modules/tweaks/manifests/cleanup.pp
@@ +1,2 @@
> +# Clean up things on start
> +class tweaks::cleanup {

Just to say somewhere, I've never been a fan of puppet doing this work for us, but I don't see an easy way to change atm, and we're already doing it on other test slaves so unless you have a better, short-term solution, not even followup-bug blocking

::: modules/vnc/templates/Xsession.conf.erb
@@ +2,5 @@
> +
> +start on started xvfb
> +
> +script
> +  su - -c "DISPLAY=:2 /etc/X11/Xsession" <%= scope.lookupvar('::config::builder_username') %>

as commented earlier, this should be DISPLAY=:0

::: modules/vnc/templates/x11vnc.conf.erb
@@ +2,5 @@
> +
> +manual
> +
> +script
> +  su - -c "x11vnc -display :2 -rfbauth ~/.vnc/passwd -forever -shared" <%= scope.lookupvar('::config::builder_username') %>

same display here

::: modules/vnc/templates/xvfb.conf.erb
@@ +6,5 @@
> +
> +# wait until puppet finishes
> +start on stopped puppet
> +
> +exec Xvfb :2 -screen 0 <%= scope.lookupvar('::screenresolution::talos::width') %>x<%= scope.lookupvar('::screenresolution::talos::height') %>x<%= scope.lookupvar('::screenresolution::talos::xvfb_depth') %>

and here

Comment 41

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

> > +Exec=gnome-terminal -x python /usr/local/bin/runslave.py
> 
> I'd be happier if we called the tools python here, and used this as a
> template, but it *does not* block landing this, and we can revisit later.

runslave.py is specifically designed to run in any Python, so this isn't a big deal either way.

> discussed on IRC, Darwin is not actually used here, since there are no
> Darwin puppetAgain configs that instantiate this class, and this class is
> only used on non-test-machines.
> 
> So in a followup we can either rip it out until we have a Darwin builder, or
> leave it presuming it will be fine ;-)

I'd prefer the latter.  Reasoning that begins with "OS X is not used in machine type Y, so this isn't necessary" should be avoided, in general.  Then when we do use X and Y together, there will be less to do to make it work.

> to avoid affecting performance I'd rather a script run at startup that does
> all this if its been > a day since last purge/check. Before buildbot starts
> up.

That's what rail's patch does!

> We can even have a "force update" type flag too for when we know we
> need/want to.

That's /etc/.repo-flag

> The idea is also to avoid bumping packages while stuff is running that uses
> them. Luckily short-term we have a static snapshot.

Puppet runs only occur before Buildbot starts.  And yes, this is based on the assumption of a static snapshot.  This is parallel to the yum cache management on CentOS.  'apt-get update' is not the same as 'yum upgrade' :)

Comment 42

[Rail Aliiev [:rail]]

Comment on attachment 703013 [details] [diff] [review]
Ubuntu 12.04 v1.0

http://hg.mozilla.org/build/puppet/rev/6fea3e96d17c

interdiff: https://gist.github.com/4597418

Comment 43

[Rail Aliiev [:rail]]

Comment on attachment 704906 [details] [diff] [review]
puppet-manifests

http://hg.mozilla.org/build/puppet-manifests/rev/f886f578cb6c

Comment 44

[Rail Aliiev [:rail]]

Comment on attachment 704613 [details] [diff] [review]
configs

http://hg.mozilla.org/build/buildbot-configs/rev/341cdbd67c41

Comment 45

[Kim Moir [:kmoir] ET]

in production

Comment 46

[Ed Morley [:emorley]]

TBPL part is in production.

Comment 47

[Rail Aliiev [:rail]]

I believe all done here. \o/