Page MenuHomePhabricator
Create Task
Maniphest T87132

Make Puppet repository pass lenient and strict lint checks
Closed, ResolvedPublic
Actions

Description

For all changes in the Puppet repository, the lint checks operations-puppet-puppetlint-lenient and puppetlint-strict are run.

Those fail all the time, thus a) they are being ignored for the most part and b) they require developers and reviewers who are interested in them to go to the details page and search for the names of the files that a change touched to see if (new) warnings were added.

The repository should be made compliant with the lint checks or, where those are too severe or, when importing external modules, comparability with upstream is more important than linting, they should be disabled for those cases either in the repository (preferred) or in the Jenkins job definition.

Details

Show related patches Customize query in gerrit

Related Objects
Search...

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Aklapper added a project: ops-requests.Jan 18 2015, 6:31 PM
Dzahn added a project: acl*sre-team.Jan 28 2015, 6:18 PM
Dzahn set Security to None.
Dzahn subscribed.Jan 28 2015, 6:21 PM

The repository should be made compliant with the lint checks ..

Yes. that. It's just an ungoing and very long task. We have been making tons of lint changes over the last couple years to get closer to that goal. The purpose is to be able to finally pass those checks and make them vote in the glorious future. We are just not there yet. I have stats though how much better we got when it comes to ratio between code lines and warnings. So don't give up on this, don't remove them, it just takes time.

gerritbot added a project: Patch-For-Review.Feb 3 2015, 2:34 PM
gerritbot subscribed.

Change 188206 had a related patch set uploaded (by Hashar):
fix another 28 puppet linter warnings

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

Patch-For-Review

hashar subscribed.Feb 3 2015, 2:34 PM
hashar mentioned this in T88430: Migrate operations/puppet.git to use a recent version of puppet-lint from rubygems.Feb 3 2015, 2:39 PM

I filled a task to propose a bump of puppet-lint from 0.3.2 (as provided by debian packages) to 1.1.0 (provided by latest ruby gem): T88430.

gerritbot added a comment.Feb 3 2015, 5:52 PM

Change 188206 merged by Alexandros Kosiaris:
fix another 28 puppet linter warnings

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

Dzahn removed a project: ops-requests.Feb 3 2015, 7:08 PM
hashar mentioned this in rOPUP3daa9f7a8ae6: fix another 28 puppet linter warnings.Feb 3 2015, 9:34 PM
faidon closed subtask T88430: Migrate operations/puppet.git to use a recent version of puppet-lint from rubygems as Resolved.Feb 5 2015, 10:08 AM
hashar added a comment.Feb 5 2015, 12:11 PM

Faidon has backported puppet-lint 1.1.0 from Ubuntu Vivid in our apt for both Precise and Trusty. I am upgrading puppet-lint on the CI slaves.

gerritbot added a comment.Feb 5 2015, 1:29 PM

Change 188375 had a related patch set uploaded (by Hashar):
Move puppet-lint options to .puppet-lint.rc

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

Patch-For-Review

gerritbot added a comment.Feb 5 2015, 1:29 PM

Change 188805 had a related patch set uploaded (by Hashar):
puppet-lint: ignore some var in single quoted strings

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

Patch-For-Review

hashar added a comment.Feb 5 2015, 1:33 PM

https://gerrit.wikimedia.org/r/188375 brings it a bunch of ignore we had in the rakefile and expose them in .puppet-lint.rc. That let us discards a lot of warnings and errors.

https://gerrit.wikimedia.org/r/188805 fix the five remaining false positive errors and make the job operations-puppet-puppetlint-lenient to pass for the first time ever (i.e. no error reported).

Andrew subscribed.Feb 5 2015, 9:04 PM

Thank you for waiting on a proper .deb package :) I've approved those patches but will wait until Antoine is back @ work before merging.

hashar added a comment.Feb 6 2015, 12:18 AM

@Andrew All CI slaves are now using puppet-lint 1.1.0 :-] So unless my patches have a weird side effect on prod they are good to land :-]

Andrew claimed this task.Feb 8 2015, 9:50 PM
Andrew triaged this task as Medium priority.
gerritbot added a comment.Feb 9 2015, 8:07 PM

Change 188375 merged by Andrew Bogott:
Move puppet-lint options to .puppet-lint.rc

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

hashar mentioned this in rOPUPccbfd6d07410: Move puppet-lint options to .puppet-lint.rc.Feb 9 2015, 8:07 PM
gerritbot added a comment.Feb 9 2015, 8:09 PM

Change 188805 merged by Andrew Bogott:
puppet-lint: ignore some var in single quoted strings

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

Andrew mentioned this in rOPUPb319e5ca2ad7: puppet-lint: ignore some var in single quoted strings.Feb 9 2015, 8:09 PM
hashar added a comment.EditedFeb 9 2015, 9:02 PM

With the operations/puppet.git patches merged above, puppet-lint v1.1.0 no more reports any error. Thus, the Jenkins job operations-puppet-puppetlint-lenient is passing!

It is up to SRE team to now have the Job result to vote -1 on error. Can be done by removing the following lines from integration/config.git zuul/layout.yaml

- name: operations-puppet-puppetlint-lenient
  voting: false

Then +2 and deploy the Zuul configuration change.

I have pointed the ops list to this comment.

scfc added a comment.Feb 9 2015, 9:30 PM

Nicely done, merci!

If Jenkins would vote -1, would that reject merges? I. e., if in an emergency SRE needs to merge a change even if it doesn't lint, would they be able to do that?

hashar added a comment.Feb 9 2015, 9:34 PM
In T87132#1026067, @scfc wrote:

Nicely done, merci!

If Jenkins would vote -1, would that reject merges? I. e., if in an emergency SRE needs to merge a change even if it doesn't lint, would they be able to do that?

Yes they would be able. Just have to drop jenkins-bot from the list of reviewers, vote Verified +2 and hit "submit patch". Additionally on jenkins-bot never merge patches on ops/puppet , it is always a human action.

Matanya subscribed.Feb 9 2015, 9:35 PM
Matanya added a comment.Feb 9 2015, 9:40 PM
This comment was removed by Matanya.
gerritbot added a comment.Feb 9 2015, 9:49 PM

Change 189589 had a related patch set uploaded (by Matanya):
zuul: Make jenkins vote on patches not passing lint check

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

Patch-For-Review

gerritbot added a comment.Feb 9 2015, 9:54 PM

Change 189589 had a related patch set uploaded (by Krinkle):
zuul: Make operations-puppet-puppetlint-lenient voting

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

Patch-For-Review

gerritbot added a comment.Feb 12 2015, 8:25 AM

Change 189589 merged by jenkins-bot:
zuul: Make operations-puppet-puppetlint-lenient voting

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

hashar mentioned this in rCICFcb2844b92acd: zuul: Make operations-puppet-puppetlint-lenient voting.Feb 12 2015, 8:27 AM
hashar added a comment.Feb 12 2015, 8:27 AM

The Jenkins job (puppet-lint lenient) now complains whenever a puppet-lint error is detected. Huge thanks to everyone that made this possible.

Still have to fix up or ignore the warnings though so leaving this task open.

Andrew reassigned this task from Andrew to hashar.Feb 22 2015, 2:14 AM
gerritbot added a comment.Mar 18 2015, 9:25 PM

Change 197759 had a related patch set uploaded (by Tim Landscheidt):
WIP: Mute warnings about case statements without default matches

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

gerritbot added a comment.Mar 19 2015, 10:25 PM

Change 198116 had a related patch set uploaded (by Tim Landscheidt):
Ignore warnings about URLs without modules for private repository

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

gerritbot added a comment.Mar 20 2015, 12:52 AM

Change 198170 had a related patch set uploaded (by Tim Landscheidt):
Disable class_inherits_from_params_class puppet-lint checks

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

hashar merged a task: T93642: fix failures of jenkins job operations-puppet-puppetlint-strict.Mar 23 2015, 10:16 PM
hashar mentioned this in T93645: align puppet-lint config with coding style.
hashar added subscribers: Krenair, JanZerebecki.
Dzahn added a comment.Mar 23 2015, 10:18 PM

we fixed all remaining tab errors and enabled checks for it

https://gerrit.wikimedia.org/r/#/c/198814/

gerritbot added a comment.Apr 3 2015, 12:30 AM

Change 197759 merged by Dzahn:
Ignore some warnings about case statements without default matches

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

Dzahn mentioned this in rOPUP48f6d3e7c3b4: Ignore some warnings about case statements without default matches.Apr 3 2015, 12:30 AM
gerritbot added a comment.Apr 3 2015, 9:37 PM

Change 198170 merged by Dzahn:
Disable class_inherits_from_params_class puppet-lint checks

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

Dzahn mentioned this in rOPUP8da9def4893d: Disable class_inherits_from_params_class puppet-lint checks.Apr 3 2015, 9:37 PM
gerritbot added a comment.Apr 4 2015, 2:05 AM

Change 201884 had a related patch set uploaded (by Tim Landscheidt):
apache: Mute warnings about right-to-left relationships

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

hashar removed hashar as the assignee of this task.Apr 15 2015, 12:51 PM
gerritbot added a comment.Apr 24 2015, 12:24 AM

Change 201884 merged by Dzahn:
apache: Mute warnings about right-to-left relationships

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

scfc mentioned this in rOPUPab0887d9f680: apache: Mute warnings about right-to-left relationships.Apr 24 2015, 12:25 AM
fgiunchedi subscribed.Jul 20 2015, 11:02 AM

current status: puppet lenient is voting, strict isn't yet. Is it worth to chase strict warnings too?

Matanya added a comment.Jul 21 2015, 10:00 AM

from my POV: yes.

scfc added a comment.Jul 21 2015, 5:14 PM

I think as well that strict lint checks should be enforced, and as https://integration.wikimedia.org/ci/job/operations-puppet-puppetlint-strict/24810/console shows the only major issue left is "puppet:// URL without modules/ found (puppet_url_without_modules)" (cf. https://gerrit.wikimedia.org/r/#/c/198116/ for a change (to be rebased) to fix that; another approach would be to ignore the warning globally).

gerritbot added a comment.Jul 23 2015, 1:37 AM

Change 226459 had a related patch set uploaded (by Tim Landscheidt):
cassandra: Fix strict puppet-lint warnings

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

gerritbot added a comment.Jul 23 2015, 2:02 AM

Change 226463 had a related patch set uploaded (by Tim Landscheidt):
statsdlb: Fix strict puppet-lint check

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

gerritbot added a comment.Jul 23 2015, 11:21 PM

Change 226459 merged by Yuvipanda:
cassandra: Fix strict puppet-lint warnings

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

yuvipanda mentioned this in rOPUP67ecf8361033: cassandra: Fix strict puppet-lint warnings.Jul 23 2015, 11:21 PM
scfc closed subtask T106768: Test that/which self-hosted puppet masters resemble production as Declined.Jul 24 2015, 12:44 AM
gerritbot added a comment.Jul 29 2015, 6:13 PM

Change 198116 merged by Dzahn:
Ignore warnings about URLs without modules for private repository

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

hashar removed a project: Patch-For-Review.Jul 29 2015, 7:47 PM

From a recent puppet lint strict run, seems the left over is migrating legacy manifests to puppet modules.

Dzahn mentioned this in rOPUP18b747e8a108: Ignore warnings about URLs without modules for private repository.Jul 30 2015, 10:16 PM
gerritbot added a comment.Aug 2 2015, 7:27 PM

Change 228682 had a related patch set uploaded (by Tim Landscheidt):
Ignore warnings about URLs without modules for volatile directory

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

gerritbot added a project: Patch-For-Review.Aug 2 2015, 7:27 PM
gerritbot added a comment.Aug 2 2015, 7:52 PM

Change 228712 had a related patch set uploaded (by Tim Landscheidt):
haproxy: Move check_haproxy to module itself

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

gerritbot added a comment.Aug 21 2015, 10:50 PM

Change 233073 had a related patch set uploaded (by Tim Landscheidt):
cassandra: Mute strict puppet-lint warnings

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

gerritbot added a comment.Aug 24 2015, 8:32 AM

Change 233073 merged by Alexandros Kosiaris:
cassandra: Mute strict puppet-lint warnings

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

akosiaris mentioned this in rOPUPc9798c74d1fb: cassandra: Mute strict puppet-lint warnings.Aug 24 2015, 8:36 AM
gerritbot added a comment.Sep 23 2015, 9:13 PM

Change 228712 merged by Yuvipanda:
haproxy: Move check_haproxy to module itself

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

yuvipanda mentioned this in rOPUP8fe2025b0c0f: haproxy: Move check_haproxy to module itself.Sep 23 2015, 9:14 PM
gerritbot added a comment.Sep 23 2015, 10:03 PM

Change 228682 merged by Dzahn:
Ignore warnings about URLs without modules for volatile directory

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

Dzahn mentioned this in rOPUP4bfb54a224f5: Ignore warnings about URLs without modules for volatile directory.Sep 23 2015, 10:03 PM
gerritbot added a comment.Sep 23 2015, 10:10 PM

Change 226463 merged by Dzahn:
statsdlb: Fix strict puppet-lint check

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

Dzahn mentioned this in rOPUP64834910daf4: statsdlb: Fix strict puppet-lint check.Sep 23 2015, 10:11 PM
Andrew added a comment.Oct 2 2015, 2:45 PM

I would like us to just disable puppet_url_without_modules-check for now, and then make the strict test voting. That will lock in all the lint fixes that we've made heretofore.

If that happens we can create a new task about turning that test back on and fixing violations... if we care :)

gerritbot added a comment.Oct 2 2015, 2:45 PM

Change 243177 had a related patch set uploaded (by Andrew Bogott):
puppet-lint: Turn on --no-puppet_url_without_modules-check

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

hashar added a comment.Oct 2 2015, 2:48 PM
In T87132#1697034, @Andrew wrote:

I would like us to just disable puppet_url_without_modules-check for now, and then make the strict test voting. That will lock in all the lint fixes that we've made heretofore.

If that happens we can create a new task about turning that test back on and fixing violations... if we care :)

Sounds good to me

gerritbot added a comment.Oct 2 2015, 2:50 PM

Change 243185 had a related patch set uploaded (by Hashar):
operations/puppet now has strict puppet-lint

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

gerritbot added a comment.Oct 2 2015, 3:00 PM

Change 243177 merged by Andrew Bogott:
puppet-lint: Turn on --no-puppet_url_without_modules-check

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

Andrew mentioned this in rOPUPc41b180660e9: puppet-lint: Turn on --no-puppet_url_without_modules-check.Oct 2 2015, 3:00 PM
gerritbot added a comment.Oct 5 2015, 2:38 PM

Change 243185 merged by jenkins-bot:
operations/puppet now has strict puppet-lint

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

hashar mentioned this in rCICFd944bbbd40c0: operations/puppet now has strict puppet-lint.Oct 5 2015, 2:38 PM
hashar added a comment.Oct 5 2015, 3:34 PM

CI is now voting on puppet-lint error/warnings. Seems all the repository has been cleaned up :-}

gerritbot added a comment.Oct 5 2015, 8:46 PM

Change 243803 had a related patch set uploaded (by Dzahn):
lint: re-enable 'variable not enclosed' check

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

gerritbot added a comment.Oct 5 2015, 9:42 PM

Change 243803 merged by Dzahn:
lint: re-enable 'variable not enclosed' check

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

Dzahn mentioned this in rOPUP495910a061b1: lint: re-enable 'variable not enclosed' check.Oct 5 2015, 9:43 PM
• chasemp subscribed.Oct 21 2015, 11:32 PM

it seems like this is working now...but the blocked by tasks are not closed?

scfc removed subtasks: T107672: Move misc::maintenance into a module, T107671: Move misc::udp2log into a module, T107670: Move role::otrs into a module.Oct 22 2015, 12:10 AM
• chasemp closed this task as Resolved.Oct 22 2015, 12:11 AM
• chasemp claimed this task.

well played @scfc

scfc added a comment.Oct 22 2015, 12:13 AM

(Prior to c41b180660e96270e9d90cd66634caa90e0e7029, --no-puppet_url_without_modules-check was not enabled, so the blocking tasks made sense then. I have removed them as blockers; they are still valid in their own right.)

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits