Closed Bug 1316702 Opened 3 years ago Closed 3 years ago

refactor scriptworker puppet to allow for shared scriptworker module

Categories

(Release Engineering :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: aki, Assigned: aki)

References

Details

Attachments

(2 files, 2 obsolete files)

Attached patch one_scriptworker.diff (obsolete) — Splinter Review
This also installs the latest scriptworker+signingscript, as featured on date.
I'm going to have to also change the nagios configs.
Attachment #8809560 - Flags: review?(bugspam.Callek)
Attached patch nagios.diffSplinter Review
I think this will allow us to add the same alert to other scriptworker instances more easily.
Attachment #8809574 - Flags: review?(arich)
Comment on attachment 8809574 [details] [diff] [review]
nagios.diff

I' assuming the regex for the proc check stays the same.
Attachment #8809574 - Flags: review?(arich) → review+
Attached patch puppet2.diff (obsolete) — Splinter Review
This is the same patch, but I bump the signing scriptworker timeout to 30min to avoid the `date` blue exception retries.
Attachment #8809560 - Attachment is obsolete: true
Attachment #8809560 - Flags: review?(bugspam.Callek)
Attachment #8809614 - Flags: review?(bugspam.Callek)
Attached patch puppet3.diffSplinter Review
same patch, but removes the old nagios config.
Attachment #8809614 - Attachment is obsolete: true
Attachment #8809614 - Flags: review?(bugspam.Callek)
Attachment #8809616 - Flags: review?(bugspam.Callek)
Comment on attachment 8809616 [details] [diff] [review]
puppet3.diff

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

Looks good! A few nits/comments though.

I didn't fine-tooth-comb this review, (e.g. for lint issues, or for referenced-but-missing vars, typos, etc), I didn't notice anything but just being explicit on how thorough I was.

::: modules/scriptworker/manifests/instance.pp
@@ +87,5 @@
> +        'create_gpg_homedirs':
> +            require => [Python35::Virtualenv["${basedir}"],
> +                        Git::Repo["scriptworker-${git_key_repo_dir}"],
> +                        File["${basedir}/scriptworker.yaml"]],
> +            command => "${basedir}/bin/create_initial_gpg_homedirs ${basedir}/scriptworker.yaml",            subscribe => File["${git_pubkey_dir}"],

nit "subscribe" on a newline

::: modules/scriptworker/templates/scriptworker.yaml.erb
@@ +55,5 @@
> +        - sha256:31035ed23eba3ede02b988be39027668d965b9fc45b74b932b2338a4e7936cf9
> +        - sha256:7320c720c770e9f93df26f7da742db72b334b7ded77539fb240fc4a28363de5a
> +        - sha256:9db282317340838f0015335d74ed56c4ee0dbad588be33e6999928a181548587
> +    docker-image:
> +        - sha256:74c5a18ce1768605ce9b1b5f009abac1ff11b55a007e2d03733cd6e95847c747

This is an odd place/way to hardcode this allowlist, but I won't block this main deployment on it.

::: modules/signing_scriptworker/manifests/init.pp
@@ +42,2 @@
>                    "requests==2.11.1",
> +                  "scriptworker==0.10.0a6",

iirc you already tagged 0.10.0a7, but you already have stamp=me for version bumps here.
Attachment #8809616 - Flags: review?(bugspam.Callek) → review+
(In reply to Justin Wood (:Callek) from comment #5)
> ::: modules/scriptworker/templates/scriptworker.yaml.erb
> @@ +55,5 @@
> > +        - sha256:31035ed23eba3ede02b988be39027668d965b9fc45b74b932b2338a4e7936cf9
> > +        - sha256:7320c720c770e9f93df26f7da742db72b334b7ded77539fb240fc4a28363de5a
> > +        - sha256:9db282317340838f0015335d74ed56c4ee0dbad588be33e6999928a181548587
> > +    docker-image:
> > +        - sha256:74c5a18ce1768605ce9b1b5f009abac1ff11b55a007e2d03733cd6e95847c747
> 
> This is an odd place/way to hardcode this allowlist, but I won't block this
> main deployment on it.

This allows us to update the allowlist out-of-band of a new scriptworker release, if and when it becomes necessary.
https://hg.mozilla.org/build/puppet/rev/0c36a8bb13908559078d5f5487db6c94ebf01eea
bug 1316702 - refactor scriptworker puppet to allow for shared scriptworker module. r=callek
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
See Also: → 1321513
Component: General Automation → General
You need to log in before you can comment on or make changes to this bug.