From 1dcb0255592e4b5ece3eb727341a5734402ef068 Mon Sep 17 00:00:00 2001 From: Brendan Burns Date: Thu, 11 Dec 2014 13:00:26 -0800 Subject: [PATCH 1/2] Handle PD already being attached to the machine. --- pkg/cloudprovider/gce/gce.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/pkg/cloudprovider/gce/gce.go b/pkg/cloudprovider/gce/gce.go index a3fde21749e..469a9878c34 100644 --- a/pkg/cloudprovider/gce/gce.go +++ b/pkg/cloudprovider/gce/gce.go @@ -376,6 +376,21 @@ func (gce *GCECloud) AttachDisk(diskName string, readOnly bool) error { } attachedDisk := gce.convertDiskToAttachedDisk(disk, readWrite) _, err = gce.service.Instances.AttachDisk(gce.projectID, gce.zone, gce.instanceID, attachedDisk).Do() + if err != nil { + // Check if the disk is already attached to this instance. We do this only + // in the error case, since it is expected to be exceptional. + instance, err := gce.service.Instances.Get(gce.projectID, gce.zone, gce.instanceID).Do() + if err != nil { + return err + } + for _, disk := range instance.Disks { + if disk.InitializeParams.DiskName == diskName { + // Disk is already attached, we're good to go. + return nil + } + } + + } return err } From 3da84e18447c4618d56296eb2a7473339e34608e Mon Sep 17 00:00:00 2001 From: Brendan Burns Date: Thu, 11 Dec 2014 16:27:58 -0800 Subject: [PATCH 2/2] Fix GCE-PD so that it works even if the PD is already attached. --- pkg/volume/gce_util.go | 16 ++++++++----- pkg/volume/mount_utils.go | 40 +++++++++++++++++++++++++++++++ pkg/volume/mount_utils_windows.go | 28 ++++++++++++++++++++++ pkg/volume/volume.go | 20 +++++++++++----- 4 files changed, 92 insertions(+), 12 deletions(-) create mode 100644 pkg/volume/mount_utils.go create mode 100644 pkg/volume/mount_utils_windows.go diff --git a/pkg/volume/gce_util.go b/pkg/volume/gce_util.go index 781e9b1e34d..9682579ad3c 100644 --- a/pkg/volume/gce_util.go +++ b/pkg/volume/gce_util.go @@ -72,19 +72,23 @@ func (util *GCEDiskUtil) AttachDisk(GCEPD *GCEPersistentDisk) error { } globalPDPath := makeGlobalPDName(GCEPD.RootDir, GCEPD.PDName, GCEPD.ReadOnly) // Only mount the PD globally once. - _, err = os.Stat(globalPDPath) - if os.IsNotExist(err) { - err = os.MkdirAll(globalPDPath, 0750) - if err != nil { + mountpoint, err := isMountPoint(globalPDPath) + if err != nil { + if os.IsNotExist(err) { + if err := os.MkdirAll(globalPDPath, 0750); err != nil { + return err + } + mountpoint = false + } else { return err } + } + if !mountpoint { err = GCEPD.mounter.Mount(devicePath, globalPDPath, GCEPD.FSType, flags, "") if err != nil { os.RemoveAll(globalPDPath) return err } - } else if err != nil { - return err } return nil } diff --git a/pkg/volume/mount_utils.go b/pkg/volume/mount_utils.go new file mode 100644 index 00000000000..3a0cdc587f2 --- /dev/null +++ b/pkg/volume/mount_utils.go @@ -0,0 +1,40 @@ +// +build !windows + +/* +Copyright 2014 Google Inc. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package volume + +import ( + "os" + "syscall" +) + +// Determine if a directory is a mountpoint, by comparing the device for the directory +// with the device for it's parent. If they are the same, it's not a mountpoint, if they're +// different, it is. +func isMountPoint(file string) (bool, error) { + stat, err := os.Stat(file) + if err != nil { + return false, err + } + rootStat, err := os.Lstat(file + "/..") + if err != nil { + return false, err + } + // If the directory has the same device as parent, then it's not a mountpoint. + return stat.Sys().(*syscall.Stat_t).Dev != rootStat.Sys().(*syscall.Stat_t).Dev, nil +} diff --git a/pkg/volume/mount_utils_windows.go b/pkg/volume/mount_utils_windows.go new file mode 100644 index 00000000000..3250b08ee27 --- /dev/null +++ b/pkg/volume/mount_utils_windows.go @@ -0,0 +1,28 @@ +// +build windows + +/* +Copyright 2014 Google Inc. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package volume + +import ( + "fmt" +) + +// Dummy implementation for Windows +func isMountPoint(file string) (bool, error) { + return false, fmt.Errorf("unimplemented") +} diff --git a/pkg/volume/volume.go b/pkg/volume/volume.go index fcf53870f47..0007a10c47a 100644 --- a/pkg/volume/volume.go +++ b/pkg/volume/volume.go @@ -235,11 +235,15 @@ func (PD *GCEPersistentDisk) GetPath() string { // Attaches the disk and bind mounts to the volume path. func (PD *GCEPersistentDisk) SetUp() error { // TODO: handle failed mounts here. - if _, err := os.Stat(PD.GetPath()); !os.IsNotExist(err) { + mountpoint, err := isMountPoint(PD.GetPath()) + glog.V(4).Infof("PersistentDisk set up: %s %v %v", PD.GetPath(), mountpoint, err) + if err != nil && !os.IsNotExist(err) { + return err + } + if mountpoint { return nil } - err := PD.util.AttachDisk(PD) - if err != nil { + if err := PD.util.AttachDisk(PD); err != nil { return err } flags := uintptr(0) @@ -265,6 +269,13 @@ func (PD *GCEPersistentDisk) SetUp() error { // Unmounts the bind mount, and detaches the disk only if the PD // resource was the last reference to that disk on the kubelet. func (PD *GCEPersistentDisk) TearDown() error { + mountpoint, err := isMountPoint(PD.GetPath()) + if err != nil { + return err + } + if !mountpoint { + return os.RemoveAll(PD.GetPath()) + } devicePath, refCount, err := PD.mounter.RefCount(PD) if err != nil { return err @@ -276,9 +287,6 @@ func (PD *GCEPersistentDisk) TearDown() error { if err := os.RemoveAll(PD.GetPath()); err != nil { return err } - if err != nil { - return err - } // If refCount is 1, then all bind mounts have been removed, and the // remaining reference is the global mount. It is safe to detach. if refCount == 1 {