There have been changes and improvements to keyholder in the phab repo that have not been incorporated into puppet. This is creating duplicate work.
We should figure out a way to use the upstream repo in puppet.
There have been changes and improvements to keyholder in the phab repo that have not been incorporated into puppet. This is creating duplicate work.
We should figure out a way to use the upstream repo in puppet.
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Open | None | T203003 Keyholder phab repo duplicate work | |||
| Resolved | faidon | T203108 Create keyholder gerrit repo | |||
| Resolved | • mmodell | T210674 Point keyholder github mirror to gerrit |
Thanks for filing this! I lost about an hour debugging and (re-)fixing the above issue today, so +1 to everything you said :)
Let's sort this out and all other potential cases like it and avoid doing this kind of fork in any of our projecs again.
I agree that we should use the upstream repo from puppet and I should have refactored the puppet code as soon as I created rKEYHOLDER. The question then is, do we install via debian packaging or directly via GIT?
Thank you @faidon to have managed to import the git history into operations/software/keyholder that will be very helpful in the future I guess :]
I guess we can close rKEYHOLDER. Seems to me keyholder code will be moved out of operations/puppet to operations/software/keyholder where development has been occurring recently.
That is all correct, so yes we should.
Other than that and to answer @greg's question, the next steps would be:
Change 588054 had a related patch set uploaded (by Hashar; owner: Hashar):
[integration/config@master] Add debian-glue for operations/software/keyholder
Change 588054 merged by jenkins-bot:
[integration/config@master] Add debian-glue for operations/software/keyholder
Change 588055 had a related patch set uploaded (by Hashar; owner: Hashar):
[operations/software/keyholder@debian] Initial debianization
Eventually today I went with the issue of having to restart Keyholder and reharm after a change in the yaml configuration. The version in operations/software/keyholder supports reloading them without having to reharm (just signal SIGHUP). So I guess I am going to cookie lick the Debian packaging :]
@faidon @thcipriani I could use a tag in the repository though. 0.1 1.0 or whatever work for you.
The master branch of operations/software/keyholder is not ready for a release at this time, so please don't tag, package or deploy this at this state. There are a bunch of pending changes in Gerrit for about a year, plus more that I've queued up locally (because it's hard to manage dozens of dependent git commits with Gerrit…). If y'all are willing to review these I can clean them up and prepare a release; if not, then I can pick this up and make some progress. Let me know!
I forgot to check for open changes, thank you for the notification. I guess it is time for some reviews and +2 :].
There are a bunch of pending changes in Gerrit for about a year, plus more that I've queued up locally (because it's hard to manage dozens of dependent git commits with Gerrit…).
Somehow I have missed that part. The repository is requiring changes to be fast forward in order to be submitted/merged, I would like to change it to Rebase-If-Necessary like we did for operations/puppet (T224033). Can be done at https://gerrit.wikimedia.org/r/#/admin/projects/operations/software/keyholder
The long list of patches is not ideal either unless they are strictly related. A patch in the middle of the chain might be held because some parent change is pending review. While it is easy to cherry pick them as needed.
The repo on Phabricator has been closed meanwhile. I wonder if there are things left to do here now in 2023.
One can compare the content of https://phabricator.wikimedia.org/source/keyholder.git with https://gerrit.wikimedia.org/r/operations/software/keyholder
Also per Faidon in T203003#6048360, there was also a series of pending open change in Gerrit + a few that were on his local machine which would be great to integrate.
Then I think the issue is having the ssh knowledge needed to review the code (even though certainly a lot of patches do not need that very specific knowledge).
@thcipriani Hi! We are reviewing the backlog and we are wondering if you are still interested in pursuing this.
This isn't a priority for me.
There's still divergence between https://gerrit.wikimedia.org/r/operations/software/keyholder and what actually gets used in prod (afaict from looking at puppet code). But the last example of that divergence causing issues is from 8 years ago when I filed this task.
Up to you all if you want pursue aligning these. If not, I'm happy to help archive the gerrit repo to avoid confusion.