From cbb945e76a78fe1ae157dacb748557de0c31691d Mon Sep 17 00:00:00 2001 From: Sean Chittenden Date: Mon, 6 Jun 2016 12:48:20 -0700 Subject: [PATCH] Move `structs.CheckID` to a new top-level package, `types`. Per discussion w/ @slackpad, move this type to its own top-level package --- command/agent/agent.go | 39 ++++++----- command/agent/agent_endpoint.go | 11 ++-- command/agent/check.go | 15 ++--- command/agent/check_test.go | 107 +++++++++++++++---------------- command/agent/local.go | 35 +++++----- command/agent/structs.go | 5 +- command/agent/util.go | 8 +-- consul/catalog_endpoint.go | 3 +- consul/state/state_store.go | 9 ++- consul/state/state_store_test.go | 3 +- consul/structs/structs.go | 20 +++--- types/README.md | 39 ----------- types/checks.go | 5 -- 13 files changed, 124 insertions(+), 175 deletions(-) delete mode 100644 types/README.md delete mode 100644 types/checks.go diff --git a/command/agent/agent.go b/command/agent/agent.go index 138f1799c5..ecdfb21dd7 100644 --- a/command/agent/agent.go +++ b/command/agent/agent.go @@ -19,7 +19,6 @@ import ( "github.com/hashicorp/consul/consul/state" "github.com/hashicorp/consul/consul/structs" "github.com/hashicorp/consul/lib" - "github.com/hashicorp/consul/types" "github.com/hashicorp/serf/coordinate" "github.com/hashicorp/serf/serf" ) @@ -75,19 +74,19 @@ type Agent struct { state localState // checkMonitors maps the check ID to an associated monitor - checkMonitors map[types.CheckID]*CheckMonitor + checkMonitors map[structs.CheckID]*CheckMonitor // checkHTTPs maps the check ID to an associated HTTP check - checkHTTPs map[types.CheckID]*CheckHTTP + checkHTTPs map[structs.CheckID]*CheckHTTP // checkTCPs maps the check ID to an associated TCP check - checkTCPs map[types.CheckID]*CheckTCP + checkTCPs map[structs.CheckID]*CheckTCP // checkTTLs maps the check ID to an associated check TTL - checkTTLs map[types.CheckID]*CheckTTL + checkTTLs map[structs.CheckID]*CheckTTL // checkDockers maps the check ID to an associated Docker Exec based check - checkDockers map[types.CheckID]*CheckDocker + checkDockers map[structs.CheckID]*CheckDocker // checkLock protects updates to the check* maps checkLock sync.Mutex @@ -177,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[types.CheckID]*CheckMonitor), - checkTTLs: make(map[types.CheckID]*CheckTTL), - checkHTTPs: make(map[types.CheckID]*CheckHTTP), - checkTCPs: make(map[types.CheckID]*CheckTCP), - checkDockers: make(map[types.CheckID]*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{}), @@ -725,7 +724,7 @@ 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 types.CheckID) error { +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) @@ -792,7 +791,7 @@ func (a *Agent) AddService(service *structs.NodeService, chkTypes CheckTypes, pe } check := &structs.HealthCheck{ Node: a.config.NodeName, - CheckID: types.CheckID(checkID), + CheckID: structs.CheckID(checkID), Name: fmt.Sprintf("Service '%s' check", service.Service), Status: structs.HealthCritical, Notes: chkType.Notes, @@ -999,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 types.CheckID, persist bool) error { +func (a *Agent) RemoveCheck(checkID structs.CheckID, persist bool) error { // Validate CheckID if checkID == "" { return fmt.Errorf("CheckID missing") @@ -1042,7 +1041,7 @@ func (a *Agent) RemoveCheck(checkID types.CheckID, 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 types.CheckID, status, output string) error { +func (a *Agent) UpdateCheck(checkID structs.CheckID, status, output string) error { a.checkLock.Lock() defer a.checkLock.Unlock() @@ -1130,7 +1129,7 @@ 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 types.CheckID) error { +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) { @@ -1394,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[types.CheckID]*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[types.CheckID]*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) types.CheckID { - return types.CheckID(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 83e2e94cbe..729fa88fcd 100644 --- a/command/agent/agent_endpoint.go +++ b/command/agent/agent_endpoint.go @@ -7,7 +7,6 @@ import ( "strings" "github.com/hashicorp/consul/consul/structs" - "github.com/hashicorp/consul/types" "github.com/hashicorp/serf/coordinate" "github.com/hashicorp/serf/serf" ) @@ -131,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 := types.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 } @@ -140,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 := types.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 @@ -150,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 := types.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 @@ -160,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 := types.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 @@ -213,7 +212,7 @@ func (s *HTTPServer) AgentCheckUpdate(resp http.ResponseWriter, req *http.Reques update.Output[:CheckBufSize], CheckBufSize, total) } - checkID := types.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/check.go b/command/agent/check.go index a0102b9914..578517a50a 100644 --- a/command/agent/check.go +++ b/command/agent/check.go @@ -16,7 +16,6 @@ import ( docker "github.com/fsouza/go-dockerclient" "github.com/hashicorp/consul/consul/structs" "github.com/hashicorp/consul/lib" - "github.com/hashicorp/consul/types" "github.com/hashicorp/go-cleanhttp" ) @@ -91,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 types.CheckID, status, output string) + UpdateCheck(checkID structs.CheckID, status, output string) } // CheckMonitor is used to periodically invoke a script to @@ -99,7 +98,7 @@ type CheckNotifier interface { // nagios plugins and expects the output in the same format. type CheckMonitor struct { Notify CheckNotifier - CheckID types.CheckID + CheckID structs.CheckID Script string Interval time.Duration Timeout time.Duration @@ -232,7 +231,7 @@ func (c *CheckMonitor) check() { // automatically set to critical. type CheckTTL struct { Notify CheckNotifier - CheckID types.CheckID + CheckID structs.CheckID TTL time.Duration Logger *log.Logger @@ -323,7 +322,7 @@ type persistedCheck struct { // expiration timestamp which is used to determine staleness on later // agent restarts. type persistedCheckState struct { - CheckID types.CheckID + CheckID structs.CheckID Output string Status string Expires int64 @@ -337,7 +336,7 @@ type persistedCheckState struct { // or if the request returns an error type CheckHTTP struct { Notify CheckNotifier - CheckID types.CheckID + CheckID structs.CheckID HTTP string Interval time.Duration Timeout time.Duration @@ -463,7 +462,7 @@ func (c *CheckHTTP) check() { // The check is critical if the connection returns an error type CheckTCP struct { Notify CheckNotifier - CheckID types.CheckID + CheckID structs.CheckID TCP string Interval time.Duration Timeout time.Duration @@ -554,7 +553,7 @@ type DockerClient interface { // with nagios plugins and expects the output in the same format. type CheckDocker struct { Notify CheckNotifier - CheckID types.CheckID + CheckID structs.CheckID Script string DockerContainerID string Shell string diff --git a/command/agent/check_test.go b/command/agent/check_test.go index 53d203eebe..2e23c75c2e 100644 --- a/command/agent/check_test.go +++ b/command/agent/check_test.go @@ -18,16 +18,15 @@ import ( docker "github.com/fsouza/go-dockerclient" "github.com/hashicorp/consul/consul/structs" "github.com/hashicorp/consul/testutil" - "github.com/hashicorp/consul/types" ) type MockNotify struct { - state map[types.CheckID]string - updates map[types.CheckID]int - output map[types.CheckID]string + state map[structs.CheckID]string + updates map[structs.CheckID]int + output map[structs.CheckID]string } -func (m *MockNotify) UpdateCheck(id types.CheckID, 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 @@ -36,13 +35,13 @@ func (m *MockNotify) UpdateCheck(id types.CheckID, status, output string) { func expectStatus(t *testing.T, script, status string) { mock := &MockNotify{ - state: make(map[types.CheckID]string), - updates: make(map[types.CheckID]int), - output: make(map[types.CheckID]string), + state: make(map[structs.CheckID]string), + updates: make(map[structs.CheckID]int), + output: make(map[structs.CheckID]string), } check := &CheckMonitor{ Notify: mock, - CheckID: types.CheckID("foo"), + CheckID: structs.CheckID("foo"), Script: script, Interval: 10 * time.Millisecond, Logger: log.New(os.Stderr, "", log.LstdFlags), @@ -85,13 +84,13 @@ func TestCheckMonitor_BadCmd(t *testing.T) { func TestCheckMonitor_Timeout(t *testing.T) { mock := &MockNotify{ - state: make(map[types.CheckID]string), - updates: make(map[types.CheckID]int), - output: make(map[types.CheckID]string), + state: make(map[structs.CheckID]string), + updates: make(map[structs.CheckID]int), + output: make(map[structs.CheckID]string), } check := &CheckMonitor{ Notify: mock, - CheckID: types.CheckID("foo"), + CheckID: structs.CheckID("foo"), Script: "sleep 1 && exit 0", Interval: 10 * time.Millisecond, Timeout: 5 * time.Millisecond, @@ -115,13 +114,13 @@ func TestCheckMonitor_Timeout(t *testing.T) { func TestCheckMonitor_RandomStagger(t *testing.T) { mock := &MockNotify{ - state: make(map[types.CheckID]string), - updates: make(map[types.CheckID]int), - output: make(map[types.CheckID]string), + state: make(map[structs.CheckID]string), + updates: make(map[structs.CheckID]int), + output: make(map[structs.CheckID]string), } check := &CheckMonitor{ Notify: mock, - CheckID: types.CheckID("foo"), + CheckID: structs.CheckID("foo"), Script: "exit 0", Interval: 25 * time.Millisecond, Logger: log.New(os.Stderr, "", log.LstdFlags), @@ -144,13 +143,13 @@ func TestCheckMonitor_RandomStagger(t *testing.T) { func TestCheckMonitor_LimitOutput(t *testing.T) { mock := &MockNotify{ - state: make(map[types.CheckID]string), - updates: make(map[types.CheckID]int), - output: make(map[types.CheckID]string), + state: make(map[structs.CheckID]string), + updates: make(map[structs.CheckID]int), + output: make(map[structs.CheckID]string), } check := &CheckMonitor{ Notify: mock, - CheckID: types.CheckID("foo"), + CheckID: structs.CheckID("foo"), Script: "od -N 81920 /dev/urandom", Interval: 25 * time.Millisecond, Logger: log.New(os.Stderr, "", log.LstdFlags), @@ -169,13 +168,13 @@ func TestCheckMonitor_LimitOutput(t *testing.T) { func TestCheckTTL(t *testing.T) { mock := &MockNotify{ - state: make(map[types.CheckID]string), - updates: make(map[types.CheckID]int), - output: make(map[types.CheckID]string), + state: make(map[structs.CheckID]string), + updates: make(map[structs.CheckID]int), + output: make(map[structs.CheckID]string), } check := &CheckTTL{ Notify: mock, - CheckID: types.CheckID("foo"), + CheckID: structs.CheckID("foo"), TTL: 100 * time.Millisecond, Logger: log.New(os.Stderr, "", log.LstdFlags), } @@ -230,13 +229,13 @@ func mockHTTPServer(responseCode int) *httptest.Server { func expectHTTPStatus(t *testing.T, url string, status string) { mock := &MockNotify{ - state: make(map[types.CheckID]string), - updates: make(map[types.CheckID]int), - output: make(map[types.CheckID]string), + state: make(map[structs.CheckID]string), + updates: make(map[structs.CheckID]int), + output: make(map[structs.CheckID]string), } check := &CheckHTTP{ Notify: mock, - CheckID: types.CheckID("foo"), + CheckID: structs.CheckID("foo"), HTTP: url, Interval: 10 * time.Millisecond, Logger: log.New(os.Stderr, "", log.LstdFlags), @@ -330,14 +329,14 @@ func TestCheckHTTPTimeout(t *testing.T) { defer server.Close() mock := &MockNotify{ - state: make(map[types.CheckID]string), - updates: make(map[types.CheckID]int), - output: make(map[types.CheckID]string), + state: make(map[structs.CheckID]string), + updates: make(map[structs.CheckID]int), + output: make(map[structs.CheckID]string), } check := &CheckHTTP{ Notify: mock, - CheckID: types.CheckID("bar"), + CheckID: structs.CheckID("bar"), HTTP: server.URL, Timeout: 5 * time.Millisecond, Interval: 10 * time.Millisecond, @@ -361,7 +360,7 @@ func TestCheckHTTPTimeout(t *testing.T) { func TestCheckHTTP_disablesKeepAlives(t *testing.T) { check := &CheckHTTP{ - CheckID: types.CheckID("foo"), + CheckID: structs.CheckID("foo"), HTTP: "http://foo.bar/baz", Interval: 10 * time.Second, Logger: log.New(os.Stderr, "", log.LstdFlags), @@ -396,13 +395,13 @@ func mockTCPServer(network string) net.Listener { func expectTCPStatus(t *testing.T, tcp string, status string) { mock := &MockNotify{ - state: make(map[types.CheckID]string), - updates: make(map[types.CheckID]int), - output: make(map[types.CheckID]string), + state: make(map[structs.CheckID]string), + updates: make(map[structs.CheckID]int), + output: make(map[structs.CheckID]string), } check := &CheckTCP{ Notify: mock, - CheckID: types.CheckID("foo"), + CheckID: structs.CheckID("foo"), TCP: tcp, Interval: 10 * time.Millisecond, Logger: log.New(os.Stderr, "", log.LstdFlags), @@ -576,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[types.CheckID]string), - updates: make(map[types.CheckID]int), - output: make(map[types.CheckID]string), + state: make(map[structs.CheckID]string), + updates: make(map[structs.CheckID]int), + output: make(map[structs.CheckID]string), } check := &CheckDocker{ Notify: mock, - CheckID: types.CheckID("foo"), + CheckID: structs.CheckID("foo"), Script: "/health.sh", DockerContainerID: "54432bad1fc7", Shell: "/bin/sh", @@ -636,13 +635,13 @@ func TestDockerCheckWhenExecInfoFails(t *testing.T) { func TestDockerCheckDefaultToSh(t *testing.T) { os.Setenv("SHELL", "") mock := &MockNotify{ - state: make(map[types.CheckID]string), - updates: make(map[types.CheckID]int), - output: make(map[types.CheckID]string), + state: make(map[structs.CheckID]string), + updates: make(map[structs.CheckID]int), + output: make(map[structs.CheckID]string), } check := &CheckDocker{ Notify: mock, - CheckID: types.CheckID("foo"), + CheckID: structs.CheckID("foo"), Script: "/health.sh", DockerContainerID: "54432bad1fc7", Interval: 10 * time.Millisecond, @@ -660,14 +659,14 @@ func TestDockerCheckDefaultToSh(t *testing.T) { func TestDockerCheckUseShellFromEnv(t *testing.T) { mock := &MockNotify{ - state: make(map[types.CheckID]string), - updates: make(map[types.CheckID]int), - output: make(map[types.CheckID]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: types.CheckID("foo"), + CheckID: structs.CheckID("foo"), Script: "/health.sh", DockerContainerID: "54432bad1fc7", Interval: 10 * time.Millisecond, @@ -686,13 +685,13 @@ func TestDockerCheckUseShellFromEnv(t *testing.T) { func TestDockerCheckTruncateOutput(t *testing.T) { mock := &MockNotify{ - state: make(map[types.CheckID]string), - updates: make(map[types.CheckID]int), - output: make(map[types.CheckID]string), + state: make(map[structs.CheckID]string), + updates: make(map[structs.CheckID]int), + output: make(map[structs.CheckID]string), } check := &CheckDocker{ Notify: mock, - CheckID: types.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 66a05721c2..11d85271c8 100644 --- a/command/agent/local.go +++ b/command/agent/local.go @@ -12,7 +12,6 @@ import ( "github.com/hashicorp/consul/consul" "github.com/hashicorp/consul/consul/structs" "github.com/hashicorp/consul/lib" - "github.com/hashicorp/consul/types" ) const ( @@ -57,12 +56,12 @@ type localState struct { serviceTokens map[string]string // Checks tracks the local checks - checks map[types.CheckID]*structs.HealthCheck - checkStatus map[types.CheckID]syncStatus - checkTokens map[types.CheckID]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[types.CheckID]*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 @@ -80,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[types.CheckID]*structs.HealthCheck) - l.checkStatus = make(map[types.CheckID]syncStatus) - l.checkTokens = make(map[types.CheckID]string) - l.deferCheck = make(map[types.CheckID]*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) } @@ -194,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 types.CheckID) 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 types.CheckID) string { +func (l *localState) checkToken(id structs.CheckID) string { token := l.checkTokens[id] if token == "" { token = l.config.ACLToken @@ -227,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 types.CheckID) { +func (l *localState) RemoveCheck(checkID structs.CheckID) { l.Lock() defer l.Unlock() @@ -238,7 +237,7 @@ func (l *localState) RemoveCheck(checkID types.CheckID) { } // UpdateCheck is used to update the status of a check -func (l *localState) UpdateCheck(checkID types.CheckID, status, output string) { +func (l *localState) UpdateCheck(checkID structs.CheckID, status, output string) { l.Lock() defer l.Unlock() @@ -283,8 +282,8 @@ func (l *localState) UpdateCheck(checkID types.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[types.CheckID]*structs.HealthCheck { - checks := make(map[types.CheckID]*structs.HealthCheck) +func (l *localState) Checks() map[structs.CheckID]*structs.HealthCheck { + checks := make(map[structs.CheckID]*structs.HealthCheck) l.RLock() defer l.RUnlock() @@ -407,7 +406,7 @@ func (l *localState) setSyncState() error { } // Index the remote health checks to improve efficiency - checkIndex := make(map[types.CheckID]*structs.HealthCheck, len(checks)) + checkIndex := make(map[structs.CheckID]*structs.HealthCheck, len(checks)) for _, check := range checks { checkIndex[check.CheckID] = check } @@ -547,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 types.CheckID) error { +func (l *localState) deleteCheck(id structs.CheckID) error { if id == "" { return fmt.Errorf("CheckID missing") } @@ -620,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 types.CheckID) 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 442b4b2965..8ccdfd1fc3 100644 --- a/command/agent/structs.go +++ b/command/agent/structs.go @@ -2,7 +2,6 @@ package agent import ( "github.com/hashicorp/consul/consul/structs" - "github.com/hashicorp/consul/types" ) // ServiceDefinition is used to JSON decode the Service definitions @@ -45,7 +44,7 @@ func (s *ServiceDefinition) CheckTypes() (checks CheckTypes) { // ChecKDefinition is used to JSON decode the Check definitions type CheckDefinition struct { - ID types.CheckID + ID structs.CheckID Name string Notes string ServiceID string @@ -67,7 +66,7 @@ func (c *CheckDefinition) HealthCheck(node string) *structs.HealthCheck { health.Status = c.Status } if health.CheckID == "" && health.Name != "" { - health.CheckID = types.CheckID(health.Name) + health.CheckID = structs.CheckID(health.Name) } return health } diff --git a/command/agent/util.go b/command/agent/util.go index 3916c70c97..152939ee17 100644 --- a/command/agent/util.go +++ b/command/agent/util.go @@ -12,7 +12,7 @@ import ( "strconv" "time" - "github.com/hashicorp/consul/types" + "github.com/hashicorp/consul/consul/structs" "github.com/hashicorp/go-msgpack/codec" ) @@ -72,9 +72,9 @@ func stringHash(s string) string { return fmt.Sprintf("%x", md5.Sum([]byte(s))) } -// checkIDHash returns a simple md5sum for a types.CheckID. -func checkIDHash(checkID types.CheckID) string { - return stringHash(string(checkID)) +// 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 diff --git a/consul/catalog_endpoint.go b/consul/catalog_endpoint.go index 06f81993b9..9b5d2b47cd 100644 --- a/consul/catalog_endpoint.go +++ b/consul/catalog_endpoint.go @@ -6,7 +6,6 @@ import ( "github.com/armon/go-metrics" "github.com/hashicorp/consul/consul/structs" - "github.com/hashicorp/consul/types" ) // Catalog endpoint is used to manipulate the service catalog @@ -58,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 = types.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 bf5334a327..1d4adabbc2 100644 --- a/consul/state/state_store.go +++ b/consul/state/state_store.go @@ -7,7 +7,6 @@ import ( "time" "github.com/hashicorp/consul/consul/structs" - "github.com/hashicorp/consul/types" "github.com/hashicorp/go-memdb" "github.com/hashicorp/serf/coordinate" ) @@ -595,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 []types.CheckID + var cids []structs.CheckID for check := checks.Next(); check != nil; check = checks.Next() { cids = append(cids, check.(*structs.HealthCheck).CheckID) } @@ -918,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 []types.CheckID + var cids []structs.CheckID for check := checks.Next(); check != nil; check = checks.Next() { cids = append(cids, check.(*structs.HealthCheck).CheckID) } @@ -1120,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 string, id types.CheckID) error { +func (s *StateStore) DeleteCheck(idx uint64, node string, id structs.CheckID) error { tx := s.db.Txn(true) defer tx.Abort() @@ -1137,7 +1136,7 @@ func (s *StateStore) DeleteCheck(idx uint64, node string, id types.CheckID) erro // 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 string, id types.CheckID) 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 99ee6c7b54..9f9c60d696 100644 --- a/consul/state/state_store_test.go +++ b/consul/state/state_store_test.go @@ -12,7 +12,6 @@ import ( "github.com/hashicorp/consul/consul/structs" "github.com/hashicorp/consul/lib" - "github.com/hashicorp/consul/types" "github.com/hashicorp/serf/coordinate" ) @@ -83,7 +82,7 @@ func testRegisterService(t *testing.T, s *StateStore, idx uint64, nodeID, servic } func testRegisterCheck(t *testing.T, s *StateStore, idx uint64, - nodeID string, serviceID string, checkID types.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 c69db8ea26..379dfcd963 100644 --- a/consul/structs/structs.go +++ b/consul/structs/structs.go @@ -8,7 +8,6 @@ import ( "time" "github.com/hashicorp/consul/acl" - "github.com/hashicorp/consul/types" "github.com/hashicorp/go-msgpack/codec" "github.com/hashicorp/serf/coordinate" ) @@ -182,7 +181,7 @@ type DeregisterRequest struct { Datacenter string Node string ServiceID string - CheckID types.CheckID + CheckID CheckID WriteRequest } @@ -365,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 types.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 + 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/types/README.md b/types/README.md deleted file mode 100644 index da662f4a1c..0000000000 --- a/types/README.md +++ /dev/null @@ -1,39 +0,0 @@ -# Consul `types` Package - -The Go language has a strong type system built into the language. The -`types` package corrals named types into a single package that is terminal in -`go`'s import graph. The `types` package should not have any downstream -dependencies. Each subsystem that defines its own set of types exists in its -own file, but all types are defined in the same package. - -# Why - -> Everything should be made as simple as possible, but not simpler. - -`string` is a useful container and underlying type for identifiers, however -the `string` type is effectively opaque to the compiler in terms of how a -given string is intended to be used. For instance, there is nothing -preventing the following from happening: - -```go -// `map` of Widgets, looked up by ID -var widgetLookup map[string]*Widget -// ... -var widgetID string = "widgetID" -w, found := widgetLookup[widgetID] - -// Bad! -var widgetName string = "name of widget" -w, found := widgetLookup[widgetName] -``` - -but this class of problem is entirely preventable: - -```go -type WidgetID string -var widgetLookup map[WidgetID]*Widget -var widgetName -``` - -TL;DR: intentions and idioms aren't statically checked by compilers. The -`types` package uses Go's strong type system to prevent this class of bug. diff --git a/types/checks.go b/types/checks.go deleted file mode 100644 index 25a136b4f4..0000000000 --- a/types/checks.go +++ /dev/null @@ -1,5 +0,0 @@ -package types - -// CheckID is a strongly typed string used to uniquely represent a Consul -// Check on an Agent (a CheckID is not globally unique). -type CheckID string