better config verification

Verify cron values when parsing; also verify depends_on relationships,
meaning that we can assume later in the code that there are no cyclic
dependencies to get us into recursive trouble.

Also remove the `priority` field from the configuration as it's of marginal
use since we now have `depends_on` which wasn't present in the
original `supervisord` configuration from which this is derived.

cmd/gopm/testdata/cyclic-depends.txt

@@ -0,0 +1,30 @@
+# Test that a configuration with cyclic dependencies is an error.
+
+# Start it up. It should fail immediately with an error.
+gopm -c gopmconfig --quit-delay 0 &
+wait
+stderr 'Error loading configuration  	error:cyclic dependency involving ("b" and "a")|("a" and "b")'
+
+-- gopmconfig/config.cue --
+package config
+
+grpc_server: {
+	network: "unix"
+	address: "gopm.socket"
+}
+programs: {
+	a: {
+		command: " "
+		depends_on: ["b", "c"]
+	}
+	b: {
+		command: " "
+		depends_on: ["d", "a"]
+	}
+	c: {
+		command: " "
+	}
+	d: {
+		command: " "
+	}
+}

cmd/gopm/testdata/non-existent-depends.txt

@@ -0,0 +1,20 @@
+# Test that a configuration with dependencies that don't exist is an error
+
+# Start it up. It should fail immediately with an error.
+gopm -c gopmconfig --quit-delay 0 &
+wait
+stderr 'Error loading configuration  	error:program "a" has dependency on non-existent program "b"'
+
+-- gopmconfig/config.cue --
+package config
+
+grpc_server: {
+	network: "unix"
+	address: "gopm.socket"
+}
+programs: {
+	a: {
+		command: " "
+		depends_on: ["b"]
+	}
+}

config/config.go

@@ -13,6 +13,7 @@ import (
 	"cuelang.org/go/cue/cuecontext"
 	"cuelang.org/go/cue/errors"
 	"cuelang.org/go/cue/load"
+	"github.com/robfig/cron/v3"
 )
 
 // Load loads the configuration at the given directory and returns it.
@@ -48,6 +49,13 @@ func Load(configDir string, root string) (*Config, error) {
 	insts := load.Instances([]string{"."}, &load.Config{
 		Dir: filepath.Join(wd, configDir),
 	})
+	for _, inst := range insts {
+		if err := inst.Err; err != nil {
+			// TODO print to log file instead of directly to stderr.
+			errors.Print(os.Stderr, err, nil)
+			return nil, err
+		}
+	}
 	vals, err := ctx.BuildInstances(insts)
 	if err != nil {
 		return nil, fmt.Errorf("cannot build instances: %v", err)
@@ -98,9 +106,40 @@ func Load(configDir string, root string) (*Config, error) {
 	if err := val.Decode(&cfg); err != nil {
 		return nil, fmt.Errorf("cannot decode into config: %v", err)
 	}
+
+	if err := cfg.verifyDependencies(); err != nil {
+		return nil, err
+	}
 	return &cfg, nil
 }
 
+func (cfg *Config) verifyDependencies() error {
+	for _, p := range cfg.Programs {
+		if err := cfg.checkCycle(p, make(map[*Program]bool)); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func (cfg *Config) checkCycle(p *Program, visiting map[*Program]bool) error {
+	visiting[p] = true
+	for _, dep := range p.DependsOn {
+		p1 := cfg.Programs[dep]
+		if p1 == nil {
+			return fmt.Errorf("program %q has dependency on non-existent program %q", p.Name, dep)
+		}
+		if visiting[p1] {
+			return fmt.Errorf("cyclic dependency involving %q and %q", p.Name, p1.Name)
+		}
+		if err := cfg.checkCycle(p1, visiting); err != nil {
+			return err
+		}
+	}
+	visiting[p] = false
+	return nil
+}
+
 type localSchema struct {
 	// config holds the #Config definition
 	config cue.Value
@@ -173,11 +212,10 @@ type Program struct {
 	Environment             map[string]string `json:"environment"`
 	User                    string            `json:"user"`
 	ExitCodes               []int             `json:"exit_codes"`
-	Priority                int               `json:"priority"`
 	RestartPause            Duration          `json:"restart_pause"`
 	StartRetries            int               `json:"start_retries"`
 	StartSeconds            Duration          `json:"start_seconds"`
-	Cron                    string            `json:"cron"`
+	Cron                    CronSchedule      `json:"cron,omitempty"`
 	AutoStart               bool              `json:"auto_start"`
 	AutoRestart             *bool             `json:"auto_restart,omitempty"`
 	RestartDirectoryMonitor string            `json:"restart_directory_monitor"`
@@ -214,3 +252,28 @@ func (d *Duration) UnmarshalJSON(bytes []byte) error {
 	d.D = v
 	return nil
 }
+
+type CronSchedule struct {
+	Schedule cron.Schedule
+	String   string
+}
+
+func (sched *CronSchedule) UnmarshalJSON(data []byte) error {
+	var s string
+	if err := json.Unmarshal(data, &s); err != nil {
+		return err
+	}
+	if s == "" {
+		sched.Schedule = nil
+		sched.String = ""
+		return nil
+	}
+	parser := cron.NewParser(cron.Second | cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow | cron.Descriptor)
+	schedule, err := parser.Parse(s)
+	if err != nil {
+		return fmt.Errorf("cannot parse cron entry: %v", err)
+	}
+	sched.Schedule = schedule
+	sched.String = s
+	return nil
+}

config/schema.cue

@@ -87,12 +87,6 @@ import (
 	// user holds the username to run the process as.
 	user?: string
 
-	// priority holds the startup-order priority of the process.
-	// In the absence of a "depends_on" relationship, this
-	// determines the startup and shutdown order of processes.
-	// Higher priority programs will start up later and shut down earlier.
-	priority?:      int
-
 	// restart_pause holds the length of time to wait after a program
 	// has exited before auto-restarting it.
 	restart_pause?: int
@@ -158,7 +152,6 @@ import (
 		directory: *runtime.cwd | _
 		shell: *"/bin/sh" | _
 		exit_codes: *[0, 2] | _
-		priority: *999 | _
 		start_retries: *3 | _
 		start_seconds: *"1s" | _
 		auto_start: *true | _

process/process.go

@@ -169,17 +169,11 @@ func (p *process) updateConfig(config *config.Program) {
 // add this process to crontab
 func (p *process) addToCron() {
 	cfg := p.config
-	if cfg.Cron == "" {
+	if cfg.Cron.Schedule == nil {
 		return
 	}
-	schedule, err := cronParser.Parse(cfg.Cron)
-	if err != nil {
-		p.log.Error("Invalid cron entry", zap.String("cron", cfg.Cron), zap.Error(err))
-		return
-	}
-
-	p.log.Info("Scheduling process with cron", zap.String("cron", cfg.Cron))
-	id := scheduler.Schedule(schedule, cron.FuncJob(func() {
+	p.log.Info("Scheduling process with cron", zap.String("cron", cfg.Cron.String))
+	id := scheduler.Schedule(cfg.Cron.Schedule, cron.FuncJob(func() {
 		p.log.Debug("Running scheduled process")
 		if !p.isRunning() {
 			p.start(false)
@@ -190,16 +184,11 @@ func (p *process) addToCron() {
 }
 
 func (p *process) removeFromCron() {
-	if p.config != nil {
-		s := p.config.Cron
-		if len(s) == 0 {
-			return
-		}
+	if p.config.Cron.Schedule != nil {
+		p.log.Info("Removing process from cron schedule")
+		scheduler.Remove(p.cronID)
+		p.cronID = 0
 	}
-
-	p.log.Info("Removing process from cron schedule")
-	scheduler.Remove(p.cronID)
-	p.cronID = 0
 }
 
 // start start the process