Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Field renames #315

Merged
merged 2 commits into from
Apr 10, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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'")
Expand All @@ -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'")
Expand Down
4 changes: 2 additions & 2 deletions config/template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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)
Expand Down
16 changes: 5 additions & 11 deletions config/testdata/test.json5
Original file line number Diff line number Diff line change
@@ -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",
Expand Down Expand Up @@ -32,7 +34,7 @@
"restarts": "unlimited"
},
{
"name": "taskD",
"name": "periodicTaskD",
"exec": "/bin/taskD",
"frequency": "1s"
},
Expand All @@ -55,17 +57,9 @@
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": [
watches: [
{
"name": "upstreamA",
"poll": 11,
Expand Down
12 changes: 6 additions & 6 deletions core/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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`"})
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"level": "DEBUG",
"format": "text"
},
"services": [
jobs: [
{
"name": "app",
"port": 8000,
Expand Down
4 changes: 2 additions & 2 deletions integration_tests/fixtures/app/containerpilot.json5
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"level": "DEBUG",
"format": "text"
},
"services": [
jobs: [
{
"name": "app",
"port": 8000,
Expand Down Expand Up @@ -41,7 +41,7 @@
exec: "/reload-app.sh"
}
],
"backends": [
watches: [
{
"name": "nginx",
"poll": 7,
Expand Down
4 changes: 2 additions & 2 deletions integration_tests/fixtures/nginx/etc/nginx-with-consul.json5
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"level": "DEBUG",
"format": "text"
},
"services": [
jobs: [
{
"name": "nginx",
"port": 80,
Expand Down Expand Up @@ -36,7 +36,7 @@
]
}
],
"backends": [
watches: [
{
"name": "app",
"poll": 1
Expand Down
2 changes: 1 addition & 1 deletion integration_tests/fixtures/zombie_maker/app.json5
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"consul": "consul:8500",
"services": [
jobs: [
{
"name": "zombies",
"port": 8000,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"level": "DEBUG",
"format": "text"
},
"services": [
jobs: [
{
"name": "app",
"port": 8000,
Expand Down Expand Up @@ -41,7 +41,7 @@
exec: "/reload-app.sh"
}
],
"backends": [
watches: [
{
"name": "nginx",
"poll": 7,
Expand Down
2 changes: 1 addition & 1 deletion integration_tests/tests/test_envvars/containerpilot.json5
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"consul": "127.0.0.1:8500",
"services": [
jobs: [
{
"name": "testenvvar",
"port": 8500,
Expand Down
4 changes: 2 additions & 2 deletions integration_tests/tests/test_tasks/containerpilot.json5
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"level": "DEBUG",
"format": "text"
},
"services": [
jobs: [
{
"name": "app",
"port": 8000,
Expand Down Expand Up @@ -60,7 +60,7 @@
exec: "/reload-app.sh"
}
],
"backends": [
watches: [
{
"name": "nginx",
"poll": 7,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"level": "DEBUG",
"format": "text"
},
"services": [
jobs: [
{
"name": "app",
"port": 8000,
Expand Down
8 changes: 4 additions & 4 deletions jobs/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
14 changes: 7 additions & 7 deletions jobs/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}]`)
Expand All @@ -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.")
Expand All @@ -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'.")
Expand Down Expand Up @@ -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'")
}

Expand Down
6 changes: 3 additions & 3 deletions jobs/jobs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down