From 80f3a516328712ce4b53bc15f2c85ef41caafbab Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Thu, 6 Apr 2017 16:52:31 -0400 Subject: [PATCH] Remove exec from Watches, add Status(Un)Healthy things Watch publishes --- config/config_test.go | 22 ++++++-- config/template_test.go | 12 ++--- config/testdata/test.json5 | 12 +++-- core/app_test.go | 11 +--- discovery/consul/consul.go | 11 ++-- discovery/consul/consul_test.go | 10 ++-- discovery/discovery.go | 2 +- .../fixtures/app/containerpilot.json5 | 22 ++++---- .../nginx/etc/nginx-with-consul.json5 | 17 ++++-- .../containerpilot.json5 | 21 ++++++-- .../tests/test_tasks/containerpilot.json5 | 21 ++++++-- tests/mocks/discovery.go | 15 ++++-- watches/config.go | 33 +----------- watches/config_test.go | 34 +++++------- watches/testdata/TestWatchesParse.json5 | 6 +-- watches/watches.go | 27 +++++----- watches/watches_test.go | 52 ++++++++----------- 17 files changed, 164 insertions(+), 164 deletions(-) diff --git a/config/config_test.go b/config/config_test.go index dd7214b5..2173f745 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -55,7 +55,7 @@ func TestValidConfigJobs(t *testing.T) { t.Fatalf("unexpected error in LoadConfig: %v", err) } - if len(cfg.Jobs) != 8 { + if len(cfg.Jobs) != 10 { t.Fatalf("expected 8 services but got %v", cfg.Jobs) } job0 := cfg.Jobs[0] @@ -103,8 +103,20 @@ func TestValidConfigJobs(t *testing.T) { assertEqual(t, job6.Restarts, nil, "expected '%v' for job6.Restarts but got '%v'") job7 := cfg.Jobs[7] - assertEqual(t, job7.Name, "containerpilot", "expected '%v' for job7.Name but got '%v'") - assertEqual(t, job7.Port, 9000, "expected '%v' for job7.Port but got '%v'") + assertEqual(t, job7.Name, "onChange-upstreamA", "expected '%v' for job7.Name but got '%v'") + assertEqual(t, job7.Port, 0, "expected '%v' for job7.Port but got '%v'") + assertEqual(t, job7.Frequency, "", "expected '%v' for job7.Frequency but got '%v'") + assertEqual(t, job7.Restarts, nil, "expected '%v' for job7.Restarts but got '%v'") + + job8 := cfg.Jobs[8] + assertEqual(t, job8.Name, "onChange-upstreamB", "expected '%v' for job8.Name but got '%v'") + assertEqual(t, job8.Port, 0, "expected '%v' for job8.Port but got '%v'") + assertEqual(t, job8.Frequency, "", "expected '%v' for job8.Frequency but got '%v'") + assertEqual(t, job8.Restarts, nil, "expected '%v' for job8.Restarts but got '%v'") + + job9 := cfg.Jobs[9] + assertEqual(t, job9.Name, "containerpilot", "expected '%v' for job9.Name but got '%v'") + assertEqual(t, job9.Port, 9000, "expected '%v' for job9.Port but got '%v'") } // telemetry.Config @@ -136,10 +148,10 @@ func TestValidConfigWatches(t *testing.T) { } watch0 := cfg.Watches[0] watch1 := cfg.Watches[1] - assertEqual(t, watch0.Name, "upstreamA.watch", "expected '%v' for Name, but got '%v'") + assertEqual(t, watch0.Name, "watch.upstreamA", "expected '%v' for Name, but got '%v'") assertEqual(t, watch0.Poll, 11, "expected '%v' for Poll, but got '%v'") assertEqual(t, watch0.Tag, "dev", "expected '%v' for Tag, but got '%v'") - assertEqual(t, watch1.Name, "upstreamB.watch", "expected '%v' for Name, but got '%v'") + assertEqual(t, watch1.Name, "watch.upstreamB", "expected '%v' for Name, but got '%v'") assertEqual(t, watch1.Poll, 79, "expected '%v' for Poll, but got '%v'") assertEqual(t, watch1.Tag, "", "expected '%v' for Tag, but got '%v'") diff --git a/config/template_test.go b/config/template_test.go index 9dd6342e..999a9ce5 100644 --- a/config/template_test.go +++ b/config/template_test.go @@ -50,10 +50,7 @@ func TestRenderConfigFileStdout(t *testing.T) { var testJSON = `{ "consul": "consul:8500", - "backends": [{ - "name": "upstreamA", - "poll": 11, - "onChange": "/bin/to/onChangeEvent/for/upstream/A.sh"}]}` + "backends": [{"name": "upstreamA", "poll": 11}]}` // Render to file defer os.Remove("testJSON.json") @@ -86,10 +83,7 @@ func TestRenderedConfigIsParseable(t *testing.T) { var testJSON = `{ "consul": "consul:8500", - "backends": [{ - "name": "upstreamA{{.TESTRENDERCONFIGISPARSEABLE}}", - "poll": 11, - "onChange": "/bin/to/onChangeEvent/for/upstream/A.sh"}]}` + "backends": [{"name": "upstreamA{{.TESTRENDERCONFIGISPARSEABLE}}", "poll": 11}]}` os.Setenv("TESTRENDERCONFIGISPARSEABLE", "-ok") template, _ := renderConfigTemplate(testJSON) @@ -98,7 +92,7 @@ func TestRenderedConfigIsParseable(t *testing.T) { t.Fatalf("unexpected error in LoadConfig: %v", err) } name := config.Watches[0].Name - if name != "upstreamA-ok.watch" { + if name != "watch.upstreamA-ok" { t.Fatalf("expected Watches[0] name to be upstreamA-ok but got %s", name) } } diff --git a/config/testdata/test.json5 b/config/testdata/test.json5 index 066931e6..8db28179 100644 --- a/config/testdata/test.json5 +++ b/config/testdata/test.json5 @@ -55,19 +55,25 @@ source: "serviceA", event: "stopped" } + }, + { + name: "onChange-upstreamA", + exec: "/bin/to/onChangeEvent/for/upstream/A.sh {{.TEST}}", + }, + { + name: "onChange-upstreamB", + exec: "/bin/to/onChangeEvent/for/upstream/B.sh {{.ENV_NOT_FOUND}}" } ], "backends": [ { "name": "upstreamA", "poll": 11, - "onChange": "/bin/to/onChangeEvent/for/upstream/A.sh {{.TEST}}", "tag": "dev" }, { "name": "upstreamB", - "poll": 79, - "onChange": "/bin/to/onChangeEvent/for/upstream/B.sh {{.ENV_NOT_FOUND}}" + "poll": 79 } ], "telemetry": { diff --git a/core/app_test.go b/core/app_test.go index ba04f4f3..e29068fa 100644 --- a/core/app_test.go +++ b/core/app_test.go @@ -39,19 +39,12 @@ func TestServiceConfigRequiredFields(t *testing.T) { func TestBackendConfigRequiredFields(t *testing.T) { // Missing `name` - var testJSON = `{"consul": "consul:8500", "backends": [ - {"name": "", "poll": 30, "onChange": "true"}]}` + var testJSON = `{"consul": "consul:8500", "backends": [{"name": "", "poll": 30}]}` validateParseError(t, testJSON, []string{"`name`"}) // Missing `poll` - testJSON = `{"consul": "consul:8500", "backends": [ - {"name": "name", "onChange": "true"}]}` + testJSON = `{"consul": "consul:8500", "backends": [{"name": "name"}]}` validateParseError(t, testJSON, []string{"`poll`"}) - - // Missing `onChange` - testJSON = `{"consul": "consul:8500", "backends": [ - {"name": "name", "poll": 19 }]}` - validateParseError(t, testJSON, []string{"`onChange`"}) } func TestInvalidConfigNoConfigFlag(t *testing.T) { diff --git a/discovery/consul/consul.go b/discovery/consul/consul.go index 89d6f5df..2182ad5a 100644 --- a/discovery/consul/consul.go +++ b/discovery/consul/consul.go @@ -175,19 +175,22 @@ func (c *Consul) registerCheck(service discovery.ServiceDefinition) error { var upstreams = make(map[string][]*consul.ServiceEntry) // CheckForUpstreamChanges runs the health check -func (c Consul) CheckForUpstreamChanges(backendName, backendTag string) bool { +func (c Consul) CheckForUpstreamChanges(backendName, backendTag string) (didChange, isHealthy bool) { services, meta, err := c.Health().Service(backendName, backendTag, true, nil) if err != nil { log.Warnf("failed to query %v: %s [%v]", backendName, err, meta) - return false + return false, false } - didChange := compareForChange(upstreams[backendName], services) + if len(services) > 0 { + isHealthy = true + } + didChange = compareForChange(upstreams[backendName], services) if didChange || len(services) == 0 { // We don't want to cause an onChange event the first time we read-in // but we do want to make sure we've written the key for this map upstreams[backendName] = services } - return didChange + return didChange, isHealthy } // Compare the two arrays to see if the address or port has changed diff --git a/discovery/consul/consul_test.go b/discovery/consul/consul_test.go index 5d115c6c..d2626065 100644 --- a/discovery/consul/consul_test.go +++ b/discovery/consul/consul_test.go @@ -104,19 +104,19 @@ func testConsulCheckForChanges(t *testing.T) { consul, _ := NewConsulConfig(testServer.HTTPAddr) service := generateServiceDefinition(backend) id := service.ID - if consul.CheckForUpstreamChanges(backend, "") { + if changed, _ := consul.CheckForUpstreamChanges(backend, ""); changed { t.Fatalf("First read of %s should show `false` for change", id) } consul.SendHeartbeat(service) // force registration and 1st heartbeat - if !consul.CheckForUpstreamChanges(backend, "") { + if changed, _ := consul.CheckForUpstreamChanges(backend, ""); !changed { t.Errorf("%v should have changed after first health check TTL", id) } - if consul.CheckForUpstreamChanges(backend, "") { + if changed, _ := consul.CheckForUpstreamChanges(backend, ""); changed { t.Errorf("%v should not have changed without TTL expiring", id) } consul.Agent().UpdateTTL(id, "expired", "critical") - if !consul.CheckForUpstreamChanges(backend, "") { + if changed, _ := consul.CheckForUpstreamChanges(backend, ""); !changed { t.Errorf("%v should have changed after TTL expired.", id) } } @@ -135,7 +135,7 @@ func testConsulEnableTagOverride(t *testing.T) { }, } id := service.ID - if consul.CheckForUpstreamChanges(backend, "") { + if changed, _ := consul.CheckForUpstreamChanges(backend, ""); changed { t.Fatalf("First read of %s should show `false` for change", id) } consul.SendHeartbeat(service) // force registration diff --git a/discovery/discovery.go b/discovery/discovery.go index 26d4c3c6..f55a0dbd 100644 --- a/discovery/discovery.go +++ b/discovery/discovery.go @@ -5,7 +5,7 @@ import log "github.com/Sirupsen/logrus" // Backend is an interface which all service discovery backends must implement type Backend interface { SendHeartbeat(service *ServiceDefinition) - CheckForUpstreamChanges(backendName string, backendTag string) bool + CheckForUpstreamChanges(backendName string, backendTag string) (bool, bool) MarkForMaintenance(service *ServiceDefinition) Deregister(service *ServiceDefinition) } diff --git a/integration_tests/fixtures/app/containerpilot.json5 b/integration_tests/fixtures/app/containerpilot.json5 index f1f955be..c6d263b8 100644 --- a/integration_tests/fixtures/app/containerpilot.json5 +++ b/integration_tests/fixtures/app/containerpilot.json5 @@ -25,32 +25,30 @@ "exec": "/reload-app.sh" }, { - "name": "reload-for-nginx", - "when": { - "source": "watch.nginx", - "event": "changed" + name: "reload-for-nginx", + when: { + source: "watch.nginx", + event: "changed" }, - "exec": "/reload-app.sh" + exec: "/reload-app.sh" }, { - "name": "reload-for-app", - "when": { - "source": "watch.app", - "event": "changed" + name: "reload-for-app", + when: { + source: "watch.app", + event: "changed" }, - "exec": "/reload-app.sh" + exec: "/reload-app.sh" } ], "backends": [ { "name": "nginx", "poll": 7, - "onChange": "/reload-app.sh" }, { "name": "app", "poll": 5, - "onChange": "/reload-app.sh", "tag": "application" } ], diff --git a/integration_tests/fixtures/nginx/etc/nginx-with-consul.json5 b/integration_tests/fixtures/nginx/etc/nginx-with-consul.json5 index 76b72381..0d91855d 100644 --- a/integration_tests/fixtures/nginx/etc/nginx-with-consul.json5 +++ b/integration_tests/fixtures/nginx/etc/nginx-with-consul.json5 @@ -23,16 +23,23 @@ "exec": ["consul-template", "-once", "-retry", "3s", "-consul", "consul:8500", "-template", "/etc/nginx-consul.ctmpl:/etc/nginx/nginx.conf"], + }, + { + name: "onChange", + when: { + source: "watch.app", + event: "changed" + }, + exec: [ + "consul-template", "-once", "-consul", "consul:8500", "-template", + "/etc/nginx-consul.ctmpl:/etc/nginx/nginx.conf:nginx -s reload" + ] } ], "backends": [ { "name": "app", - "poll": 1, - "onChange": [ - "consul-template", "-once", "-consul", "consul:8500", "-template", - "/etc/nginx-consul.ctmpl:/etc/nginx/nginx.conf:nginx -s reload" - ] + "poll": 1 } ] } diff --git a/integration_tests/tests/test_discovery_consul/containerpilot.json5 b/integration_tests/tests/test_discovery_consul/containerpilot.json5 index 273364bd..7d178f97 100644 --- a/integration_tests/tests/test_discovery_consul/containerpilot.json5 +++ b/integration_tests/tests/test_discovery_consul/containerpilot.json5 @@ -23,19 +23,32 @@ { name: "preStart", exec: "/reload-app.sh" + }, + { + name: "reload-for-nginx", + when: { + source: "watch.nginx", + event: "changed" + }, + exec: "/reload-app.sh" + }, + { + name: "reload-for-app", + when: { + source: "watch.app", + event: "changed" + }, + exec: "/reload-app.sh" } ], "backends": [ { "name": "nginx", "poll": 7, - "onChange": "/reload-app.sh" }, { "name": "app", - "poll": 5, - "onChange": "/reload-app.sh", - "tag": "application" + "poll": 5 } ] } diff --git a/integration_tests/tests/test_tasks/containerpilot.json5 b/integration_tests/tests/test_tasks/containerpilot.json5 index 3e73ac2c..81c5b9cc 100644 --- a/integration_tests/tests/test_tasks/containerpilot.json5 +++ b/integration_tests/tests/test_tasks/containerpilot.json5 @@ -42,19 +42,32 @@ { name: "preStart", exec: "/reload-app.sh" + }, + { + name: "reload-for-nginx", + when: { + source: "watch.nginx", + event: "changed" + }, + exec: "/reload-app.sh" + }, + { + name: "reload-for-app", + when: { + source: "watch.app", + event: "changed" + }, + exec: "/reload-app.sh" } ], "backends": [ { "name": "nginx", "poll": 7, - "onChange": "/reload-app.sh" }, { "name": "app", - "poll": 5, - "onChange": "/reload-app.sh", - "tag": "application" + "poll": 5 } ], "telemetry": { diff --git a/tests/mocks/discovery.go b/tests/mocks/discovery.go index 53eb49ce..292f3414 100644 --- a/tests/mocks/discovery.go +++ b/tests/mocks/discovery.go @@ -4,7 +4,8 @@ import "github.com/joyent/containerpilot/discovery" // NoopDiscoveryBackend is a mock discovery.Backend type NoopDiscoveryBackend struct { - Val bool + Val bool + lastVal bool } // SendHeartbeat (required for mock interface) @@ -13,9 +14,15 @@ func (noop *NoopDiscoveryBackend) SendHeartbeat(service *discovery.ServiceDefini } // CheckForUpstreamChanges will return the public Val field to mock -// whether a change has occurred. -func (noop *NoopDiscoveryBackend) CheckForUpstreamChanges(backend, tag string) bool { - return noop.Val +// whether a change has occurred. Will not report a change on the second +// check unless the Val has been updated externally by the test rig +func (noop *NoopDiscoveryBackend) CheckForUpstreamChanges(backend, tag string) (didChange, isHealthy bool) { + if noop.lastVal != noop.Val { + didChange = true + } + noop.lastVal = noop.Val + isHealthy = noop.Val + return didChange, isHealthy } // MarkForMaintenance (required for mock interface) diff --git a/watches/config.go b/watches/config.go index 471f7e59..fbd83bbe 100644 --- a/watches/config.go +++ b/watches/config.go @@ -2,10 +2,7 @@ package watches import ( "fmt" - "time" - log "github.com/Sirupsen/logrus" - "github.com/joyent/containerpilot/commands" "github.com/joyent/containerpilot/discovery" "github.com/joyent/containerpilot/utils" ) @@ -14,12 +11,8 @@ import ( type Config struct { Name string `mapstructure:"name"` serviceName string - Poll int `mapstructure:"poll"` // time in seconds - Exec interface{} `mapstructure:"onChange"` - exec *commands.Command + Poll int `mapstructure:"poll"` // time in seconds Tag string `mapstructure:"tag"` - Timeout string `mapstructure:"timeout"` - timeout time.Duration discoveryService discovery.Backend } @@ -47,33 +40,11 @@ func (cfg *Config) Validate(disc discovery.Backend) error { } cfg.serviceName = cfg.Name - cfg.Name = cfg.Name + ".watch" - - if cfg.Exec == nil { - // TODO v3: this error message is tied to existing config syntax - return fmt.Errorf("`onChange` is required in watch %s", cfg.serviceName) - } - if cfg.Timeout == "" { - cfg.Timeout = fmt.Sprintf("%ds", cfg.Poll) - } - timeout, err := utils.GetTimeout(cfg.Timeout) - if err != nil { - return fmt.Errorf("could not parse `timeout` in watch %s: %v", cfg.serviceName, err) - } - cfg.timeout = timeout + cfg.Name = "watch." + cfg.Name if cfg.Poll < 1 { return fmt.Errorf("`poll` must be > 0 in watch %s", cfg.serviceName) } - cmd, err := commands.NewCommand(cfg.Exec, cfg.timeout, - log.Fields{"watch": cfg.Name}) - if err != nil { - // TODO v3: this error message is tied to existing config syntax - return fmt.Errorf("could not parse `onChange` in watch %s: %s", - cfg.serviceName, err) - } - cmd.Name = cfg.Name - cfg.exec = cmd cfg.discoveryService = disc return nil } diff --git a/watches/config_test.go b/watches/config_test.go index 02270949..6aeaae8e 100644 --- a/watches/config_test.go +++ b/watches/config_test.go @@ -16,14 +16,19 @@ func TestWatchesParse(t *testing.T) { if err != nil { t.Fatal(err) } - assert.Equal(t, watches[0].exec.Exec, "/bin/upstreamA.sh", - "expected %v for exec.Exec got %v") - assert.Equal(t, watches[0].exec.Args, []string{"A1", "A2"}, - "expected %v for exec.Args got %v") - assert.Equal(t, watches[1].exec.Exec, "/bin/upstreamB.sh", - "expected %v for exec.Exec got %v") - assert.Equal(t, watches[1].exec.Args, []string{"B1", "B2"}, - "expected %v for exec.Args got %v") + assert.Equal(t, watches[0].serviceName, "upstreamA", + "expected %v for serviceName got %v") + assert.Equal(t, watches[0].Name, "watch.upstreamA", + "expected %v for Name got %v") + assert.Equal(t, watches[0].Poll, 11, + "expected %v for Poll got %v") + + assert.Equal(t, watches[1].serviceName, "upstreamB", + "expected %v for serviceName got %v") + assert.Equal(t, watches[1].Name, "watch.upstreamB", + "expected %v for Name got %v") + assert.Equal(t, watches[1].Poll, 79, + "expected %v for Poll got %v") } func TestWatchesConfigError(t *testing.T) { @@ -31,18 +36,7 @@ func TestWatchesConfigError(t *testing.T) { _, err := NewConfigs(tests.DecodeRawToSlice(`[{"name": ""}]`), nil) assert.Error(t, err, "`name` must not be blank") - _, err = NewConfigs(tests.DecodeRawToSlice(`[{"name": "myName"}]`), nil) - assert.Error(t, err, "`onChange` is required in watch myName") - - _, err = NewConfigs(tests.DecodeRawToSlice(`[{"name": "myName", "onChange": "", "poll": 1}]`), nil) - assert.Error(t, err, "could not parse `onChange` in watch myName: received zero-length argument") - - _, err = NewConfigs(tests.DecodeRawToSlice( - `[{"name": "myName", "onChange": "true", "poll": 1, "timeout": "xx"}]`), nil) - assert.Error(t, err, - "could not parse `timeout` in watch myName: time: invalid duration xx") - _, err = NewConfigs(tests.DecodeRawToSlice( - `[{"name": "myName", "onChange": "true", "timeout": ""}]`), nil) + `[{"name": "myName"}]`), nil) assert.Error(t, err, "`poll` must be > 0 in watch myName") } diff --git a/watches/testdata/TestWatchesParse.json5 b/watches/testdata/TestWatchesParse.json5 index 0163b50f..a96e0fee 100644 --- a/watches/testdata/TestWatchesParse.json5 +++ b/watches/testdata/TestWatchesParse.json5 @@ -2,12 +2,10 @@ { "name": "upstreamA", "poll": 11, - "onChange": ["/bin/upstreamA.sh", "A1", "A2"], "tag": "dev" }, { - "name": "upstreamB", - "poll": 79, - "onChange": "/bin/upstreamB.sh B1 B2" + name: "upstreamB", + poll: 79 } ] diff --git a/watches/watches.go b/watches/watches.go index ae722005..3dc4cc0e 100644 --- a/watches/watches.go +++ b/watches/watches.go @@ -5,20 +5,17 @@ import ( "fmt" "time" - "github.com/joyent/containerpilot/commands" "github.com/joyent/containerpilot/discovery" "github.com/joyent/containerpilot/events" ) const eventBufferSize = 1000 -// Watch represents a task to execute when something changes +// Watch represents an event to signal when something changes type Watch struct { Name string serviceName string tag string - exec *commands.Command - startupTimeout int // TODO v3: we don't have configuration for this yet poll int discoveryService discovery.Backend @@ -31,7 +28,6 @@ func NewWatch(cfg *Config) *Watch { Name: cfg.Name, serviceName: cfg.serviceName, tag: cfg.Tag, - exec: cfg.exec, poll: cfg.Poll, discoveryService: cfg.discoveryService, } @@ -52,15 +48,10 @@ func FromConfigs(cfgs []*Config) []*Watch { // CheckForUpstreamChanges checks the service discovery endpoint for any changes // in a dependent backend. Returns true when there has been a change. -func (watch *Watch) CheckForUpstreamChanges() bool { +func (watch *Watch) CheckForUpstreamChanges() (bool, bool) { return watch.discoveryService.CheckForUpstreamChanges(watch.serviceName, watch.tag) } -// OnChange runs the Watch's executable -func (watch *Watch) OnChange(ctx context.Context) { - watch.exec.Run(ctx, watch.Bus) -} - // Run executes the event loop for the Watch func (watch *Watch) Run(bus *events.EventBus) { watch.Subscribe(bus) @@ -76,9 +67,16 @@ func (watch *Watch) Run(bus *events.EventBus) { event := <-watch.Rx switch event { case events.Event{events.TimerExpired, timerSource}: - changed := watch.CheckForUpstreamChanges() - if changed { - watch.OnChange(ctx) + didChange, isHealthy := watch.CheckForUpstreamChanges() + if didChange { + watch.Bus.Publish(events.Event{events.StatusChanged, watch.Name}) + // we only send the StatusHealthy and StatusUnhealthy + // events if there was a change + if isHealthy { + watch.Bus.Publish(events.Event{events.StatusHealthy, watch.Name}) + } else { + watch.Bus.Publish(events.Event{events.StatusUnhealthy, watch.Name}) + } } case events.Event{events.Quit, watch.Name}, @@ -88,7 +86,6 @@ func (watch *Watch) Run(bus *events.EventBus) { close(watch.Rx) cancel() watch.Flush <- true - watch.exec.CloseLogs() return } } diff --git a/watches/watches_test.go b/watches/watches_test.go index f4291581..c7760870 100644 --- a/watches/watches_test.go +++ b/watches/watches_test.go @@ -4,52 +4,46 @@ import ( "fmt" "testing" - log "github.com/Sirupsen/logrus" + "github.com/joyent/containerpilot/discovery" "github.com/joyent/containerpilot/events" "github.com/joyent/containerpilot/tests/mocks" ) -var noop = &mocks.NoopDiscoveryBackend{Val: true} - -func TestWatchExecOk(t *testing.T) { - log.SetLevel(log.WarnLevel) // suppress test noise +func TestWatchPollOk(t *testing.T) { cfg := &Config{ - Name: "mywatchOk", - Exec: "./testdata/test.sh doStuff --debug", - Timeout: "100ms", - Poll: 1, + Name: "mywatchOk", + Poll: 1, } - got := runWatchTest(cfg, 5) - poll := events.Event{events.TimerExpired, "mywatchOk.watch.poll"} - exitOk := events.Event{events.ExitSuccess, "mywatchOk.watch"} - if got[exitOk] != 2 || got[poll] != 2 || got[events.QuitByClose] != 1 { - t.Fatalf("expected 2 successful poll events but got %v", got) + // this discovery backend will always return true when we check + // it for changed + got := runWatchTest(cfg, 5, &mocks.NoopDiscoveryBackend{Val: true}) + poll := events.Event{events.TimerExpired, "watch.mywatchOk.poll"} + changed := events.Event{events.StatusChanged, "watch.mywatchOk"} + healthy := events.Event{events.StatusHealthy, "watch.mywatchOk"} + if got[changed] != 1 || got[poll] != 2 || got[healthy] != 1 { + t.Fatalf("expected 2 successful StatusHealthy events but got %v", got) } } -func TestWatchExecFail(t *testing.T) { - log.SetLevel(log.WarnLevel) // suppress test noise +func TestWatchPollFail(t *testing.T) { cfg := &Config{ - Name: "mywatchFail", - Exec: "./testdata/test.sh failStuff", - Timeout: "100ms", - Poll: 1, + Name: "mywatchFail", + Poll: 1, } - got := runWatchTest(cfg, 7) - poll := events.Event{events.TimerExpired, "mywatchFail.watch.poll"} - exitOk := events.Event{events.ExitFailed, "mywatchFail.watch"} - errMsg := events.Event{events.Error, "mywatchFail.watch: exit status 255"} - if got[exitOk] != 2 || got[poll] != 2 || - got[events.QuitByClose] != 1 || got[errMsg] != 2 { - t.Fatalf("expected 2 failed poll events but got %v", got) + got := runWatchTest(cfg, 3, &mocks.NoopDiscoveryBackend{Val: false}) + poll := events.Event{events.TimerExpired, "watch.mywatchFail.poll"} + changed := events.Event{events.StatusChanged, "watch.mywatchFail"} + unhealthy := events.Event{events.StatusUnhealthy, "watch.mywatchFail"} + if got[changed] != 0 || got[poll] != 2 || got[unhealthy] != 0 { + t.Fatalf("expected 2 failed poll events without changes, but got %v", got) } } -func runWatchTest(cfg *Config, count int) map[events.Event]int { +func runWatchTest(cfg *Config, count int, disc discovery.Backend) map[events.Event]int { bus := events.NewEventBus() ds := mocks.NewDebugSubscriber(bus, count) ds.Run(0) - cfg.Validate(noop) + cfg.Validate(disc) watch := NewWatch(cfg) watch.Run(bus)