Merge pull request #34961 from kubernetes/revert-32485-e2e-provisioning-class

Automatic merge from submit-queue

Revert "Add e2e tests for storageclass"

Reverts kubernetes/kubernetes#32485

This PR broke the multizone tests on GCE/GKE, and broke the AWS tests, e.g.:
* https://k8s-testgrid.appspot.com/google-aws
* https://k8s-testgrid.appspot.com/google-gke#gke-multizone
* https://k8s-testgrid.appspot.com/google-gke#gci-gke-multizone
Suggest revert. 

Fixes #34794

cc @k8s-oncall

pkg/cloudprovider/providers/gce/gce.go

@@ -2562,21 +2562,6 @@ func (gce *GCECloud) GetAutoLabelsForPD(name string, zone string) (map[string]st
 	return labels, nil
 }
 
-// TestDisk checks that a disk has given type. It should be used only for
-// testing!
-func (gce *GCECloud) TestDisk(diskName, volumeType string) error {
-	disk, err := gce.getDiskByNameUnknownZone(diskName)
-	if err != nil {
-		return err
-	}
-
-	if strings.HasSuffix(disk.Type, volumeType) {
-		return nil
-	}
-
-	return fmt.Errorf("unexpected disk type %q, expected suffix %q", disk.Type, volumeType)
-}
-
 func (gce *GCECloud) AttachDisk(diskName string, nodeName types.NodeName, readOnly bool) error {
 	instanceName := mapNodeNameToInstanceName(nodeName)
 	instance, err := gce.getInstanceByName(instanceName)
@@ -2660,7 +2645,6 @@ func (gce *GCECloud) findDiskByName(diskName string, zone string) (*gceDisk, err
 			Zone: lastComponent(disk.Zone),
 			Name: disk.Name,
 			Kind: disk.Kind,
-			Type: disk.Type,
 		}
 		return d, nil
 	}
@@ -2781,7 +2765,6 @@ type gceDisk struct {
 	Zone string
 	Name string
 	Kind string
-	Type string
 }
 
 // Gets the named instances, returning cloudprovider.InstanceNotFound if any instance is not found

test/e2e/volume_provisioning.go

@@ -17,13 +17,8 @@ limitations under the License.
 package e2e
 
 import (
-	"fmt"
-	"strings"
 	"time"
 
-	"github.com/aws/aws-sdk-go/aws/session"
-	"github.com/aws/aws-sdk-go/service/ec2"
-
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/api/resource"
 	"k8s.io/kubernetes/pkg/api/unversioned"
@@ -35,35 +30,16 @@ import (
 	. "github.com/onsi/gomega"
 )
 
-type storageClassTest struct {
+const (
-	name           string
+	// Requested size of the volume
-	cloudProviders []string
+	requestedSize = "1500Mi"
-	provisioner    string
+	// Expected size of the volume is 2GiB, because all three supported cloud
-	parameters     map[string]string
+	// providers allocate volumes in 1GiB chunks.
-	claimSize      string
+	expectedSize = "2Gi"
-	expectedSize   string
+)
-	pvCheck        func(volume *api.PersistentVolume) error
-}
 
-func testDynamicProvisioning(t storageClassTest, client *client.Client, claim *api.PersistentVolumeClaim, class *storage.StorageClass) {
+func testDynamicProvisioning(client *client.Client, claim *api.PersistentVolumeClaim) {
-	if class != nil {
+	err := framework.WaitForPersistentVolumeClaimPhase(api.ClaimBound, client, claim.Namespace, claim.Name, framework.Poll, framework.ClaimProvisionTimeout)
-		By("creating a StorageClass " + class.Name)
-		class, err := client.Storage().StorageClasses().Create(class)
-		defer func() {
-			framework.Logf("deleting storage class %s", class.Name)
-			client.Storage().StorageClasses().Delete(class.Name)
-		}()
-		Expect(err).NotTo(HaveOccurred())
-	}
-
-	By("creating a claim")
-	claim, err := client.PersistentVolumeClaims(claim.Namespace).Create(claim)
-	defer func() {
-		framework.Logf("deleting claim %s/%s", claim.Namespace, claim.Name)
-		client.PersistentVolumeClaims(claim.Namespace).Delete(claim.Name)
-	}()
-	Expect(err).NotTo(HaveOccurred())
-	err = framework.WaitForPersistentVolumeClaimPhase(api.ClaimBound, client, claim.Namespace, claim.Name, framework.Poll, framework.ClaimProvisionTimeout)
 	Expect(err).NotTo(HaveOccurred())
 
 	By("checking the claim")
@@ -76,28 +52,21 @@ func testDynamicProvisioning(t storageClassTest, client *client.Client, claim *a
 	Expect(err).NotTo(HaveOccurred())
 
 	// Check sizes
-	expectedCapacity := resource.MustParse(t.expectedSize)
+	expectedCapacity := resource.MustParse(expectedSize)
 	pvCapacity := pv.Spec.Capacity[api.ResourceName(api.ResourceStorage)]
 	Expect(pvCapacity.Value()).To(Equal(expectedCapacity.Value()))
 
-	requestedCapacity := resource.MustParse(t.claimSize)
+	requestedCapacity := resource.MustParse(requestedSize)
 	claimCapacity := claim.Spec.Resources.Requests[api.ResourceName(api.ResourceStorage)]
 	Expect(claimCapacity.Value()).To(Equal(requestedCapacity.Value()))
 
 	// Check PV properties
-	By("checking the PV")
 	Expect(pv.Spec.PersistentVolumeReclaimPolicy).To(Equal(api.PersistentVolumeReclaimDelete))
 	expectedAccessModes := []api.PersistentVolumeAccessMode{api.ReadWriteOnce}
 	Expect(pv.Spec.AccessModes).To(Equal(expectedAccessModes))
 	Expect(pv.Spec.ClaimRef.Name).To(Equal(claim.ObjectMeta.Name))
 	Expect(pv.Spec.ClaimRef.Namespace).To(Equal(claim.ObjectMeta.Namespace))
 
-	// Run the checker
-	if t.pvCheck != nil {
-		err = t.pvCheck(pv)
-		Expect(err).NotTo(HaveOccurred())
-	}
-
 	// We start two pods:
 	// - The first writes 'hello word' to the /mnt/test (= the volume).
 	// - The second one runs grep 'hello world' on /mnt/test.
@@ -132,54 +101,6 @@ func testDynamicProvisioning(t storageClassTest, client *client.Client, claim *a
 	framework.ExpectNoError(framework.WaitForPersistentVolumeDeleted(client, pv.Name, 5*time.Second, 20*time.Minute))
 }
 
-// checkAWSEBS checks properties of an AWS EBS. Test framework does not
-// instantiate full AWS provider, therefore we need use ec2 API directly.
-func checkAWSEBS(volume *api.PersistentVolume, volumeType string, encrypted bool) error {
-	diskName := volume.Spec.AWSElasticBlockStore.VolumeID
-
-	client := ec2.New(session.New())
-	tokens := strings.Split(diskName, "/")
-	volumeID := tokens[len(tokens)-1]
-
-	request := &ec2.DescribeVolumesInput{
-		VolumeIds: []*string{&volumeID},
-	}
-	info, err := client.DescribeVolumes(request)
-	if err != nil {
-		return fmt.Errorf("error querying ec2 for volume %q: %v", volumeID, err)
-	}
-	if len(info.Volumes) == 0 {
-		return fmt.Errorf("no volumes found for volume %q", volumeID)
-	}
-	if len(info.Volumes) > 1 {
-		return fmt.Errorf("multiple volumes found for volume %q", volumeID)
-	}
-
-	awsVolume := info.Volumes[0]
-	if awsVolume.VolumeType == nil {
-		return fmt.Errorf("expected volume type %q, got nil", volumeType)
-	}
-	if *awsVolume.VolumeType != volumeType {
-		return fmt.Errorf("expected volume type %q, got %q", volumeType, *awsVolume.VolumeType)
-	}
-	if encrypted && awsVolume.Encrypted == nil {
-		return fmt.Errorf("expected encrypted volume, got no encryption")
-	}
-	if encrypted && !*awsVolume.Encrypted {
-		return fmt.Errorf("expected encrypted volume, got %v", *awsVolume.Encrypted)
-	}
-	return nil
-}
-
-func checkGCEPD(volume *api.PersistentVolume, volumeType string) error {
-	cloud, err := getGCECloud()
-	if err != nil {
-		return err
-	}
-	diskName := volume.Spec.GCEPersistentDisk.PDName
-	return cloud.TestDisk(diskName, volumeType)
-}
-
 var _ = framework.KubeDescribe("Dynamic provisioning", func() {
 	f := framework.NewDefaultFramework("volume-provisioning")
 
@@ -193,170 +114,45 @@ var _ = framework.KubeDescribe("Dynamic provisioning", func() {
 	})
 
 	framework.KubeDescribe("DynamicProvisioner", func() {
-		// This test checks that dynamic provisioning can provision a volume
+		It("should create and delete persistent volumes [Slow]", func() {
-		// that can be used to persist data among pods.
+			framework.SkipUnlessProviderIs("openstack", "gce", "aws", "gke")
 
-		tests := []storageClassTest{
+			By("creating a StorageClass")
-			{
+			class := newStorageClass()
-				"should provision SSD PD on GCE/GKE",
+			_, err := c.Storage().StorageClasses().Create(class)
-				[]string{"gce", "gke"},
+			defer c.Storage().StorageClasses().Delete(class.Name)
-				"kubernetes.io/gce-pd",
+			Expect(err).NotTo(HaveOccurred())
-				map[string]string{
-					"type": "pd-ssd",
-					// Check that GCE can parse "zone" parameter, however
-					// we can't create PDs in different than default zone
-					// as we don't know if we're running with Multizone=true
-					"zone": framework.TestContext.CloudConfig.Zone,
-				},
-				"1.5Gi",
-				"2Gi",
-				func(volume *api.PersistentVolume) error {
-					return checkGCEPD(volume, "pd-ssd")
-				},
-			},
-			{
-				"should provision HDD PD on GCE/GKE",
-				[]string{"gce", "gke"},
-				"kubernetes.io/gce-pd",
-				map[string]string{
-					"type": "pd-standard",
-				},
-				"1.5Gi",
-				"2Gi",
-				func(volume *api.PersistentVolume) error {
-					return checkGCEPD(volume, "pd-standard")
-				},
-			},
-			// AWS
-			{
-				"should provision gp2 EBS on AWS",
-				[]string{"aws"},
-				"kubernetes.io/aws-ebs",
-				map[string]string{
-					"type": "gp2",
-					// Check that AWS can parse "zone" parameter, however
-					// we can't create PDs in different than default zone
-					// as we don't know zone names
-					"zone": framework.TestContext.CloudConfig.Zone,
-				},
-				"1.5Gi",
-				"2Gi",
-				func(volume *api.PersistentVolume) error {
-					return checkAWSEBS(volume, "gp2", false)
-				},
-			},
-			{
-				"should provision io1 EBS on AWS",
-				[]string{"aws"},
-				"kubernetes.io/aws-ebs",
-				map[string]string{
-					"type":      "io1",
-					"iopsPerGB": "50",
-				},
-				"3.5Gi",
-				"4Gi", // 4 GiB is minimum for io1
-				func(volume *api.PersistentVolume) error {
-					return checkAWSEBS(volume, "io1", false)
-				},
-			},
-			{
-				"should provision sc1 EBS on AWS",
-				[]string{"aws"},
-				"kubernetes.io/aws-ebs",
-				map[string]string{
-					"type": "sc1",
-				},
-				"500Gi", // minimum for sc1
-				"500Gi",
-				func(volume *api.PersistentVolume) error {
-					return checkAWSEBS(volume, "sc1", false)
-				},
-			},
-			{
-				"should provision st1 EBS on AWS",
-				[]string{"aws"},
-				"kubernetes.io/aws-ebs",
-				map[string]string{
-					"type": "st1",
-				},
-				"500Gi", // minimum for st1
-				"500Gi",
-				func(volume *api.PersistentVolume) error {
-					return checkAWSEBS(volume, "st1", false)
-				},
-			},
-			{
-				"should provision encrypted EBS on AWS",
-				[]string{"aws"},
-				"kubernetes.io/aws-ebs",
-				map[string]string{
-					"encrypted": "true",
-				},
-				"1Gi",
-				"1Gi",
-				func(volume *api.PersistentVolume) error {
-					return checkAWSEBS(volume, "gp2", true)
-				},
-			},
-			// OpenStack generic tests (works on all OpenStack deployments)
-			{
-				"should provision generic Cinder volume on OpenStack",
-				[]string{"openstack"},
-				"kubernetes.io/cinder",
-				map[string]string{},
-				"1.5Gi",
-				"2Gi",
-				nil, // there is currently nothing to check on OpenStack
-			},
-			{
-				"should provision Cinder volume with empty volume type and zone on OpenStack",
-				[]string{"openstack"},
-				"kubernetes.io/cinder",
-				map[string]string{
-					"type":         "",
-					"availability": "",
-				},
-				"1.5Gi",
-				"2Gi",
-				nil, // there is currently nothing to check on OpenStack
-			},
-		}
 
-		for i, t := range tests {
+			By("creating a claim with a dynamic provisioning annotation")
-			// Beware of clojure, use local variables instead of those from
+			claim := newClaim(ns, false)
-			// outer scope
+			defer func() {
-			test := t
+				c.PersistentVolumeClaims(ns).Delete(claim.Name)
-			suffix := fmt.Sprintf("%d", i)
+			}()
-			It(test.name, func() {
+			claim, err = c.PersistentVolumeClaims(ns).Create(claim)
-				if len(t.cloudProviders) > 0 {
+			Expect(err).NotTo(HaveOccurred())
-					framework.SkipUnlessProviderIs(test.cloudProviders...)
-				}
 
-				class := newStorageClass(test, suffix)
+			testDynamicProvisioning(c, claim)
-				claim := newClaim(test, ns, suffix, false)
+		})
-				testDynamicProvisioning(test, c, claim, class)
-			})
-		}
 	})
 
 	framework.KubeDescribe("DynamicProvisioner Alpha", func() {
-		It("should provision alpha volumes [Slow]", func() {
+		It("should create and delete alpha persistent volumes [Slow]", func() {
 			framework.SkipUnlessProviderIs("openstack", "gce", "aws", "gke")
 
 			By("creating a claim with an alpha dynamic provisioning annotation")
-			test := storageClassTest{
+			claim := newClaim(ns, true)
-				name:         "alpha test",
+			defer func() {
-				claimSize:    "1500Mi",
+				c.PersistentVolumeClaims(ns).Delete(claim.Name)
-				expectedSize: "2Gi",
+			}()
-			}
+			claim, err := c.PersistentVolumeClaims(ns).Create(claim)
+			Expect(err).NotTo(HaveOccurred())
 
-			claim := newClaim(test, ns, "", true)
+			testDynamicProvisioning(c, claim)
-			testDynamicProvisioning(test, c, claim, nil)
 		})
 	})
 })
 
-func newClaim(t storageClassTest, ns, suffix string, alpha bool) *api.PersistentVolumeClaim {
+func newClaim(ns string, alpha bool) *api.PersistentVolumeClaim {
 	claim := api.PersistentVolumeClaim{
 		ObjectMeta: api.ObjectMeta{
 			GenerateName: "pvc-",
@@ -368,7 +164,7 @@ func newClaim(t storageClassTest, ns, suffix string, alpha bool) *api.Persistent
 			},
 			Resources: api.ResourceRequirements{
 				Requests: api.ResourceList{
-					api.ResourceName(api.ResourceStorage): resource.MustParse(t.claimSize),
+					api.ResourceName(api.ResourceStorage): resource.MustParse(requestedSize),
 				},
 			},
 		},
@@ -380,8 +176,9 @@ func newClaim(t storageClassTest, ns, suffix string, alpha bool) *api.Persistent
 		}
 	} else {
 		claim.Annotations = map[string]string{
-			"volume.beta.kubernetes.io/storage-class": "myclass-" + suffix,
+			"volume.beta.kubernetes.io/storage-class": "fast",
 		}
+
 	}
 
 	return &claim
@@ -427,22 +224,32 @@ func runInPodWithVolume(c *client.Client, ns, claimName, command string) {
 		},
 	}
 	pod, err := c.Pods(ns).Create(pod)
-	framework.ExpectNoError(err, "Failed to create pod: %v", err)
 	defer func() {
 		framework.ExpectNoError(c.Pods(ns).Delete(pod.Name, nil))
 	}()
+	framework.ExpectNoError(err, "Failed to create pod: %v", err)
 	framework.ExpectNoError(framework.WaitForPodSuccessInNamespaceSlow(c, pod.Name, pod.Namespace))
 }
 
-func newStorageClass(t storageClassTest, suffix string) *storage.StorageClass {
+func newStorageClass() *storage.StorageClass {
+	var pluginName string
+
+	switch {
+	case framework.ProviderIs("gke"), framework.ProviderIs("gce"):
+		pluginName = "kubernetes.io/gce-pd"
+	case framework.ProviderIs("aws"):
+		pluginName = "kubernetes.io/aws-ebs"
+	case framework.ProviderIs("openstack"):
+		pluginName = "kubernetes.io/cinder"
+	}
+
 	return &storage.StorageClass{
 		TypeMeta: unversioned.TypeMeta{
 			Kind: "StorageClass",
 		},
 		ObjectMeta: api.ObjectMeta{
-			Name: "myclass-" + suffix,
+			Name: "fast",
 		},
-		Provisioner: t.provisioner,
+		Provisioner: pluginName,
-		Parameters:  t.parameters,
 	}
 }