Page MenuHomePhabricator
Create Task
Maniphest T72068

Jenkins: Re-enable lint checks for Apache config in operations-puppet
Closed, ResolvedPublic
Actions

Description

In the past Apache configs were in the apache-config repo.

When they were there and since Tim Starling wrote the generator for redirects you had to edit redirects.dat, then generate redirects.conf from that and upload both to Gerrit. Then jenkins would check if they matched and vote on that.

Then, the Apache configs have been moved into the mediawiki module (which i'm not entirely sure i support because many redirects have nothing to do with mediawiki, but that's not the point here). The point of this bug is that now f.e. on https://gerrit.wikimedia.org/r/#/c/149890/ you can see how it's possible to change just the .dat file, not upload the .conf file, but jenkins doesn't say anything about it, where it should vote it down. That is a regression compared to for example https://gerrit.wikimedia.org/r/#/c/146334/.

Can we have the Apache lint checks back even though now the configs are in the Mediawiki module?

Version: unspecified
Severity: major

Details

Reference
bz70068
SubjectRepoBranchLines +/-
Remove operations-apache-config-lintintegration/configmaster+0 -54
mediawiki: Add test to verify redirects.conf has been regenerated from redirects.datoperations/puppetproduction+39 -1
Disable operations-apache-config-lintintegration/configmaster+1 -4
Customize query in gerrit

Related Objects
Search...

Event Timeline

• bzimport raised the priority of this task from to High.Nov 22 2014, 3:29 AM
• bzimport added a project: Continuous-Integration-Infrastructure.
• bzimport set Reference to bz70068.
Dzahn created this task.Aug 26 2014, 11:34 PM
Dzahn added a comment.Aug 26 2014, 11:36 PM

also i'm going to note that now a change as the one above turns into "puppet merge" & "apache deployment" at the same time if merged both has to be done, and this could have broken all the redirects

greg added a comment.Sep 24 2014, 9:07 PM

Do we know when this behavior changed?

hashar added a comment.Sep 26 2014, 9:01 PM

I am just noticing this bug :-/ The apache configuration got moved from the standalone repository operations/apache-config.git to puppet.

The Jenkins job need to be ported as well.

gerritbot added a comment.Sep 30 2014, 8:44 AM

Change 163812 had a related patch set uploaded by Hashar:
Reenable Apache lint check

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

gerritbot added a comment.Sep 30 2014, 8:47 AM

Change 163813 had a related patch set uploaded by Hashar:
Retrigger operations-apache-config-lint (non voting)

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

gerritbot added a comment.Sep 30 2014, 8:49 AM

Change 163813 merged by jenkins-bot:
Retrigger operations-apache-config-lint (non voting)

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

hashar added a comment.Sep 30 2014, 8:52 AM

I have adjusted the job to clone both operations/puppet.git and operations/mediawiki-config.git using Zuul cloner: https://gerrit.wikimedia.org/r/#/c/163812/ . It is deployed though there is a mass amount of sed scripts which I am not fan of.

The job pass, but I am not sure whether it catch all potentials issues that might occurs in Apache configuration files. We need to propose patches to verify the job is failing properly.

I have update Zuul configuration ( https://gerrit.wikimedia.org/r/#/c/163813/ ) to trigger the job on both puppet and mediawiki-config repositories.

hashar added a comment.Sep 30 2014, 9:00 AM

I have proposed two test changes to play with, both have a build passing when doing a noop change:

operations/puppet:
https://gerrit.wikimedia.org/r/#/c/163814/
https://integration.wikimedia.org/ci/job/operations-apache-config-lint/569/console

operations/mediawiki-config:
https://gerrit.wikimedia.org/r/#/c/163815/
https://integration.wikimedia.org/ci/job/operations-apache-config-lint/570/console

The patchsets can be reused to propose bad configurations and verify the job are failing properly. Whenever that is known to work, we can make them voting and hence vote Verified -2 in Gerrit.

gerritbot added a comment.Oct 10 2014, 1:43 PM

Change 166033 had a related patch set uploaded by Hashar:
Reenable Apache lint check

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

gerritbot added a comment.Oct 10 2014, 1:44 PM

Change 163812 abandoned by Hashar:
Reenable Apache lint check

Reason:
Migrated to integration/config.git as https://gerrit.wikimedia.org/r/#/c/166033/

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

hashar added a comment.Oct 28 2014, 2:44 PM

Patch is pending peer reviewing/testing to verify the test script would properly detects issues.

hashar raised the priority of this task from High to Needs Triage.Nov 24 2014, 7:10 AM
hashar triaged this task as High priority.
hashar lowered the priority of this task from High to Medium.Nov 24 2014, 9:22 AM
hashar moved this task from Untriaged to In-progress on the Continuous-Integration-Infrastructure board.
hashar added a project: acl*sre-team.
hashar set Security to None.
hashar added a comment.Nov 24 2014, 9:26 AM

+SRE

The patch is pending review, I have deployed the job on Sep. 30th https://integration.wikimedia.org/ci/job/operations-apache-config-lint/ but I am unsure whether it is going to catch all potential errors or is actually working properly. So meanwhile it has been non voting and will not prevent a bad apache change to sneak in.

We need people with good knowledge of our Apache configuration files to review what is being executed by the Jenkins job and +1 / validate it is fine. The change is https://gerrit.wikimedia.org/r/#/c/166033/

The relevant code: https://gerrit.wikimedia.org/r/#/c/166033/2/jjb/operations-apache-config.yaml

hashar raised the priority of this task from Medium to Needs Triage.Nov 24 2014, 9:26 AM
hashar moved this task from In-progress to Externally Blocked on the Continuous-Integration-Infrastructure board.
hashar added a comment.Dec 1 2014, 9:28 AM

I have emailed the ops list to attract some reviewers:

Hello,

The wiki Apache conf has been moved to puppet during the summer and since then the Jenkins job in charge of validating the configuration has been non voting.

I think I have managed to port it properly to run on operations/puppet.git but would need some review to ensure it is working properly and going to catch possible errors.

The job is currently running on operations/puppet but is not voting :-/ The relevant code to review is:

https://gerrit.wikimedia.org/r/#/c/166033/2/jjb/operations-apache-config.yaml

I would appreciate a few other pair of eyes to look at the crazy shell and eventually send dummy commits with errors to verify they are caught properly.

Thanks!

Task reference: https://phabricator.wikimedia.org/T72068

hashar removed hashar as the assignee of this task.Dec 1 2014, 9:30 AM
Se4598 subscribed.Dec 6 2014, 11:38 PM

FYI: Maybe T76955 is related or even blocks this task.

hashar added a comment.Dec 8 2014, 10:36 AM
In T72068#823563, @Se4598 wrote:

FYI: Maybe T76955 is related or even blocks this task.

Yup some oddity in zuul-cloner caused the Jenkins job workspace to be left in a bad state (missing git remote) which caused the job to always fail early on). I have fixed it up by deleting all the files and the job is building fine.

hashar added a subtask: T76955: zuul-cloner fails with "AttributeError: 'IterableList' object has no attribute 'origin' " (operations-apache-config-lint).Dec 8 2014, 10:46 AM
Krinkle unsubscribed.Dec 8 2014, 6:01 PM
Krinkle triaged this task as Medium priority.Dec 16 2014, 5:21 PM
scfc subscribed.Dec 17 2014, 1:51 PM
Dzahn mentioned this in T78782: remove operations-apache-config-lint on operations/mediawiki-config.Dec 17 2014, 2:14 PM
scfc added a comment.Feb 21 2015, 2:53 AM

Could the operations-apache-config-lint job be completely removed until it is enabled again?

gerritbot subscribed.Mar 4 2015, 3:53 AM

Change 194261 had a related patch set uploaded (by Krinkle):
Disable operations-apache-config-lint

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

gerritbot added a comment.Mar 4 2015, 3:55 AM

Change 194261 merged by jenkins-bot:
Disable operations-apache-config-lint

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

Krinkle mentioned this in rCICF5a5621517476: Disable operations-apache-config-lint.Mar 4 2015, 3:56 AM
Krinkle subscribed.Mar 4 2015, 3:56 AM
In T72068#1056111, @scfc wrote:

Could the operations-apache-config-lint job be completely removed until it is enabled again?

Yep, makes sense. Done.

Krinkle removed a project: Patch-For-Review.Mar 4 2015, 3:56 AM
Krinkle removed a subscriber: gerritbot.
Dzahn added a comment.Mar 24 2015, 2:00 AM

example why this check is missed: https://gerrit.wikimedia.org/r/#/c/199182/1

hashar added a comment.Mar 24 2015, 9:14 AM

Nobody bothered reviewing the patches I proposed back in October / November though :( I kind of loose interest in pushing this though.

Quoting an earlier comment on this task:

In T72068#777695, @hashar wrote:

+SRE

The patch is pending review, I have deployed the job on Sep. 30th https://integration.wikimedia.org/ci/job/operations-apache-config-lint/ but I am unsure whether it is going to catch all potential errors or is actually working properly. So meanwhile it has been non voting and will not prevent a bad apache change to sneak in.

We need people with good knowledge of our Apache configuration files to review what is being executed by the Jenkins job and +1 / validate it is fine. The change is https://gerrit.wikimedia.org/r/#/c/166033/

The relevant code: https://gerrit.wikimedia.org/r/#/c/166033/2/jjb/operations-apache-config.yaml

Or we can get the test script embeded in operations/puppet.git directly

greg added projects: Scrum-of-Scrums, Blocked-on-Operations.Mar 24 2015, 8:06 PM
Dzahn added a comment.Mar 25 2015, 4:26 AM

I'm wondering if it's an option to just go back to the separate apache-config repo we had before and run the same tests we had before. But that would be a question for Ori and _joe_.

You are right about the general review problem, but this change is also in the integration/config repository and the 2 reviewers on it are both not in ops.

Krinkle added a comment.EditedMar 25 2015, 8:57 AM

I'd recommend for someone experienced with apache config and operations/puppet (maybe @Dzahn) add an executable script to the puppet repository. Then we'll add a job that executes that. I don't think we have the resources for developing (and maintaining) such test right now.

As added bonus it would make it easy for maintainers of operations/puppet to then run it locally, too.

dduvall added a project: Release-Engineering-Team.Mar 25 2015, 5:42 PM
Dzahn added a comment.Mar 25 2015, 11:15 PM

Why do we have to write something new? I wasn't asking for a new feature, just reported a regression from what we had in the past.

fgiunchedi subscribed.Mar 30 2015, 9:42 AM

agreed if that's a regression we should automatically check whether the redirects files are synched, what do you think @Joe ?

Andrew subscribed.Apr 6 2015, 4:04 PM

Hashar, if this is still blocked on reviews then ping me on IRC this week and I'll try to get things unblocked.

gerritbot added a comment.Apr 18 2015, 4:53 AM

Change 204994 had a related patch set uploaded (by Legoktm):
redirects: Add test to verify redirects.conf has been regenerated from redirects.dat

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

gerritbot added a project: Patch-For-Review.Apr 18 2015, 4:53 AM
greg changed the task status from Open to Stalled.Apr 28 2015, 4:28 PM
hashar added a comment.May 6 2015, 4:38 PM

We talked about this task during our weekly RelEng checkin. The Jenkins job definition is quite trivial, but the testing script is a bit more challenging. The script I came up with is not robust and is hardcoded in the Jenkins job.

Ideally the test script would be directly in operations/puppet.git repo and Jenkins would simply invoke it. That offers much more liberty to define what is being tested.

JanZerebecki subscribed.May 15 2015, 5:09 PM
faidon removed projects: Blocked-on-Operations, Scrum-of-Scrums.May 20 2015, 5:21 PM
greg removed a project: Release-Engineering-Team.Sep 9 2015, 11:42 PM
Restricted Application added a subscriber: Matanya. · View Herald TranscriptSep 9 2015, 11:42 PM
gerritbot added a comment.Sep 24 2015, 12:33 AM

Change 204994 merged by Yuvipanda:
mediawiki: Add test to verify redirects.conf has been regenerated from redirects.dat

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

yuvipanda mentioned this in rOPUP09fbdf4b4e41: mediawiki: Add test to verify redirects.conf has been regenerated from….Sep 24 2015, 12:34 AM
yuvipanda subscribed.Sep 24 2015, 12:34 AM

Good Job Lego (As he asked me to type) - the redirects are checked now. There is lots of other apache configs, not sure if those also need to be checked?

Dzahn awarded a token.Sep 24 2015, 12:54 AM
Dzahn added a comment.Sep 24 2015, 1:09 AM

tested it.

just changed redirects.conf, but not .dat

https://gerrit.wikimedia.org/r/#/c/240626/1
AssertionError: redirects.conf not regenerated. Run ./refreshDomainRedirects.

then changed redirects.dat, but not .conf

https://gerrit.wikimedia.org/r/#/c/240626/3
AssertionError: redirects.conf not regenerated. Run ./refreshDomainRedirects.

then got them both in sync:
https://gerrit.wikimedia.org/r/#/c/240626/4
verified

Dzahn closed this task as Resolved.Sep 24 2015, 1:11 AM
Dzahn claimed this task.
Dzahn added a subscriber: Legoktm.

thank you @Legoktm

@yuvipanda maybe there should be more checks, yea. But since i created this just for the original regression and that works again like before i'll call it resolved

Dzahn reassigned this task from Dzahn to Legoktm.Sep 24 2015, 1:11 AM
gerritbot added a comment.Oct 6 2015, 12:43 PM

Change 243923 had a related patch set uploaded (by Hashar):
Remove operations-apache-config-lint

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

JanZerebecki added a comment.Oct 6 2015, 7:56 PM

This is not yet doing any apache linting / parse checking, which it did before. Does this need to be reopened?

Dzahn added a comment.Oct 6 2015, 8:00 PM

@JanZerebecki new task that is linked there though please?

Krinkle unsubscribed.Oct 6 2015, 8:30 PM
hashar reopened this task as Open.Oct 8 2015, 12:58 PM

We no have the redirects expansion being done. Still need to run the Apache2 configtest. I removed the Jenkins job but you can get some idea with https://gerrit.wikimedia.org/r/#/c/243923/1/jjb/operations-apache-config.yaml,unified

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 8 2015, 12:58 PM
gerritbot added a comment.Oct 8 2015, 1:01 PM

Change 243923 merged by jenkins-bot:
Remove operations-apache-config-lint

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

JanZerebecki closed this task as Resolved.Oct 8 2015, 1:58 PM

I opened T114801 for that.

Dzahn mentioned this in T124812: add icinga and watchmouse https checks for content on commons. or other wikimedia.org sites.Jan 26 2016, 7:40 PM
• MZMcBride subscribed.Jan 26 2016, 9:56 PM
hashar mentioned this in T57857: Unit tests for apache config/rewrites.Nov 16 2016, 9:27 PM
• Phabricator_maintenance removed a subscriber: yuvipanda.Jun 7 2017, 6:58 PM
thiemowmde closed subtask T114801: operations-apache-config-lint replacement doesn't check syntax as Invalid.Dec 7 2017, 3:51 PM
Legoktm reopened subtask T114801: operations-apache-config-lint replacement doesn't check syntax as Open.Dec 7 2017, 3:58 PM
Joe closed subtask T114801: operations-apache-config-lint replacement doesn't check syntax as Invalid.Mar 20 2023, 11:07 AM
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