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

Add support for initial check status #1599

Merged
merged 2 commits into from
Aug 16, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions command/agent/consul/syncer.go
Original file line number Diff line number Diff line change
Expand Up @@ -720,6 +720,7 @@ func (c *Syncer) createCheckReg(check *structs.ServiceCheck, serviceReg *consul.
default:
return nil, fmt.Errorf("check type %+q not valid", check.Type)
}
chkReg.Status = check.InitialStatus
return &chkReg, nil
}

Expand Down
13 changes: 9 additions & 4 deletions command/agent/consul/syncer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,11 @@ const (
var (
logger = log.New(os.Stdout, "", log.LstdFlags)
check1 = structs.ServiceCheck{
Name: "check-foo-1",
Type: structs.ServiceCheckTCP,
Interval: 30 * time.Second,
Timeout: 5 * time.Second,
Name: "check-foo-1",
Type: structs.ServiceCheckTCP,
Interval: 30 * time.Second,
Timeout: 5 * time.Second,
InitialStatus: "passing",
}
check2 = structs.ServiceCheck{
Name: "check1",
Expand Down Expand Up @@ -86,6 +87,10 @@ func TestCheckRegistration(t *testing.T) {
t.Fatalf("expected: %v, actual: %v", expected, check3Reg.HTTP)
}

expected = "passing"
if check1Reg.Status != expected {
t.Fatalf("expected: %v, actual: %v", expected, check1Reg.Status)
}
}

func TestConsulServiceRegisterServices(t *testing.T) {
Expand Down
1 change: 1 addition & 0 deletions jobspec/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -770,6 +770,7 @@ func parseChecks(service *structs.Service, checkObjs *ast.ObjectList) error {
"port",
"command",
"args",
"initial_status",
}
if err := checkHCLKeys(co.Val, valid); err != nil {
return multierror.Prefix(err, "check ->")
Expand Down
14 changes: 14 additions & 0 deletions jobspec/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -525,3 +525,17 @@ func TestIncorrectKey(t *testing.T) {
t.Fatalf("Expected collision error; got %v", err)
}
}

func TestServiceCheckInitialStatus(t *testing.T) {

path, err := filepath.Abs(filepath.Join("./test-fixtures", "service-check-initial-status.hcl"))
if err != nil {
t.Fatalf("Can't get absolute path for file: %s", err)
}

_, err = ParseFile(path)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also check that parsing the file resulted in the check that we are expecting by using reflect.DeepEquals

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay! it turns out it was actually broken without the mapstructure tag :x

if err != nil {
t.Fatalf("Expected no error; got %v", err)
}
}
22 changes: 22 additions & 0 deletions jobspec/test-fixtures/service-check-initial-status.hcl
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
job "foo" {

group "group" {
count = 1

task "task" {

service {
tags = ["foo", "bar"]

check {
name = "check-name"
type = "http"
interval = "10s"
timeout = "2s"
initial_status = "passing"
}
}
}
}
}

31 changes: 22 additions & 9 deletions nomad/structs/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"time"

"github.com/gorhill/cronexpr"
"github.com/hashicorp/consul/api"
"github.com/hashicorp/go-multierror"
"github.com/hashicorp/go-version"
"github.com/hashicorp/nomad/helper/args"
Expand Down Expand Up @@ -1591,15 +1592,16 @@ const (
// The ServiceCheck data model represents the consul health check that
// Nomad registers for a Task
type ServiceCheck struct {
Name string // Name of the check, defaults to id
Type string // Type of the check - tcp, http, docker and script
Command string // Command is the command to run for script checks
Args []string // Args is a list of argumes for script checks
Path string // path of the health check url for http type check
Protocol string // Protocol to use if check is http, defaults to http
PortLabel string `mapstructure:"port"` // The port to use for tcp/http checks
Interval time.Duration // Interval of the check
Timeout time.Duration // Timeout of the response from the check before consul fails the check
Name string // Name of the check, defaults to id
Type string // Type of the check - tcp, http, docker and script
Command string // Command is the command to run for script checks
Args []string // Args is a list of argumes for script checks
Path string // path of the health check url for http type check
Protocol string // Protocol to use if check is http, defaults to http
PortLabel string `mapstructure:"port"` // The port to use for tcp/http checks
Interval time.Duration // Interval of the check
Timeout time.Duration // Timeout of the response from the check before consul fails the check
InitialStatus string // Initial status of the check
}

func (sc *ServiceCheck) Copy() *ServiceCheck {
Expand Down Expand Up @@ -1653,6 +1655,17 @@ func (sc *ServiceCheck) validate() error {
return fmt.Errorf("interval (%v) can not be lower than %v", sc.Interval, minCheckInterval)
}

switch sc.InitialStatus {
case "":
case api.HealthUnknown:
case api.HealthPassing:
case api.HealthWarning:
case api.HealthCritical:
default:
return fmt.Errorf(`invalid initial check state (%s), must be one of "%s", "%s", "%s", or "%s"`, sc.InitialStatus, api.HealthUnknown, api.HealthPassing, api.HealthWarning, api.HealthCritical)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

invalid initial check state (%s), must be one of %q, %q, %q, %q or empty


}

return nil
}

Expand Down
44 changes: 44 additions & 0 deletions nomad/structs/structs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"testing"
"time"

"github.com/hashicorp/consul/api"
"github.com/hashicorp/go-multierror"
)

Expand Down Expand Up @@ -364,6 +365,49 @@ func TestTask_Validate_Services(t *testing.T) {
}
}

func TestTask_Validate_Service_Check(t *testing.T) {

check1 := ServiceCheck{
Name: "check-name",
Type: ServiceCheckTCP,
Interval: 10 * time.Second,
Timeout: 2 * time.Second,
}

err := check1.validate()
if err != nil {
t.Fatal("err: %v", err)
}

check1.InitialStatus = "foo"
err = check1.validate()
if err == nil {
t.Fatal("Expected an error")
}

if !strings.Contains(err.Error(), "invalid initial check state (foo)") {
t.Fatalf("err: %v", err)
}

check1.InitialStatus = api.HealthCritical
err = check1.validate()
if err != nil {
t.Fatalf("err: %v", err)
}

check1.InitialStatus = api.HealthPassing
err = check1.validate()
if err != nil {
t.Fatalf("err: %v", err)
}

check1.InitialStatus = ""
err = check1.validate()
if err != nil {
t.Fatalf("err: %v", err)
}
}

func TestTask_Validate_LogConfig(t *testing.T) {
task := &Task{
LogConfig: DefaultLogConfig(),
Expand Down