Clean up gitserver clonePipelineRoutine

Problem statement

There's a nice TODO item in clonePipelineRoutine that seems like an easy first issue: remove the pipeline's dependency on the upstream gitserver server instance.

https://github.com/sourcegraph/sourcegraph/blob/5666861dfa305e9b12bea12f05763275c08a26c5/cmd/gitserver/internal/server.go#L353

Things to learn/practice during the implementation

Before pushing up some code, I wanted to make sure I understand what I'm doing clearly. The least coupling we can do, the better!

clonePipelineRoutine uses server for a few things:

• Easy: p.s.Logger is the same thing as p.logger, so we can just replace that.
• p.s.doClone — the pipeline needs to invoke actually cloning the repository.
• p.s.acquireCloneLimiter — the server managers rate limits on cloning
• p.s.setLastNonFatal records an error into the repos PG database.
• Related: cloneJobConsumer should acquire it's own lock (see https://github.com/sourcegraph/sourcegraph/pull/24892)

We could just make an interface with those methods and pass the interface instead of the server, abstracting some complexity away, but I'm not so sure if that improves code clarity much.

type GitServerCloner interface {
    AcquireCloneLimit(argType) returnType
    SetLastNonLethal(argType) returnType
    DoClone(argType) returnType
}

The core curious piece is that because [server.cloneRepo](https://github.com/sourcegraph/sourcegraph/blob/5666861dfa305e9b12bea12f05763275c08a26c5/cmd/gitserver/internal/server.go#L1325 can clone synchronously, without using the pipeline, we can't move all of the logic (rate limiters and doClone) into the pipeline.

Suggested approach

Alternatively, we could make clonePipelineRoutine responsible for all actual clone execution stuff. To do that, we could give it access to a few things, and move the doClone part into it.

Server will be responsible for the HTTP API, and for putting clone jobs on the queue, or executing them inline. clonePipeline will listen to the queue as it does now, and expose doClone to clone in a blocking way.

To make the shift gradual, we can leave the isClonable and it's RPS limiter on the server, or move it along too.

type clonePipelineRoutine struct {
    ...
    // setLastNonFatal, setStatus and friends
    db database.GitserverRepoStore
    
    // since Server starts it's RPS Limiter for isClonable operations, we'll want server to share that thing with us.
    rpsLimiter ...
    ...
}

// move the actual clone in clonePipelineRoutine. When 
func (p* clonePipelineroutine) doClone() ...

The only thing that clonePipeline will need that it does not have today will be Perforce to EnqueueChangelistMappingJob. It doesn't look right in doClone today to me, either — perhaps we can extract it out?

@eseliger, you contributed to gitserver quite a lot. How does this look to you? Is it worth spending time, or is it okay as it is?

Estimated amount of work

Um, not sure. Looks like an M? ;)

Contributors

1. Check out contributing guidelines.
2. Join the Sourcegraph Community Space on Slack and join the #help channel where the Sourcegraph team can help you!
3. Check out the good first issues board to find more curated issues.