github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-30 15:05:27 +00:00
Code Issues Projects Releases Wiki Activity

IsVolumeAttachedToNode() renamed to GetAttachState(), and returns 3 states instead of combining "uncertain" and "detached" into "false"

Browse Source
This commit is contained in:
Cheng Xing 2020-09-07 17:24:00 -07:00
parent a61743b125
commit d9a629fe3a
7 changed files with 119 additions and 90 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
pkg/controller/volume/attachdetach/attach_detach_controller.go
View File

@ -461,7 +461,12 @@ func (adc *attachDetachController) populateDesiredStateOfWorld() error {
err)
continue
}
if adc.actualStateOfWorld.IsVolumeAttachedToNode(volumeName, nodeName) {
attachState := adc.actualStateOfWorld.GetAttachState(volumeName, nodeName)
if attachState == cache.AttachStateAttached {
klog.V(10).Infof("Volume %q is attached to node %q. Marking as attached in ActualStateOfWorld",
volumeName,
nodeName,
)
devicePath, err := adc.getNodeVolumeDevicePath(volumeName, nodeName)
if err != nil {
klog.Errorf("Failed to find device path: %v", err)

4
pkg/controller/volume/attachdetach/attach_detach_controller_test.go
View File

@ -116,8 +116,8 @@ func Test_AttachDetachControllerStateOfWolrdPopulators_Positive(t *testing.T) {
for _, node := range nodes {
nodeName := types.NodeName(node.Name)
for _, attachedVolume := range node.Status.VolumesAttached {
found := adc.actualStateOfWorld.IsVolumeAttachedToNode(attachedVolume.Name, nodeName)
if !found {
attachedState := adc.actualStateOfWorld.GetAttachState(attachedVolume.Name, nodeName)
if attachedState != cache.AttachStateAttached {
t.Fatalf("Run failed with error. Node %s, volume %s not found", nodeName, attachedVolume.Name)
}
}

48
pkg/controller/volume/attachdetach/cache/actual_state_of_world.go vendored
View File

@ -96,11 +96,13 @@ type ActualStateOfWorld interface {
// nodes, the volume is also deleted.
DeleteVolumeNode(volumeName v1.UniqueVolumeName, nodeName types.NodeName)
// IsVolumeAttachedToNode returns true if the specified volume/node combo exists
// in the underlying store indicating the specified volume is attached to
// the specified node, and false if either the combo does not exist, or the
// attached state is marked as uncertain.
IsVolumeAttachedToNode(volumeName v1.UniqueVolumeName, nodeName types.NodeName) bool
// GetAttachState returns the attach state for the given volume-node
// combination.
// Returns AttachStateAttached if the specified volume/node combo exists in
// the underlying store indicating the specified volume is attached to the
// specified node, AttachStateDetached if the combo does not exist, or
// AttachStateUncertain if the attached state is marked as uncertain.
GetAttachState(volumeName v1.UniqueVolumeName, nodeName types.NodeName) AttachState
// GetAttachedVolumes generates and returns a list of volumes/node pairs
// reflecting which volumes might attached to which nodes based on the
@ -154,6 +156,31 @@ type AttachedVolume struct {
DetachRequestedTime time.Time
}
// AttachState represents the attach state of a volume to a node known to the
// Actual State of World.
// This type is used as external representation of attach state (specifically
// as the return type of GetAttachState only); the state is represented
// differently in the internal cache implementation.
type AttachState int
const (
// AttachStateAttached represents the state in which the volume is attached to
// the node.
AttachStateAttached AttachState = iota
// AttachStateUncertain represents the state in which the Actual State of World
// does not know whether the volume is attached to the node.
AttachStateUncertain
// AttachStateDetached represents the state in which the volume is not
// attached to the node.
AttachStateDetached
)
func (s AttachState) String() string {
return []string{"Attached", "Uncertain", "Detached"}[s]
}
// NewActualStateOfWorld returns a new instance of ActualStateOfWorld.
func NewActualStateOfWorld(volumePluginMgr *volume.VolumePluginMgr) ActualStateOfWorld {
return &actualStateOfWorld{
@ -531,19 +558,22 @@ func (asw *actualStateOfWorld) DeleteVolumeNode(
asw.removeVolumeFromReportAsAttached(volumeName, nodeName)
}
func (asw *actualStateOfWorld) IsVolumeAttachedToNode(
volumeName v1.UniqueVolumeName, nodeName types.NodeName) bool {
func (asw *actualStateOfWorld) GetAttachState(
volumeName v1.UniqueVolumeName, nodeName types.NodeName) AttachState {
asw.RLock()
defer asw.RUnlock()
volumeObj, volumeExists := asw.attachedVolumes[volumeName]
if volumeExists {
if node, nodeExists := volumeObj.nodesAttachedTo[nodeName]; nodeExists {
return node.attachedConfirmed
if node.attachedConfirmed {
return AttachStateAttached
}
return AttachStateUncertain
}
}
return false
return AttachStateDetached
}
func (asw *actualStateOfWorld) GetAttachedVolumes() []AttachedVolume {

96
pkg/controller/volume/attachdetach/cache/actual_state_of_world_test.go vendored
View File

@ -47,9 +47,9 @@ func Test_AddVolumeNode_Positive_NewVolumeNewNode(t *testing.T) {
t.Fatalf("AddVolumeNode failed. Expected: <no error> Actual: <%v>", err)
}
volumeNodeComboExists := asw.IsVolumeAttachedToNode(generatedVolumeName, nodeName)
if !volumeNodeComboExists {
t.Fatalf("%q/%q volume/node combo does not exist, it should.", generatedVolumeName, nodeName)
volumeNodeComboState := asw.GetAttachState(generatedVolumeName, nodeName)
if volumeNodeComboState != AttachStateAttached {
t.Fatalf("%q/%q volume/node combo is marked %q; expected 'Attached'.", generatedVolumeName, nodeName, volumeNodeComboState)
}
attachedVolumes := asw.GetAttachedVolumes()
@ -82,9 +82,9 @@ func Test_AddVolumeNode_Positive_NewVolumeNewNodeWithFalseAttached(t *testing.T)
t.Fatalf("AddVolumeNode failed. Expected: <no error> Actual: <%v>", err)
}
volumeNodeComboExists := asw.IsVolumeAttachedToNode(generatedVolumeName, nodeName)
if volumeNodeComboExists {
t.Fatalf("%q/%q volume/node combo does exist, it should not.", generatedVolumeName, nodeName)
volumeNodeComboState := asw.GetAttachState(generatedVolumeName, nodeName)
if volumeNodeComboState != AttachStateUncertain {
t.Fatalf("%q/%q volume/node combo is marked %q, expected 'Uncertain'.", generatedVolumeName, nodeName, volumeNodeComboState)
}
allVolumes := asw.GetAttachedVolumes()
@ -131,9 +131,9 @@ func Test_AddVolumeNode_Positive_NewVolumeNewNodeWithFalseAttached(t *testing.T)
generatedVolumeName2)
}
volumeNodeComboExists = asw.IsVolumeAttachedToNode(generatedVolumeName, nodeName)
if !volumeNodeComboExists {
t.Fatalf("%q/%q combo does not exist, it should.", generatedVolumeName, nodeName)
volumeNodeComboState = asw.GetAttachState(generatedVolumeName, nodeName)
if volumeNodeComboState != AttachStateAttached {
t.Fatalf("%q/%q volume/node combo is marked %q; expected 'Attached'.", generatedVolumeName, nodeName, volumeNodeComboState)
}
attachedVolumes := asw.GetAttachedVolumes()
@ -182,9 +182,9 @@ func Test_AddVolumeNode_Positive_NewVolumeTwoNodesWithFalseAttached(t *testing.T
t.Fatalf("AddVolumeNode failed. Expected: <no error> Actual: <%v>", err)
}
volumeNodeComboExists := asw.IsVolumeAttachedToNode(generatedVolumeName, node1Name)
if volumeNodeComboExists {
t.Fatalf("%q/%q volume/node combo does exist, it should not.", generatedVolumeName, node1Name)
volumeNodeComboState := asw.GetAttachState(generatedVolumeName, node1Name)
if volumeNodeComboState != AttachStateUncertain {
t.Fatalf("%q/%q volume/node combo is marked %q, expected 'Uncertain'.", generatedVolumeName, node1Name, volumeNodeComboState)
}
generatedVolumeName2, add2Err := asw.AddVolumeNode(volumeName, volumeSpec, node2Name, devicePath, true)
@ -201,9 +201,9 @@ func Test_AddVolumeNode_Positive_NewVolumeTwoNodesWithFalseAttached(t *testing.T
generatedVolumeName2)
}
volumeNodeComboExists = asw.IsVolumeAttachedToNode(generatedVolumeName, node2Name)
if !volumeNodeComboExists {
t.Fatalf("%q/%q combo does not exist, it should.", generatedVolumeName, node2Name)
volumeNodeComboState = asw.GetAttachState(generatedVolumeName, node2Name)
if volumeNodeComboState != AttachStateAttached {
t.Fatalf("%q/%q volume/node combo is marked %q; expected 'Attached'.", generatedVolumeName, node2Name, volumeNodeComboState)
}
attachedVolumes := asw.GetAttachedVolumes()
@ -268,14 +268,14 @@ func Test_AddVolumeNode_Positive_ExistingVolumeNewNode(t *testing.T) {
generatedVolumeName2)
}
volumeNode1ComboExists := asw.IsVolumeAttachedToNode(generatedVolumeName1, node1Name)
if !volumeNode1ComboExists {
t.Fatalf("%q/%q volume/node combo does not exist, it should.", generatedVolumeName1, node1Name)
volumeNode1ComboState := asw.GetAttachState(generatedVolumeName1, node1Name)
if volumeNode1ComboState != AttachStateAttached {
t.Fatalf("%q/%q volume/node combo is marked %q; expected 'Attached'.", generatedVolumeName1, node1Name, volumeNode1ComboState)
}
volumeNode2ComboExists := asw.IsVolumeAttachedToNode(generatedVolumeName1, node2Name)
if !volumeNode2ComboExists {
t.Fatalf("%q/%q volume/node combo does not exist, it should.", generatedVolumeName1, node2Name)
volumeNode2ComboState := asw.GetAttachState(generatedVolumeName1, node2Name)
if volumeNode2ComboState != AttachStateAttached {
t.Fatalf("%q/%q volume/node combo is marked %q; expected 'Attached'.", generatedVolumeName1, node2Name, volumeNode2ComboState)
}
attachedVolumes := asw.GetAttachedVolumes()
@ -317,9 +317,9 @@ func Test_AddVolumeNode_Positive_ExistingVolumeExistingNode(t *testing.T) {
generatedVolumeName2)
}
volumeNodeComboExists := asw.IsVolumeAttachedToNode(generatedVolumeName1, nodeName)
if !volumeNodeComboExists {
t.Fatalf("%q/%q volume/node combo does not exist, it should.", generatedVolumeName1, nodeName)
volumeNodeComboState := asw.GetAttachState(generatedVolumeName1, nodeName)
if volumeNodeComboState != AttachStateAttached {
t.Fatalf("%q/%q volume/node combo is marked %q; expected 'Attached'.", generatedVolumeName1, nodeName, volumeNodeComboState)
}
attachedVolumes := asw.GetAttachedVolumes()
@ -350,9 +350,9 @@ func Test_DeleteVolumeNode_Positive_VolumeExistsNodeExists(t *testing.T) {
asw.DeleteVolumeNode(generatedVolumeName, nodeName)
// Assert
volumeNodeComboExists := asw.IsVolumeAttachedToNode(generatedVolumeName, nodeName)
if volumeNodeComboExists {
t.Fatalf("%q/%q volume/node combo exists, it should not.", generatedVolumeName, nodeName)
volumeNodeComboState := asw.GetAttachState(generatedVolumeName, nodeName)
if volumeNodeComboState != AttachStateDetached {
t.Fatalf("%q/%q volume/node combo is marked %q, expected 'Detached'.", generatedVolumeName, nodeName, volumeNodeComboState)
}
attachedVolumes := asw.GetAttachedVolumes()
@ -374,9 +374,9 @@ func Test_DeleteVolumeNode_Positive_VolumeDoesntExistNodeDoesntExist(t *testing.
asw.DeleteVolumeNode(volumeName, nodeName)
// Assert
volumeNodeComboExists := asw.IsVolumeAttachedToNode(volumeName, nodeName)
if volumeNodeComboExists {
t.Fatalf("%q/%q volume/node combo exists, it should not.", volumeName, nodeName)
volumeNodeComboState := asw.GetAttachState(volumeName, nodeName)
if volumeNodeComboState != AttachStateDetached {
t.Fatalf("%q/%q volume/node combo is marked %q, expected 'Detached'.", volumeName, nodeName, volumeNodeComboState)
}
attachedVolumes := asw.GetAttachedVolumes()
@ -417,14 +417,14 @@ func Test_DeleteVolumeNode_Positive_TwoNodesOneDeleted(t *testing.T) {
asw.DeleteVolumeNode(generatedVolumeName1, node1Name)
// Assert
volumeNodeComboExists := asw.IsVolumeAttachedToNode(generatedVolumeName1, node1Name)
if volumeNodeComboExists {
t.Fatalf("%q/%q volume/node combo exists, it should not.", generatedVolumeName1, node1Name)
volumeNodeComboState := asw.GetAttachState(generatedVolumeName1, node1Name)
if volumeNodeComboState != AttachStateDetached {
t.Fatalf("%q/%q volume/node combo is marked %q, expected 'Detached'.", generatedVolumeName1, node1Name, volumeNodeComboState)
}
volumeNodeComboExists = asw.IsVolumeAttachedToNode(generatedVolumeName1, node2Name)
if !volumeNodeComboExists {
t.Fatalf("%q/%q volume/node combo does not exist, it should.", generatedVolumeName1, node2Name)
volumeNodeComboState = asw.GetAttachState(generatedVolumeName1, node2Name)
if volumeNodeComboState != AttachStateAttached {
t.Fatalf("%q/%q volume/node combo is marked %q; expected 'Attached'.", generatedVolumeName1, node2Name, volumeNodeComboState)
}
attachedVolumes := asw.GetAttachedVolumes()
@ -436,7 +436,7 @@ func Test_DeleteVolumeNode_Positive_TwoNodesOneDeleted(t *testing.T) {
}
// Populates data struct with one volume/node entry.
// Calls IsVolumeAttachedToNode() to verify entry.
// Calls GetAttachState() to verify entry.
// Verifies the populated volume/node entry exists.
func Test_VolumeNodeExists_Positive_VolumeExistsNodeExists(t *testing.T) {
// Arrange
@ -452,11 +452,11 @@ func Test_VolumeNodeExists_Positive_VolumeExistsNodeExists(t *testing.T) {
}
// Act
volumeNodeComboExists := asw.IsVolumeAttachedToNode(generatedVolumeName, nodeName)
volumeNodeComboState := asw.GetAttachState(generatedVolumeName, nodeName)
// Assert
if !volumeNodeComboExists {
t.Fatalf("%q/%q volume/node combo does not exist, it should.", generatedVolumeName, nodeName)
if volumeNodeComboState != AttachStateAttached {
t.Fatalf("%q/%q volume/node combo is marked %q; expected 'Attached'.", generatedVolumeName, nodeName, volumeNodeComboState)
}
attachedVolumes := asw.GetAttachedVolumes()
@ -468,7 +468,7 @@ func Test_VolumeNodeExists_Positive_VolumeExistsNodeExists(t *testing.T) {
}
// Populates data struct with one volume1/node1 entry.
// Calls IsVolumeAttachedToNode() with volume1/node2.
// Calls GetAttachState() with volume1/node2.
// Verifies requested entry does not exist, but populated entry does.
func Test_VolumeNodeExists_Positive_VolumeExistsNodeDoesntExist(t *testing.T) {
// Arrange
@ -485,11 +485,11 @@ func Test_VolumeNodeExists_Positive_VolumeExistsNodeDoesntExist(t *testing.T) {
}
// Act
volumeNodeComboExists := asw.IsVolumeAttachedToNode(generatedVolumeName, node2Name)
volumeNodeComboState := asw.GetAttachState(generatedVolumeName, node2Name)
// Assert
if volumeNodeComboExists {
t.Fatalf("%q/%q volume/node combo exists, it should not.", generatedVolumeName, node2Name)
if volumeNodeComboState != AttachStateDetached {
t.Fatalf("%q/%q volume/node combo is marked %q, expected 'Detached'.", generatedVolumeName, node2Name, volumeNodeComboState)
}
attachedVolumes := asw.GetAttachedVolumes()
@ -500,7 +500,7 @@ func Test_VolumeNodeExists_Positive_VolumeExistsNodeDoesntExist(t *testing.T) {
verifyAttachedVolume(t, attachedVolumes, generatedVolumeName, string(volumeName), node1Name, devicePath, true /* expectedMountedByNode */, false /* expectNonZeroDetachRequestedTime */)
}
// Calls IsVolumeAttachedToNode() on empty data struct.
// Calls GetAttachState() on empty data struct.
// Verifies requested entry does not exist.
func Test_VolumeNodeExists_Positive_VolumeAndNodeDontExist(t *testing.T) {
// Arrange
@ -510,11 +510,11 @@ func Test_VolumeNodeExists_Positive_VolumeAndNodeDontExist(t *testing.T) {
nodeName := types.NodeName("node-name")
// Act
volumeNodeComboExists := asw.IsVolumeAttachedToNode(volumeName, nodeName)
volumeNodeComboState := asw.GetAttachState(volumeName, nodeName)
// Assert
if volumeNodeComboExists {
t.Fatalf("%q/%q volume/node combo exists, it should not.", volumeName, nodeName)
if volumeNodeComboState != AttachStateDetached {
t.Fatalf("%q/%q volume/node combo is marked %q, expected 'Detached'.", volumeName, nodeName, volumeNodeComboState)
}
attachedVolumes := asw.GetAttachedVolumes()

16
pkg/controller/volume/attachdetach/reconciler/reconciler.go
View File

@ -169,15 +169,8 @@ func (rc *reconciler) reconcile() {
// Check the ActualStateOfWorld again to avoid issuing an unnecessary
// detach.
// See https://github.com/kubernetes/kubernetes/issues/93902
attachedVolumesForNode := rc.actualStateOfWorld.GetAttachedVolumesForNode(attachedVolume.NodeName)
stillAttached := false
for _, volForNode := range attachedVolumesForNode {
if volForNode.VolumeName == attachedVolume.VolumeName {
stillAttached = true
break
}
}
if !stillAttached {
attachState := rc.actualStateOfWorld.GetAttachState(attachedVolume.VolumeName, attachedVolume.NodeName)
if attachState == cache.AttachStateDetached {
if klog.V(5).Enabled() {
klog.Infof(attachedVolume.GenerateMsgDetailed("Volume detached--skipping", ""))
}
@ -269,10 +262,11 @@ func (rc *reconciler) attachDesiredVolumes() {
// Because the attach operation updates the ActualStateOfWorld before
// marking itself complete, IsOperationPending() must be checked before
// IsVolumeAttachedToNode() to guarantee the ActualStateOfWorld is
// GetAttachState() to guarantee the ActualStateOfWorld is
// up-to-date when it's read.
// See https://github.com/kubernetes/kubernetes/issues/93902
if rc.actualStateOfWorld.IsVolumeAttachedToNode(volumeToAttach.VolumeName, volumeToAttach.NodeName) {
attachState := rc.actualStateOfWorld.GetAttachState(volumeToAttach.VolumeName, volumeToAttach.NodeName)
if attachState == cache.AttachStateAttached {
// Volume/Node exists, touch it to reset detachRequestedTime
if klog.V(5).Enabled() {
klog.Infof(volumeToAttach.GenerateMsgDetailed("Volume attached--touching", ""))

36
pkg/controller/volume/attachdetach/reconciler/reconciler_test.go
View File

@ -577,9 +577,9 @@ func Test_Run_OneVolumeAttachAndDetachUncertainNodesWithReadWriteOnce(t *testing
}
time.Sleep(1 * time.Second)
// Volume is added to asw. Because attach operation fails, volume should not reported as attached to the node.
// Volume is added to asw. Because attach operation fails, volume should not be reported as attached to the node.
waitForVolumeAddedToNode(t, generatedVolumeName, nodeName1, asw)
verifyVolumeAttachedToNode(t, generatedVolumeName, nodeName1, true, asw)
verifyVolumeAttachedToNode(t, generatedVolumeName, nodeName1, cache.AttachStateAttached, asw)
verifyVolumeReportedAsAttachedToNode(t, generatedVolumeName, nodeName1, true, asw)
// When volume is added to the node, it is set to mounted by default. Then the status will be updated by checking node status VolumeInUse.
@ -596,7 +596,7 @@ func Test_Run_OneVolumeAttachAndDetachUncertainNodesWithReadWriteOnce(t *testing
t.Fatalf("AddPod failed. Expected: <no error> Actual: <%v>", podAddErr)
}
waitForVolumeAttachedToNode(t, generatedVolumeName, nodeName2, asw)
verifyVolumeAttachedToNode(t, generatedVolumeName, nodeName2, true, asw)
verifyVolumeAttachedToNode(t, generatedVolumeName, nodeName2, cache.AttachStateAttached, asw)
}
@ -643,9 +643,9 @@ func Test_Run_OneVolumeAttachAndDetachTimeoutNodesWithReadWriteOnce(t *testing.T
t.Fatalf("AddPod failed. Expected: <no error> Actual: <%v>", podAddErr)
}
// Volume is added to asw. Because attach operation fails, volume should not reported as attached to the node.
// Volume is added to asw. Because attach operation fails, volume should not be reported as attached to the node.
waitForVolumeAddedToNode(t, generatedVolumeName, nodeName1, asw)
verifyVolumeAttachedToNode(t, generatedVolumeName, nodeName1, false, asw)
verifyVolumeAttachedToNode(t, generatedVolumeName, nodeName1, cache.AttachStateUncertain, asw)
verifyVolumeReportedAsAttachedToNode(t, generatedVolumeName, nodeName1, false, asw)
// When volume is added to the node, it is set to mounted by default. Then the status will be updated by checking node status VolumeInUse.
@ -662,7 +662,7 @@ func Test_Run_OneVolumeAttachAndDetachTimeoutNodesWithReadWriteOnce(t *testing.T
t.Fatalf("AddPod failed. Expected: <no error> Actual: <%v>", podAddErr)
}
waitForVolumeAttachedToNode(t, generatedVolumeName, nodeName2, asw)
verifyVolumeAttachedToNode(t, generatedVolumeName, nodeName2, true, asw)
verifyVolumeAttachedToNode(t, generatedVolumeName, nodeName2, cache.AttachStateAttached, asw)
}
@ -1048,7 +1048,8 @@ func waitForVolumeAttachedToNode(
err := retryWithExponentialBackOff(
time.Duration(500*time.Millisecond),
func() (bool, error) {
if asw.IsVolumeAttachedToNode(volumeName, nodeName) {
attachState := asw.GetAttachState(volumeName, nodeName)
if attachState == cache.AttachStateAttached {
return true, nil
}
t.Logf(
@ -1060,7 +1061,8 @@ func waitForVolumeAttachedToNode(
},
)
if err != nil && !asw.IsVolumeAttachedToNode(volumeName, nodeName) {
attachState := asw.GetAttachState(volumeName, nodeName)
if err != nil && attachState != cache.AttachStateAttached {
t.Fatalf(
"Volume <%v> is not attached to node <%v>.",
volumeName,
@ -1141,19 +1143,17 @@ func verifyVolumeAttachedToNode(
t *testing.T,
volumeName v1.UniqueVolumeName,
nodeName k8stypes.NodeName,
isAttached bool,
expectedAttachState cache.AttachState,
asw cache.ActualStateOfWorld,
) {
result := asw.IsVolumeAttachedToNode(volumeName, nodeName)
if result == isAttached {
return
attachState := asw.GetAttachState(volumeName, nodeName)
if attachState != expectedAttachState {
t.Fatalf("Check volume <%v> is attached to node <%v>, got %v, expected %v",
volumeName,
nodeName,
attachState,
expectedAttachState)
}
t.Fatalf("Check volume <%v> is attached to node <%v>, got %v, expected %v",
volumeName,
nodeName,
result,
isAttached)
}
func verifyVolumeReportedAsAttachedToNode(

2
pkg/volume/testing/testing.go
View File

@ -27,8 +27,8 @@ import (
"testing"
"time"
"k8s.io/mount-utils"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/mount-utils"
"k8s.io/utils/exec"
testingexec "k8s.io/utils/exec/testing"
utilstrings "k8s.io/utils/strings"