Sourcegraph App (single-binary branch) #46547
Conversation
|
❌ Problem: the label |
|
Codenotify: Notifying subscribers in CODENOTIFY files for diff 02c985e...a0f7a95.
|
|
Codenotify: Notifying subscribers in OWNERS files for diff 02c985e...a0f7a95.
|
|
❌ Problem: the label |
|
❌ Problem: the label |
|
❌ Problem: the label |
There was a problem hiding this comment.
I started reviewing, but this is a monster change. Your rationale in the description makes sense to me, written by SQS, reviewed by you so it is ready to land. I am approving to approve the CI gods. Perfect time to land this large change since we have a whole month before next branch cut just in case we missed something.
| func LoadConfig() { | ||
| searcherURL = env.Get("SEARCHER_URL", "k8s+http://searcher:3181", "searcher server URL") | ||
| highlight.LoadConfig() | ||
| symbols.LoadConfig() |
There was a problem hiding this comment.
side-note: this is an inconsistency @abeatrix recently pointed out to me when we were pairing. symbols is the only service discovery user which isn't setup in this file. It is also the only service discovery we do which isn't advertised through the serviceconnections
| shared.Main(func(_ database.DB, _ conftypes.UnifiedWatchable) enterprise.Services { | ||
| return enterprise.DefaultServices() | ||
| }) | ||
| osscmd.DeprecatedSingleServiceMainOSS(shared.Service) |
There was a problem hiding this comment.
If you look at the func definition (follow the go-to-def chain) you'll see a comment about this:
basically the idea is that we should stop shipping individual binaries (and images) for each service, instead we can have sourcegraph --as=gitserver run the gitserver service and so on. One image for all services.
Basically marked as deprecated just to signal this change is coming, and not to add new services using this approach.
Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
…s in the same process Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
Add support for multi-platform builds to enterprise/cmd/sourcegraph/build.sh. Used without any command-line options, it will build for the current platform, preserving current behavior. With `--target` command-line options, it will build for the specified target(s). Because of OS-dependent coding in various places, windows builds fail. Different architectures work (arm64 and amd64, for example). Sample usage: build.sh --target=linux/amd64 --target darwin/arm64
Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
…ingleServiceMain Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
…me process Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
…and dev deployments only Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
Builds and packages Sourcegraph App for: - macOS (Universal .zip with single binary) - Linux (amd64 .zip with single binary, .deb, .rpm) Uploads to: - Google Cloud Storage (`sourcegraph-app-releases` bucket) - Homebrew (temporary repo https://github.com/sourcegraph/homebrew-sourcegraph-app while we consider whether to use sourcegraph/homebrew-sourcegraph) - GitHub releases on https://github.com/sourcegraph/sourcegraph-app (temporary repo while we consider whether to post releases on the main sourcegraph/sourcegraph repository) All this occurs when you push a commit to the `app/release-snapshot` branch with `git push -f origin HEAD:app/release-snapshot`. This is sufficient for now. In the future, we can automate nightlies and releases.
Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
* executor: improve error logging when running commands
Prior to this change one could get ambiguous errors when executor does its
startup checks, especially in the case of the single-binary deployment where
some of these binaries may not e.g. be installed:
```
ERROR service run/util.go:40 Failed to get src-cli version {"error": "exit status 126"}
```
This change makes it so that the error at least includes what the command output was:
```
ERROR service run/util.go:41 Failed to get src-cli version {"error": "'src version -client-only': error: exit status 126 output: No preset version installed for command src\nPlease install a version by running one of the following:\n\nasdf install golang 1.19.3\n\nor add one of the following versions in your config file at /Users/stephen@sourcegraph.com/work/sourcegraph/.tool-versions\ngolang 1.18.1\n"}
```
Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
See executorcmd package comment for details. Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
In `singleprogram` mode package `conf` always behaves as a server (there is only one process, after all) - but `internal/singleprogram` is imported by pkg `svcmain` and as a result this init would cause `DEPLOY_TYPE` to incorrectly always be `single-program` in all deployment types. To fix this Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
`EXECUTOR_QUEUE_DISABLE_ACCESS_TOKEN_INSECURE` may now also be used in `sourcegraph/server` as well, to aid in our usage of it in regression tests. An `_INSECURE` suffix has been added to the env var to highlight that it is insecure to use this option. Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
…ncing) Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
|
|
| @@ -159,8 +145,5 @@ func Main() { | |||
| }, | |||
| } | |||
|
|
|||
| if err := app.RunContext(context.Background(), os.Args); err != nil { | |||
| println(err.Error()) | |||
| os.Exit(1) | |||
There was a problem hiding this comment.
- This may be the reason CI is hanging
| log.SentrySink{ | ||
| ClientOptions: sentry.ClientOptions{SampleRate: 0.2}, | ||
| }, |
There was a problem hiding this comment.
- Wasn't enabled for executors
| }, | ||
| ), | ||
| ) | ||
| logger := log.Scoped("sourcegraph", "Sourcegraph") |
There was a problem hiding this comment.
Mis-scoped for non-single-binary?
| } | ||
|
|
||
| env.Lock() | ||
| env.HandleHelpFlag() |
There was a problem hiding this comment.
Conflicts with executor's own help flag.
| // Start the debug server. The ready boolean state it publishes will become true when *all* | ||
| // services report ready. | ||
| var allReadyWG sync.WaitGroup | ||
| go debugserver.NewServerRoutine(allReady, allDebugserverEndpoints...).Start() |
There was a problem hiding this comment.
Executor spawns debug server on its own (it's not just a service entrypoint)
This branch is the
mainbranch of the Sourcegraph App / single-binary product.Most of these changes were authored by sqs originally, I picked them apart piece-by-piece, reviewed them, and improved them. There are cases where code can certainly be improved (as indicated by TODOs), and will follow up / improve that code. My primary goal has been to identify how quickly we can merge these changes (to stop diverging from
mainfurther, as at least 3 people are building upon these changes) without regressing on any behavior.Reviewing
All commits in this PR were effectively authored by one person, and reviewed by another - either formally or informally. I have made a judgement call that this is sufficient for merge. Feedback is still very welcome (discussion of this approach in Slack).
Once the 4.4 branch cut occurs, this will be stamped and merged into
main. We are doing this after the 4.4 branch cut to reduce the risk of any issues that may arise.The people working on #app are temporarily treating this as our
mainbranch until then, and only reviewed/approved changes are being merged into this branch - excluding changes to get CI/tests passing where appropriate.Test plan
In some cases new tests were added, in others existing tests provide coverage, in others the only way to verify was through manual testing due to the nature of the changes.
Before merging into
mainI will ensure amain-dry-runbranch passes on CI, executed via thesgtool.Should any issues arise from merging these changes, we will do our best to "roll forward" (i.e. fix the issues as our #1 priority) not revert, due to the potential for introducing merge conflicts across the company.