diff --git a/agent/unit_state.go b/agent/unit_state.go index 79dc9f6b5..a93f13698 100644 --- a/agent/unit_state.go +++ b/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) diff --git a/agent/unit_state_test.go b/agent/unit_state_test.go index e773b319f..0a239c623 100644 --- a/agent/unit_state_test.go +++ b/agent/unit_state_test.go @@ -227,6 +227,7 @@ func TestDefaultPublisher(t *testing.T) { UnitName: "foo.service", ActiveState: "active", MachineID: "xyz", + UnitHash: "quickbrownfox", }, nil, []*unit.UnitState{ @@ -234,6 +235,7 @@ func TestDefaultPublisher(t *testing.T) { UnitName: "foo.service", ActiveState: "active", MachineID: "xyz", + UnitHash: "quickbrownfox", }, }, }, @@ -243,6 +245,7 @@ func TestDefaultPublisher(t *testing.T) { &unit.UnitState{ UnitName: "foo.service", ActiveState: "active", + UnitHash: "quickbrownfox", }, nil, []*unit.UnitState{}, @@ -255,6 +258,7 @@ func TestDefaultPublisher(t *testing.T) { unit.UnitState{ UnitName: "foo.service", ActiveState: "active", + UnitHash: "quickbrownfox", }, }, []*unit.UnitState{}, diff --git a/registry/unit_state.go b/registry/unit_state.go index fef6590be..c8a13bf11 100644 --- a/registry/unit_state.go +++ b/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, diff --git a/registry/unit_state_test.go b/registry/unit_state_test.go index 12e62421b..a97dc24ee 100644 --- a/registry/unit_state_test.go +++ b/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