luxolus
/
open-consul
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

consul/state: refactor tnxService to avoid missed cases 

Handling errors at the end of a log switch/case block is somewhat
brittle. This block included a couple cases where errors were ignored,
but it was not obvious the way it was written.

This change moves all error handling into each case block. There is
still potentially one case where err is ignored, which will be handled
in a follow up.
This commit is contained in:
Daniel Nephin 2020-05-19 16:48:17 -04:00
parent 3e76ebbd49
commit 6e3a7b0aa8
1 changed files with 32 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

58
agent/consul/state/txn.go
View File

@ -210,64 +210,62 @@ func (s *Store) txnNode(tx *memdb.Txn, idx uint64, op *structs.TxnNodeOp) (struc
// txnService handles all Service-related operations.
func (s *Store) txnService(tx *memdb.Txn, idx uint64, op *structs.TxnServiceOp) (structs.TxnResults, error) {
var entry *structs.NodeService
var err error
switch op.Verb {
case api.ServiceGet:
entry, err = s.getNodeServiceTxn(tx, op.Node, op.Service.ID, &op.Service.EnterpriseMeta)
if entry == nil && err == nil {
entry, err := s.getNodeServiceTxn(tx, op.Node, op.Service.ID, &op.Service.EnterpriseMeta)
switch {
case err != nil:
return nil, err
case entry == nil:
return nil, fmt.Errorf("service %q on node %q doesn't exist", op.Service.ID, op.Node)
default:
return structs.TxnResults{&structs.TxnResult{Service: entry}}, nil
}
case api.ServiceSet:
err = s.ensureServiceTxn(tx, idx, op.Node, &op.Service)
if err != nil {
if err := s.ensureServiceTxn(tx, idx, op.Node, &op.Service); err != nil {
return nil, err
}
entry, err = s.getNodeServiceTxn(tx, op.Node, op.Service.ID, &op.Service.EnterpriseMeta)
entry, err := s.getNodeServiceTxn(tx, op.Node, op.Service.ID, &op.Service.EnterpriseMeta)
return newTxnResultFromNodeServiceEntry(entry), err
case api.ServiceCAS:
var ok bool
ok, err = s.ensureServiceCASTxn(tx, idx, op.Node, &op.Service)
ok, err := s.ensureServiceCASTxn(tx, idx, op.Node, &op.Service)
// TODO: err != nil case is ignored
if !ok && err == nil {
err = fmt.Errorf("failed to set service %q on node %q, index is stale", op.Service.ID, op.Node)
break
err := fmt.Errorf("failed to set service %q on node %q, index is stale", op.Service.ID, op.Node)
return nil, err
}
entry, err = s.getNodeServiceTxn(tx, op.Node, op.Service.ID, &op.Service.EnterpriseMeta)
entry, err := s.getNodeServiceTxn(tx, op.Node, op.Service.ID, &op.Service.EnterpriseMeta)
return newTxnResultFromNodeServiceEntry(entry), err
case api.ServiceDelete:
err = s.deleteServiceTxn(tx, idx, op.Node, op.Service.ID, &op.Service.EnterpriseMeta)
err := s.deleteServiceTxn(tx, idx, op.Node, op.Service.ID, &op.Service.EnterpriseMeta)
return nil, err
case api.ServiceDeleteCAS:
var ok bool
ok, err = s.deleteServiceCASTxn(tx, idx, op.Service.ModifyIndex, op.Node, op.Service.ID, &op.Service.EnterpriseMeta)
ok, err := s.deleteServiceCASTxn(tx, idx, op.Service.ModifyIndex, op.Node, op.Service.ID, &op.Service.EnterpriseMeta)
if !ok && err == nil {
return nil, fmt.Errorf("failed to delete service %q on node %q, index is stale", op.Service.ID, op.Node)
}
return nil, err
default:
return nil, fmt.Errorf("unknown Service verb %q", op.Verb)
}
if err != nil {
return nil, err
}
// For a GET we keep the value, otherwise we clone and blank out the
// value (we have to clone so we don't modify the entry being used by
// the state store).
if entry != nil {
if op.Verb == api.ServiceGet {
result := structs.TxnResult{Service: entry}
return structs.TxnResults{&result}, nil
// newTxnResultFromNodeServiceEntry returns a TxnResults with a single result,
// a copy of entry. The entry is copied to prevent modification of the state
// store.
func newTxnResultFromNodeServiceEntry(entry *structs.NodeService) structs.TxnResults {
if entry == nil {
return nil
}
clone := *entry
result := structs.TxnResult{Service: &clone}
return structs.TxnResults{&result}, nil
}
return nil, nil
return structs.TxnResults{&result}
}
// txnCheck handles all Check-related operations.