Page MenuHomePhabricator

Puppet failure on phabricator-prod-1001.devtools.eqiad1.wikimedia.cloud
Closed, ResolvedPublic

Description

phabricator-prod-1001.devtools.eqiad1.wikimedia.cloud fails puppet:

The last Puppet run was at Thu Oct  7 10:59:14 UTC 2021

Running it locally yields:

Error: Could not retrieve catalog from remote server: Error 500 on SERVER:
Server Error:Evaluation Error: Error while evaluating a Resource Statement, Evaluation Error:

Error while evaluating a Function Call, Duplicate declaration:

  Package[mariadb-client] is already declared at (file: /etc/puppet/modules/mariadb/manifests/packages.pp, line: 6);
  cannot redeclare
    (file: /etc/puppet/modules/phabricator/manifests/logmail.pp, line: 58)
    (file: /etc/puppet/modules/phabricator/manifests/logmail.pp, line: 58, column: 5)
    (file: /etc/puppet/modules/profile/manifests/phabricator/main.pp, line: 509)

  on node phabricator-prod-1001.devtools.eqiad1.wikimedia.cloud

modules/mariadb/manifests/packages.pp hasn't been touched in a while and has:

class mariadb::packages { 

    package { [
        'mariadb-client',
        'mariadb-server',
        'percona-toolkit',
        # 'percona-xtrabackup',
    ]:
        ensure => present,
    }
}

While modules/phabricator/manifests/logmail.pp uses ensure_packages(['mariadb-client']) which would work even in case of a duplicate.

So I guess something changed in the order the classes are loaded, phabricator::logmail happening first, then when mariadb::packages is applied it fails due to the package already having been defined.

On WMCS the phabricator role uses profile::mariadb::generic_server which comes after the Phabricator profiles and the client has ordering = manifest. I can't find the root cause for the breakage but maybe it was previously broken?

Event Timeline

The sole lead I have would be change in ensure_packages behavior. stdlib has been updated roughly at the same time from 7.0.1 to 8.1.0. Done by https://gerrit.wikimedia.org/r/c/operations/puppet/+/726872 on Oct 7th at 11:10 UTC.

Maybe because there is a mismatch between present and installed states? https://gerrit.wikimedia.org/r/c/operations/puppet/+/726872/7/modules/stdlib/lib/puppet/parser/functions/ensure_packages.rb

+ @jbond and @dcaro they surely know more about puppet internal and made some follow up change adding ensure_packages at various places. Potentially related upstream code:

Maybe the packages in mariadb::packages should instead be ensured installed or we pass to ensure_packages ensure => present instead of the new default of installed.

The issue (I think) is that the ensure_packages is running before the package declaration, and thus creates a new package resource, then the package declaration (in packages.pp) comes after and fails.

The fix would be to change the package declaration in the packages.pp to ensure_packages too, that way it does not matter which one runs first, and given that it does nothing else than just declare the packages, there's no downside (if it had to add extra parameters or pin versions then it would be different).

I'll try to cook a patch, one sec

Change 731348 had a related patch set uploaded (by David Caro; author: David Caro):

[operations/puppet@production] mariadb::packages: use ensure_packages

https://gerrit.wikimedia.org/r/731348

Change 731348 merged by David Caro:

[operations/puppet@production] mariadb::packages: use ensure_packages

https://gerrit.wikimedia.org/r/731348

Yep, that made the trick! \o/

dcaro@phabricator-prod-1001:~$ sudo run-puppet-agent
Info: Using configured environment 'production'
Info: Retrieving pluginfacts
Info: Retrieving plugin
Info: Retrieving locales
Info: Loading facts
Info: Caching catalog for phabricator-prod-1001.devtools.eqiad1.wikimedia.cloud
Info: Applying configuration version '(1cb5ece067) David Caro - mariadb::packages: use ensure_packages'
Notice: /Stage[main]/Profile::Base::Certificates/Package[wmf-certificates]/ensure: created
...
Notice: Applied catalog in 17.16 seconds
mmodell assigned this task to dcaro.

Thanks for the fix!