Skip to content

Commit

Permalink
Chase casting types.CheckID to a string into the state_store.
Browse files Browse the repository at this point in the history
It turns out the indexer can only use strings as arguments when
creating a query.  Cast `types.CheckID` to a `string` before calling
into `memdb`.

Ideally the indexer would be smart enough to do this at compile-time,
but I need to look into how to do this without reflection and the
runtime package.  For the time being statically cast `types.CheckID`
to a `string` at the call sites.
  • Loading branch information
sean- committed Jun 7, 2016
1 parent 63adcbd commit e9a2f5b
Show file tree
Hide file tree
Showing 12 changed files with 80 additions and 55 deletions.
19 changes: 11 additions & 8 deletions command/agent/agent_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (

"github.com/hashicorp/consul/consul/structs"
"github.com/hashicorp/consul/testutil"
"github.com/hashicorp/consul/types"
"github.com/hashicorp/serf/serf"
)

Expand Down Expand Up @@ -61,7 +62,7 @@ func TestHTTPAgentChecks(t *testing.T) {
if err != nil {
t.Fatalf("Err: %v", err)
}
val := obj.(map[string]*structs.HealthCheck)
val := obj.(map[types.CheckID]*structs.HealthCheck)
if len(val) != 1 {
t.Fatalf("bad checks: %v", obj)
}
Expand Down Expand Up @@ -294,21 +295,22 @@ func TestHTTPAgentRegisterCheck(t *testing.T) {
}

// Ensure we have a check mapping
if _, ok := srv.agent.state.Checks()["test"]; !ok {
checkID := types.CheckID("test")
if _, ok := srv.agent.state.Checks()[checkID]; !ok {
t.Fatalf("missing test check")
}

if _, ok := srv.agent.checkTTLs["test"]; !ok {
if _, ok := srv.agent.checkTTLs[checkID]; !ok {
t.Fatalf("missing test check ttl")
}

// Ensure the token was configured
if token := srv.agent.state.CheckToken("test"); token == "" {
if token := srv.agent.state.CheckToken(checkID); token == "" {
t.Fatalf("missing token")
}

// By default, checks start in critical state.
state := srv.agent.state.Checks()["test"]
state := srv.agent.state.Checks()[checkID]
if state.Status != structs.HealthCritical {
t.Fatalf("bad: %v", state)
}
Expand Down Expand Up @@ -343,15 +345,16 @@ func TestHTTPAgentRegisterCheckPassing(t *testing.T) {
}

// Ensure we have a check mapping
if _, ok := srv.agent.state.Checks()["test"]; !ok {
checkID := types.CheckID("test")
if _, ok := srv.agent.state.Checks()[checkID]; !ok {
t.Fatalf("missing test check")
}

if _, ok := srv.agent.checkTTLs["test"]; !ok {
if _, ok := srv.agent.checkTTLs[checkID]; !ok {
t.Fatalf("missing test check ttl")
}

state := srv.agent.state.Checks()["test"]
state := srv.agent.state.Checks()[checkID]
if state.Status != structs.HealthPassing {
t.Fatalf("bad: %v", state)
}
Expand Down
12 changes: 6 additions & 6 deletions command/agent/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,17 +192,17 @@ func (l *localState) Services() map[string]*structs.NodeService {
return services
}

// 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 {
// CheckToken is used to return the configured health check token for a
// Check, or if none is configured, the default agent ACL token.
func (l *localState) CheckToken(checkID types.CheckID) string {
l.RLock()
defer l.RUnlock()
return l.checkToken(id)
return l.checkToken(checkID)
}

// checkToken returns an ACL token associated with a check.
func (l *localState) checkToken(id types.CheckID) string {
token := l.checkTokens[id]
func (l *localState) checkToken(checkID types.CheckID) string {
token := l.checkTokens[checkID]
if token == "" {
token = l.config.ACLToken
}
Expand Down
3 changes: 2 additions & 1 deletion command/agent/session_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/hashicorp/consul/consul"
"github.com/hashicorp/consul/consul/structs"
"github.com/hashicorp/consul/types"
)

const (
Expand Down Expand Up @@ -38,7 +39,7 @@ func (s *HTTPServer) SessionCreate(resp http.ResponseWriter, req *http.Request)
Op: structs.SessionCreate,
Session: structs.Session{
Node: s.agent.config.NodeName,
Checks: []string{consul.SerfCheckID},
Checks: []types.CheckID{consul.SerfCheckID},
LockDelay: 15 * time.Second,
Behavior: structs.SessionKeysRelease,
TTL: "",
Expand Down
5 changes: 3 additions & 2 deletions command/agent/session_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

"github.com/hashicorp/consul/consul"
"github.com/hashicorp/consul/consul/structs"
"github.com/hashicorp/consul/types"
)

func TestSessionCreate(t *testing.T) {
Expand Down Expand Up @@ -38,7 +39,7 @@ func TestSessionCreate(t *testing.T) {
raw := map[string]interface{}{
"Name": "my-cool-session",
"Node": srv.agent.config.NodeName,
"Checks": []string{consul.SerfCheckID, "consul"},
"Checks": []types.CheckID{consul.SerfCheckID, "consul"},
"LockDelay": "20s",
}
enc.Encode(raw)
Expand Down Expand Up @@ -86,7 +87,7 @@ func TestSessionCreateDelete(t *testing.T) {
raw := map[string]interface{}{
"Name": "my-cool-session",
"Node": srv.agent.config.NodeName,
"Checks": []string{consul.SerfCheckID, "consul"},
"Checks": []types.CheckID{consul.SerfCheckID, "consul"},
"LockDelay": "20s",
"Behavior": structs.SessionKeysDelete,
}
Expand Down
2 changes: 1 addition & 1 deletion command/agent/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func (s *ServiceDefinition) CheckTypes() (checks CheckTypes) {
return
}

// ChecKDefinition is used to JSON decode the Check definitions
// CheckDefinition is used to JSON decode the Check definitions
type CheckDefinition struct {
ID types.CheckID
Name string
Expand Down
2 changes: 1 addition & 1 deletion command/maint.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func (c *MaintCommand) Run(args []string) int {
c.Ui.Output(" Name: " + nodeName)
c.Ui.Output(" Reason: " + check.Notes)
c.Ui.Output("")
} else if strings.HasPrefix(check.CheckID, "_service_maintenance:") {
} else if strings.HasPrefix(string(check.CheckID), "_service_maintenance:") {
c.Ui.Output("Service:")
c.Ui.Output(" ID: " + check.ServiceID)
c.Ui.Output(" Reason: " + check.Notes)
Expand Down
3 changes: 2 additions & 1 deletion consul/fsm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ 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/go-uuid"
"github.com/hashicorp/raft"
)
Expand Down Expand Up @@ -860,7 +861,7 @@ func TestFSM_SessionCreate_Destroy(t *testing.T) {
Session: structs.Session{
ID: generateUUID(),
Node: "foo",
Checks: []string{"web"},
Checks: []types.CheckID{"web"},
},
}
buf, err := structs.Encode(structs.SessionRequestType, req)
Expand Down
15 changes: 8 additions & 7 deletions consul/leader.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,19 @@ import (
"github.com/armon/go-metrics"
"github.com/hashicorp/consul/consul/agent"
"github.com/hashicorp/consul/consul/structs"
"github.com/hashicorp/consul/types"
"github.com/hashicorp/raft"
"github.com/hashicorp/serf/serf"
)

const (
SerfCheckID = "serfHealth"
SerfCheckName = "Serf Health Status"
SerfCheckAliveOutput = "Agent alive and reachable"
SerfCheckFailedOutput = "Agent not live or unreachable"
ConsulServiceID = "consul"
ConsulServiceName = "consul"
newLeaderEvent = "consul:new-leader"
SerfCheckID types.CheckID = "serfHealth"
SerfCheckName = "Serf Health Status"
SerfCheckAliveOutput = "Agent alive and reachable"
SerfCheckFailedOutput = "Agent not live or unreachable"
ConsulServiceID = "consul"
ConsulServiceName = "consul"
newLeaderEvent = "consul:new-leader"
)

// monitorLeadership is used to monitor if we acquire or lose our role
Expand Down
18 changes: 9 additions & 9 deletions consul/state/state_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ type IndexEntry struct {
// store and thus it is not exported.
type sessionCheck struct {
Node string
CheckID string
CheckID types.CheckID
Session string
}

Expand Down Expand Up @@ -969,7 +969,7 @@ func (s *StateStore) EnsureCheck(idx uint64, hc *structs.HealthCheck) error {
func (s *StateStore) ensureCheckTxn(tx *memdb.Txn, idx uint64, watches *DumbWatchManager,
hc *structs.HealthCheck) error {
// Check if we have an existing health check
existing, err := tx.First("checks", "id", hc.Node, hc.CheckID)
existing, err := tx.First("checks", "id", hc.Node, string(hc.CheckID))
if err != nil {
return fmt.Errorf("failed health check lookup: %s", err)
}
Expand Down Expand Up @@ -1014,7 +1014,7 @@ func (s *StateStore) ensureCheckTxn(tx *memdb.Txn, idx uint64, watches *DumbWatc

// Delete any sessions for this check if the health is critical.
if hc.Status == structs.HealthCritical {
mappings, err := tx.Get("session_checks", "node_check", hc.Node, hc.CheckID)
mappings, err := tx.Get("session_checks", "node_check", hc.Node, string(hc.CheckID))
if err != nil {
return fmt.Errorf("failed session checks lookup: %s", err)
}
Expand Down Expand Up @@ -1120,13 +1120,13 @@ 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, checkID types.CheckID) error {
tx := s.db.Txn(true)
defer tx.Abort()

// Call the check deletion
watches := NewDumbWatchManager(s.tableWatches)
if err := s.deleteCheckTxn(tx, idx, watches, node, id); err != nil {
if err := s.deleteCheckTxn(tx, idx, watches, node, checkID); err != nil {
return err
}

Expand All @@ -1137,9 +1137,9 @@ 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, checkID types.CheckID) error {
// Try to retrieve the existing health check.
hc, err := tx.First("checks", "id", node, id)
hc, err := tx.First("checks", "id", node, string(checkID))
if err != nil {
return fmt.Errorf("check lookup failed: %s", err)
}
Expand All @@ -1156,7 +1156,7 @@ func (s *StateStore) deleteCheckTxn(tx *memdb.Txn, idx uint64, watches *DumbWatc
}

// Delete any sessions for this check.
mappings, err := tx.Get("session_checks", "node_check", node, id)
mappings, err := tx.Get("session_checks", "node_check", node, string(checkID))
if err != nil {
return fmt.Errorf("failed session checks lookup: %s", err)
}
Expand Down Expand Up @@ -1413,7 +1413,7 @@ func (s *StateStore) sessionCreateTxn(tx *memdb.Txn, idx uint64, sess *structs.S

// Go over the session checks and ensure they exist.
for _, checkID := range sess.Checks {
check, err := tx.First("checks", "id", sess.Node, checkID)
check, err := tx.First("checks", "id", sess.Node, string(checkID))
if err != nil {
return fmt.Errorf("failed check lookup: %s", err)
}
Expand Down
16 changes: 8 additions & 8 deletions consul/state/state_store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func testRegisterCheck(t *testing.T, s *StateStore, idx uint64,

tx := s.db.Txn(false)
defer tx.Abort()
c, err := tx.First("checks", "id", nodeID, checkID)
c, err := tx.First("checks", "id", nodeID, string(checkID))
if err != nil {
t.Fatalf("err: %s", err)
}
Expand Down Expand Up @@ -2151,7 +2151,7 @@ func TestStateStore_SessionCreate_SessionGet(t *testing.T) {
sess = &structs.Session{
ID: testUUID(),
Node: "node1",
Checks: []string{"check1"},
Checks: []types.CheckID{"check1"},
}
err = s.SessionCreate(3, sess)
if err == nil || !strings.Contains(err.Error(), "Missing check") {
Expand All @@ -2176,7 +2176,7 @@ func TestStateStore_SessionCreate_SessionGet(t *testing.T) {
sess2 := &structs.Session{
ID: testUUID(),
Node: "node1",
Checks: []string{"check1", "check2"},
Checks: []types.CheckID{"check1", "check2"},
}
if err := s.SessionCreate(6, sess2); err != nil {
t.Fatalf("err: %s", err)
Expand Down Expand Up @@ -2210,7 +2210,7 @@ func TestStateStore_SessionCreate_SessionGet(t *testing.T) {
for i, check := 0, checks.Next(); check != nil; i, check = i+1, checks.Next() {
expectCheck := &sessionCheck{
Node: "node1",
CheckID: fmt.Sprintf("check%d", i+1),
CheckID: types.CheckID(fmt.Sprintf("check%d", i+1)),
Session: sess2.ID,
}
if actual := check.(*sessionCheck); !reflect.DeepEqual(actual, expectCheck) {
Expand Down Expand Up @@ -2408,7 +2408,7 @@ func TestStateStore_Session_Snapshot_Restore(t *testing.T) {
ID: session1,
Node: "node1",
Behavior: structs.SessionKeysDelete,
Checks: []string{"check1"},
Checks: []types.CheckID{"check1"},
},
&structs.Session{
ID: testUUID(),
Expand Down Expand Up @@ -2625,7 +2625,7 @@ func TestStateStore_Session_Invalidate_DeleteService(t *testing.T) {
session := &structs.Session{
ID: testUUID(),
Node: "foo",
Checks: []string{"api"},
Checks: []types.CheckID{"api"},
}
if err := s.SessionCreate(14, session); err != nil {
t.Fatalf("err: %v", err)
Expand Down Expand Up @@ -2673,7 +2673,7 @@ func TestStateStore_Session_Invalidate_Critical_Check(t *testing.T) {
session := &structs.Session{
ID: testUUID(),
Node: "foo",
Checks: []string{"bar"},
Checks: []types.CheckID{"bar"},
}
if err := s.SessionCreate(14, session); err != nil {
t.Fatalf("err: %v", err)
Expand Down Expand Up @@ -2720,7 +2720,7 @@ func TestStateStore_Session_Invalidate_DeleteCheck(t *testing.T) {
session := &structs.Session{
ID: testUUID(),
Node: "foo",
Checks: []string{"bar"},
Checks: []types.CheckID{"bar"},
}
if err := s.SessionCreate(14, session); err != nil {
t.Fatalf("err: %v", err)
Expand Down
2 changes: 1 addition & 1 deletion consul/structs/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -618,7 +618,7 @@ type Session struct {
ID string
Name string
Node string
Checks []string
Checks []types.CheckID
LockDelay time.Duration
Behavior SessionBehavior // What to do when session is invalidated
TTL string
Expand Down
Loading

0 comments on commit e9a2f5b

Please sign in to comment.