Skip to content

Commit

Permalink
Fix wrapped volume race
Browse files Browse the repository at this point in the history 
This fixes race conditions in configmap, secret, downwardapi & git_repo
volume plugins.
wrappedVolumeSpec vars used by volume mounters and unmounters contained
a pointer to api.Volume structs which were being patched by
NewWrapperMounter/NewWrapperUnmounter, causing race condition during
volume mounts.
  • Loading branch information
@ivan4th
ivan4th committed Jul 27, 2016
1 parent ed3a29b commit df1e925
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 20 deletions.
18 changes: 10 additions & 8 deletions pkg/volume/configmap/configmap.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,12 +121,14 @@ func (sv *configMapVolume) GetAttributes() volume.Attributes {
}
}

// This is the spec for the volume that this plugin wraps.
var wrappedVolumeSpec = volume.Spec{
// This should be on a tmpfs instead of the local disk; the problem is
// charging the memory for the tmpfs to the right cgroup. We should make
// this a tmpfs when we can do the accounting correctly.
Volume: &api.Volume{VolumeSource: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}}},
func wrappedVolumeSpec() volume.Spec {
// This is the spec for the volume that this plugin wraps.
return volume.Spec{
// This should be on a tmpfs instead of the local disk; the problem is
// charging the memory for the tmpfs to the right cgroup. We should make
// this a tmpfs when we can do the accounting correctly.
Volume: &api.Volume{VolumeSource: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}}},
}
}

func (b *configMapVolumeMounter) SetUp(fsGroup *int64) error {
Expand All @@ -137,7 +139,7 @@ func (b *configMapVolumeMounter) SetUpAt(dir string, fsGroup *int64) error {
glog.V(3).Infof("Setting up volume %v for pod %v at %v", b.volName, b.pod.UID, dir)

// Wrap EmptyDir, let it do the setup.
wrapped, err := b.plugin.host.NewWrapperMounter(b.volName, wrappedVolumeSpec, &b.pod, *b.opts)
wrapped, err := b.plugin.host.NewWrapperMounter(b.volName, wrappedVolumeSpec(), &b.pod, *b.opts)
if err != nil {
return err
}
Expand Down Expand Up @@ -236,7 +238,7 @@ func (c *configMapVolumeUnmounter) TearDownAt(dir string) error {
glog.V(3).Infof("Tearing down volume %v for pod %v at %v", c.volName, c.podUID, dir)

// Wrap EmptyDir, let it do the teardown.
wrapped, err := c.plugin.host.NewWrapperUnmounter(c.volName, wrappedVolumeSpec, c.podUID)
wrapped, err := c.plugin.host.NewWrapperUnmounter(c.volName, wrappedVolumeSpec(), c.podUID)
if err != nil {
return err
}
Expand Down
10 changes: 6 additions & 4 deletions pkg/volume/downwardapi/downwardapi.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,10 @@ type downwardAPIPlugin struct {

var _ volume.VolumePlugin = &downwardAPIPlugin{}

var wrappedVolumeSpec = volume.Spec{
Volume: &api.Volume{VolumeSource: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{Medium: api.StorageMediumMemory}}},
func wrappedVolumeSpec() volume.Spec {
return volume.Spec{
Volume: &api.Volume{VolumeSource: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{Medium: api.StorageMediumMemory}}},
}
}

func (plugin *downwardAPIPlugin) Init(host volume.VolumeHost) error {
Expand Down Expand Up @@ -144,7 +146,7 @@ func (b *downwardAPIVolumeMounter) SetUp(fsGroup *int64) error {
func (b *downwardAPIVolumeMounter) SetUpAt(dir string, fsGroup *int64) error {
glog.V(3).Infof("Setting up a downwardAPI volume %v for pod %v/%v at %v", b.volName, b.pod.Namespace, b.pod.Name, dir)
// Wrap EmptyDir. Here we rely on the idempotency of the wrapped plugin to avoid repeatedly mounting
wrapped, err := b.plugin.host.NewWrapperMounter(b.volName, wrappedVolumeSpec, b.pod, *b.opts)
wrapped, err := b.plugin.host.NewWrapperMounter(b.volName, wrappedVolumeSpec(), b.pod, *b.opts)
if err != nil {
glog.Errorf("Couldn't setup downwardAPI volume %v for pod %v/%v: %s", b.volName, b.pod.Namespace, b.pod.Name, err.Error())
return err
Expand Down Expand Up @@ -233,7 +235,7 @@ func (c *downwardAPIVolumeUnmounter) TearDownAt(dir string) error {
glog.V(3).Infof("Tearing down volume %v for pod %v at %v", c.volName, c.podUID, dir)

// Wrap EmptyDir, let it do the teardown.
wrapped, err := c.plugin.host.NewWrapperUnmounter(c.volName, wrappedVolumeSpec, c.podUID)
wrapped, err := c.plugin.host.NewWrapperUnmounter(c.volName, wrappedVolumeSpec(), c.podUID)
if err != nil {
return err
}
Expand Down
10 changes: 6 additions & 4 deletions pkg/volume/git_repo/git_repo.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,10 @@ type gitRepoPlugin struct {

var _ volume.VolumePlugin = &gitRepoPlugin{}

var wrappedVolumeSpec = volume.Spec{
Volume: &api.Volume{VolumeSource: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}}},
func wrappedVolumeSpec() volume.Spec {
return volume.Spec{
Volume: &api.Volume{VolumeSource: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}}},
}
}

const (
Expand Down Expand Up @@ -155,7 +157,7 @@ func (b *gitRepoVolumeMounter) SetUpAt(dir string, fsGroup *int64) error {
}

// Wrap EmptyDir, let it do the setup.
wrapped, err := b.plugin.host.NewWrapperMounter(b.volName, wrappedVolumeSpec, &b.pod, b.opts)
wrapped, err := b.plugin.host.NewWrapperMounter(b.volName, wrappedVolumeSpec(), &b.pod, b.opts)
if err != nil {
return err
}
Expand Down Expand Up @@ -237,7 +239,7 @@ func (c *gitRepoVolumeUnmounter) TearDown() error {
func (c *gitRepoVolumeUnmounter) TearDownAt(dir string) error {

// Wrap EmptyDir, let it do the teardown.
wrapped, err := c.plugin.host.NewWrapperUnmounter(c.volName, wrappedVolumeSpec, c.podUID)
wrapped, err := c.plugin.host.NewWrapperUnmounter(c.volName, wrappedVolumeSpec(), c.podUID)
if err != nil {
return err
}
Expand Down
10 changes: 6 additions & 4 deletions pkg/volume/secret/secret.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,10 @@ type secretPlugin struct {

var _ volume.VolumePlugin = &secretPlugin{}

var wrappedVolumeSpec = volume.Spec{
Volume: &api.Volume{VolumeSource: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{Medium: api.StorageMediumMemory}}},
func wrappedVolumeSpec() volume.Spec {
return volume.Spec{
Volume: &api.Volume{VolumeSource: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{Medium: api.StorageMediumMemory}}},
}
}

func getPath(uid types.UID, volName string, host volume.VolumeHost) string {
Expand Down Expand Up @@ -150,7 +152,7 @@ func (b *secretVolumeMounter) SetUpAt(dir string, fsGroup *int64) error {
glog.V(3).Infof("Setting up volume %v for pod %v at %v", b.volName, b.pod.UID, dir)

// Wrap EmptyDir, let it do the setup.
wrapped, err := b.plugin.host.NewWrapperMounter(b.volName, wrappedVolumeSpec, &b.pod, *b.opts)
wrapped, err := b.plugin.host.NewWrapperMounter(b.volName, wrappedVolumeSpec(), &b.pod, *b.opts)
if err != nil {
return err
}
Expand Down Expand Up @@ -249,7 +251,7 @@ func (c *secretVolumeUnmounter) TearDownAt(dir string) error {
glog.V(3).Infof("Tearing down volume %v for pod %v at %v", c.volName, c.podUID, dir)

// Wrap EmptyDir, let it do the teardown.
wrapped, err := c.plugin.host.NewWrapperUnmounter(c.volName, wrappedVolumeSpec, c.podUID)
wrapped, err := c.plugin.host.NewWrapperUnmounter(c.volName, wrappedVolumeSpec(), c.podUID)
if err != nil {
return err
}
Expand Down

0 comments on commit df1e925

Please sign in to comment.