diff --git a/pkg/volume/cinder/attacher.go b/pkg/volume/cinder/attacher.go index 8bb4e67feea..e3f031c6721 100644 --- a/pkg/volume/cinder/attacher.go +++ b/pkg/volume/cinder/attacher.go @@ -30,7 +30,8 @@ import ( ) type cinderDiskAttacher struct { - host volume.VolumeHost + host volume.VolumeHost + cinderProvider CinderProvider } var _ volume.Attacher = &cinderDiskAttacher{} @@ -42,7 +43,14 @@ const ( ) func (plugin *cinderPlugin) NewAttacher() (volume.Attacher, error) { - return &cinderDiskAttacher{host: plugin.host}, nil + cinder, err := getCloudProvider(plugin.host.GetCloudProvider()) + if err != nil { + return nil, err + } + return &cinderDiskAttacher{ + host: plugin.host, + cinderProvider: cinder, + }, nil } func (attacher *cinderDiskAttacher) Attach(spec *volume.Spec, hostName string) (string, error) { @@ -53,11 +61,7 @@ func (attacher *cinderDiskAttacher) Attach(spec *volume.Spec, hostName string) ( volumeID := volumeSource.VolumeID - cloud, err := getCloudProvider(attacher.host.GetCloudProvider()) - if err != nil { - return "", err - } - instances, res := cloud.Instances() + instances, res := attacher.cinderProvider.Instances() if !res { return "", fmt.Errorf("failed to list openstack instances") } @@ -68,7 +72,7 @@ func (attacher *cinderDiskAttacher) Attach(spec *volume.Spec, hostName string) ( if ind := strings.LastIndex(instanceid, "/"); ind >= 0 { instanceid = instanceid[(ind + 1):] } - attached, err := cloud.DiskIsAttached(volumeID, instanceid) + attached, err := attacher.cinderProvider.DiskIsAttached(volumeID, instanceid) if err != nil { // Log error and continue with attach glog.Warningf( @@ -80,7 +84,7 @@ func (attacher *cinderDiskAttacher) Attach(spec *volume.Spec, hostName string) ( // Volume is already attached to node. glog.Infof("Attach operation is successful. volume %q is already attached to node %q.", volumeID, instanceid) } else { - _, err = cloud.AttachDisk(instanceid, volumeID) + _, err = attacher.cinderProvider.AttachDisk(instanceid, volumeID) if err == nil { glog.Infof("Attach operation successful: volume %q attached to node %q.", volumeID, instanceid) } else { @@ -89,7 +93,7 @@ func (attacher *cinderDiskAttacher) Attach(spec *volume.Spec, hostName string) ( } } - devicePath, err := cloud.GetAttachmentDiskPath(instanceid, volumeID) + devicePath, err := attacher.cinderProvider.GetAttachmentDiskPath(instanceid, volumeID) if err != nil { glog.Infof("Attach volume %q to instance %q failed with %v", volumeID, instanceid, err) return "", err @@ -181,22 +185,26 @@ func (attacher *cinderDiskAttacher) MountDevice(spec *volume.Spec, devicePath st } type cinderDiskDetacher struct { - host volume.VolumeHost + mounter mount.Interface + cinderProvider CinderProvider } var _ volume.Detacher = &cinderDiskDetacher{} func (plugin *cinderPlugin) NewDetacher() (volume.Detacher, error) { - return &cinderDiskDetacher{host: plugin.host}, nil + cinder, err := getCloudProvider(plugin.host.GetCloudProvider()) + if err != nil { + return nil, err + } + return &cinderDiskDetacher{ + mounter: plugin.host.GetMounter(), + cinderProvider: cinder, + }, nil } func (detacher *cinderDiskDetacher) Detach(deviceMountPath string, hostName string) error { volumeID := path.Base(deviceMountPath) - cloud, err := getCloudProvider(detacher.host.GetCloudProvider()) - if err != nil { - return err - } - instances, res := cloud.Instances() + instances, res := detacher.cinderProvider.Instances() if !res { return fmt.Errorf("failed to list openstack instances") } @@ -205,7 +213,7 @@ func (detacher *cinderDiskDetacher) Detach(deviceMountPath string, hostName stri instanceid = instanceid[(ind + 1):] } - attached, err := cloud.DiskIsAttached(volumeID, instanceid) + attached, err := detacher.cinderProvider.DiskIsAttached(volumeID, instanceid) if err != nil { // Log error and continue with detach glog.Errorf( @@ -219,7 +227,7 @@ func (detacher *cinderDiskDetacher) Detach(deviceMountPath string, hostName stri return nil } - if err = cloud.DetachDisk(instanceid, volumeID); err != nil { + if err = detacher.cinderProvider.DetachDisk(instanceid, volumeID); err != nil { glog.Errorf("Error detaching volume %q: %v", volumeID, err) return err } @@ -249,9 +257,8 @@ func (detacher *cinderDiskDetacher) WaitForDetach(devicePath string, timeout tim } func (detacher *cinderDiskDetacher) UnmountDevice(deviceMountPath string) error { - mounter := detacher.host.GetMounter() volume := path.Base(deviceMountPath) - if err := unmountPDAndRemoveGlobalPath(deviceMountPath, mounter); err != nil { + if err := unmountPDAndRemoveGlobalPath(deviceMountPath, detacher.mounter); err != nil { glog.Errorf("Error unmounting %q: %v", volume, err) } diff --git a/pkg/volume/cinder/attacher_test.go b/pkg/volume/cinder/attacher_test.go new file mode 100644 index 00000000000..cf146b029b8 --- /dev/null +++ b/pkg/volume/cinder/attacher_test.go @@ -0,0 +1,449 @@ +/* +Copyright 2016 The Kubernetes Authors 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 cinder + +import ( + "errors" + "testing" + + "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/cloudprovider" + "k8s.io/kubernetes/pkg/volume" + volumetest "k8s.io/kubernetes/pkg/volume/testing" + + "github.com/golang/glog" +) + +func TestGetDeviceName_Volume(t *testing.T) { + plugin := newPlugin() + name := "my-cinder-volume" + spec := createVolSpec(name, false) + + deviceName, err := plugin.GetVolumeName(spec) + if err != nil { + t.Errorf("GetDeviceName error: %v", err) + } + if deviceName != name { + t.Errorf("GetDeviceName error: expected %s, got %s", name, deviceName) + } +} + +func TestGetDeviceName_PersistentVolume(t *testing.T) { + plugin := newPlugin() + name := "my-cinder-pv" + spec := createPVSpec(name, true) + + deviceName, err := plugin.GetVolumeName(spec) + if err != nil { + t.Errorf("GetDeviceName error: %v", err) + } + if deviceName != name { + t.Errorf("GetDeviceName error: expected %s, got %s", name, deviceName) + } +} + +// One testcase for TestAttachDetach table test below +type testcase struct { + name string + // For fake GCE: + attach attachCall + detach detachCall + diskIsAttached diskIsAttachedCall + diskPath diskPathCall + t *testing.T + + instanceID string + // Actual test to run + test func(test *testcase) (string, error) + // Expected return of the test + expectedDevice string + expectedError error +} + +func TestAttachDetach(t *testing.T) { + diskName := "disk" + instanceID := "instance" + readOnly := false + spec := createVolSpec(diskName, readOnly) + attachError := errors.New("Fake attach error") + detachError := errors.New("Fake detach error") + diskCheckError := errors.New("Fake DiskIsAttached error") + diskPathError := errors.New("Fake GetAttachmentDiskPath error") + tests := []testcase{ + // Successful Attach call + { + name: "Attach_Positive", + instanceID: instanceID, + diskIsAttached: diskIsAttachedCall{diskName, instanceID, false, nil}, + attach: attachCall{diskName, instanceID, "", nil}, + diskPath: diskPathCall{diskName, instanceID, "/dev/sda", nil}, + test: func(testcase *testcase) (string, error) { + attacher := newAttacher(testcase) + return attacher.Attach(spec, instanceID) + }, + expectedDevice: "/dev/sda", + }, + + // Disk is already attached + { + name: "Attach_Positive_AlreadyAttached", + instanceID: instanceID, + diskIsAttached: diskIsAttachedCall{diskName, instanceID, true, nil}, + diskPath: diskPathCall{diskName, instanceID, "/dev/sda", nil}, + test: func(testcase *testcase) (string, error) { + attacher := newAttacher(testcase) + return attacher.Attach(spec, instanceID) + }, + expectedDevice: "/dev/sda", + }, + + // DiskIsAttached fails and Attach succeeds + { + name: "Attach_Positive_CheckFails", + instanceID: instanceID, + diskIsAttached: diskIsAttachedCall{diskName, instanceID, false, diskCheckError}, + attach: attachCall{diskName, instanceID, "", nil}, + diskPath: diskPathCall{diskName, instanceID, "/dev/sda", nil}, + test: func(testcase *testcase) (string, error) { + attacher := newAttacher(testcase) + return attacher.Attach(spec, instanceID) + }, + expectedDevice: "/dev/sda", + }, + + // Attach call fails + { + name: "Attach_Negative", + instanceID: instanceID, + diskIsAttached: diskIsAttachedCall{diskName, instanceID, false, diskCheckError}, + attach: attachCall{diskName, instanceID, "/dev/sda", attachError}, + test: func(testcase *testcase) (string, error) { + attacher := newAttacher(testcase) + return attacher.Attach(spec, instanceID) + }, + expectedError: attachError, + }, + + // GetAttachmentDiskPath call fails + { + name: "Attach_Negative_DiskPatchFails", + instanceID: instanceID, + diskIsAttached: diskIsAttachedCall{diskName, instanceID, false, diskCheckError}, + attach: attachCall{diskName, instanceID, "", nil}, + diskPath: diskPathCall{diskName, instanceID, "", diskPathError}, + test: func(testcase *testcase) (string, error) { + attacher := newAttacher(testcase) + return attacher.Attach(spec, instanceID) + }, + expectedError: diskPathError, + }, + + // Detach succeeds + { + name: "Detach_Positive", + instanceID: instanceID, + diskIsAttached: diskIsAttachedCall{diskName, instanceID, true, nil}, + detach: detachCall{diskName, instanceID, nil}, + test: func(testcase *testcase) (string, error) { + detacher := newDetacher(testcase) + return "", detacher.Detach(diskName, instanceID) + }, + }, + + // Disk is already detached + { + name: "Detach_Positive_AlreadyDetached", + instanceID: instanceID, + diskIsAttached: diskIsAttachedCall{diskName, instanceID, false, nil}, + test: func(testcase *testcase) (string, error) { + detacher := newDetacher(testcase) + return "", detacher.Detach(diskName, instanceID) + }, + }, + + // Detach succeeds when DiskIsAttached fails + { + name: "Detach_Positive_CheckFails", + instanceID: instanceID, + diskIsAttached: diskIsAttachedCall{diskName, instanceID, false, diskCheckError}, + detach: detachCall{diskName, instanceID, nil}, + test: func(testcase *testcase) (string, error) { + detacher := newDetacher(testcase) + return "", detacher.Detach(diskName, instanceID) + }, + }, + + // Detach fails + { + name: "Detach_Negative", + instanceID: instanceID, + diskIsAttached: diskIsAttachedCall{diskName, instanceID, false, diskCheckError}, + detach: detachCall{diskName, instanceID, detachError}, + test: func(testcase *testcase) (string, error) { + detacher := newDetacher(testcase) + return "", detacher.Detach(diskName, instanceID) + }, + expectedError: detachError, + }, + } + + for _, testcase := range tests { + testcase.t = t + device, err := testcase.test(&testcase) + if err != testcase.expectedError { + t.Errorf("%s failed: expected err=%q, got %q", testcase.name, testcase.expectedError.Error(), err.Error()) + } + if device != testcase.expectedDevice { + t.Errorf("%s failed: expected device=%q, got %q", testcase.name, testcase.expectedDevice, device) + } + t.Logf("Test %q succeeded", testcase.name) + } +} + +// newPlugin creates a new gcePersistentDiskPlugin with fake cloud, NewAttacher +// and NewDetacher won't work. +func newPlugin() *cinderPlugin { + host := volumetest.NewFakeVolumeHost("/tmp", nil, nil, "") + plugins := ProbeVolumePlugins() + plugin := plugins[0] + plugin.Init(host) + return plugin.(*cinderPlugin) +} + +func newAttacher(testcase *testcase) *cinderDiskAttacher { + return &cinderDiskAttacher{ + host: nil, + cinderProvider: testcase, + } +} + +func newDetacher(testcase *testcase) *cinderDiskDetacher { + return &cinderDiskDetacher{ + cinderProvider: testcase, + } +} + +func createVolSpec(name string, readOnly bool) *volume.Spec { + return &volume.Spec{ + Volume: &api.Volume{ + VolumeSource: api.VolumeSource{ + Cinder: &api.CinderVolumeSource{ + VolumeID: name, + ReadOnly: readOnly, + }, + }, + }, + } +} + +func createPVSpec(name string, readOnly bool) *volume.Spec { + return &volume.Spec{ + PersistentVolume: &api.PersistentVolume{ + Spec: api.PersistentVolumeSpec{ + PersistentVolumeSource: api.PersistentVolumeSource{ + Cinder: &api.CinderVolumeSource{ + VolumeID: name, + ReadOnly: readOnly, + }, + }, + }, + }, + } +} + +// Fake GCE implementation + +type attachCall struct { + diskName string + instanceID string + retDeviceName string + ret error +} + +type detachCall struct { + devicePath string + instanceID string + ret error +} + +type diskIsAttachedCall struct { + diskName, instanceID string + isAttached bool + ret error +} + +type diskPathCall struct { + diskName, instanceID string + retPath string + ret error +} + +func (testcase *testcase) AttachDisk(instanceID string, diskName string) (string, error) { + expected := &testcase.attach + + if expected.diskName == "" && expected.instanceID == "" { + // testcase.attach looks uninitialized, test did not expect to call + // AttachDisk + testcase.t.Errorf("Unexpected AttachDisk call!") + return "", errors.New("Unexpected AttachDisk call!") + } + + if expected.diskName != diskName { + testcase.t.Errorf("Unexpected AttachDisk call: expected diskName %s, got %s", expected.diskName, diskName) + return "", errors.New("Unexpected AttachDisk call: wrong diskName") + } + + if expected.instanceID != instanceID { + testcase.t.Errorf("Unexpected AttachDisk call: expected instanceID %s, got %s", expected.instanceID, instanceID) + return "", errors.New("Unexpected AttachDisk call: wrong instanceID") + } + + glog.V(4).Infof("AttachDisk call: %s, %s, returning %q, %v", diskName, instanceID, expected.retDeviceName, expected.ret) + + return expected.retDeviceName, expected.ret +} + +func (testcase *testcase) DetachDisk(instanceID string, partialDiskId string) error { + expected := &testcase.detach + + if expected.devicePath == "" && expected.instanceID == "" { + // testcase.detach looks uninitialized, test did not expect to call + // DetachDisk + testcase.t.Errorf("Unexpected DetachDisk call!") + return errors.New("Unexpected DetachDisk call!") + } + + if expected.devicePath != partialDiskId { + testcase.t.Errorf("Unexpected DetachDisk call: expected partialDiskId %s, got %s", expected.devicePath, partialDiskId) + return errors.New("Unexpected DetachDisk call: wrong diskName") + } + + if expected.instanceID != instanceID { + testcase.t.Errorf("Unexpected DetachDisk call: expected instanceID %s, got %s", expected.instanceID, instanceID) + return errors.New("Unexpected DetachDisk call: wrong instanceID") + } + + glog.V(4).Infof("DetachDisk call: %s, %s, returning %v", partialDiskId, instanceID, expected.ret) + + return expected.ret +} + +func (testcase *testcase) DiskIsAttached(diskName, instanceID string) (bool, error) { + expected := &testcase.diskIsAttached + + if expected.diskName == "" && expected.instanceID == "" { + // testcase.diskIsAttached looks uninitialized, test did not expect to + // call DiskIsAttached + testcase.t.Errorf("Unexpected DiskIsAttached call!") + return false, errors.New("Unexpected DiskIsAttached call!") + } + + if expected.diskName != diskName { + testcase.t.Errorf("Unexpected DiskIsAttached call: expected diskName %s, got %s", expected.diskName, diskName) + return false, errors.New("Unexpected DiskIsAttached call: wrong diskName") + } + + if expected.instanceID != instanceID { + testcase.t.Errorf("Unexpected DiskIsAttached call: expected instanceID %s, got %s", expected.instanceID, instanceID) + return false, errors.New("Unexpected DiskIsAttached call: wrong instanceID") + } + + glog.V(4).Infof("DiskIsAttached call: %s, %s, returning %v, %v", diskName, instanceID, expected.isAttached, expected.ret) + + return expected.isAttached, expected.ret +} + +func (testcase *testcase) GetAttachmentDiskPath(instanceID string, diskName string) (string, error) { + expected := &testcase.diskPath + if expected.diskName == "" && expected.instanceID == "" { + // testcase.diskPath looks uninitialized, test did not expect to + // call GetAttachmentDiskPath + testcase.t.Errorf("Unexpected GetAttachmentDiskPath call!") + return "", errors.New("Unexpected GetAttachmentDiskPath call!") + } + + if expected.diskName != diskName { + testcase.t.Errorf("Unexpected GetAttachmentDiskPath call: expected diskName %s, got %s", expected.diskName, diskName) + return "", errors.New("Unexpected GetAttachmentDiskPath call: wrong diskName") + } + + if expected.instanceID != instanceID { + testcase.t.Errorf("Unexpected GetAttachmentDiskPath call: expected instanceID %s, got %s", expected.instanceID, instanceID) + return "", errors.New("Unexpected GetAttachmentDiskPath call: wrong instanceID") + } + + glog.V(4).Infof("GetAttachmentDiskPath call: %s, %s, returning %v, %v", diskName, instanceID, expected.retPath, expected.ret) + + return expected.retPath, expected.ret +} + +func (testcase *testcase) CreateVolume(name string, size int, tags *map[string]string) (volumeName string, err error) { + return "", errors.New("Not implemented") +} + +func (testcase *testcase) GetDevicePath(diskId string) string { + return "" +} + +func (testcase *testcase) InstanceID() (string, error) { + return testcase.instanceID, nil +} + +func (testcase *testcase) DeleteVolume(volumeName string) error { + return errors.New("Not implemented") +} + +func (testcase *testcase) GetAutoLabelsForPD(name string) (map[string]string, error) { + return map[string]string{}, errors.New("Not implemented") +} + +func (testcase *testcase) Instances() (cloudprovider.Instances, bool) { + return &instances{testcase.instanceID}, true +} + +// Implementation of fake cloudprovider.Instances +type instances struct { + instanceID string +} + +func (instances *instances) NodeAddresses(name string) ([]api.NodeAddress, error) { + return []api.NodeAddress{}, errors.New("Not implemented") +} + +func (instances *instances) ExternalID(name string) (string, error) { + return "", errors.New("Not implemented") +} + +func (instances *instances) InstanceID(name string) (string, error) { + return instances.instanceID, nil +} + +func (instances *instances) InstanceType(name string) (string, error) { + return "", errors.New("Not implemented") +} + +func (instances *instances) List(filter string) ([]string, error) { + return []string{}, errors.New("Not implemented") +} + +func (instances *instances) AddSSHKeyToAllInstances(user string, keyData []byte) error { + return errors.New("Not implemented") +} + +func (instances *instances) CurrentNodeName(hostname string) (string, error) { + return "", errors.New("Not implemented") +}