1185909 - Update the puppet runner manifest to handle windows installs

Status: RESOLVED FIXED

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

Description

[Rob Thijssen [:grenade (EET/UTC+0300)]]

the current runner manifest (https://hg.mozilla.org/build/puppet/file/fcd376586df5/modules/runner/manifests/init.pp) handles ubuntu only, extend this to install runner on windows.

Comment 1

[Rob Thijssen [:grenade (EET/UTC+0300)]]

I had a look at GPO today and there isn't a lot of work. It's just a bit finicky and I'd like to clean it up a bit. It looks like just a collection of python bits and batch files that set environment variables.
- under Puppet (as opposed to GPO), it makes sense to set the env vars using the puppet mechanisms for doing so.
- the implementation relies on specific paths (c:\opt, c:\mozilla-build, etc). This is a little ugly on Windows but we should probably leave it unchanged for the gpo to puppet migration and look to improve it after we get it working
- gpo starts runner with a login trigger for the build account. I think it would be a lot nicer to set it up to run as a native windows service, but again, maybe better to just get it working, and improve as further patches.

Comment 2

[Mark Cornmesser [:markco]]

Attachment: BUG1185909-WIP.patch

This what I have for this so far.
mrrrgn: As far as the tasks, should I just put OS conditional language in the task manifests and point the content for clobber, buildbot, and halt? And I am summing i will need to put all those into erb(s)? 

With the runner.cfg, Is all we need there is the halt task and the set the PATH variable? Should I point the other directory paths to paths that Windows can use?

Also this currently set up on B-2008-ix-0001 if anyone wants to poke at it.

Comment 3

[Morgan Phillips [:mrrrgn]]

Comment on attachment 8643929 [details] [diff] [review]
BUG1185909-WIP.patch

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

Looking to be in the right direction so far. We should leave out cleanslate for now. That sounds right for the runner.cfg, whichever fields will make it match what we currently have on the windows machines will be great. As for tasks, yes, unfortunately platform specific things are being baked into tasks via templates at the moment. Another option, since windows requires .bat files anyway, is to just rewrite any simple stuff as a windows only file.

::: modules/cleanslate/manifests/settings.pp
@@ -6,4 @@
>          'CentOS', 'Ubuntu', 'Darwin': {
>              $root = '/opt/cleanslate'
>          }
> -

I think we should leave cleanslate only on Linux/OSX for now.

Comment 4

[Morgan Phillips [:mrrrgn]]

(In reply to Mark Cornmesser [:markco] from comment #2)
> Created attachment 8643929 [details] [diff] [review]
> BUG1185909-WIP.patch
> 
> This what I have for this so far.
> mrrrgn: As far as the tasks, should I just put OS conditional language in
> the task manifests and point the content for clobber, buildbot, and halt?
> And I am summing i will need to put all those into erb(s)? 
> 
> With the runner.cfg, Is all we need there is the halt task and the set the
> PATH variable? Should I point the other directory paths to paths that
> Windows can use?
> 
> Also this currently set up on B-2008-ix-0001 if anyone wants to poke at it.

Just to clarify, I think http://hg.mozilla.org/build/puppet/file/eca827980bad/modules/buildslave/manifests/startup/runner.pp is a good place to put platform specific tasks. Since the windows tasks are batch scripts, it should be fine to make them completely separate from their *NIX counterparts. Then you'll just be able to include them under a case::operatingsystem of Windows.

Comment 5

[Mark Cornmesser [:markco]]

Just an update. I have all the parts are being deployed, but when runner.py is ran the following traceback is returned: 
"Thu 08/13/2015  8:10:55.30 - About to run runner.py"
  File "c:\opt\runner\Scripts\runner.exe", line 1
SyntaxError: Non-ASCII character '\x90' in file c:\opt\runner\Scripts\runner.exe
 on line 1, but no encoding declared; see http://www.python.org/peps/pep-0263.ht
ml for details
"Thu 08/13/2015  8:10:55.88 - runner.py finished"

Comment 6

[Mark Cornmesser [:markco]]

mrrrgn: any thoughts on the traceback in comment 5?

Comment 7

[Morgan Phillips [:mrrrgn]]

(In reply to Mark Cornmesser [:markco] from comment #6)
> mrrrgn: any thoughts on the traceback in comment 5?

Ahh, I need to push out an updated runner package. This is a solved issue with runner on Windows. Will let you know when I get out the new package.

Comment 8

[Morgan Phillips [:mrrrgn]]

(In reply to Mark Cornmesser [:markco] from comment #6)
> mrrrgn: any thoughts on the traceback in comment 5?

Can you give me the hostname of the machine where this happened so that I can look into it?

Comment 9

[Morgan Phillips [:mrrrgn]]

Comment on attachment 8643929 [details] [diff] [review]
BUG1185909-WIP.patch

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

I goofed the review here. It needs more work. I left comments on the problematic areas.

::: modules/runner/files/0-clobber.bat
@@ +15,5 @@
> +
> +REM running this via 'bash' is critical - bash adds a bunch of items to PATH
> +REM which the build steps expect to find.  We pass the --twistd-cmd pointing
> +REM to the appropriate twistd executable for the active Buildbot version
> +"%MOZILLABUILD%\msys\bin\bash" --login -c "cd /c/builds/moz2_slave;'%BUILDBOT_PATH%\Scripts\python' /c/opt/clobberer.py -s scripts -s buildprops.json --url 'https://api.pub.build.mozilla.org/clobberer/lastclobber/all' -v"

It looks like clobberer is not being installed in the opt directory, so this task will fail. We need clobberer.py in /c/opt

::: modules/runner/manifests/init.pp
@@ +30,4 @@
>              recurse => true,
>              purge   => true;
>          "${runner::settings::root}/runner.cfg":
> +            before  => $runner_service,

We need to modify the contents of runner.cfg so that it looks like what we have on windows now. The contents of runner.cfg need to match: https://pastebin.mozilla.org/8842705

::: modules/runner/templates/buildbot.bat.erb
@@ +23,5 @@
> +
> +REM running this via 'bash' is critical - bash adds a bunch of items to PATH
> +REM which the build steps expect to find.  We pass the --twistd-cmd pointing
> +REM to the appropriate twistd executable for the active Buildbot version
> +"%MOZILLABUILD%\msys\bin\bash" --login -c "'%BUILDBOT_PATH%\Scripts\python' /c/opt/runslave.py --twistd-cmd '%BUILDBOT_PATH%\Scripts\twistd.py'"

runslave.py doesn't appear to be installed at /c/opt/ here. Instead, this line should read: "%MOZILLABUILD%\msys\bin\bash" --login -c "'%BUILDBOT_PATH%\Scripts\python' /c/Users/All\ Users/puppetagain/runslave.py --twistd-cmd '%BUILDBOT_PATH%\Scripts\twistd.py'"

Also, we need to add SET IDLEIZER_HALT_ON_IDLE=true

::: modules/runner/templates/start-runner.bat.erb
@@ +37,5 @@
> +
> +:start
> +echo "%date% %time% - About to run runner.py"
> +
> +"%MOZILLABUILD%\Python27\python.exe" c:\opt\runner\Scripts\runner.exe -c c:\opt\runner\runner.cfg c:\opt\runner\tasks.d

python.exe shouldn't call runner.exe instead use:
%BUILDBOT_PATH%\Scripts\runner -n 5 -H -c c:\opt\runner\runner.cfg c:\opt\runner\tasks.d 2> c:\tmp\runner.log

Comment 10

[Rob Thijssen [:grenade (EET/UTC+0300)]]

Comment on attachment 8643929 [details] [diff] [review]
BUG1185909-WIP.patch

I feel out of my depth on this one. I'm going to defer to mrrrgn.

Comment 11

[Mark Cornmesser [:markco]]

Attachment: BUG1185909-WIP.patch

Comment 12

[Mark Cornmesser [:markco]]

Attachment: BUG1185909-WIP.patch

Comment 13

[Mark Cornmesser [:markco]]

Attachment: BUG1185909-WIP.patch

This is almost there... I think. Tested this and runner starts and builds complete successfully. 

Not submitting for review yet. When I try to run Puppet in AWS with these changes the runner schedule task fails: 

ERROR: No mapping between account names and security IDs was done.
(9,42):UserId:y-2008-ec2-golden\cltbld

Comment 14

[Morgan Phillips [:mrrrgn]]

Comment on attachment 8651851 [details] [diff] [review]
BUG1185909-WIP.patch

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

Mostly there, let's remove the extra task_hook.py and try to find a common clobberer.py. Also, the halt.bat seems to be stubbed out. Great work on this!!!! :]

::: modules/runner/files/clobberer.py
@@ +1,1 @@
> +#!/usr/bin/python

This will work, but we should try to see if there's already another copy living on the machine and use that one if possible. Looks like it lives in hg.mozilla.org/build/tools/clobberer http://hg.mozilla.org/build/tools/file/dbca4fa3f6a9/clobberer

::: modules/runner/files/task_hook.py
@@ +1,1 @@
> +#!/usr/bin/env python

This already lives in the runner module as influxdb_hook.py http://hg.mozilla.org/build/puppet/file/39edb742127c/modules/runner/files/influxdb_hook.py it can be deployed the same as on Linux/Mac.

::: modules/runner/templates/tasks/clobber.bat.erb
@@ +15,5 @@
> +
> +REM running this via 'bash' is critical - bash adds a bunch of items to PATH
> +REM which the build steps expect to find.  We pass the --twistd-cmd pointing
> +REM to the appropriate twistd executable for the active Buildbot version
> +"%MOZILLABUILD%\msys\bin\bash" --login -c "cd /c/builds/moz2_slave;'%BUILDBOT_PATH%\Scripts\python' /c/opt/clobberer.py -s scripts -s buildprops.json --url 'https://api.pub.build.mozilla.org/clobberer/lastclobber/all' -v"

We can modify the /c/opt/clobberer.py here to point to an existing clobberer if there is one. See the comment above about that.

::: modules/runner/templates/tasks/halt.bat.erb
@@ +2,5 @@
> +
> +call "C:/mozilla-build/bbpath.bat"
> +echo "Buildbot virtualenv: %BUILDBOT_PATH%"
> +
> +echo shutdown /r /t 0 /f

This could just be a file. It only needs `shutdown /r /t 0 /f`

Comment 15

[Mark Cornmesser [:markco]]

Attachment: Bug1185909.patch

Comment 16

[Mark Cornmesser [:markco]]

Attachment: Bug1185909.patch

Comment 17

[Morgan Phillips [:mrrrgn]]

(In reply to Mark Cornmesser [:markco] from comment #16)
> Created attachment 8653641 [details] [diff] [review]
> Bug1185909.patch

Oh, any reason the flag disappeared? I was about to get on this.

Comment 18

[Mark Cornmesser [:markco]]

I need to make some adjustments for AWS instances. Sorry for the early flag.

Comment 19

[Mark Cornmesser [:markco]]

Attachment: Bug1185909.patch

Comment 20

[Morgan Phillips [:mrrrgn]]

Comment on attachment 8655238 [details] [diff] [review]
Bug1185909.patch

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

<3 this !

::: modules/runner/manifests/settings.pp
@@ +14,5 @@
> +            include buildslave
> +            include buildslave::install
> +            include packages::mozilla::python27
> +
> +            $runner_path = "C:\\opt\\runner"

Mi gusta !

::: modules/runner/manifests/tasks/clobber.pp
@@ +15,5 @@
> +            }
> +            # Clobberer.py is needed before it is checkout for use by the clobber.bat
> +            exec {
> +                "get_clobberer.py" :
> +                    command => "C:\\mozilla-build\\wget\\wget.exe  hg.mozilla.org/build/tools/raw-file/tip/clobberer/clobberer.py",

Awesome idea !

Comment 21

[Mark Cornmesser [:markco]]

Comment on attachment 8655238 [details] [diff] [review]
Bug1185909.patch

 https://hg.mozilla.org/build/puppet/rev/147e1d4df78f

Comment 22

[Mark Cornmesser [:markco]]

Attachment: Bug1185909-2.patch

Correcting variables in task.pp and replacing bbpath.bat references. 

Callek: with the variables, is that what you mentioned this morning?

Comment 23

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

Comment on attachment 8658921 [details] [diff] [review]
Bug1185909-2.patch

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

:thumbsup:

Comment 24

[Mark Cornmesser [:markco]]

Comment on attachment 8658921 [details] [diff] [review]
Bug1185909-2.patch

https://hg.mozilla.org/build/puppet/rev/c81d540a57dc
https://hg.mozilla.org/build/puppet/rev/c156dace4805