Merge pull request #27280 from jsafrane/aws-cinder-attach-test

Automatic merge from submit-queue

Add AWS volume plugin attach tests.

@kubernetes/sig-storage 

This it a test, it does not really matter if it catches 1.3 train or the next one.

pkg/volume/aws_ebs/attacher.go

@@ -24,13 +24,15 @@ import (
 	"time"
 
 	"github.com/golang/glog"
+	"k8s.io/kubernetes/pkg/cloudprovider/providers/aws"
 	"k8s.io/kubernetes/pkg/util/exec"
 	"k8s.io/kubernetes/pkg/util/mount"
 	"k8s.io/kubernetes/pkg/volume"
 )
 
 type awsElasticBlockStoreAttacher struct {
-	host volume.VolumeHost
+	host       volume.VolumeHost
+	awsVolumes aws.Volumes
 }
 
 var _ volume.Attacher = &awsElasticBlockStoreAttacher{}
@@ -38,7 +40,15 @@ var _ volume.Attacher = &awsElasticBlockStoreAttacher{}
 var _ volume.AttachableVolumePlugin = &awsElasticBlockStorePlugin{}
 
 func (plugin *awsElasticBlockStorePlugin) NewAttacher() (volume.Attacher, error) {
-	return &awsElasticBlockStoreAttacher{host: plugin.host}, nil
+	awsCloud, err := getCloudProvider(plugin.host.GetCloudProvider())
+	if err != nil {
+		return nil, err
+	}
+
+	return &awsElasticBlockStoreAttacher{
+		host:       plugin.host,
+		awsVolumes: awsCloud,
+	}, nil
 }
 
 func (attacher *awsElasticBlockStoreAttacher) Attach(spec *volume.Spec, hostName string) (string, error) {
@@ -49,14 +59,9 @@ func (attacher *awsElasticBlockStoreAttacher) Attach(spec *volume.Spec, hostName
 
 	volumeID := volumeSource.VolumeID
 
-	awsCloud, err := getCloudProvider(attacher.host.GetCloudProvider())
-	if err != nil {
-		return "", err
-	}
-
 	// awsCloud.AttachDisk checks if disk is already attached to node and
 	// succeeds in that case, so no need to do that separately.
-	devicePath, err := awsCloud.AttachDisk(volumeID, hostName, readOnly)
+	devicePath, err := attacher.awsVolumes.AttachDisk(volumeID, hostName, readOnly)
 	if err != nil {
 		glog.Errorf("Error attaching volume %q: %+v", volumeID, err)
 		return "", err
@@ -156,23 +161,28 @@ func (attacher *awsElasticBlockStoreAttacher) MountDevice(spec *volume.Spec, dev
 }
 
 type awsElasticBlockStoreDetacher struct {
-	host volume.VolumeHost
+	mounter    mount.Interface
+	awsVolumes aws.Volumes
 }
 
 var _ volume.Detacher = &awsElasticBlockStoreDetacher{}
 
 func (plugin *awsElasticBlockStorePlugin) NewDetacher() (volume.Detacher, error) {
-	return &awsElasticBlockStoreDetacher{host: plugin.host}, nil
+	awsCloud, err := getCloudProvider(plugin.host.GetCloudProvider())
+	if err != nil {
+		return nil, err
+	}
+
+	return &awsElasticBlockStoreDetacher{
+		mounter:    plugin.host.GetMounter(),
+		awsVolumes: awsCloud,
+	}, nil
 }
 
 func (detacher *awsElasticBlockStoreDetacher) Detach(deviceMountPath string, hostName string) error {
 	volumeID := path.Base(deviceMountPath)
 
-	awsCloud, err := getCloudProvider(detacher.host.GetCloudProvider())
-	if err != nil {
-		return err
-	}
-	attached, err := awsCloud.DiskIsAttached(volumeID, hostName)
+	attached, err := detacher.awsVolumes.DiskIsAttached(volumeID, hostName)
 	if err != nil {
 		// Log error and continue with detach
 		glog.Errorf(
@@ -186,7 +196,7 @@ func (detacher *awsElasticBlockStoreDetacher) Detach(deviceMountPath string, hos
 		return nil
 	}
 
-	if _, err = awsCloud.DetachDisk(volumeID, hostName); err != nil {
+	if _, err = detacher.awsVolumes.DetachDisk(volumeID, hostName); err != nil {
 		glog.Errorf("Error detaching volumeID %q: %v", volumeID, err)
 		return err
 	}
@@ -215,9 +225,8 @@ func (detacher *awsElasticBlockStoreDetacher) WaitForDetach(devicePath string, t
 }
 
 func (detacher *awsElasticBlockStoreDetacher) 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)
 	}
 

pkg/volume/aws_ebs/attacher_test.go

@@ -0,0 +1,332 @@
+/*
+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 aws_ebs
+
+import (
+	"errors"
+	"testing"
+
+	"k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/cloudprovider/providers/aws"
+	"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-aws-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-aws-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 AWS:
+	attach         attachCall
+	detach         detachCall
+	diskIsAttached diskIsAttachedCall
+	t              *testing.T
+
+	// 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")
+	tests := []testcase{
+		// Successful Attach call
+		{
+			name:   "Attach_Positive",
+			attach: attachCall{diskName, instanceID, readOnly, "/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",
+			attach: attachCall{diskName, instanceID, readOnly, "", attachError},
+			test: func(testcase *testcase) (string, error) {
+				attacher := newAttacher(testcase)
+				return attacher.Attach(spec, instanceID)
+			},
+			expectedError: attachError,
+		},
+
+		// Detach succeeds
+		{
+			name:           "Detach_Positive",
+			diskIsAttached: diskIsAttachedCall{diskName, instanceID, true, nil},
+			detach:         detachCall{diskName, instanceID, "/dev/sda", nil},
+			test: func(testcase *testcase) (string, error) {
+				detacher := newDetacher(testcase)
+				return "", detacher.Detach(diskName, instanceID)
+			},
+		},
+
+		// Disk is already detached
+		{
+			name:           "Detach_Positive_AlreadyDetached",
+			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",
+			diskIsAttached: diskIsAttachedCall{diskName, instanceID, false, diskCheckError},
+			detach:         detachCall{diskName, instanceID, "/dev/sda", nil},
+			test: func(testcase *testcase) (string, error) {
+				detacher := newDetacher(testcase)
+				return "", detacher.Detach(diskName, instanceID)
+			},
+		},
+
+		// Detach fails
+		{
+			name:           "Detach_Negative",
+			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() *awsElasticBlockStorePlugin {
+	host := volumetest.NewFakeVolumeHost("/tmp", nil, nil, "")
+	plugins := ProbeVolumePlugins()
+	plugin := plugins[0]
+	plugin.Init(host)
+	return plugin.(*awsElasticBlockStorePlugin)
+}
+
+func newAttacher(testcase *testcase) *awsElasticBlockStoreAttacher {
+	return &awsElasticBlockStoreAttacher{
+		host:       nil,
+		awsVolumes: testcase,
+	}
+}
+
+func newDetacher(testcase *testcase) *awsElasticBlockStoreDetacher {
+	return &awsElasticBlockStoreDetacher{
+		awsVolumes: testcase,
+	}
+}
+
+func createVolSpec(name string, readOnly bool) *volume.Spec {
+	return &volume.Spec{
+		Volume: &api.Volume{
+			VolumeSource: api.VolumeSource{
+				AWSElasticBlockStore: &api.AWSElasticBlockStoreVolumeSource{
+					VolumeID: name,
+					ReadOnly: readOnly,
+				},
+			},
+		},
+	}
+}
+
+func createPVSpec(name string, readOnly bool) *volume.Spec {
+	return &volume.Spec{
+		PersistentVolume: &api.PersistentVolume{
+			Spec: api.PersistentVolumeSpec{
+				PersistentVolumeSource: api.PersistentVolumeSource{
+					AWSElasticBlockStore: &api.AWSElasticBlockStoreVolumeSource{
+						VolumeID: name,
+						ReadOnly: readOnly,
+					},
+				},
+			},
+		},
+	}
+}
+
+// Fake AWS implementation
+
+type attachCall struct {
+	diskName      string
+	instanceID    string
+	readOnly      bool
+	retDeviceName string
+	ret           error
+}
+
+type detachCall struct {
+	diskName      string
+	instanceID    string
+	retDeviceName string
+	ret           error
+}
+
+type diskIsAttachedCall struct {
+	diskName, instanceID string
+	isAttached           bool
+	ret                  error
+}
+
+func (testcase *testcase) AttachDisk(diskName string, instanceID string, readOnly bool) (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")
+	}
+
+	if expected.readOnly != readOnly {
+		testcase.t.Errorf("Unexpected AttachDisk call: expected readOnly %v, got %v", expected.readOnly, readOnly)
+		return "", errors.New("Unexpected AttachDisk call: wrong readOnly")
+	}
+
+	glog.V(4).Infof("AttachDisk call: %s, %s, %v, returning %q, %v", diskName, instanceID, readOnly, expected.retDeviceName, expected.ret)
+
+	return expected.retDeviceName, expected.ret
+}
+
+func (testcase *testcase) DetachDisk(diskName string, instanceID string) (string, error) {
+	expected := &testcase.detach
+
+	if expected.diskName == "" && 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.diskName != diskName {
+		testcase.t.Errorf("Unexpected DetachDisk call: expected diskName %s, got %s", expected.diskName, diskName)
+		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 %q, %v", diskName, instanceID, expected.retDeviceName, expected.ret)
+
+	return expected.retDeviceName, 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) CreateDisk(volumeOptions *aws.VolumeOptions) (volumeName string, err error) {
+	return "", errors.New("Not implemented")
+}
+
+func (testcase *testcase) DeleteDisk(volumeName string) (bool, error) {
+	return false, errors.New("Not implemented")
+}
+
+func (testcase *testcase) GetVolumeLabels(volumeName string) (map[string]string, error) {
+	return map[string]string{}, errors.New("Not implemented")
+}
+
+func (testcase *testcase) GetDiskPath(volumeName string) (string, error) {
+	return "", errors.New("Not implemented")
+}

pkg/volume/gce_pd/attacher_test.go

@@ -31,7 +31,7 @@ import (
 func TestGetDeviceName_Volume(t *testing.T) {
 	plugin := newPlugin()
 	name := "my-pd-volume"
-	spec := createVSpec(name, false)
+	spec := createVolSpec(name, false)
 
 	deviceName, err := plugin.GetVolumeName(spec)
 	if err != nil {
@@ -75,7 +75,7 @@ func TestAttachDetach(t *testing.T) {
 	diskName := "disk"
 	instanceID := "instance"
 	readOnly := false
-	spec := createVSpec(diskName, readOnly)
+	spec := createVolSpec(diskName, readOnly)
 	attachError := errors.New("Fake attach error")
 	detachError := errors.New("Fake detach error")
 	diskCheckError := errors.New("Fake DiskIsAttached error")
@@ -222,7 +222,7 @@ func newDetacher(testcase *testcase) *gcePersistentDiskDetacher {
 	}
 }
 
-func createVSpec(name string, readOnly bool) *volume.Spec {
+func createVolSpec(name string, readOnly bool) *volume.Spec {
 	return &volume.Spec{
 		Volume: &api.Volume{
 			VolumeSource: api.VolumeSource{