Page MenuHomePhabricator

Add --no-autoloader_layout-check to operations-puppet-puppetlint-lenient
Closed, DeclinedPublic

Description

in jenkins job/operations-puppet-puppetlint-lenient is it possible to add to puppet-lint:

--no-autoloader_layout-check and skip this one check

but only if file is under ./manifests/role/ and still let it run if it is under ./modules/ ?

reason is that in role class we will never be in autoload layout and always fail with a lot of "not in autoload module layout (autoloader_layout)" while the check DOES make sense for anything in actual module structure under ./modules/

references:

https://integration.wikimedia.org/ci/job/operations-puppet-puppetlint-lenient/10629/console

http://puppet-lint.com/checks/autoloader_layout/

Details

Reference
bz73117

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 3:56 AM
bzimport set Reference to bz73117.
bzimport added a subscriber: Unknown Object (MLST).

In the operations/puppet.git repository, you should be able to pass the options to puppet-lint by adding them in /.puppet-lint.rc

There is only one there:

$ cat .puppet-lint.rc 
--no-class_parameter_defaults-check
$

We can not set the parameters on different path, though we could have two different puppet-lint run.

Would it make sense to migrate role class to autloading layout as well?

also see T1289 . it might be a duplicate. i don't think T1289 should be resolved as originally requested though. let's not remove the entire check please.

Dzahn added a subscriber: mmodell.
Krinkle removed a subscriber: Unknown Object (MLST).
Krinkle removed a subscriber: Krinkle.
Krinkle renamed this task from jenkins - operations-puppet-puppetlint-lenient - --no-autoloader_layout-check to Add --no-autoloader_layout-check to operations-puppet-puppetlint-lenient.Jan 8 2015, 12:22 PM
Krinkle updated the task description. (Show Details)

This is up to SRE to adjust the /.puppet-lint.rc file in operations/puppet.git.

The lint check is currently disabled in /.puppet-lint.rc

I don't think it can be ignored for a specific file hierarchy. So one would need to reenable it then add ignore rules in the files. Something like:

# lint:ignore:single_quote_string_with_variables
'${name}',
# lint:endignore

An example is: https://gerrit.wikimedia.org/r/#/c/188805/2/manifests/role/gerrit.pp,unified

It is up to SRE / puppet volunteers to figure it out and propose a patchset. CI just runs puppet-lint :-]

That is done in the repository puppet-lint configuration file:

/.puppet-lint.rc
--no-autoloader_layout-check

When i wrote this task i assumed we'd never have the role classes in autoload layout, but now we do, since manifests/role/ moved to modules/role/manifests/.

..but we still can't remove this exception mostly because mariadb, openstack/nova and eventlogging have multiple classes in a single file and would have to be split up properly.

which i tried for example here but there was resistance to doing that (https://gerrit.wikimedia.org/r/#/c/315343/)

Yeah I noticed that. Then T119042 was to migrate everything out of manifests/role so I guess it is now easy to make all the modules to respect the autoloader layout. That would be T93645

There is only 51 warnings right now:

$ bundle exec rake puppetlint|wc -l
51

lot related to modules/role/manifests/mariadb.pp which IIRC is going to be removed.

Yes, but my last comment was about how it's unfortunately not easy even though that migration out of manifests/role happened, i could not get those things merged.

Yes, that is exactly what i said, pretty much all of that depends on mariadb, the other 2 are openstack and eventlogging. i once had patches for all of them...

I can refactor the eventlogging stuff if you need

Thanks Ottomata, that would be great.