Do not swallow error in volume

This commit is contained in:
Harry Zhang 2017-01-19 14:30:13 +08:00
parent c1ecedf44d
commit 70941f65bf
2 changed files with 65 additions and 8 deletions

View File

@ -452,25 +452,28 @@ func (asw *actualStateOfWorld) addVolumeToReportAsAttached(
// needs to be updated again by the node status updater.
// If the specifed node does not exist in the nodesToUpdateStatusFor list, log the error and return
// This is an internal function and caller should acquire and release the lock
func (asw *actualStateOfWorld) updateNodeStatusUpdateNeeded(nodeName types.NodeName, needed bool) {
func (asw *actualStateOfWorld) updateNodeStatusUpdateNeeded(nodeName types.NodeName, needed bool) error {
nodeToUpdate, nodeToUpdateExists := asw.nodesToUpdateStatusFor[nodeName]
if !nodeToUpdateExists {
// should not happen
glog.Errorf(
"Failed to set statusUpdateNeeded to needed %t because nodeName=%q does not exist",
needed,
nodeName)
return
errMsg := fmt.Sprintf("Failed to set statusUpdateNeeded to needed %t because nodeName=%q does not exist",
needed, nodeName)
glog.Errorf(errMsg)
return fmt.Errorf(errMsg)
}
nodeToUpdate.statusUpdateNeeded = needed
asw.nodesToUpdateStatusFor[nodeName] = nodeToUpdate
return nil
}
func (asw *actualStateOfWorld) SetNodeStatusUpdateNeeded(nodeName types.NodeName) {
asw.Lock()
defer asw.Unlock()
asw.updateNodeStatusUpdateNeeded(nodeName, true)
if err := asw.updateNodeStatusUpdateNeeded(nodeName, true); err != nil {
glog.Errorf("Failed to update statusUpdateNeeded field in actual state of world: %v", err)
}
}
func (asw *actualStateOfWorld) DeleteVolumeNode(
@ -589,7 +592,9 @@ func (asw *actualStateOfWorld) GetVolumesToReportAttached() map[types.NodeName][
// When GetVolumesToReportAttached is called by node status updater, the current status
// of this node will be updated, so set the flag statusUpdateNeeded to false indicating
// the current status is already updated.
asw.updateNodeStatusUpdateNeeded(nodeName, false)
if err := asw.updateNodeStatusUpdateNeeded(nodeName, false); err != nil {
glog.Errorf("Failed to update statusUpdateNeeded field when getting volumes: %v", err)
}
}
return volumesToReportAttached

View File

@ -1113,6 +1113,58 @@ func Test_SetNodeStatusUpdateNeededError(t *testing.T) {
}
}
// Test_updateNodeStatusUpdateNeeded expects statusUpdateNeeded to be properly updated if
// updateNodeStatusUpdateNeeded is called on a node that exists in the actual state of the world
func Test_updateNodeStatusUpdateNeeded(t *testing.T) {
// Arrange
volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t)
asw := &actualStateOfWorld{
attachedVolumes: make(map[v1.UniqueVolumeName]attachedVolume),
nodesToUpdateStatusFor: make(map[types.NodeName]nodeToUpdateStatusFor),
volumePluginMgr: volumePluginMgr,
}
nodeName := types.NodeName("node-1")
nodeToUpdate := nodeToUpdateStatusFor{
nodeName: nodeName,
statusUpdateNeeded: true,
volumesToReportAsAttached: make(map[v1.UniqueVolumeName]v1.UniqueVolumeName),
}
asw.nodesToUpdateStatusFor[nodeName] = nodeToUpdate
// Act
err := asw.updateNodeStatusUpdateNeeded(nodeName, false)
// Assert
if err != nil {
t.Fatalf("updateNodeStatusUpdateNeeded should not return error, but got: %v", err)
}
nodesToUpdateStatusFor := asw.GetNodesToUpdateStatusFor()
if nodesToUpdateStatusFor[nodeName].statusUpdateNeeded {
t.Fatalf("nodesToUpdateStatusFor should be updated to: false, but got: true")
}
}
// Test_updateNodeStatusUpdateNeededError expects statusUpdateNeeded to report error if
// updateNodeStatusUpdateNeeded is called on a node that does not exist in the actual state of the world
func Test_updateNodeStatusUpdateNeededError(t *testing.T) {
// Arrange
volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t)
asw := &actualStateOfWorld{
attachedVolumes: make(map[v1.UniqueVolumeName]attachedVolume),
nodesToUpdateStatusFor: make(map[types.NodeName]nodeToUpdateStatusFor),
volumePluginMgr: volumePluginMgr,
}
nodeName := types.NodeName("node-1")
// Act
err := asw.updateNodeStatusUpdateNeeded(nodeName, false)
// Assert
if err == nil {
t.Fatalf("updateNodeStatusUpdateNeeded should return error, but got nothing")
}
}
func verifyAttachedVolume(
t *testing.T,
attachedVolumes []AttachedVolume,