From eb7c78b05339927447b7ad9a331af2097e580e6d Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Thu, 6 Apr 2017 11:53:47 -0400 Subject: [PATCH 1/2] implement 'when' config, eliminating preStart/preStop/postStop --- config/testdata/test.json5 | 8 -------- 1 file changed, 8 deletions(-) diff --git a/config/testdata/test.json5 b/config/testdata/test.json5 index 8db28179..91fe62c1 100644 --- a/config/testdata/test.json5 +++ b/config/testdata/test.json5 @@ -55,14 +55,6 @@ 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": [ From 1382e23245af05aa72992e0b9d1c923a050910be Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Fri, 7 Apr 2017 09:28:40 -0400 Subject: [PATCH 2/2] rename 'services' config field to 'jobs', 'backends' to 'watches' --- config/config.go | 8 ++++---- config/config_test.go | 6 +++--- config/template_test.go | 4 ++-- config/testdata/test.json5 | 8 +++++--- core/app_test.go | 12 ++++++------ .../app/containerpilot-with-coprocess.json5 | 2 +- .../fixtures/app/containerpilot.json5 | 4 ++-- .../fixtures/nginx/etc/nginx-with-consul.json5 | 4 ++-- integration_tests/fixtures/zombie_maker/app.json5 | 2 +- .../test_discovery_consul/containerpilot.json5 | 4 ++-- .../tests/test_envvars/containerpilot.json5 | 2 +- .../tests/test_tasks/containerpilot.json5 | 4 ++-- .../tests/test_telemetry/containerpilot.json5 | 2 +- jobs/config.go | 8 ++++---- jobs/config_test.go | 14 +++++++------- jobs/jobs.go | 6 +++--- 16 files changed, 46 insertions(+), 44 deletions(-) diff --git a/config/config.go b/config/config.go index 61cd7c15..89d24db7 100644 --- a/config/config.go +++ b/config/config.go @@ -289,15 +289,15 @@ func decodeConfig(configMap map[string]interface{}, result *rawConfig) error { result.stopTimeout = stopTimeout result.logConfig = &logConfig result.control = configMap["control"] - result.jobs = decodeArray(configMap["services"]) - result.watches = decodeArray(configMap["backends"]) + result.jobs = decodeArray(configMap["jobs"]) + result.watches = decodeArray(configMap["watches"]) result.telemetry = configMap["telemetry"] delete(configMap, "logging") delete(configMap, "control") delete(configMap, "stopTimeout") - delete(configMap, "services") - delete(configMap, "backends") + delete(configMap, "jobs") + delete(configMap, "watches") delete(configMap, "telemetry") var unused []string for key := range configMap { diff --git a/config/config_test.go b/config/config_test.go index 2173f745..13877fe6 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -47,7 +47,7 @@ func TestValidConfigHealthChecks(t *testing.T) { assertEqual(t, check1.Timeout, "2s", "expected '%v' for check1.Timeout, but got '%v'") } -// services.Config +// jobs.Config func TestValidConfigJobs(t *testing.T) { os.Setenv("TEST", "HELLO") cfg, err := LoadConfig(testJSON) @@ -56,7 +56,7 @@ func TestValidConfigJobs(t *testing.T) { } if len(cfg.Jobs) != 10 { - t.Fatalf("expected 8 services but got %v", cfg.Jobs) + t.Fatalf("expected 8 jobs but got %v", cfg.Jobs) } job0 := cfg.Jobs[0] assertEqual(t, job0.Name, "serviceA", "expected '%v' for job0.Name but got '%v'") @@ -79,7 +79,7 @@ func TestValidConfigJobs(t *testing.T) { assertEqual(t, job2.Restarts, "unlimited", "expected '%v' for job2.Restarts but got '%v'") job3 := cfg.Jobs[3] - assertEqual(t, job3.Name, "taskD", "expected '%v' for job3.Name but got '%v'") + assertEqual(t, job3.Name, "periodicTaskD", "expected '%v' for job3.Name but got '%v'") assertEqual(t, job3.Port, 0, "expected '%v' for job3.Port but got '%v'") assertEqual(t, job3.Frequency, "1s", "expected '%v' for job3.Frequency but got '%v'") assertEqual(t, job3.Restarts, nil, "expected '%v' for job3.Restarts but got '%v'") diff --git a/config/template_test.go b/config/template_test.go index 999a9ce5..1c8bd0de 100644 --- a/config/template_test.go +++ b/config/template_test.go @@ -50,7 +50,7 @@ func TestRenderConfigFileStdout(t *testing.T) { var testJSON = `{ "consul": "consul:8500", - "backends": [{"name": "upstreamA", "poll": 11}]}` + watches: [{"name": "upstreamA", "poll": 11}]}` // Render to file defer os.Remove("testJSON.json") @@ -83,7 +83,7 @@ func TestRenderedConfigIsParseable(t *testing.T) { var testJSON = `{ "consul": "consul:8500", - "backends": [{"name": "upstreamA{{.TESTRENDERCONFIGISPARSEABLE}}", "poll": 11}]}` + watches: [{"name": "upstreamA{{.TESTRENDERCONFIGISPARSEABLE}}", "poll": 11}]}` os.Setenv("TESTRENDERCONFIGISPARSEABLE", "-ok") template, _ := renderConfigTemplate(testJSON) diff --git a/config/testdata/test.json5 b/config/testdata/test.json5 index 91fe62c1..7c9a3061 100644 --- a/config/testdata/test.json5 +++ b/config/testdata/test.json5 @@ -1,8 +1,10 @@ { "consul": "consul:8500", "stopTimeout": 5, - "services": [ + "jobs": [ { + // although these are all jobs, we're naming these jobs "service", + // "coprocess", "task", "prestart", etc. to make their role clear "name": "serviceA", "port": 8080, "interfaces": "inet", @@ -32,7 +34,7 @@ "restarts": "unlimited" }, { - "name": "taskD", + "name": "periodicTaskD", "exec": "/bin/taskD", "frequency": "1s" }, @@ -57,7 +59,7 @@ } } ], - "backends": [ + watches: [ { "name": "upstreamA", "poll": 11, diff --git a/core/app_test.go b/core/app_test.go index e29068fa..8e09cfe0 100644 --- a/core/app_test.go +++ b/core/app_test.go @@ -18,32 +18,32 @@ TODO v3: a LOT of the these tests should be moved to the config package func TestServiceConfigRequiredFields(t *testing.T) { // Missing `name` - var testJSON = `{"consul": "consul:8500", "services": [ + var testJSON = `{"consul": "consul:8500", jobs: [ {"name": "", "port": 8080, "poll": 30, "ttl": 19 }]}` validateParseError(t, testJSON, []string{"`name`"}) // Missing `poll` - testJSON = `{"consul": "consul:8500", "services": [ + testJSON = `{"consul": "consul:8500", jobs: [ {"name": "name", "port": 8080, "ttl": 19}]}` validateParseError(t, testJSON, []string{"`poll`"}) // Missing `ttl` - testJSON = `{"consul": "consul:8500", "services": [ + testJSON = `{"consul": "consul:8500", jobs: [ {"name": "name", "port": 8080, "poll": 19}]}` validateParseError(t, testJSON, []string{"`ttl`"}) - testJSON = `{"consul": "consul:8500", "services": [ + testJSON = `{"consul": "consul:8500", jobs: [ {"name": "name", "poll": 19, "ttl": 19}]}` validateParseError(t, testJSON, []string{"`port`"}) } func TestBackendConfigRequiredFields(t *testing.T) { // Missing `name` - var testJSON = `{"consul": "consul:8500", "backends": [{"name": "", "poll": 30}]}` + var testJSON = `{"consul": "consul:8500", watches: [{"name": "", "poll": 30}]}` validateParseError(t, testJSON, []string{"`name`"}) // Missing `poll` - testJSON = `{"consul": "consul:8500", "backends": [{"name": "name"}]}` + testJSON = `{"consul": "consul:8500", watches: [{"name": "name"}]}` validateParseError(t, testJSON, []string{"`poll`"}) } diff --git a/integration_tests/fixtures/app/containerpilot-with-coprocess.json5 b/integration_tests/fixtures/app/containerpilot-with-coprocess.json5 index 936bb693..350f8e59 100644 --- a/integration_tests/fixtures/app/containerpilot-with-coprocess.json5 +++ b/integration_tests/fixtures/app/containerpilot-with-coprocess.json5 @@ -4,7 +4,7 @@ "level": "DEBUG", "format": "text" }, - "services": [ + jobs: [ { "name": "app", "port": 8000, diff --git a/integration_tests/fixtures/app/containerpilot.json5 b/integration_tests/fixtures/app/containerpilot.json5 index c6d263b8..3f109a89 100644 --- a/integration_tests/fixtures/app/containerpilot.json5 +++ b/integration_tests/fixtures/app/containerpilot.json5 @@ -4,7 +4,7 @@ "level": "DEBUG", "format": "text" }, - "services": [ + jobs: [ { "name": "app", "port": 8000, @@ -41,7 +41,7 @@ exec: "/reload-app.sh" } ], - "backends": [ + watches: [ { "name": "nginx", "poll": 7, diff --git a/integration_tests/fixtures/nginx/etc/nginx-with-consul.json5 b/integration_tests/fixtures/nginx/etc/nginx-with-consul.json5 index 0d91855d..13f9b6b8 100644 --- a/integration_tests/fixtures/nginx/etc/nginx-with-consul.json5 +++ b/integration_tests/fixtures/nginx/etc/nginx-with-consul.json5 @@ -4,7 +4,7 @@ "level": "DEBUG", "format": "text" }, - "services": [ + jobs: [ { "name": "nginx", "port": 80, @@ -36,7 +36,7 @@ ] } ], - "backends": [ + watches: [ { "name": "app", "poll": 1 diff --git a/integration_tests/fixtures/zombie_maker/app.json5 b/integration_tests/fixtures/zombie_maker/app.json5 index 7afae7e5..e351d4b6 100644 --- a/integration_tests/fixtures/zombie_maker/app.json5 +++ b/integration_tests/fixtures/zombie_maker/app.json5 @@ -1,6 +1,6 @@ { "consul": "consul:8500", - "services": [ + jobs: [ { "name": "zombies", "port": 8000, diff --git a/integration_tests/tests/test_discovery_consul/containerpilot.json5 b/integration_tests/tests/test_discovery_consul/containerpilot.json5 index 7d178f97..d093694b 100644 --- a/integration_tests/tests/test_discovery_consul/containerpilot.json5 +++ b/integration_tests/tests/test_discovery_consul/containerpilot.json5 @@ -4,7 +4,7 @@ "level": "DEBUG", "format": "text" }, - "services": [ + jobs: [ { "name": "app", "port": 8000, @@ -41,7 +41,7 @@ exec: "/reload-app.sh" } ], - "backends": [ + watches: [ { "name": "nginx", "poll": 7, diff --git a/integration_tests/tests/test_envvars/containerpilot.json5 b/integration_tests/tests/test_envvars/containerpilot.json5 index d3940445..a577b414 100644 --- a/integration_tests/tests/test_envvars/containerpilot.json5 +++ b/integration_tests/tests/test_envvars/containerpilot.json5 @@ -1,6 +1,6 @@ { "consul": "127.0.0.1:8500", - "services": [ + jobs: [ { "name": "testenvvar", "port": 8500, diff --git a/integration_tests/tests/test_tasks/containerpilot.json5 b/integration_tests/tests/test_tasks/containerpilot.json5 index 81c5b9cc..2619b3c0 100644 --- a/integration_tests/tests/test_tasks/containerpilot.json5 +++ b/integration_tests/tests/test_tasks/containerpilot.json5 @@ -4,7 +4,7 @@ "level": "DEBUG", "format": "text" }, - "services": [ + jobs: [ { "name": "app", "port": 8000, @@ -60,7 +60,7 @@ exec: "/reload-app.sh" } ], - "backends": [ + watches: [ { "name": "nginx", "poll": 7, diff --git a/integration_tests/tests/test_telemetry/containerpilot.json5 b/integration_tests/tests/test_telemetry/containerpilot.json5 index 171ed4e1..2b0c6b88 100644 --- a/integration_tests/tests/test_telemetry/containerpilot.json5 +++ b/integration_tests/tests/test_telemetry/containerpilot.json5 @@ -4,7 +4,7 @@ "level": "DEBUG", "format": "text" }, - "services": [ + jobs: [ { "name": "app", "port": 8000, diff --git a/jobs/config.go b/jobs/config.go index fe296cd0..fb3941a5 100644 --- a/jobs/config.go +++ b/jobs/config.go @@ -78,7 +78,7 @@ func NewConfigs(raw []interface{}, disc discovery.Backend) ([]*Config, error) { return jobs, nil } if err := utils.DecodeRaw(raw, &jobs); err != nil { - return nil, fmt.Errorf("service configuration error: %v", err) + return nil, fmt.Errorf("job configuration error: %v", err) } stopDependencies := make(map[string]string) for _, job := range jobs { @@ -135,15 +135,15 @@ func (cfg *Config) validateDiscovery(disc discovery.Backend) error { // if port isn't set then we won't do any discovery for this job if cfg.Port == 0 { if cfg.Heartbeat > 0 || cfg.TTL > 0 { - return fmt.Errorf("`heartbeat` and `ttl` may not be set in service `%s` if `port` is not set", cfg.Name) + return fmt.Errorf("`heartbeat` and `ttl` may not be set in job `%s` if `port` is not set", cfg.Name) } return nil } if cfg.Heartbeat < 1 { - return fmt.Errorf("`poll` must be > 0 in service `%s` when `port` is set", cfg.Name) + return fmt.Errorf("`poll` must be > 0 in job `%s` when `port` is set", cfg.Name) } if cfg.TTL < 1 { - return fmt.Errorf("`ttl` must be > 0 in service `%s` when `port` is set", cfg.Name) + return fmt.Errorf("`ttl` must be > 0 in job `%s` when `port` is set", cfg.Name) } cfg.heartbeatInterval = time.Duration(cfg.Heartbeat) * time.Second if err := cfg.AddDiscoveryConfig(disc); err != nil { diff --git a/jobs/config_test.go b/jobs/config_test.go index 6fcaf1f0..26d6b247 100644 --- a/jobs/config_test.go +++ b/jobs/config_test.go @@ -89,14 +89,14 @@ func TestJobConfigValidateName(t *testing.T) { func TestJobConfigValidateDiscovery(t *testing.T) { _, err := NewConfigs(tests.DecodeRawToSlice(`[{"name": "myName", "port": 80}]`), noop) - assert.Error(t, err, "`poll` must be > 0 in service `myName` when `port` is set") + assert.Error(t, err, "`poll` must be > 0 in job `myName` when `port` is set") _, err = NewConfigs(tests.DecodeRawToSlice(`[{"name": "myName", "port": 80, "poll": 1}]`), noop) - assert.Error(t, err, "`ttl` must be > 0 in service `myName` when `port` is set") + assert.Error(t, err, "`ttl` must be > 0 in job `myName` when `port` is set") _, err = NewConfigs(tests.DecodeRawToSlice(`[{"name": "myName", "poll": 1, "ttl": 1}]`), noop) assert.Error(t, err, - "`heartbeat` and `ttl` may not be set in service `myName` if `port` is not set") + "`heartbeat` and `ttl` may not be set in job `myName` if `port` is not set") // no health check shouldn't return an error raw := tests.DecodeRawToSlice(`[{"name": "myName", "poll": 1, "ttl": 1, "port": 80}]`) @@ -109,7 +109,7 @@ func TestJobsConsulExtrasEnableTagOverride(t *testing.T) { testCfg, _ := ioutil.ReadFile(fmt.Sprintf("./testdata/%s.json5", t.Name())) jobs, err := NewConfigs(tests.DecodeRawToSlice(string(testCfg)), nil) if err != nil { - t.Fatalf("could not parse service JSON: %s", err) + t.Fatalf("could not parse job JSON: %s", err) } if jobs[0].definition.ConsulExtras.EnableTagOverride != true { t.Errorf("ConsulExtras should have had EnableTagOverride set to true.") @@ -128,7 +128,7 @@ func TestJobsConsulExtrasDeregisterCriticalServiceAfter(t *testing.T) { testCfg, _ := ioutil.ReadFile(fmt.Sprintf("./testdata/%s.json5", t.Name())) jobs, err := NewConfigs(tests.DecodeRawToSlice(string(testCfg)), nil) if err != nil { - t.Fatalf("could not parse service JSON: %s", err) + t.Fatalf("could not parse job JSON: %s", err) } if jobs[0].definition.ConsulExtras.DeregisterCriticalServiceAfter != "40m" { t.Errorf("ConsulExtras should have had DeregisterCriticalServiceAfter set to '40m'.") @@ -165,8 +165,8 @@ func TestJobConfigValidateFrequency(t *testing.T) { "unable to parse frequency 'xx': time: invalid duration xx") testCfg := tests.DecodeRawToSlice(`[{"exec": "/bin/taskE", "frequency": "1ms"}]`) - service, _ := NewConfigs(testCfg, nil) - assert.Equal(t, service[0].execTimeout, service[0].freqInterval, + job, _ := NewConfigs(testCfg, nil) + assert.Equal(t, job[0].execTimeout, job[0].freqInterval, "expected execTimeout '%v' to equal frequency '%v'") } diff --git a/jobs/jobs.go b/jobs/jobs.go index 07018e4b..84b2041d 100644 --- a/jobs/jobs.go +++ b/jobs/jobs.go @@ -77,21 +77,21 @@ func FromConfigs(cfgs []*Config) []*Job { return jobs } -// SendHeartbeat sends a heartbeat for this service +// SendHeartbeat sends a heartbeat for this Job's service func (job *Job) SendHeartbeat() { if job.discoveryCatalog != nil || job.Definition != nil { job.discoveryCatalog.SendHeartbeat(job.Definition) } } -// MarkForMaintenance marks this service for maintenance +// MarkForMaintenance marks this Job's service for maintenance func (job *Job) MarkForMaintenance() { if job.discoveryCatalog != nil || job.Definition != nil { job.discoveryCatalog.MarkForMaintenance(job.Definition) } } -// Deregister will deregister this instance of the service +// Deregister will deregister this instance of Job's service func (job *Job) Deregister() { if job.discoveryCatalog != nil || job.Definition != nil { job.discoveryCatalog.Deregister(job.Definition)