sg: add ability to define checks per commandset (#23020)

This allows us to check whether services are running before we boot up
the environment. This should reduce the number of "why is service X failing?".
At least that's the hope.

The changes to the config file format are backwards compatible. See the
`UnmarshalYAML` method on `Commandset`.

It also updates the checks to make sure that they work for users of the
docker-compose setup.

dev/sg/config.go

@@ -20,6 +20,10 @@ func ParseConfigFile(name string) (*Config, error) {
 		return nil, errors.Wrap(err, "reading configuration file")
 	}
 
+	return ParseConfig(data)
+}
+
+func ParseConfig(data []byte) (*Config, error) {
 	var conf Config
 	if err := yaml.Unmarshal(data, &conf); err != nil {
 		return nil, err
@@ -30,6 +34,11 @@ func ParseConfigFile(name string) (*Config, error) {
 		conf.Commands[name] = cmd
 	}
 
+	for name, cmd := range conf.Commandsets {
+		cmd.Name = name
+		conf.Commandsets[name] = cmd
+	}
+
 	for name, cmd := range conf.Tests {
 		cmd.Name = name
 		conf.Tests[name] = cmd
@@ -119,12 +128,38 @@ type Check struct {
 	FailMessage string `yaml:"failMessage"`
 }
 
+type Commandset struct {
+	Name     string   `yaml:"-"`
+	Commands []string `yaml:"commands"`
+	Checks   []string `yaml:"checks"`
+}
+
+// UnmarshalYAML implements the Unmarshaler interface.
+func (c *Commandset) UnmarshalYAML(unmarshal func(interface{}) error) error {
+	// To be backwards compatible we first try to unmarshal as a simple list.
+	var list []string
+	if err := unmarshal(&list); err == nil {
+		c.Commands = list
+		return nil
+	}
+
+	// If it's not a list we try to unmarshal it as a Commandset. In order to
+	// not recurse infinitely (calling UnmarshalYAML over and over) we create a
+	// temporary type alias.
+	type rawCommandset Commandset
+	if err := unmarshal((*rawCommandset)(c)); err != nil {
+		return err
+	}
+
+	return nil
+}
+
 type Config struct {
-	Env         map[string]string   `yaml:"env"`
-	Commands    map[string]Command  `yaml:"commands"`
-	Commandsets map[string][]string `yaml:"commandsets"`
-	Tests       map[string]Command  `yaml:"tests"`
-	Checks      map[string]Check    `yaml:"checks"`
+	Env         map[string]string      `yaml:"env"`
+	Commands    map[string]Command     `yaml:"commands"`
+	Commandsets map[string]*Commandset `yaml:"commandsets"`
+	Tests       map[string]Command     `yaml:"tests"`
+	Checks      map[string]Check       `yaml:"checks"`
 }
 
 // Merges merges the top-level entries of two Config objects, with the receiver

dev/sg/config_test.go

@@ -0,0 +1,81 @@
+package main
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+)
+
+func TestParseConfig(t *testing.T) {
+	input := `
+env:
+  SRC_REPOS_DIR: $HOME/.sourcegraph/repos
+
+commands:
+  frontend:
+    cmd: ulimit -n 10000 && .bin/frontend
+    install: go build -o .bin/frontend github.com/sourcegraph/sourcegraph/cmd/frontend
+    checkBinary: .bin/frontend
+    env:
+      CONFIGURATION_MODE: server
+    watch:
+      - lib
+
+checks:
+  docker:
+    cmd: docker version
+    failMessage: "Failed to run 'docker version'. Please make sure Docker is running."
+
+commandsets:
+  oss:
+    - frontend
+    - gitserver
+  enterprise:
+    checks:
+      - docker
+    commands:
+      - frontend
+      - gitserver
+`
+
+	have, err := ParseConfig([]byte(input))
+	if err != nil {
+		t.Errorf("unexpected error: %s", err)
+	}
+
+	want := &Config{
+		Env: map[string]string{"SRC_REPOS_DIR": "$HOME/.sourcegraph/repos"},
+		Commands: map[string]Command{
+			"frontend": {
+				Name:        "frontend",
+				Cmd:         "ulimit -n 10000 && .bin/frontend",
+				Install:     "go build -o .bin/frontend github.com/sourcegraph/sourcegraph/cmd/frontend",
+				CheckBinary: ".bin/frontend",
+				Env:         map[string]string{"CONFIGURATION_MODE": "server"},
+				Watch:       []string{"lib"},
+			},
+		},
+		Commandsets: map[string]*Commandset{
+			"oss": {
+				Name:     "oss",
+				Commands: []string{"frontend", "gitserver"},
+			},
+			"enterprise": {
+				Name:     "enterprise",
+				Commands: []string{"frontend", "gitserver"},
+				Checks:   []string{"docker"},
+			},
+		},
+		Checks: map[string]Check{
+			"docker": {
+				Name:        "docker",
+				Cmd:         "docker version",
+				FailMessage: "Failed to run 'docker version'. Please make sure Docker is running.",
+			},
+		},
+	}
+
+	if diff := cmp.Diff(want, have); diff != "" {
+		t.Fatalf("wrong config. (-want +got):\n%s", diff)
+	}
+}

dev/sg/main.go

@@ -257,14 +257,14 @@ func main() {
 	}
 }
 
-// conf is the global config. If a command needs to access it, it *must* call
+// globalConf is the global config. If a command needs to access it, it *must* call
 // `parseConf` before.
-var conf *Config
+var globalConf *Config
 
 // parseConf parses the config file and the optional overwrite file.
 // Iear the conf has already been parsed it's a noop.
 func parseConf(confFile, overwriteFile string) (bool, output.FancyLine) {
-	if conf != nil {
+	if globalConf != nil {
 		return true, output.FancyLine{}
 	}
 
@@ -284,7 +284,7 @@ func parseConf(confFile, overwriteFile string) (bool, output.FancyLine) {
 		overwriteFile = filepath.Join(repoRoot, overwriteFile)
 	}
 
-	conf, err = ParseConfigFile(confFile)
+	globalConf, err = ParseConfigFile(confFile)
 	if err != nil {
 		return false, output.Linef("", output.StyleWarning, "Failed to parse %s%s%s%s as configuration file:%s\n%s\n", output.StyleBold, confFile, output.StyleReset, output.StyleWarning, output.StyleReset, err)
 	}
@@ -294,7 +294,7 @@ func parseConf(confFile, overwriteFile string) (bool, output.FancyLine) {
 		if err != nil {
 			return false, output.Linef("", output.StyleWarning, "Failed to parse %s%s%s%s as overwrites configuration file:%s\n%s\n", output.StyleBold, overwriteFile, output.StyleReset, output.StyleWarning, output.StyleReset, err)
 		}
-		conf.Merge(overwriteConf)
+		globalConf.Merge(overwriteConf)
 	}
 
 	return true, output.FancyLine{}
@@ -317,15 +317,35 @@ func runSetExec(ctx context.Context, args []string) error {
 		return flag.ErrHelp
 	}
 
-	names, ok := conf.Commandsets[args[0]]
+	set, ok := globalConf.Commandsets[args[0]]
 	if !ok {
 		out.WriteLine(output.Linef("", output.StyleWarning, "ERROR: commandset %q not found :(\n", args[0]))
 		return flag.ErrHelp
 	}
 
-	cmds := make([]Command, 0, len(names))
-	for _, name := range names {
-		cmd, ok := conf.Commands[name]
+	var checks []Check
+	for _, name := range set.Checks {
+		check, ok := globalConf.Checks[name]
+		if !ok {
+			out.WriteLine(output.Linef("", output.StyleWarning, "WARNING: check %s not found in config\n", name))
+			continue
+		}
+		checks = append(checks, check)
+	}
+
+	ok, err := runChecks(ctx, checks...)
+	if err != nil {
+		out.WriteLine(output.Linef("", output.StyleWarning, "ERROR: checks could not be run: %s\n", err))
+	}
+
+	if !ok {
+		out.WriteLine(output.Linef("", output.StyleWarning, "ERROR: checks did not pass, aborting start of commandset %s\n", set.Name))
+		return nil
+	}
+
+	cmds := make([]Command, 0, len(set.Commands))
+	for _, name := range set.Commands {
+		cmd, ok := globalConf.Commands[name]
 		if !ok {
 			return errors.Errorf("command %q not found in commandset %q", name, args[0])
 		}
@@ -348,7 +368,7 @@ func testExec(ctx context.Context, args []string) error {
 		return flag.ErrHelp
 	}
 
-	cmd, ok := conf.Tests[args[0]]
+	cmd, ok := globalConf.Tests[args[0]]
 	if !ok {
 		out.WriteLine(output.Linef("", output.StyleWarning, "ERROR: test suite %q not found :(\n", args[0]))
 		return flag.ErrHelp
@@ -389,7 +409,7 @@ func runExec(ctx context.Context, args []string) error {
 		return flag.ErrHelp
 	}
 
-	cmd, ok := conf.Commands[args[0]]
+	cmd, ok := globalConf.Commands[args[0]]
 	if !ok {
 		out.WriteLine(output.Linef("", output.StyleWarning, "ERROR: command %q not found :(\n", args[0]))
 		return flag.ErrHelp
@@ -405,7 +425,12 @@ func doctorExec(ctx context.Context, args []string) error {
 		os.Exit(1)
 	}
 
-	return runChecks(ctx, conf.Checks)
+	var checks []Check
+	for _, c := range globalConf.Checks {
+		checks = append(checks, c)
+	}
+	_, err := runChecks(ctx, checks...)
+	return err
 }
 
 func liveExec(ctx context.Context, args []string) error {
@@ -561,11 +586,11 @@ func printRunUsage(c *ffcli.Command) string {
 	// error, because we should never error when the user wants --help output.
 	_, _ = parseConf(*configFlag, *overwriteConfigFlag)
 
-	if conf != nil {
+	if globalConf != nil {
 		fmt.Fprintf(&out, "\n")
 		fmt.Fprintf(&out, "AVAILABLE COMMANDS IN %s%s%s\n", output.StyleBold, *configFlag, output.StyleReset)
 
-		for name := range conf.Commands {
+		for name := range globalConf.Commands {
 			fmt.Fprintf(&out, "  %s\n", name)
 		}
 	}
@@ -615,11 +640,11 @@ func printTestUsage(c *ffcli.Command) string {
 	// error, because we should never error when the user wants --help output.
 	_, _ = parseConf(*configFlag, *overwriteConfigFlag)
 
-	if conf != nil {
+	if globalConf != nil {
 		fmt.Fprintf(&out, "\n")
 		fmt.Fprintf(&out, "AVAILABLE TESTSUITES IN %s%s%s\n", output.StyleBold, *configFlag, output.StyleReset)
 
-		for name := range conf.Tests {
+		for name := range globalConf.Tests {
 			fmt.Fprintf(&out, "  %s\n", name)
 		}
 	}
@@ -636,11 +661,11 @@ func printRunSetUsage(c *ffcli.Command) string {
 	// Attempt to parse config so we can list available sets, but don't fail on
 	// error, because we should never error when the user wants --help output.
 	_, _ = parseConf(*configFlag, *overwriteConfigFlag)
-	if conf != nil {
+	if globalConf != nil {
 		fmt.Fprintf(&out, "\n")
 		fmt.Fprintf(&out, "AVAILABLE COMMANDSETS IN %s%s%s\n", output.StyleBold, *configFlag, output.StyleReset)
 
-		for name := range conf.Commandsets {
+		for name := range globalConf.Commandsets {
 			fmt.Fprintf(&out, "  %s\n", name)
 		}
 	}

dev/sg/run.go

@@ -18,6 +18,7 @@ import (
 	"github.com/rjeczalik/notify"
 
 	// TODO - deduplicate me
+	"github.com/sourcegraph/sourcegraph/dev/sg/internal/command"
 	"github.com/sourcegraph/sourcegraph/dev/sg/root"
 	"github.com/sourcegraph/sourcegraph/lib/output"
 )
@@ -182,7 +183,7 @@ func runWatch(ctx context.Context, cmd Command, root string, reload <-chan struc
 
 			c := exec.CommandContext(ctx, "bash", "-c", cmd.Install)
 			c.Dir = root
-			c.Env = makeEnv(conf.Env, cmd.Env)
+			c.Env = makeEnv(globalConf.Env, cmd.Env)
 
 			cmdOut, err := c.CombinedOutput()
 			if err != nil {
@@ -314,7 +315,7 @@ func startCmd(ctx context.Context, dir string, cmd Command) (*startedCmd, error)
 
 	sc.Cmd = exec.CommandContext(commandCtx, "bash", "-c", cmd.Cmd)
 	sc.Cmd.Dir = dir
-	sc.Cmd.Env = makeEnv(conf.Env, cmd.Env)
+	sc.Cmd.Env = makeEnv(globalConf.Env, cmd.Env)
 
 	logger := newCmdLogger(cmd.Name, out)
 	if cmd.IgnoreStdout {
@@ -491,7 +492,7 @@ func runTest(ctx context.Context, cmd Command, args []string) error {
 
 	c := exec.CommandContext(commandCtx, "bash", "-c", strings.Join(cmdArgs, " "))
 	c.Dir = root
-	c.Env = makeEnv(conf.Env, cmd.Env)
+	c.Env = makeEnv(globalConf.Env, cmd.Env)
 	c.Stdout = os.Stdout
 	c.Stderr = os.Stderr
 
@@ -500,23 +501,21 @@ func runTest(ctx context.Context, cmd Command, args []string) error {
 	return c.Run()
 }
 
-func runChecks(ctx context.Context, checks map[string]Check) error {
-	root, err := root.RepositoryRoot()
-	if err != nil {
-		return err
-	}
+func runChecks(ctx context.Context, checks ...Check) (bool, error) {
+	success := true
 
 	for _, check := range checks {
 		commandCtx, cancel := context.WithCancel(ctx)
 		defer cancel()
 
 		c := exec.CommandContext(commandCtx, "bash", "-c", check.Cmd)
-		c.Dir = root
-		c.Env = makeEnv(conf.Env)
+		c.Env = makeEnv(globalConf.Env)
 
 		p := out.Pending(output.Linef(output.EmojiLightbulb, output.StylePending, "Running check %q...", check.Name))
 
-		if cmdOut, err := c.CombinedOutput(); err != nil {
+		if cmdOut, err := command.RunInRoot(c); err != nil {
+			success = false
+
 			p.Complete(output.Linef(output.EmojiFailure, output.StyleWarning, "Check %q failed: %s", check.Name, err))
 
 			out.WriteLine(output.Linef("", output.StyleWarning, "%s", check.FailMessage))
@@ -535,7 +534,7 @@ func runChecks(ctx context.Context, checks map[string]Check) error {
 		}
 	}
 
-	return nil
+	return success, nil
 }
 
 // prefixSuffixSaver is an io.Writer which retains the first N bytes

dev/sg/sg.config.example.yaml

@@ -91,6 +91,15 @@ commands:
       NODE_ENV: development
       NODE_OPTIONS: "--max_old_space_size=4096"
 
+checks:
+  docker:
+    cmd: docker version
+    failMessage: "Failed to run 'docker version'. Please make sure Docker is running."
+
+  redis:
+    cmd: redis-cli -p 6379 PING
+    failMessage: 'Failed to connect to Redis on port 6379. Please make sure Redis is running.'
+
 commandsets:
   minimal:
     - frontend
@@ -102,6 +111,16 @@ commandsets:
     - gitserver
     - web
 
+  fancy:
+    checks:
+      - docker
+      - redis
+    commands:
+      - frontend
+      - repo-updater
+      - gitserver
+      - web
+
 tests:
   # These can be run with `sg test [name]`
   # Every command is run from the repository root.

sg.config.yaml

@@ -1,4 +1,11 @@
 env:
+  PGPORT: 5432
+  PGHOST: localhost
+  PGUSER: sourcegraph
+  PGPASSWORD: sourcegraph
+  PGDATABASE: sourcegraph
+  PGSSLMODE: disable
+
   SRC_REPOS_DIR: $HOME/.sourcegraph/repos
   SRC_LOG_LEVEL: info
   SRC_LOG_FORMAT: condensed
@@ -500,76 +507,91 @@ checks:
     failMessage: "Failed to run 'docker version'. Please make sure Docker is running."
 
   redis:
-    cmd: redis-cli -p 6379 PING
+    cmd: (command -v redis-cli && redis-cli -p 6379 PING) || docker-compose -f dev/redis-postgres.yml exec -T redis redis-cli PING
     failMessage: 'Failed to connect to Redis on port 6379. Please make sure Redis is running.'
 
   postgres:
-    cmd: psql -c 'SELECT 1;'
+    cmd: (command -v psql && psql -c 'SELECT 1;') || docker-compose -f dev/redis-postgres.yml exec -T postgresql psql -U ${PGUSER} -c 'select 1;'
     failMessage: 'Failed to connect to Postgres database. Make sure environment variables are setup correctly so that psql can connect.'
 
 commandsets:
   # TODO: Should we be able to define "env" vars _per set_?
 
   oss:
-    - frontend
-    - worker
-    - repo-updater
-    - gitserver
-    - searcher
-    - symbols
-    - query-runner
-    - web
-    - caddy
-    - docsite
-    - syntect_server
-    - github-proxy
-    - zoekt-indexserver-0
-    - zoekt-indexserver-1
-    - zoekt-webserver-0
-    - zoekt-webserver-1
+    checks:
+      - docker
+      - redis
+      - postgres
+    commands:
+      - frontend
+      - worker
+      - repo-updater
+      - gitserver
+      - searcher
+      - symbols
+      - query-runner
+      - web
+      - caddy
+      - docsite
+      - syntect_server
+      - github-proxy
+      - zoekt-indexserver-0
+      - zoekt-indexserver-1
+      - zoekt-webserver-0
+      - zoekt-webserver-1
 
   enterprise: &enterprise_set
-    - enterprise-frontend
-    - enterprise-worker
-    - enterprise-repo-updater
-    - enterprise-web
-    - gitserver
-    - searcher
-    - symbols
-    - query-runner
-    - caddy
-    - docsite
-    - syntect_server
-    - github-proxy
-    - zoekt-indexserver-0
-    - zoekt-indexserver-1
-    - zoekt-webserver-0
-    - zoekt-webserver-1
-    - executor-queue
+    checks:
+      - docker
+      - redis
+      - postgres
+    commands:
+      - enterprise-frontend
+      - enterprise-worker
+      - enterprise-repo-updater
+      - enterprise-web
+      - gitserver
+      - searcher
+      - symbols
+      - query-runner
+      - caddy
+      - docsite
+      - syntect_server
+      - github-proxy
+      - zoekt-indexserver-0
+      - zoekt-indexserver-1
+      - zoekt-webserver-0
+      - zoekt-webserver-1
+      - executor-queue
 
   default: *enterprise_set
 
   enterprise-codeintel:
-    - enterprise-frontend
-    - enterprise-worker
-    - enterprise-repo-updater
-    - enterprise-web
-    - gitserver
-    - searcher
-    - symbols
-    - query-runner
-    - caddy
-    - docsite
-    - syntect_server
-    - github-proxy
-    - zoekt-indexserver-0
-    - zoekt-indexserver-1
-    - zoekt-webserver-0
-    - zoekt-webserver-1
-    - minio
-    - executor-queue
-    - precise-code-intel-worker
-    - codeintel-executor
+    checks:
+      - docker
+      - redis
+      - postgres
+    commands:
+      - enterprise-frontend
+      - enterprise-worker
+      - enterprise-repo-updater
+      - enterprise-web
+      - gitserver
+      - searcher
+      - symbols
+      - query-runner
+      - caddy
+      - docsite
+      - syntect_server
+      - github-proxy
+      - zoekt-indexserver-0
+      - zoekt-indexserver-1
+      - zoekt-webserver-0
+      - zoekt-webserver-1
+      - minio
+      - executor-queue
+      - precise-code-intel-worker
+      - codeintel-executor
 
   enterprise-codeinsights:
     # Add the following overwrites to your sg.config.overwrite.yaml to get
@@ -579,56 +601,71 @@ commandsets:
     #     DISABLE_CODE_INSIGHTS_HISTORICAL: false
     #     DISABLE_CODE_INSIGHTS: false
     #
-    - enterprise-frontend
-    - enterprise-worker
-    - enterprise-repo-updater
-    - enterprise-web
-    - gitserver
-    - searcher
-    - symbols
-    - query-runner
-    - caddy
-    - docsite
-    - syntect_server
-    - github-proxy
-    - zoekt-indexserver-0
-    - zoekt-indexserver-1
-    - zoekt-webserver-0
-    - zoekt-webserver-1
-    - codeinsights-db
+    checks:
+      - docker
+      - redis
+      - postgres
+    commands:
+      - enterprise-frontend
+      - enterprise-worker
+      - enterprise-repo-updater
+      - enterprise-web
+      - gitserver
+      - searcher
+      - symbols
+      - query-runner
+      - caddy
+      - docsite
+      - syntect_server
+      - github-proxy
+      - zoekt-indexserver-0
+      - zoekt-indexserver-1
+      - zoekt-webserver-0
+      - zoekt-webserver-1
+      - codeinsights-db
 
   api-only:
-    - enterprise-frontend
-    - enterprise-worker
-    - enterprise-repo-updater
-    - gitserver
-    - searcher
-    - symbols
-    - github-proxy
-    - zoekt-indexserver-0
-    - zoekt-indexserver-1
-    - zoekt-webserver-0
-    - zoekt-webserver-1
+    checks:
+      - docker
+      - redis
+      - postgres
+    commands:
+      - enterprise-frontend
+      - enterprise-worker
+      - enterprise-repo-updater
+      - gitserver
+      - searcher
+      - symbols
+      - github-proxy
+      - zoekt-indexserver-0
+      - zoekt-indexserver-1
+      - zoekt-webserver-0
+      - zoekt-webserver-1
 
   batches:
-    - enterprise-frontend
-    - enterprise-worker
-    - enterprise-repo-updater
-    - enterprise-web
-    - gitserver
-    - searcher
-    - symbols
-    - query-runner
-    - caddy
-    - docsite
-    - syntect_server
-    - github-proxy
-    - zoekt-indexserver-0
-    - zoekt-indexserver-1
-    - zoekt-webserver-0
-    - zoekt-webserver-1
-    - executor-queue
-    - batches-executor
+    checks:
+      - docker
+      - redis
+      - postgres
+    commands:
+      - enterprise-frontend
+      - enterprise-worker
+      - enterprise-repo-updater
+      - enterprise-web
+      - gitserver
+      - searcher
+      - symbols
+      - query-runner
+      - caddy
+      - docsite
+      - syntect_server
+      - github-proxy
+      - zoekt-indexserver-0
+      - zoekt-indexserver-1
+      - zoekt-webserver-0
+      - zoekt-webserver-1
+      - executor-queue
+      - batches-executor
 
 tests:
   # These can be run with `sg test [name]`