agent,registry: reenable check for nil hash in unit state publisher

Long time ago there were checks for nil unit hashes in
agent.UnitStatePublisher.publishOne and registry.UnitStateToModel, but
given that units could enter the system without populated hashes, the
checks were disabled: 11949e0 ("agent: publish unit state regardless of
hash"). However the hashes are now always populated, so the corresponding
assertions should be reenabled.

Fixes #989

agent/unit_state.go

@@ -207,12 +207,9 @@ func newPublisher(reg registry.Registry, ttl time.Duration) publishFunc {
 			// Sanity check - don't want to publish incomplete UnitStates
 			// TODO(jonboulle): consider teasing apart a separate UnitState-like struct
 			// so we can rely on a UnitState always being fully hydrated?
-
-			// See https://github.com/coreos/fleet/issues/720
-			//if len(us.UnitHash) == 0 {
-			//	log.Errorf("Refusing to push UnitState(%s), no UnitHash: %#v", name, us)
-
-			if len(us.MachineID) == 0 {
+			if len(us.UnitHash) == 0 {
+				log.Errorf("Refusing to push UnitState(%s), no UnitHash: %#v", name, us)
+			} else if len(us.MachineID) == 0 {
 				log.Errorf("Refusing to push UnitState(%s), no MachineID: %#v", name, us)
 			} else {
 				log.Debugf("Pushing UnitState(%s) to Registry: %#v", name, us)

registry/unit_state.go

@@ -224,10 +224,9 @@ func unitStateToModel(us *unit.UnitState) *unitStateModel {
 	}
 
 	// Refuse to create a UnitState without a Hash
-	// See https://github.com/coreos/fleet/issues/720
-	//if len(us.UnitHash) == 0 {
-	//	return nil
-	//}
+	if len(us.UnitHash) == 0 {
+		return nil
+	}
 
 	usm := unitStateModel{
 		LoadState:   us.LoadState,

registry/unit_state_test.go

@@ -111,14 +111,13 @@ func TestSaveUnitState(t *testing.T) {
 		t.Fatalf("SaveUnitState of nil state should fail but acted unexpectedly!")
 	}
 
-	// Saving unit state with no hash should succeed for now, but should fail
-	// in the future. See https://github.com/coreos/fleet/issues/720.
-	//r.SaveUnitState(j, us, time.Second)
-	//if len(e.sets) != 1 || e.deletes == nil {
-	//	t.Logf("sets: %#v", e.sets)
-	//	t.Logf("deletes: %#v", e.deletes)
-	//	t.Fatalf("SaveUnitState on UnitState with no hash acted unexpectedly!")
-	//}
+	// Saving unit state with no hash should should fail.
+	r.SaveUnitState(j, us, time.Second)
+	if len(e.sets) != 0 || e.deletes != nil {
+		t.Logf("sets: %#v", e.sets)
+		t.Logf("deletes: %#v", e.deletes)
+		t.Fatalf("SaveUnitState on UnitState with no hash acted unexpectedly!")
+	}
 
 	us.UnitHash = "quickbrownfox"
 	r.SaveUnitState(j, us, time.Second)
@@ -195,8 +194,7 @@ func TestUnitStateToModel(t *testing.T) {
 			want: nil,
 		},
 		{
-			// Unit state with no hash and no machineID is OK
-			// See https://github.com/coreos/fleet/issues/720
+			// Unit state with no hash and no machineID is not OK
 			in: &unit.UnitState{
 				LoadState:   "foo",
 				ActiveState: "bar",
@@ -205,13 +203,7 @@ func TestUnitStateToModel(t *testing.T) {
 				UnitHash:    "",
 				UnitName:    "name",
 			},
-			want: &unitStateModel{
-				LoadState:    "foo",
-				ActiveState:  "bar",
-				SubState:     "baz",
-				MachineState: nil,
-				UnitHash:     "",
-			},
+			want: nil,
 		},
 		{
 			// Unit state with hash but no machineID is OK