Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix concurrent map write access in Portworx create volume call #76341

Merged
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -72,12 +72,14 @@ func (util *portworxVolumeUtil) CreateVolume(p *portworxVolumeProvisioner) (stri
}

// Pass all parameters as volume labels for Portworx server-side processing
if len(p.options.Parameters) > 0 {
spec.VolumeLabels = p.options.Parameters
} else {
if spec.VolumeLabels == nil {
This conversation was marked as resolved by liggitt

This comment has been minimized.

Copy link
@liggitt

liggitt Apr 10, 2019

Member

this doesn't seem to protect against concurrent access... was the issue concurrency or assigning to a nil map?

This comment has been minimized.

Copy link
@harsh-px

harsh-px Apr 11, 2019

Author Contributor

@liggitt thanks for reviewing this.

Before this change, reference to p.options.Parameters (which is the shared map) was getting saved in spec.VolumeLabels and line 103 below updates this map. When multiple PVCs are getting provisioned, this causes concurrent map writes. The golang panic will say fatal: concurrent map read and write.

Another problem you can see from the below for loop is p.options.Parameters will start accumulate annotations of all PVCs sharing a StorageClass.

Let me know if you have further questions.

This comment has been minimized.

Copy link
@liggitt

liggitt Apr 15, 2019

Member

I see, thanks

spec.VolumeLabels = make(map[string]string, 0)
}

for k, v := range p.options.Parameters {
spec.VolumeLabels[k] = v
}

// Update the requested size in the spec
spec.Size = uint64(requestGiB * volumehelpers.GiB)

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.