From 0857e93d0bdc2302ca9efd9dd182b389740e46eb Mon Sep 17 00:00:00 2001 From: Sean Chittenden Date: Mon, 6 Jun 2016 01:53:30 -0700 Subject: [PATCH] Float a type balloon. Some strings are square pegs in round holes. This experiment was brought about because of variable naming confusion where name and checkIDs were interchanged. Gave CheckID an Qualified Type Name and chased downstream changes. --- command/agent/agent.go | 48 +++++++------- command/agent/agent_endpoint.go | 17 ++--- command/agent/agent_test.go | 8 +-- command/agent/check.go | 14 ++-- command/agent/check_test.go | 106 +++++++++++++++---------------- command/agent/local.go | 38 +++++------ command/agent/structs.go | 4 +- command/agent/util.go | 6 ++ consul/catalog_endpoint.go | 2 +- consul/state/state_store.go | 8 +-- consul/state/state_store_test.go | 2 +- consul/structs/structs.go | 19 +++--- consul/structs/structs_test.go | 3 +- 13 files changed, 143 insertions(+), 132 deletions(-) diff --git a/command/agent/agent.go b/command/agent/agent.go index d4e7faa50..ecdfb21dd 100644 --- a/command/agent/agent.go +++ b/command/agent/agent.go @@ -74,19 +74,19 @@ type Agent struct { state localState // checkMonitors maps the check ID to an associated monitor - checkMonitors map[string]*CheckMonitor + checkMonitors map[structs.CheckID]*CheckMonitor // checkHTTPs maps the check ID to an associated HTTP check - checkHTTPs map[string]*CheckHTTP + checkHTTPs map[structs.CheckID]*CheckHTTP // checkTCPs maps the check ID to an associated TCP check - checkTCPs map[string]*CheckTCP + checkTCPs map[structs.CheckID]*CheckTCP // checkTTLs maps the check ID to an associated check TTL - checkTTLs map[string]*CheckTTL + checkTTLs map[structs.CheckID]*CheckTTL // checkDockers maps the check ID to an associated Docker Exec based check - checkDockers map[string]*CheckDocker + checkDockers map[structs.CheckID]*CheckDocker // checkLock protects updates to the check* maps checkLock sync.Mutex @@ -176,11 +176,11 @@ func Create(config *Config, logOutput io.Writer) (*Agent, error) { config: config, logger: log.New(logOutput, "", log.LstdFlags), logOutput: logOutput, - checkMonitors: make(map[string]*CheckMonitor), - checkTTLs: make(map[string]*CheckTTL), - checkHTTPs: make(map[string]*CheckHTTP), - checkTCPs: make(map[string]*CheckTCP), - checkDockers: make(map[string]*CheckDocker), + checkMonitors: make(map[structs.CheckID]*CheckMonitor), + checkTTLs: make(map[structs.CheckID]*CheckTTL), + checkHTTPs: make(map[structs.CheckID]*CheckHTTP), + checkTCPs: make(map[structs.CheckID]*CheckTCP), + checkDockers: make(map[structs.CheckID]*CheckDocker), eventCh: make(chan serf.UserEvent, 1024), eventBuf: make([]*UserEvent, 256), shutdownCh: make(chan struct{}), @@ -696,7 +696,7 @@ func (a *Agent) purgeService(serviceID string) error { // persistCheck saves a check definition to the local agent's state directory func (a *Agent) persistCheck(check *structs.HealthCheck, chkType *CheckType) error { - checkPath := filepath.Join(a.config.DataDir, checksDir, stringHash(check.CheckID)) + checkPath := filepath.Join(a.config.DataDir, checksDir, checkIDHash(check.CheckID)) // Create the persisted check wrapped := persistedCheck{ @@ -724,8 +724,8 @@ func (a *Agent) persistCheck(check *structs.HealthCheck, chkType *CheckType) err } // purgeCheck removes a persisted check definition file from the data dir -func (a *Agent) purgeCheck(checkID string) error { - checkPath := filepath.Join(a.config.DataDir, checksDir, stringHash(checkID)) +func (a *Agent) purgeCheck(checkID structs.CheckID) error { + checkPath := filepath.Join(a.config.DataDir, checksDir, checkIDHash(checkID)) if _, err := os.Stat(checkPath); err == nil { return os.Remove(checkPath) } @@ -791,7 +791,7 @@ func (a *Agent) AddService(service *structs.NodeService, chkTypes CheckTypes, pe } check := &structs.HealthCheck{ Node: a.config.NodeName, - CheckID: checkID, + CheckID: structs.CheckID(checkID), Name: fmt.Sprintf("Service '%s' check", service.Service), Status: structs.HealthCritical, Notes: chkType.Notes, @@ -998,7 +998,7 @@ func (a *Agent) AddCheck(check *structs.HealthCheck, chkType *CheckType, persist // RemoveCheck is used to remove a health check. // The agent will make a best effort to ensure it is deregistered -func (a *Agent) RemoveCheck(checkID string, persist bool) error { +func (a *Agent) RemoveCheck(checkID structs.CheckID, persist bool) error { // Validate CheckID if checkID == "" { return fmt.Errorf("CheckID missing") @@ -1041,7 +1041,7 @@ func (a *Agent) RemoveCheck(checkID string, persist bool) error { // UpdateCheck is used to update the status of a check. // This can only be used with checks of the TTL type. -func (a *Agent) UpdateCheck(checkID, status, output string) error { +func (a *Agent) UpdateCheck(checkID structs.CheckID, status, output string) error { a.checkLock.Lock() defer a.checkLock.Unlock() @@ -1090,7 +1090,7 @@ func (a *Agent) persistCheckState(check *CheckTTL, status, output string) error } // Write the state to the file - file := filepath.Join(dir, stringHash(check.CheckID)) + file := filepath.Join(dir, checkIDHash(check.CheckID)) if err := ioutil.WriteFile(file, buf, 0600); err != nil { return fmt.Errorf("failed writing file %q: %s", file, err) } @@ -1101,7 +1101,7 @@ func (a *Agent) persistCheckState(check *CheckTTL, status, output string) error // loadCheckState is used to restore the persisted state of a check. func (a *Agent) loadCheckState(check *structs.HealthCheck) error { // Try to read the persisted state for this check - file := filepath.Join(a.config.DataDir, checkStateDir, stringHash(check.CheckID)) + file := filepath.Join(a.config.DataDir, checkStateDir, checkIDHash(check.CheckID)) buf, err := ioutil.ReadFile(file) if err != nil { if os.IsNotExist(err) { @@ -1129,8 +1129,8 @@ func (a *Agent) loadCheckState(check *structs.HealthCheck) error { } // purgeCheckState is used to purge the state of a check from the data dir -func (a *Agent) purgeCheckState(checkID string) error { - file := filepath.Join(a.config.DataDir, checkStateDir, stringHash(checkID)) +func (a *Agent) purgeCheckState(checkID structs.CheckID) error { + file := filepath.Join(a.config.DataDir, checkStateDir, checkIDHash(checkID)) err := os.Remove(file) if os.IsNotExist(err) { return nil @@ -1393,22 +1393,22 @@ func (a *Agent) unloadChecks() error { // snapshotCheckState is used to snapshot the current state of the health // checks. This is done before we reload our checks, so that we can properly // restore into the same state. -func (a *Agent) snapshotCheckState() map[string]*structs.HealthCheck { +func (a *Agent) snapshotCheckState() map[structs.CheckID]*structs.HealthCheck { return a.state.Checks() } // restoreCheckState is used to reset the health state based on a snapshot. // This is done after we finish the reload to avoid any unnecessary flaps // in health state and potential session invalidations. -func (a *Agent) restoreCheckState(snap map[string]*structs.HealthCheck) { +func (a *Agent) restoreCheckState(snap map[structs.CheckID]*structs.HealthCheck) { for id, check := range snap { a.state.UpdateCheck(id, check.Status, check.Output) } } // serviceMaintCheckID returns the ID of a given service's maintenance check -func serviceMaintCheckID(serviceID string) string { - return fmt.Sprintf("%s:%s", serviceMaintCheckPrefix, serviceID) +func serviceMaintCheckID(serviceID string) structs.CheckID { + return structs.CheckID(fmt.Sprintf("%s:%s", serviceMaintCheckPrefix, serviceID)) } // EnableServiceMaintenance will register a false health check against the given diff --git a/command/agent/agent_endpoint.go b/command/agent/agent_endpoint.go index b897d5242..729fa88fc 100644 --- a/command/agent/agent_endpoint.go +++ b/command/agent/agent_endpoint.go @@ -2,12 +2,13 @@ package agent import ( "fmt" - "github.com/hashicorp/consul/consul/structs" - "github.com/hashicorp/serf/coordinate" - "github.com/hashicorp/serf/serf" "net/http" "strconv" "strings" + + "github.com/hashicorp/consul/consul/structs" + "github.com/hashicorp/serf/coordinate" + "github.com/hashicorp/serf/serf" ) type AgentSelf struct { @@ -129,7 +130,7 @@ func (s *HTTPServer) AgentRegisterCheck(resp http.ResponseWriter, req *http.Requ } func (s *HTTPServer) AgentDeregisterCheck(resp http.ResponseWriter, req *http.Request) (interface{}, error) { - checkID := strings.TrimPrefix(req.URL.Path, "/v1/agent/check/deregister/") + checkID := structs.CheckID(strings.TrimPrefix(req.URL.Path, "/v1/agent/check/deregister/")) if err := s.agent.RemoveCheck(checkID, true); err != nil { return nil, err } @@ -138,7 +139,7 @@ func (s *HTTPServer) AgentDeregisterCheck(resp http.ResponseWriter, req *http.Re } func (s *HTTPServer) AgentCheckPass(resp http.ResponseWriter, req *http.Request) (interface{}, error) { - checkID := strings.TrimPrefix(req.URL.Path, "/v1/agent/check/pass/") + checkID := structs.CheckID(strings.TrimPrefix(req.URL.Path, "/v1/agent/check/pass/")) note := req.URL.Query().Get("note") if err := s.agent.UpdateCheck(checkID, structs.HealthPassing, note); err != nil { return nil, err @@ -148,7 +149,7 @@ func (s *HTTPServer) AgentCheckPass(resp http.ResponseWriter, req *http.Request) } func (s *HTTPServer) AgentCheckWarn(resp http.ResponseWriter, req *http.Request) (interface{}, error) { - checkID := strings.TrimPrefix(req.URL.Path, "/v1/agent/check/warn/") + checkID := structs.CheckID(strings.TrimPrefix(req.URL.Path, "/v1/agent/check/warn/")) note := req.URL.Query().Get("note") if err := s.agent.UpdateCheck(checkID, structs.HealthWarning, note); err != nil { return nil, err @@ -158,7 +159,7 @@ func (s *HTTPServer) AgentCheckWarn(resp http.ResponseWriter, req *http.Request) } func (s *HTTPServer) AgentCheckFail(resp http.ResponseWriter, req *http.Request) (interface{}, error) { - checkID := strings.TrimPrefix(req.URL.Path, "/v1/agent/check/fail/") + checkID := structs.CheckID(strings.TrimPrefix(req.URL.Path, "/v1/agent/check/fail/")) note := req.URL.Query().Get("note") if err := s.agent.UpdateCheck(checkID, structs.HealthCritical, note); err != nil { return nil, err @@ -211,7 +212,7 @@ func (s *HTTPServer) AgentCheckUpdate(resp http.ResponseWriter, req *http.Reques update.Output[:CheckBufSize], CheckBufSize, total) } - checkID := strings.TrimPrefix(req.URL.Path, "/v1/agent/check/update/") + checkID := structs.CheckID(strings.TrimPrefix(req.URL.Path, "/v1/agent/check/update/")) if err := s.agent.UpdateCheck(checkID, update.Status, update.Output); err != nil { return nil, err } diff --git a/command/agent/agent_test.go b/command/agent/agent_test.go index baf5c5dc5..c7331aa7a 100644 --- a/command/agent/agent_test.go +++ b/command/agent/agent_test.go @@ -919,7 +919,7 @@ func TestAgent_PersistCheck(t *testing.T) { Interval: 10 * time.Second, } - file := filepath.Join(agent.config.DataDir, checksDir, stringHash(check.CheckID)) + file := filepath.Join(agent.config.DataDir, checksDir, checkIDHash(check.CheckID)) // Not persisted if not requested if err := agent.AddCheck(check, chkType, false, ""); err != nil { @@ -1014,7 +1014,7 @@ func TestAgent_PurgeCheck(t *testing.T) { Status: structs.HealthPassing, } - file := filepath.Join(agent.config.DataDir, checksDir, stringHash(check.CheckID)) + file := filepath.Join(agent.config.DataDir, checksDir, checkIDHash(check.CheckID)) if err := agent.AddCheck(check, nil, true, ""); err != nil { t.Fatalf("err: %v", err) } @@ -1074,7 +1074,7 @@ func TestAgent_PurgeCheckOnDuplicate(t *testing.T) { } defer agent2.Shutdown() - file := filepath.Join(agent.config.DataDir, checksDir, stringHash(check1.CheckID)) + file := filepath.Join(agent.config.DataDir, checksDir, checkIDHash(check1.CheckID)) if _, err := os.Stat(file); err == nil { t.Fatalf("should have removed persisted check") } @@ -1465,7 +1465,7 @@ func TestAgent_loadChecks_checkFails(t *testing.T) { } // Check to make sure the check was persisted - checkHash := stringHash(check.CheckID) + checkHash := checkIDHash(check.CheckID) checkPath := filepath.Join(config.DataDir, checksDir, checkHash) if _, err := os.Stat(checkPath); err != nil { t.Fatalf("err: %s", err) diff --git a/command/agent/check.go b/command/agent/check.go index 515fade40..578517a50 100644 --- a/command/agent/check.go +++ b/command/agent/check.go @@ -90,7 +90,7 @@ func (c *CheckType) IsDocker() bool { // to notify when a check has a status update. The update // should take care to be idempotent. type CheckNotifier interface { - UpdateCheck(checkID, status, output string) + UpdateCheck(checkID structs.CheckID, status, output string) } // CheckMonitor is used to periodically invoke a script to @@ -98,7 +98,7 @@ type CheckNotifier interface { // nagios plugins and expects the output in the same format. type CheckMonitor struct { Notify CheckNotifier - CheckID string + CheckID structs.CheckID Script string Interval time.Duration Timeout time.Duration @@ -231,7 +231,7 @@ func (c *CheckMonitor) check() { // automatically set to critical. type CheckTTL struct { Notify CheckNotifier - CheckID string + CheckID structs.CheckID TTL time.Duration Logger *log.Logger @@ -322,7 +322,7 @@ type persistedCheck struct { // expiration timestamp which is used to determine staleness on later // agent restarts. type persistedCheckState struct { - CheckID string + CheckID structs.CheckID Output string Status string Expires int64 @@ -336,7 +336,7 @@ type persistedCheckState struct { // or if the request returns an error type CheckHTTP struct { Notify CheckNotifier - CheckID string + CheckID structs.CheckID HTTP string Interval time.Duration Timeout time.Duration @@ -462,7 +462,7 @@ func (c *CheckHTTP) check() { // The check is critical if the connection returns an error type CheckTCP struct { Notify CheckNotifier - CheckID string + CheckID structs.CheckID TCP string Interval time.Duration Timeout time.Duration @@ -553,7 +553,7 @@ type DockerClient interface { // with nagios plugins and expects the output in the same format. type CheckDocker struct { Notify CheckNotifier - CheckID string + CheckID structs.CheckID Script string DockerContainerID string Shell string diff --git a/command/agent/check_test.go b/command/agent/check_test.go index 798301422..2e23c75c2 100644 --- a/command/agent/check_test.go +++ b/command/agent/check_test.go @@ -21,12 +21,12 @@ import ( ) type MockNotify struct { - state map[string]string - updates map[string]int - output map[string]string + state map[structs.CheckID]string + updates map[structs.CheckID]int + output map[structs.CheckID]string } -func (m *MockNotify) UpdateCheck(id, status, output string) { +func (m *MockNotify) UpdateCheck(id structs.CheckID, status, output string) { m.state[id] = status old := m.updates[id] m.updates[id] = old + 1 @@ -35,13 +35,13 @@ func (m *MockNotify) UpdateCheck(id, status, output string) { func expectStatus(t *testing.T, script, status string) { mock := &MockNotify{ - state: make(map[string]string), - updates: make(map[string]int), - output: make(map[string]string), + state: make(map[structs.CheckID]string), + updates: make(map[structs.CheckID]int), + output: make(map[structs.CheckID]string), } check := &CheckMonitor{ Notify: mock, - CheckID: "foo", + CheckID: structs.CheckID("foo"), Script: script, Interval: 10 * time.Millisecond, Logger: log.New(os.Stderr, "", log.LstdFlags), @@ -84,13 +84,13 @@ func TestCheckMonitor_BadCmd(t *testing.T) { func TestCheckMonitor_Timeout(t *testing.T) { mock := &MockNotify{ - state: make(map[string]string), - updates: make(map[string]int), - output: make(map[string]string), + state: make(map[structs.CheckID]string), + updates: make(map[structs.CheckID]int), + output: make(map[structs.CheckID]string), } check := &CheckMonitor{ Notify: mock, - CheckID: "foo", + CheckID: structs.CheckID("foo"), Script: "sleep 1 && exit 0", Interval: 10 * time.Millisecond, Timeout: 5 * time.Millisecond, @@ -114,13 +114,13 @@ func TestCheckMonitor_Timeout(t *testing.T) { func TestCheckMonitor_RandomStagger(t *testing.T) { mock := &MockNotify{ - state: make(map[string]string), - updates: make(map[string]int), - output: make(map[string]string), + state: make(map[structs.CheckID]string), + updates: make(map[structs.CheckID]int), + output: make(map[structs.CheckID]string), } check := &CheckMonitor{ Notify: mock, - CheckID: "foo", + CheckID: structs.CheckID("foo"), Script: "exit 0", Interval: 25 * time.Millisecond, Logger: log.New(os.Stderr, "", log.LstdFlags), @@ -143,13 +143,13 @@ func TestCheckMonitor_RandomStagger(t *testing.T) { func TestCheckMonitor_LimitOutput(t *testing.T) { mock := &MockNotify{ - state: make(map[string]string), - updates: make(map[string]int), - output: make(map[string]string), + state: make(map[structs.CheckID]string), + updates: make(map[structs.CheckID]int), + output: make(map[structs.CheckID]string), } check := &CheckMonitor{ Notify: mock, - CheckID: "foo", + CheckID: structs.CheckID("foo"), Script: "od -N 81920 /dev/urandom", Interval: 25 * time.Millisecond, Logger: log.New(os.Stderr, "", log.LstdFlags), @@ -168,13 +168,13 @@ func TestCheckMonitor_LimitOutput(t *testing.T) { func TestCheckTTL(t *testing.T) { mock := &MockNotify{ - state: make(map[string]string), - updates: make(map[string]int), - output: make(map[string]string), + state: make(map[structs.CheckID]string), + updates: make(map[structs.CheckID]int), + output: make(map[structs.CheckID]string), } check := &CheckTTL{ Notify: mock, - CheckID: "foo", + CheckID: structs.CheckID("foo"), TTL: 100 * time.Millisecond, Logger: log.New(os.Stderr, "", log.LstdFlags), } @@ -229,13 +229,13 @@ func mockHTTPServer(responseCode int) *httptest.Server { func expectHTTPStatus(t *testing.T, url string, status string) { mock := &MockNotify{ - state: make(map[string]string), - updates: make(map[string]int), - output: make(map[string]string), + state: make(map[structs.CheckID]string), + updates: make(map[structs.CheckID]int), + output: make(map[structs.CheckID]string), } check := &CheckHTTP{ Notify: mock, - CheckID: "foo", + CheckID: structs.CheckID("foo"), HTTP: url, Interval: 10 * time.Millisecond, Logger: log.New(os.Stderr, "", log.LstdFlags), @@ -329,14 +329,14 @@ func TestCheckHTTPTimeout(t *testing.T) { defer server.Close() mock := &MockNotify{ - state: make(map[string]string), - updates: make(map[string]int), - output: make(map[string]string), + state: make(map[structs.CheckID]string), + updates: make(map[structs.CheckID]int), + output: make(map[structs.CheckID]string), } check := &CheckHTTP{ Notify: mock, - CheckID: "bar", + CheckID: structs.CheckID("bar"), HTTP: server.URL, Timeout: 5 * time.Millisecond, Interval: 10 * time.Millisecond, @@ -360,7 +360,7 @@ func TestCheckHTTPTimeout(t *testing.T) { func TestCheckHTTP_disablesKeepAlives(t *testing.T) { check := &CheckHTTP{ - CheckID: "foo", + CheckID: structs.CheckID("foo"), HTTP: "http://foo.bar/baz", Interval: 10 * time.Second, Logger: log.New(os.Stderr, "", log.LstdFlags), @@ -395,13 +395,13 @@ func mockTCPServer(network string) net.Listener { func expectTCPStatus(t *testing.T, tcp string, status string) { mock := &MockNotify{ - state: make(map[string]string), - updates: make(map[string]int), - output: make(map[string]string), + state: make(map[structs.CheckID]string), + updates: make(map[structs.CheckID]int), + output: make(map[structs.CheckID]string), } check := &CheckTCP{ Notify: mock, - CheckID: "foo", + CheckID: structs.CheckID("foo"), TCP: tcp, Interval: 10 * time.Millisecond, Logger: log.New(os.Stderr, "", log.LstdFlags), @@ -575,13 +575,13 @@ func (d *fakeDockerClientWithExecInfoErrors) InspectExec(id string) (*docker.Exe func expectDockerCheckStatus(t *testing.T, dockerClient DockerClient, status string, output string) { mock := &MockNotify{ - state: make(map[string]string), - updates: make(map[string]int), - output: make(map[string]string), + state: make(map[structs.CheckID]string), + updates: make(map[structs.CheckID]int), + output: make(map[structs.CheckID]string), } check := &CheckDocker{ Notify: mock, - CheckID: "foo", + CheckID: structs.CheckID("foo"), Script: "/health.sh", DockerContainerID: "54432bad1fc7", Shell: "/bin/sh", @@ -635,13 +635,13 @@ func TestDockerCheckWhenExecInfoFails(t *testing.T) { func TestDockerCheckDefaultToSh(t *testing.T) { os.Setenv("SHELL", "") mock := &MockNotify{ - state: make(map[string]string), - updates: make(map[string]int), - output: make(map[string]string), + state: make(map[structs.CheckID]string), + updates: make(map[structs.CheckID]int), + output: make(map[structs.CheckID]string), } check := &CheckDocker{ Notify: mock, - CheckID: "foo", + CheckID: structs.CheckID("foo"), Script: "/health.sh", DockerContainerID: "54432bad1fc7", Interval: 10 * time.Millisecond, @@ -659,14 +659,14 @@ func TestDockerCheckDefaultToSh(t *testing.T) { func TestDockerCheckUseShellFromEnv(t *testing.T) { mock := &MockNotify{ - state: make(map[string]string), - updates: make(map[string]int), - output: make(map[string]string), + state: make(map[structs.CheckID]string), + updates: make(map[structs.CheckID]int), + output: make(map[structs.CheckID]string), } os.Setenv("SHELL", "/bin/bash") check := &CheckDocker{ Notify: mock, - CheckID: "foo", + CheckID: structs.CheckID("foo"), Script: "/health.sh", DockerContainerID: "54432bad1fc7", Interval: 10 * time.Millisecond, @@ -685,13 +685,13 @@ func TestDockerCheckUseShellFromEnv(t *testing.T) { func TestDockerCheckTruncateOutput(t *testing.T) { mock := &MockNotify{ - state: make(map[string]string), - updates: make(map[string]int), - output: make(map[string]string), + state: make(map[structs.CheckID]string), + updates: make(map[structs.CheckID]int), + output: make(map[structs.CheckID]string), } check := &CheckDocker{ Notify: mock, - CheckID: "foo", + CheckID: structs.CheckID("foo"), Script: "/health.sh", DockerContainerID: "54432bad1fc7", Shell: "/bin/sh", diff --git a/command/agent/local.go b/command/agent/local.go index ed67258c5..11d85271c 100644 --- a/command/agent/local.go +++ b/command/agent/local.go @@ -56,12 +56,12 @@ type localState struct { serviceTokens map[string]string // Checks tracks the local checks - checks map[string]*structs.HealthCheck - checkStatus map[string]syncStatus - checkTokens map[string]string + checks map[structs.CheckID]*structs.HealthCheck + checkStatus map[structs.CheckID]syncStatus + checkTokens map[structs.CheckID]string // Used to track checks that are being deferred - deferCheck map[string]*time.Timer + deferCheck map[structs.CheckID]*time.Timer // consulCh is used to inform of a change to the known // consul nodes. This may be used to retry a sync run @@ -79,10 +79,10 @@ func (l *localState) Init(config *Config, logger *log.Logger) { l.services = make(map[string]*structs.NodeService) l.serviceStatus = make(map[string]syncStatus) l.serviceTokens = make(map[string]string) - l.checks = make(map[string]*structs.HealthCheck) - l.checkStatus = make(map[string]syncStatus) - l.checkTokens = make(map[string]string) - l.deferCheck = make(map[string]*time.Timer) + l.checks = make(map[structs.CheckID]*structs.HealthCheck) + l.checkStatus = make(map[structs.CheckID]syncStatus) + l.checkTokens = make(map[structs.CheckID]string) + l.deferCheck = make(map[structs.CheckID]*time.Timer) l.consulCh = make(chan struct{}, 1) l.triggerCh = make(chan struct{}, 1) } @@ -193,14 +193,14 @@ func (l *localState) Services() map[string]*structs.NodeService { // CheckToken is used to return the configured health check token, or // if none is configured, the default agent ACL token. -func (l *localState) CheckToken(id string) string { +func (l *localState) CheckToken(id structs.CheckID) string { l.RLock() defer l.RUnlock() return l.checkToken(id) } // checkToken returns an ACL token associated with a check. -func (l *localState) checkToken(id string) string { +func (l *localState) checkToken(id structs.CheckID) string { token := l.checkTokens[id] if token == "" { token = l.config.ACLToken @@ -226,7 +226,7 @@ func (l *localState) AddCheck(check *structs.HealthCheck, token string) { // RemoveCheck is used to remove a health check from the local state. // The agent will make a best effort to ensure it is deregistered -func (l *localState) RemoveCheck(checkID string) { +func (l *localState) RemoveCheck(checkID structs.CheckID) { l.Lock() defer l.Unlock() @@ -237,7 +237,7 @@ func (l *localState) RemoveCheck(checkID string) { } // UpdateCheck is used to update the status of a check -func (l *localState) UpdateCheck(checkID, status, output string) { +func (l *localState) UpdateCheck(checkID structs.CheckID, status, output string) { l.Lock() defer l.Unlock() @@ -282,13 +282,13 @@ func (l *localState) UpdateCheck(checkID, status, output string) { // Checks returns the locally registered checks that the // agent is aware of and are being kept in sync with the server -func (l *localState) Checks() map[string]*structs.HealthCheck { - checks := make(map[string]*structs.HealthCheck) +func (l *localState) Checks() map[structs.CheckID]*structs.HealthCheck { + checks := make(map[structs.CheckID]*structs.HealthCheck) l.RLock() defer l.RUnlock() - for name, check := range l.checks { - checks[name] = check + for checkID, check := range l.checks { + checks[checkID] = check } return checks } @@ -406,7 +406,7 @@ func (l *localState) setSyncState() error { } // Index the remote health checks to improve efficiency - checkIndex := make(map[string]*structs.HealthCheck, len(checks)) + checkIndex := make(map[structs.CheckID]*structs.HealthCheck, len(checks)) for _, check := range checks { checkIndex[check.CheckID] = check } @@ -546,7 +546,7 @@ func (l *localState) deleteService(id string) error { } // deleteCheck is used to delete a service from the server -func (l *localState) deleteCheck(id string) error { +func (l *localState) deleteCheck(id structs.CheckID) error { if id == "" { return fmt.Errorf("CheckID missing") } @@ -619,7 +619,7 @@ func (l *localState) syncService(id string) error { } // syncCheck is used to sync a check to the server -func (l *localState) syncCheck(id string) error { +func (l *localState) syncCheck(id structs.CheckID) error { // Pull in the associated service if any check := l.checks[id] var service *structs.NodeService diff --git a/command/agent/structs.go b/command/agent/structs.go index 5ebc3b455..8ccdfd1fc 100644 --- a/command/agent/structs.go +++ b/command/agent/structs.go @@ -44,7 +44,7 @@ func (s *ServiceDefinition) CheckTypes() (checks CheckTypes) { // ChecKDefinition is used to JSON decode the Check definitions type CheckDefinition struct { - ID string + ID structs.CheckID Name string Notes string ServiceID string @@ -66,7 +66,7 @@ func (c *CheckDefinition) HealthCheck(node string) *structs.HealthCheck { health.Status = c.Status } if health.CheckID == "" && health.Name != "" { - health.CheckID = health.Name + health.CheckID = structs.CheckID(health.Name) } return health } diff --git a/command/agent/util.go b/command/agent/util.go index e8d802d78..152939ee1 100644 --- a/command/agent/util.go +++ b/command/agent/util.go @@ -12,6 +12,7 @@ import ( "strconv" "time" + "github.com/hashicorp/consul/consul/structs" "github.com/hashicorp/go-msgpack/codec" ) @@ -71,6 +72,11 @@ func stringHash(s string) string { return fmt.Sprintf("%x", md5.Sum([]byte(s))) } +// checkIDHash returns a simple md5sum for a structs.CheckID. +func checkIDHash(checkID structs.CheckID) string { + return fmt.Sprintf("%x", md5.Sum([]byte(string(checkID)))) +} + // FilePermissions is an interface which allows a struct to set // ownership and permissions easily on a file it describes. type FilePermissions interface { diff --git a/consul/catalog_endpoint.go b/consul/catalog_endpoint.go index ec8389dc1..9b5d2b47c 100644 --- a/consul/catalog_endpoint.go +++ b/consul/catalog_endpoint.go @@ -57,7 +57,7 @@ func (c *Catalog) Register(args *structs.RegisterRequest, reply *struct{}) error } for _, check := range args.Checks { if check.CheckID == "" && check.Name != "" { - check.CheckID = check.Name + check.CheckID = structs.CheckID(check.Name) } if check.Node == "" { check.Node = args.Node diff --git a/consul/state/state_store.go b/consul/state/state_store.go index a5b4b2f83..1d4adabbc 100644 --- a/consul/state/state_store.go +++ b/consul/state/state_store.go @@ -594,7 +594,7 @@ func (s *StateStore) deleteNodeTxn(tx *memdb.Txn, idx uint64, nodeID string) err if err != nil { return fmt.Errorf("failed check lookup: %s", err) } - var cids []string + var cids []structs.CheckID for check := checks.Next(); check != nil; check = checks.Next() { cids = append(cids, check.(*structs.HealthCheck).CheckID) } @@ -917,7 +917,7 @@ func (s *StateStore) deleteServiceTxn(tx *memdb.Txn, idx uint64, watches *DumbWa if err != nil { return fmt.Errorf("failed service check lookup: %s", err) } - var cids []string + var cids []structs.CheckID for check := checks.Next(); check != nil; check = checks.Next() { cids = append(cids, check.(*structs.HealthCheck).CheckID) } @@ -1119,7 +1119,7 @@ func (s *StateStore) parseChecks(idx uint64, iter memdb.ResultIterator) (uint64, } // DeleteCheck is used to delete a health check registration. -func (s *StateStore) DeleteCheck(idx uint64, node, id string) error { +func (s *StateStore) DeleteCheck(idx uint64, node string, id structs.CheckID) error { tx := s.db.Txn(true) defer tx.Abort() @@ -1136,7 +1136,7 @@ func (s *StateStore) DeleteCheck(idx uint64, node, id string) error { // deleteCheckTxn is the inner method used to call a health // check deletion within an existing transaction. -func (s *StateStore) deleteCheckTxn(tx *memdb.Txn, idx uint64, watches *DumbWatchManager, node, id string) error { +func (s *StateStore) deleteCheckTxn(tx *memdb.Txn, idx uint64, watches *DumbWatchManager, node string, id structs.CheckID) error { // Try to retrieve the existing health check. hc, err := tx.First("checks", "id", node, id) if err != nil { diff --git a/consul/state/state_store_test.go b/consul/state/state_store_test.go index d88275642..9f9c60d69 100644 --- a/consul/state/state_store_test.go +++ b/consul/state/state_store_test.go @@ -82,7 +82,7 @@ func testRegisterService(t *testing.T, s *StateStore, idx uint64, nodeID, servic } func testRegisterCheck(t *testing.T, s *StateStore, idx uint64, - nodeID, serviceID, checkID, state string) { + nodeID string, serviceID string, checkID structs.CheckID, state string) { chk := &structs.HealthCheck{ Node: nodeID, CheckID: checkID, diff --git a/consul/structs/structs.go b/consul/structs/structs.go index 08c0d8e5c..379dfcd96 100644 --- a/consul/structs/structs.go +++ b/consul/structs/structs.go @@ -181,7 +181,7 @@ type DeregisterRequest struct { Datacenter string Node string ServiceID string - CheckID string + CheckID CheckID WriteRequest } @@ -364,16 +364,19 @@ type NodeServices struct { Services map[string]*NodeService } +// CheckID is a strongly typed string +type CheckID string + // HealthCheck represents a single check on a given node type HealthCheck struct { Node string - CheckID string // Unique per-node ID - Name string // Check name - Status string // The current check status - Notes string // Additional notes with the status - Output string // Holds output of script runs - ServiceID string // optional associated service - ServiceName string // optional service name + CheckID CheckID // Unique per-node ID + Name string // Check name + Status string // The current check status + Notes string // Additional notes with the status + Output string // Holds output of script runs + ServiceID string // optional associated service + ServiceName string // optional service name RaftIndex } diff --git a/consul/structs/structs_test.go b/consul/structs/structs_test.go index d7672cad5..412b7f44b 100644 --- a/consul/structs/structs_test.go +++ b/consul/structs/structs_test.go @@ -202,7 +202,8 @@ func TestStructs_HealthCheck_IsSame(t *testing.T) { } check(&other.Node) - check(&other.CheckID) + checkStr := string(other.CheckID) + check(&checkStr) check(&other.Name) check(&other.Status) check(&other.Notes)