agent: publish unit state regardless of hash

When starting up, the Agent may already have some Jobs loaded. In this
case, the UnitStatePublisher will not have a cached hash for the
corresponding Unit. The UnitStatePublisher must publish state regardless
of the UnitHash being absent.

agent/unit_state.go

@@ -89,9 +89,12 @@ func (p *UnitStatePublisher) publishOne(name string, us *unit.UnitState) {
 		// 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?
-		if len(us.UnitHash) == 0 {
-			log.Errorf("Refusing to push UnitState(%s), no UnitHash: %#v", name, us)
-		} else if len(us.MachineID) == 0 {
+
+		// 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 {
 			log.Errorf("Refusing to push UnitState(%s), no MachineID: %#v", name, us)
 		} else {
 			log.V(1).Infof("Pushing UnitState(%s) to Registry: %#v", name, us)

registry/unit_state.go

@@ -148,21 +148,22 @@ func unitStateToModel(us *unit.UnitState) *unitStateModel {
 		return nil
 	}
 
+	// Refuse to create a UnitState without a Hash
+	// See https://github.com/coreos/fleet/issues/720
+	//if len(us.UnitHash) == 0 {
+	//	return nil
+	//}
+
 	usm := unitStateModel{
 		LoadState:   us.LoadState,
 		ActiveState: us.ActiveState,
 		SubState:    us.SubState,
+		UnitHash:    us.UnitHash,
 	}
 
 	if us.MachineID != "" {
 		usm.MachineState = &machine.MachineState{ID: us.MachineID}
 	}
 
-	// Refuse to create a UnitState without a Hash
-	if len(us.UnitHash) == 0 {
-		return nil
-	}
-	usm.UnitHash = us.UnitHash
-
 	return &usm
 }

registry/unit_state_test.go

@@ -61,13 +61,14 @@ func TestSaveUnitState(t *testing.T) {
 	mID := "mymachine"
 	us := unit.NewUnitState("abc", "def", "ghi", mID)
 
-	// saving unit state with no hash should fail (nil action)
-	r.SaveUnitState(j, us)
-	if len(e.sets) != 0 || e.deletes != nil {
-		t.Fatalf("SaveUnitState on UnitState with no hash acted unexpectedly!")
-		t.Logf("sets: %#v", e.sets)
-		t.Logf("deletes: %#v", e.deletes)
-	}
+	// 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)
+	//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!")
+	//}
 
 	us.UnitHash = "quickbrownfox"
 	r.SaveUnitState(j, us)
@@ -139,9 +140,10 @@ func TestUnitStateToModel(t *testing.T) {
 			want: nil,
 		},
 		{
-			// No unit states without hashes
+			// Unit state with no hash and no machineID is OK
+			// See https://github.com/coreos/fleet/issues/720
 			in:   &unit.UnitState{"foo", "bar", "baz", "", ""},
-			want: nil,
+			want: &unitStateModel{"foo", "bar", "baz", nil, ""},
 		},
 		{
 			// Unit state with hash but no machineID is OK