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 GCE-PD so that it works even if the PD is attached. #2886

Merged
merged 2 commits into from
Dec 12, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 15 additions & 0 deletions pkg/cloudprovider/gce/gce.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
16 changes: 10 additions & 6 deletions pkg/volume/gce_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I am understanding, this is just a stand-in for "is device X mounted at path Y" ? If that became easier to ask, this could become that, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. Implementation based on this:

http://stackoverflow.com/questions/10410513/function-or-a-systemcall-similar-to-mountpoint-command-in-linux

If there's a better way, I'm 100% open to it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ask because part of my volume plugins PR is a mount-tools pkg, which could answer this question, but is obviously much more heavy-weight.

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
}
Expand Down
40 changes: 40 additions & 0 deletions pkg/volume/mount_utils.go
Original file line number Diff line number Diff line change
@@ -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
}
28 changes: 28 additions & 0 deletions pkg/volume/mount_utils_windows.go
Original file line number Diff line number Diff line change
@@ -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")
}
20 changes: 14 additions & 6 deletions pkg/volume/volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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 {
Expand Down