1321513 - Add nagios checks for pushapk scriptworkers

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Johan Lorenzo [:jlorenzo]]

Nagios config files have been created on the nodes sides[1] (added in bug 1307826). Pushapkworker doesn't rely on the scriptworker shared module yet (introduced in bug 1316702). If I understand bug 1317783 comment 3 correctly, this may not happen immediately (the scriptworker module enables chain of trust AFAUI).

Therefore, I believe we can turn on nagios just like Aki did in bug 1295196

[1] https://hg.mozilla.org/build/puppet/file/8fbdd01cb464/modules/pushapkworker/templates/nagios.cfg.erb

Comment 1

[Johan Lorenzo [:jlorenzo]]

Things have changed since bug 1295196 comment 1, no more SVN repo is required. Releng folks have access to the git repos listed at [1] (and more precisely, the nagios part of it)

[1] https://mana.mozilla.org/wiki/display/SYSADMIN/Git

Comment 2

[Johan Lorenzo [:jlorenzo]]

Attachment: add_pushapk_scriptworker_in_nagios.diff

The patch is totally similar to bug 1295196. Before asking for a formal review to the folks in #moc, I have a question for you, Aki. I'm not sure I need to add the resource "pushapk-scriptworker-procs", because "scriptworker-procs" already exist. Should I create it anyway, or is it better to just add a hostgroup to "scriptworker-procs"?

Comment 3

[Aki Sasaki (not active)]

Comment on attachment 8816127 [details] [diff] [review]
add_pushapk_scriptworker_in_nagios.diff

The current idea is to have each scriptworker instance use scriptworker-procs, rather than have one *-procs check per scriptworker instance type, especially since I believe they'll look exactly the same other than the name.  If there isn't a strong reason to add another check, let's reuse the check.

Comment 4

[Johan Lorenzo [:jlorenzo]]

Attachment: add_pushapk_scriptworker_in_nagios_rev2.diff

Here's a new revision where pushapk-scriptworkers is now listed under "scriptworker-procs" instead of a newly created entry. Aki, do you have the rights to review and merge my patch? Or should I ask Amy?

Comment 5

[Johan Lorenzo [:jlorenzo]]

Comment on attachment 8823219 [details] [diff] [review]
add_pushapk_scriptworker_in_nagios_rev2.diff

diff --git a/modules/nagios/manifests/releng/hostgroups.pp b/modules/nagios/manifests/releng/hostgroups.pp
index 0986283d56..c2f4045603 100644
--- a/modules/nagios/manifests/releng/hostgroups.pp
+++ b/modules/nagios/manifests/releng/hostgroups.pp
@@ -126,6 +126,9 @@ class nagios::releng::hostgroups {
         'puppetagain-masters' => {
             alias => "puppetagain masters"
         },
+        'pushapk-scriptworkers' => {
+            alias => 'pushapk scriptworkers'
+        },
         'seamicro-admin' => {
             alias => "Seamicro Management server",
         },
diff --git a/modules/nagios/manifests/releng/scl3.pp b/modules/nagios/manifests/releng/scl3.pp
index 37798cdae6..f8a1fb31b0 100644
--- a/modules/nagios/manifests/releng/scl3.pp
+++ b/modules/nagios/manifests/releng/scl3.pp
@@ -1046,6 +1046,12 @@ class nagios::releng::scl3 {
                 'signing-scriptworkers'
             ]
         },
+        'pushapkworker-1.srv.releng.use1.mozilla.com' => {
+            contact_groups => 'build',
+            hostgroups => [
+                'pushapk-scriptworkers'
+            ]
+        },
         'buildbot-master103.bb.releng.scl3.mozilla.com' => {
             parents        => 'fw1.private.releng.scl3.mozilla.net',
             contact_groups => 'build',
diff --git a/modules/nagios/manifests/releng/services.pp b/modules/nagios/manifests/releng/services.pp
index 6c2f416685..2527712c98 100644
--- a/modules/nagios/manifests/releng/services.pp
+++ b/modules/nagios/manifests/releng/services.pp
@@ -64,6 +64,7 @@ class nagios::releng::services {
                     'pxe-servers',
                     'scl3-production-buildbot-masters',
                     'puppetagain-masters',
+                    'pushapk-scriptworkers',
                     'slaveapi',
                     'stage-servers',
                     'use1-production-buildbot-masters',
@@ -247,6 +248,7 @@ class nagios::releng::services {
                     'scl3-production-buildbot-masters',
                     'slaveapi',
                     'puppetagain-masters',
+                    'pushapk-scriptworkers',
                     'signing-servers',
                     'stage-servers',
                     'use1-production-buildbot-masters',
@@ -291,6 +293,7 @@ class nagios::releng::services {
                     'dev-buildbot-masters',
                     'mac-signing-servers',
                     'mac-partner-repack',
+                    'pushapk-scriptworkers',
                     'scl3-production-buildbot-masters',
                     'signing-servers',
                     'stage-servers',
@@ -318,6 +321,7 @@ class nagios::releng::services {
                     'dev-buildbot-masters',
                     'mac-signing-servers',
                     'mac-partner-repack',
+                    'pushapk-scriptworkers',
                     'scl3-production-buildbot-masters',
                     'signing-servers',
                     'stage-servers',
@@ -808,7 +812,8 @@ class nagios::releng::services {
             check_command => 'check_nrpe_procs_regex!/builds/scriptworker/bin/scriptworker!1!1',
             hostgroups => $::fqdn ? {
                 'nagios1.private.releng.scl3.mozilla.com' => [
-                    'signing-scriptworkers'
+                    'signing-scriptworkers',
+                    'pushapk-scriptworkers',
                 ],
                 default => [
                 ]
@@ -1007,6 +1012,7 @@ class nagios::releng::services {
                     'pxe-servers',
                     'scl3-production-buildbot-masters',
                     'puppetagain-masters',
+                    'pushapk-scriptworkers',
                     'slaveapi',
                     'use1-production-buildbot-masters',
                     'usw2-production-buildbot-masters',

Comment 6

[Johan Lorenzo [:jlorenzo]]

Attachment: add_pushapk_scriptworker_in_nagios_rev3.diff

Oops, I should've added --no-color when generating the diff.

Comment 7

[Johan Lorenzo [:jlorenzo]]

For the record, the patch has also been rebased on top of the latest master.

Comment 8

[Aki Sasaki (not active)]

Comment on attachment 8823221 [details] [diff] [review]
add_pushapk_scriptworker_in_nagios_rev3.diff

I think :arr is a better reviewer here.

Comment 9

[Amy Rich [:arr] [:arich]]

Comment on attachment 8823221 [details] [diff] [review]
add_pushapk_scriptworker_in_nagios_rev3.diff

This looks fine to me. I presume that /builds/scriptworker/bin/scriptworker already exists on the client and that the nagios config has been modified in releng puppet to include the nrpe check?

Comment 10

[Johan Lorenzo [:jlorenzo]]

The releng had been modified to support the nrpe check[1]. However, thanks for calling out `/builds/scriptworker`!

I noticed pushapk-scriptworker and signing-scriptworker don't share the same root[2]. Therefore, something needs to be changed:
a) either on the releng puppet; we make the root pushapk at /build/scriptworker
b) or on the current nagios config; we extend the regex to support other roots.

I see drawbacks in both scenarios: 
a) if you're logged on a machine, you may not know what type of scriptworker instance you have, unless you dig into its configuration
b) the regex might become a nightmare after several instances being added.

On the other had, having consistency across scriptworker instances is better, and will help setting up new ones. Moreover, if you're logged on a machine, there's great probability its FQDN refers to what type of scriptworker instance you're dealing with. Then, I'll change the puppet configuration.

[1] https://hg.mozilla.org/build/puppet/file/3e2076f7aedc/modules/pushapkworker/manifests/init.pp#l63
[2] Pushapk being defined as: https://hg.mozilla.org/build/puppet/file/3e2076f7aedc/manifests/moco-config.pp#l453
Signing-scriptworker as: https://hg.mozilla.org/build/puppet/file/3e2076f7aedc/modules/signing_scriptworker/manifests/settings.pp#l2

Comment 11

[Johan Lorenzo [:jlorenzo]]

Attachment: Bug 1321513 - Make pushapkworker live under /build/scriptworker

Review commit: https://reviewboard.mozilla.org/r/102242/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/102242/

Comment 12

[Aki Sasaki (not active)]

Comment on attachment 8823758 [details]
Bug 1321513 - Make pushapkworker live under /build/scriptworker

https://reviewboard.mozilla.org/r/102242/#review102620

At some point, we should get pushapkworker using modules/scriptworker, and we can make sure all workers using modules/scriptworker have a /builds/scriptworker root.

I think this works until that point.

Comment 13

[Johan Lorenzo [:jlorenzo]]

Comment on attachment 8823758 [details]
Bug 1321513 - Make pushapkworker live under /build/scriptworker

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/102242/diff/1-2/

Comment 14

[Johan Lorenzo [:jlorenzo]]

Even though the puppet run was passing yesterday, a nit slipped through the patch. I updated it.

The latest revision of that patch passed lint tests there: https://github.com/mozilla/build-puppet/pull/24
I tested the changes against dev-linux64-ec2-jlorenzo.dev.releng.use1.m.c:
* /build/pushapkworker doesn't exist anymore
* /build/scriptworker does and contains all the files
* `supervisorctl status pushapkworker` says the process is running
* /builds/scriptworker/logs/worker.log shows the process does what it needs to do (polling TC's queue)

I think we're good to merge and deploy attachment 8823758 [details] (which is necessary to land attachment 8823221 [details] [diff] [review]).

Comment 15

[Johan Lorenzo [:jlorenzo]]

Landed on default [1] and merged in production [2]. Puppet agent applied the changes restarted the process (which restarted properly)[3].

On the production machine, I see now /builds/scriptworker/ but not /build/pushapkworker/ anymore. This complies with [4].

[1] https://hg.mozilla.org/build/puppet/rev/1a429ca163d9
[2] https://hg.mozilla.org/build/puppet/rev/b4d2f4f777e4
[3] https://papertrailapp.com/systems/526654063/events?focus=753674615796166664
[4] https://papertrailapp.com/events?q=program%3Apuppet-agent&focus=753673798523449394

Comment 16

[Johan Lorenzo [:jlorenzo]]

Comment on attachment 8823221 [details] [diff] [review]
add_pushapk_scriptworker_in_nagios_rev3.diff

This patch was landed as revision 568ad3460d032bf498f7bf0e9239938244f64784 on master in the git repo (linked in comment 1).

Comment 17

[Johan Lorenzo [:jlorenzo]]

I connected to the releng nagios instance[1] and looked for pushapkwoker. The entry there shows:
* 7 checks (PING + the 6 new checks added with attachment 8823221 [details] [diff] [review]).
* /builds/scriptworker/bin/scriptworker is declared as running. I can tell it's also working, because this job[2] has just finished and it ran fine.
* Every check is green.

Looks like we're all set. Time to close this bug! Thanks for the help Ami and Aki!

[1] Listed at https://mana.mozilla.org/wiki/display/SYSADMIN/Nagios#Nagios-Hardware 
[2] https://tools.taskcluster.net/task-inspector/#Tmlx52SQT9uwVhi9oEqagg/

Comment 18

[Johan Lorenzo [:jlorenzo]]

(In reply to Aki Sasaki [:aki] from comment #12)
> At some point, we should get pushapkworker using modules/scriptworker, and
> we can make sure all workers using modules/scriptworker have a
> /builds/scriptworker root.

I agree. I looked more into using this module. I get the feeling I need a greater version than 0.7.1. That is to say, Chain of Trust has to be enabled. Hence, I guess the module can be added alongside bug 1317783.