Skip to content

all: Use ServiceConnections configuration for gitserver

Created by: slimsag

All services that need the gitserver details should use ServiceConnections configuration field.

This issue was originally a discussion on what approach to use for repo-updater to get the Postgres connection information. We switched to using ServiceConnections in the config. See below for original description.

Background

Multiple of our services have to talk to each-other.

Here's how it looks as of 3.2.0:

  • zoekt-indexserver -> sourcegraph-frontend-internal
  • symbols -> gitserver (via sourcegraph-frontend-internal)
  • searcher -> gitserver (via sourcegraph-frontend-internal)
  • repo-updater -> sourcegraph-frontend-internal (for config) / github-proxy / gitserver (via sourcegraph-frontend-internal)
  • query-runner -> sourcegraph-frontend-internal
  • management-console -> pgsql (has to be direct, management console must be accessible when frontend is down)
  • gitserver -> sourcegraph-frontend-internal (for config)
  • github-proxy -> sourcegraph-frontend-internal (for config)
  • sourcegraph-frontend[-internal] -> pgsql / gitserver / syntect-server / searcher / symbols / sourcegraph-frontend-internal / repo-updater / zoekt-webserver

One thing you should take note of above is that almost all services depend on sourcegraph-frontend-internal. This is because it is also Sourcegraph's configuration server, responsible for giving the current configuration to any client (service) that asks for it. There is definitely an argument for moving this configuration server out into a separate service for clarity sake, and I think we are likely to do that eventually, but for now it is a part of the frontend monolith.

Why do we use the configuration server for determining how to talk to gitserver?

Prior to having the configuration server, all services needing to talk to gitserver had to directly configure their access to it via environment variables. That meant that for:

  • symbols
  • searcher
  • repo-updater
  • sourcegraph-frontend

You would need to update the GITSERVER environment variables for each of the above k8s deployments when adding new gitservers to scale up your cluster. This was bad for two reasons:

  1. It was tedious to do, and hard for users to do correctly. This lead to us giving users scripts that would do it for them, which made scaling up the number of gitservers difficult. See our previous instructions for doing so: https://github.com/sourcegraph/deploy-sourcegraph/blob/v2.13.5/docs/configure.md#configure-gitserver-replica-count
  2. In non-Kubernetes deployments, such as deploy-sourcegraph-docker, it made configuring the number of gitservers very difficult as you do not have access to nicer k8s tooling. Every change here is 100% manual and involves e.g. SSHing into a box and running a docker command.

To resolve this, and since most of these services already had a dependency on the configuration service that the frontend provided anyway, we allowed the frontend to also pass through service connection information. How it works:

  • The frontend has environment variables like SRC_GIT_SERVERS which are configured.
  • Other services pull that information from the frontend's configuration server via conf.Get().ServiceConnections.Gitservers, and use conf.Watch() to respond to changes in this configuration.

This meant that you only needed to update the env vars on the frontend and all other services relying on gitserver would automatically handle respecting that new env var.

What should have been documented earlier

An accurate statement which I should've document earlier is:

If one Sourcegraph service wishes to talk to another, the address MUST be retrieved via conf.Get().ServiceConnections. All Sourcegraph services do this today, with exception of:

  • repo-updater talking to github-proxy, because this will be removed eventually (according to Keegan).
  • management-console talking directly to pgsql because it has no choice (the management console MUST be accessible when the frontend is down).
  • (tech debt) Gitserver address retrieval which uses an older API than conf.Get().ServiceConnections, but effectively does the exact same thing.

Recent regression in repo-updater service intercommunication

repo-updater recently began talking to pgsql directly, but it retrieves this connection information from env vars instead of via conf.Get().ServiceConnections. Before the new syncer can be enabled by default, we need to take action on one of the following choices:

  1. Make repo-updater pull the PG* information from conf.Get().ServiceConnections instead.

  2. State that repo-updater is excluded from this rule, update deploy-sourcegraph-docker with a similar change to PR #211, and then clearly document that these environment variables need to be manually update by all users of deploy-sourcegraph-docker AND all users of deploy-sourcegraph that are making use of an external postgres DB before upgrading.

  3. Decide we are not interested in this rule at all and discard it, then clearly document env var configuration needed (a) when updating to each new version of deploy-sourcegraph (and -docker) and (b) update deploy-sourcegraph (and -docker) documentation to reflect how to configure gitserver connection access via env vars when scaling up gitserver replicas. Prior to this, we would need to reach out to customers running deploy-sourcegraph-docker and confirm how much manual env var configuration they would be willing to accept as it was previously a point of contention.

@tsenart @keegancsmith Let's discuss how to move forward here (or schedule a call if you like) to discuss this further once you've had a chance to read what I've said above.

@tsenart had said on Slack:

@keegan and I were thinking to use a separate database instance eventually, when we make repo-updater more of an independent micro-service. Is this change [using ServiceConnections] really needed?

This wouldn't prohibit doing that, the env vars would just be passed via the frontend's config server.

@felixfbecker had said on Slack:

Shouldn’t we be avoiding unneeded dependencies between services?

I 100% agree with this, that is not even a question in my opinion. However:

(1) What is the proof it is unneeded? Are we willing to accept one of the other 3 options above which have different tradeoffs? (2) We do have the dependency between repo-updater and the config server (sourcegraph-frontend-internal) already due to accessing site config which repo-updater uses.