Page MenuHomePhabricator

Allow use of wmf's MW CLI scripts on snapshot hosts instead of bypassing
Open, LowPublic

Description

The multiversion/bin/expanddblist CL script exists in the operations/mediawiki-config.git repository for local development. It is an equivalent to the expanddblist script provisioned by Puppet in production on deployment and mwmaint hosts.

I noticed that recently, a usage showed up in production code. Specifically in dump-growth-mentorship.sh as added for T291966 by https://gerrit.wikimedia.org/r/c/operations/puppet/+/740371 (cc @Urbanecm @Tgr @ArielGlenn).

After a brief moment in which I thought this could "simply" be replaced with invoking expanddblist, instead of $multiversion/bin/expanddblist, I realized this was a workaround for a larger problem. The script is also using $multiversion/MWScript.php directly instead of the mwscript helper that we use elsewhere in production.

Proposal

  • Rename scap::scripts in Puppet to mediawiki::cli-helpers. There is basically nothing Scap-related left here. Some scripts are indeed used by Scap, but most are also used by a ton of other things independent from Scap.
  • Remove the various "FIXME" comments atop this manifest that have been resolved.
  • Add mediawiki::cli-helpers to profile::mediawiki::common.
  • Remove redundant other imports of this in various places.
  • Update dump-growth-mentorship.sh to use the prod expanddblist, thus restoring the status quo.
  • Update dump-growth-mentorship.sh as pilot and leading by example to use mwscript. (Others can be done later based on available testing, no rush there.)

See also:

Gotcha:

One of the helpers, sql apparently only works on maintenance and deployment hosts because the mysql client command isn't provisioned elsewhere. I suggest we either provision that package (is there a reason not to?) or move that into a separate manifest like mediawiki::maintenance::sql that provisions both that script and the needed package, instead of the current fragile situation where we sometimes set sql_scripts => absent and then separately in an entirely different file ensure_packages('default-mysql-client').

Note that snapshot::dumps::packages at least already contains this package. So the question is whether we install it on appservers/jobrunners. I don't think we need it there since sql does the same thing no matter where you run it. That's I guess a reason to place it in a separate manifest as otherwise it's hard to include the rest in profile::mediawiki::common.

Event Timeline

Why do we even have a separate expanddblist and $multiversion/bin/expanddblist with basically identical content? It will in the end execute code in the multiversion repo anyway, so the security footprint is the same. Why isn't expanddblist just an alias/redirect to the other?

Why do we even have a separate expanddblist and $multiversion/bin/expanddblist with basically identical content? It will in the end execute code in the multiversion repo anyway, so the security footprint is the same. Why isn't expanddblist just an alias/redirect to the other?

The Puppet-provided script (originally part of the "scap" module that included sync-common-all etc) came first, and those were all pure bash scripts.

There wasn't one in the wmf-config repo until I created one a while back to aid in local development.

I agree a good next step after this task would be to de-duplicate the scripts that naturally have such runtime dependency already, and indeed puppetize them merely as symlinks to files in /srv/mediawiki/bin/. I do note that there has historically been a resistence to Puppet having knowledge of what files may or may not exist in the Git clone at /srv/mediawiki as relying on what files exist there is deemed fragile but virtue of not being Puppet's own responsibility. However, at least for the scripts that explicitly depend at runtime on files in that same directory already, this is a distinction without a difference as the dependency is there either way, just less obvious.