Page MenuHomePhabricator

puppet-merge can't accept an explicit SHA1 for an --ops merge
Closed, ResolvedPublic

Description

Because of the way puppet-merge.erb is presently written, any explicit SHA1 passed to it for an ops merge will wind up also being passed to its invocation of puppet-merge.py --labsprivate -- where, of course, git rev-parse will yell at you for giving it a SHA1 that doesn't exist in that repository and things will fail.

Event Timeline

Change 560417 had a related patch set uploaded (by Crutishnyk; owner: Crutishnyk):
[wikimedia-cz/tracker@master] tests/UserProfileTests: Add tests for /users and /users/<username>

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

Sorry, I did a mistake when writing a commit message. It was for T241227

@CDanis Is this something you plan to work on? Otherwise, who do you need help with? I am trying to triage the importance of this ticket.

jbond triaged this task as Medium priority.Dec 30 2019, 10:53 AM

Change 559944 had a related patch set uploaded (by Jbond; owner: CDanis):
[operations/puppet@production] puppet-merge.py: SHA1 or explicit FETCH_HEAD is mandatory

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

@CDanis Is this something you plan to work on? Otherwise, who do you need help with? I am trying to triage the importance of this ticket.

Chris has already created a change for this, i have updated the priority and associated the change

Thanks

A simple option (thanks John): if puppet-merge.sh is given a treeish, it *only* does the ops repo or the labsprivate repo (depending on what flag was passed).

A possibly better but more work option: replace the shell-script-generated-from-ERB with a proper Python script, plus small configuration file generated from ERB (to 'configure' the remote puppetmasters to operate on). This would make it much easier to have the wrapper be smarter about argument processing, since I think you'd want to add a --labsprivate-treeish flag that accepts a value so you could still maintain the lockstep syncing of both repos.

However it's very rare that we're ever puppet-merging a non-FETCH_HEAD hash anyway, so maybe not worth it.

Change 559944 merged by CDanis:
[operations/puppet@production] puppet-merge.py: SHA1 or explicit FETCH_HEAD is mandatory

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

CDanis claimed this task.