Revert "*: combine excessive Hash -> Unit requests into a single recu…

…rsive call."

This reverts commit 091846e.

registry/job.go

@@ -101,25 +101,9 @@ func (r *EtcdRegistry) Units() ([]job.Unit, error) {
 		return nil, err
 	}
 
-	// Fetch all units by hash recursively to avoid sending N requests to Etcd.
-	hashToUnit, err := r.getAllUnitsHashMap()
-	if err != nil {
-		log.Errorf("failed fetching all Units from etcd: %v", err)
-		return nil, err
-	}
-	unitHashLookupFunc := func(hash unit.Hash) *unit.UnitFile {
-		stringHash := hash.String()
-		unit, ok := hashToUnit[stringHash]
-		if !ok {
-			log.Errorf("did not find Unit %v in list of all units", stringHash)
-			return nil
-		}
-		return unit
-	}
-
 	uMap := make(map[string]*job.Unit)
 	for _, dir := range res.Node.Nodes {
-		u, err := r.dirToUnit(dir, unitHashLookupFunc)
+		u, err := r.dirToUnit(dir)
 		if err != nil {
 			log.Errorf("Failed to parse Unit from etcd: %v", err)
 			continue
@@ -159,12 +143,12 @@ func (r *EtcdRegistry) Unit(name string) (*job.Unit, error) {
 		return nil, err
 	}
 
-	return r.dirToUnit(res.Node, r.getUnitByHash)
+	return r.dirToUnit(res.Node)
 }
 
 // dirToUnit takes a Node containing a Job's constituent objects (in child
 // nodes) and returns a *job.Unit, or any error encountered
-func (r *EtcdRegistry) dirToUnit(dir *etcd.Node, unitHashLookupFunc func(unit.Hash) *unit.UnitFile) (*job.Unit, error) {
+func (r *EtcdRegistry) dirToUnit(dir *etcd.Node) (*job.Unit, error) {
 	objKey := path.Join(dir.Key, "object")
 	var objNode *etcd.Node
 	for _, node := range dir.Nodes {
@@ -176,7 +160,7 @@ func (r *EtcdRegistry) dirToUnit(dir *etcd.Node, unitHashLookupFunc func(unit.Ha
 	if objNode == nil {
 		return nil, nil
 	}
-	u, err := r.getUnitFromObjectNode(objNode, unitHashLookupFunc)
+	u, err := r.getUnitFromObjectNode(objNode)
 	if err != nil {
 		return nil, err
 	}
@@ -270,7 +254,7 @@ func dirToHeartbeat(dir *etcd.Node) (heartbeat string) {
 // getUnitFromObject takes a *etcd.Node containing a Unit's jobModel, and
 // instantiates and returns a representative *job.Unit, transitively fetching the
 // associated UnitFile as necessary
-func (r *EtcdRegistry) getUnitFromObjectNode(node *etcd.Node, unitHashLookupFunc func(unit.Hash) *unit.UnitFile) (*job.Unit, error) {
+func (r *EtcdRegistry) getUnitFromObjectNode(node *etcd.Node) (*job.Unit, error) {
 	var err error
 	var jm jobModel
 	if err = unmarshal(node.Value, &jm); err != nil {
@@ -279,7 +263,7 @@ func (r *EtcdRegistry) getUnitFromObjectNode(node *etcd.Node, unitHashLookupFunc
 
 	var unit *unit.UnitFile
 
-	unit = unitHashLookupFunc(jm.UnitHash)
+	unit = r.getUnitByHash(jm.UnitHash)
 	if unit == nil {
 		log.Warningf("No Unit found in Registry for Job(%s)", jm.Name)
 		return nil, nil

registry/unit.go

@@ -15,8 +15,6 @@
 package registry
 
 import (
-	"strings"
-
 	etcd "github.com/coreos/fleet/Godeps/_workspace/src/github.com/coreos/etcd/client"
 
 	"github.com/coreos/fleet/log"
@@ -63,47 +61,8 @@ func (r *EtcdRegistry) getUnitByHash(hash unit.Hash) *unit.UnitFile {
 		}
 		return nil
 	}
-	return r.unitFromEtcdNode(hash, resp.Node)
-}
-
-// getAllUnitsHashMap retrieves from the Registry all Units and returns a map of hash to UnitFile
-func (r *EtcdRegistry) getAllUnitsHashMap() (map[string]*unit.UnitFile, error) {
-	key := r.prefixed(unitPrefix)
-	opts := &etcd.GetOptions{
-		Recursive: true,
-		Quorum:    true,
-	}
-	hashToUnit := map[string]*unit.UnitFile{}
-	resp, err := r.kAPI.Get(r.ctx(), key, opts)
-	if err != nil {
-		return nil, err
-	}
-
-	for _, node := range resp.Node.Nodes {
-		parts := strings.Split(node.Key, "/")
-		if len(parts) == 0 {
-			log.Errorf("key '%v' doesn't have enough parts", node.Key)
-			continue
-		}
-		stringHash := parts[len(parts)-1]
-		hash, err := unit.HashFromHexString(stringHash)
-		if err != nil {
-			log.Errorf("failed to get Hash for key '%v' with stringHash '%v': %v", node.Key, stringHash, err)
-			continue
-		}
-		unit := r.unitFromEtcdNode(hash, node)
-		if unit == nil {
-			continue
-		}
-		hashToUnit[stringHash] = unit
-	}
-
-	return hashToUnit, nil
-}
-
-func (r *EtcdRegistry) unitFromEtcdNode(hash unit.Hash, etcdNode *etcd.Node) *unit.UnitFile {
 	var um unitModel
-	if err := unmarshal(etcdNode.Value, &um); err != nil {
+	if err := unmarshal(resp.Node.Value, &um); err != nil {
 		log.Errorf("error unmarshaling Unit(%s): %v", hash, err)
 		return nil
 	}

unit/unit.go

@@ -17,7 +17,6 @@ package unit
 import (
 	"bytes"
 	"crypto/sha1"
-	"encoding/hex"
 	"fmt"
 	"io/ioutil"
 	"strings"
@@ -170,19 +169,6 @@ func (h *Hash) Empty() bool {
 	return *h == Hash{}
 }
 
-func HashFromHexString(key string) (Hash, error) {
-	h := Hash{}
-	out, err := hex.DecodeString(key)
-	if err != nil {
-		return h, err
-	}
-	if len(out) != sha1.Size {
-		return h, fmt.Errorf("size of key %q (%d) differs from SHA1 size (%d)", out, len(out), sha1.Size)
-	}
-	copy(h[:], out[:sha1.Size])
-	return h, nil
-}
-
 // UnitState encodes the current state of a unit loaded into a fleet agent
 type UnitState struct {
 	LoadState   string

unit/unit_test.go

@@ -42,14 +42,6 @@ func TestUnitHash(t *testing.T) {
 	if !eh.Empty() {
 		t.Fatalf("Empty hash check failed: %v", eh.Empty())
 	}
-
-	rehashed, err := HashFromHexString(expectHashString)
-	if err != nil {
-		t.Fatalf("HashFromHexString failed with: %v", err)
-	}
-	if rehashed != gotHash {
-		t.Fatalf("HashFromHexString not equal to original hash")
-	}
 }
 
 func TestRecognizedUnitTypes(t *testing.T) {