Volume Manager Data Structure Should Use Pod UID in Key

[saad-ali]

Problem
This issue is related to #29555 which was fixed by PR #29673. That issue fixed name collisions for non-attachable volumes (which is much more common), but not for attachable volumes.

Specifically, if a single pod references the same attachable disk via two different volume names (possibly to refer to different partitions), the names will collide.

Suggested Fix

There are three main volumemanager data structures:

• Actual State of the World (ASW)
• Desired State of the World (DSW)
• Nested Pending Operations (formerly goroutinemap)

Instead of using the same key (unique volume name) for all these data structs, the key for ASW and DSW should be a struct that includes unique volume name and unique pod name. Nested pending operations can continue to operate on unique volume name to keep the existing protection against multiple operations on the same volume.

Suggested Cleanup
Move GetVolumeName() to Attacher volumes interface (instead of required for all volumes). With PR #29673 non-attachable volumes auto-generate the volume name. Ensure that MarkVolumeAsAttached() has all the information it needs to either generate the name in either case (instead of passing in an override).

[pmorie]

Let me add to this also:

1. We should make a distinction in how we treat non-attachable volumes; they shouldn't be forced through an 'attach' phase. If a volume isn't attachable, go right to trying to mount it. This will save one iteration through the sync loop in the volume manager reconciler for non-attachable volumes and reduce mount latency for them a bit.
2. We should remove the optional volumeName arguments I added to the interface calls from the operation executor -- they're purely a symptom of (1).

[asalkeld]

If no one is working on this, I can give it a go.

[pmorie]

@asalkeld we need a couple weeks of bake-time before we do anything further here to make sure that there aren't more bugs waiting in the wings.

[jingxu97]

Agree. I think it would be good to examine again what are the necessary
information should be stored in actual and desired world.

On Thu, Jul 28, 2016 at 6:47 AM, Paul Morie notifications@github.com
wrote:

@asalkeld https://github.com/asalkeld we need a couple weeks of
bake-time before we do anything further here to make sure that there aren't
more bugs waiting in the wings.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#29695 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ASSNxRSYtJ0dC7gNVLGkkizWqtaWACn0ks5qaLLrgaJpZM4JWgvd
.

• Jing

[asalkeld]

OK @pmorie no worries, I'll attach something else.

[pmorie]

@asalkeld I appreciate the enthusiasm - check out stuff with the help-wanted label.