User:RLazarus (WMF)/New Puppet process
Material may not yet be complete, information may presently be omitted, and certain parts of the content may be subject to radical, rapid alteration. More information pertaining to this may be available on the talk page.
For submitters:
If you're already working with an SRE, or if it's clear who in SRE should review your code, you can just send your review to that person. You don't need to do anything with this page. (If your usual SRE reviewer isn't responding promptly, please raise it with them or escalate normally; don't use this process to work around them.)
If you're requesting access to production systems (via SSH or otherwise), don't use this process. Instead submit an access request.
Otherwise, if you have a patch for the Puppet repo, and need help to get it merged, this is for you.
How to get your patch reviewed
Add your patch to ###. If you'll want to coordinate a time to deploy your change and test it, ###.
TODO: Fill out a form? Add to a wiki page? Something that generates a notification for reviewers to respond to. Hopefully not a phab ticket, which feels heavyweight.
Suggestion Moritz: We could simply add a list to this subsection with 1. the name of the submitter 2. the link to Gerrit 3. a tick box whether sync up with the submitter for a time is needed or or not 4. IRC nickname to contact for that. When a patch is merged (or abandoned if there were issues), it simply gets removed from the list by the person who merged
What happens next
An SRE monitoring the queue will take a first look at your patch, and will merge it right away if they can. In most cases, that means you'll be done. Remember that this is a generalist review; the Puppet repo is large, and we aren't all experts in all parts of it. A quick and easy +2 is most likely if:
- Your commit message is clear and links to a task with context.
- Your change is simple.
- Your change is easy to verify and test.
- Your change already has a +1 from someone familiar with the code you're changing. (For example, this may be a member of your team.)
- You've already run the Puppet compiler on your change and the results look correct.
- For changes that can be tested on the Beta cluster, they've already been cherry-picked to the Beta cluster puppetserver.
If you indicated that you want to schedule a deployment time to test your change, or if the reviewer thinks it's a good idea, they'll coordinate with you to schedule a window on the deployment calendar.
If your patch is complex, it will be routed to the appropriate team for a careful review. Certain types of configuration are inherently risky to touch -- even if the actual diff is small, your patch will almost certainly still be considered "complex" if it changes any of:
- Varnish configuration (VCL) or ATS Lua. Your patch will be reviewed by the Traffic team and you should expect to coordinate the deployment with them.
- Apache configuration. Your patch will be reviewed by the Service Ops team and you should expect to coordinate the deployment with them. (Apache changes are more likely to be merged if submitted along with httpbb tests in the same patch, but this is still at the reviewer's discretion.)
The SRE handling your patch will merge it if they consider it safe to do so, or will route your patch to a reviewer from the appropriate team if they consider it necessary. This decision is up to the reviewer, since they are ultimately responsible for the stability of the cluster.
What should I do if I don't hear back?
You can expect an initial response within two business days. This response will be one of: +2ing and merging your patch, requesting specific changes, or, routing it for specialized review.
If getting a response takes longer than that, please directly contact any of the SREs listed below.
SRE reviewers
- Reuven Lazarus (IRC: rzl)
- Moritz Mühlenhoff (IRC: moritzm)
TODO: Start with the same people as the Puppet window? No more than two or maybe three, because there's not much work to do and it diffuses the responsibility. (One person can do the work easily, we just need more for OOO coverage.) TZ spread is nice but not required.
What happened to the Puppet request window?
We used to reserve two deployment windows weekly for handling simple Puppet patches, by analogy to MediaWiki's backport windows.
For the last few years, the window went unused most of the time, and most of the submitted patches didn't actually need a scheduled deployment; it was just the easiest way to find a reviewer. At the same time, more complex changes were explicitly out of scope, and we still didn't have a great process for getting them reviewed.
This mostly-asynchronous queue system is intended to be more convenient for both submitters and reviewers.