Merge pull request kubernetes#124151 from ConnorJC3/add-vac-e2e

Add Happy Path VolumeAttributesClass CSI E2E Tests

Author: k8s-ci-robot

test/e2e/feature/feature.go

@@ -363,6 +363,13 @@ var (
 	// TODO: document the feature (owning SIG, when to use this feature for a test)
 	ValidatingAdmissionPolicy = framework.WithFeature(framework.ValidFeatures.Add("ValidatingAdmissionPolicy"))
 
+	// Owner: sig-storage
+	// Tests related to VolumeAttributesClass (https://kep.k8s.io/3751)
+	//
+	// TODO: This label only requires the API storage.k8s.io/v1alpha1 and the VolumeAttributesClass feature-gate enabled.
+	// It should be removed after k/k #124350 is merged.
+	VolumeAttributesClass = framework.WithFeature(framework.ValidFeatures.Add("VolumeAttributesClass"))
+
 	// TODO: document the feature (owning SIG, when to use this feature for a test)
 	Volumes = framework.WithFeature(framework.ValidFeatures.Add("Volumes"))
 

test/e2e/framework/pv/pv.go

@@ -127,10 +127,11 @@ type PersistentVolumeClaimConfig struct {
 	// unspecified
 	ClaimSize string
 	// AccessModes defaults to RWO if unspecified
-	AccessModes      []v1.PersistentVolumeAccessMode
-	Annotations      map[string]string
-	Selector         *metav1.LabelSelector
-	StorageClassName *string
+	AccessModes               []v1.PersistentVolumeAccessMode
+	Annotations               map[string]string
+	Selector                  *metav1.LabelSelector
+	StorageClassName          *string
+	VolumeAttributesClassName *string
 	// VolumeMode defaults to nil if unspecified or specified as the empty
 	// string
 	VolumeMode *v1.PersistentVolumeMode
@@ -661,8 +662,9 @@ func MakePersistentVolumeClaim(cfg PersistentVolumeClaimConfig, ns string) *v1.P
 					v1.ResourceStorage: resource.MustParse(cfg.ClaimSize),
 				},
 			},
-			StorageClassName: cfg.StorageClassName,
-			VolumeMode:       cfg.VolumeMode,
+			StorageClassName:          cfg.StorageClassName,
+			VolumeAttributesClassName: cfg.VolumeAttributesClassName,
+			VolumeMode:                cfg.VolumeMode,
 		},
 	}
 }

test/e2e/storage/drivers/csi.go

@@ -54,6 +54,7 @@ import (
 	v1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
 	storagev1 "k8s.io/api/storage/v1"
+	storagev1alpha1 "k8s.io/api/storage/v1alpha1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -85,6 +86,11 @@ const (
 
 	// Prefix of the mock driver grpc log
 	grpcCallPrefix = "gRPCCall:"
+
+	// Parameter to use in hostpath CSI driver VolumeAttributesClass
+	// Must be passed to the driver via --accepted-mutable-parameter-names
+	hostpathCSIDriverMutableParameterName  = "e2eVacTest"
+	hostpathCSIDriverMutableParameterValue = "test-value"
 )
 
 // hostpathCSI
@@ -209,6 +215,15 @@ func (h *hostpathCSIDriver) GetSnapshotClass(ctx context.Context, config *storag
 	return utils.GenerateSnapshotClassSpec(snapshotter, parameters, ns)
 }
 
+func (h *hostpathCSIDriver) GetVolumeAttributesClass(_ context.Context, config *storageframework.PerTestConfig) *storagev1alpha1.VolumeAttributesClass {
+	return storageframework.CopyVolumeAttributesClass(&storagev1alpha1.VolumeAttributesClass{
+		DriverName: config.GetUniqueDriverName(),
+		Parameters: map[string]string{
+			hostpathCSIDriverMutableParameterName: hostpathCSIDriverMutableParameterValue,
+		},
+	}, config.Framework.Namespace.Name, "e2e-vac-hostpath")
+}
+
 func (h *hostpathCSIDriver) PrepareTest(ctx context.Context, f *framework.Framework) *storageframework.PerTestConfig {
 	// Create secondary namespace which will be used for creating driver
 	driverNamespace := utils.CreateDriverNamespace(ctx, f)
@@ -230,7 +245,9 @@ func (h *hostpathCSIDriver) PrepareTest(ctx context.Context, f *framework.Framew
 		DriverNamespace:     driverNamespace,
 	}
 
-	o := utils.PatchCSIOptions{
+	patches := []utils.PatchCSIOptions{}
+
+	patches = append(patches, utils.PatchCSIOptions{
 		OldDriverName:       h.driverInfo.Name,
 		NewDriverName:       config.GetUniqueDriverName(),
 		DriverContainerName: "hostpath",
@@ -246,11 +263,31 @@ func (h *hostpathCSIDriver) PrepareTest(ctx context.Context, f *framework.Framew
 		ProvisionerContainerName: "csi-provisioner",
 		SnapshotterContainerName: "csi-snapshotter",
 		NodeName:                 node.Name,
-	}
+	})
+
+	// VAC E2E HostPath patch
+	// Enables ModifyVolume support in the hostpath CSI driver, and adds an enabled parameter name
+	patches = append(patches, utils.PatchCSIOptions{
+		DriverContainerName:      "hostpath",
+		DriverContainerArguments: []string{"--enable-controller-modify-volume=true", "--accepted-mutable-parameter-names=e2eVacTest"},
+	})
+
+	// VAC E2E FeatureGate patches
+	// TODO: These can be removed after the VolumeAttributesClass feature is default enabled
+	patches = append(patches, utils.PatchCSIOptions{
+		DriverContainerName:      "csi-provisioner",
+		DriverContainerArguments: []string{"--feature-gates=VolumeAttributesClass=true"},
+	})
+	patches = append(patches, utils.PatchCSIOptions{
+		DriverContainerName:      "csi-resizer",
+		DriverContainerArguments: []string{"--feature-gates=VolumeAttributesClass=true"},
+	})
 
 	err = utils.CreateFromManifests(ctx, config.Framework, driverNamespace, func(item interface{}) error {
-		if err := utils.PatchCSIDeployment(config.Framework, o, item); err != nil {
-			return err
+		for _, o := range patches {
+			if err := utils.PatchCSIDeployment(config.Framework, o, item); err != nil {
+				return err
+			}
 		}
 
 		// Remove csi-external-health-monitor-agent and

test/e2e/storage/external/external.go

@@ -25,6 +25,7 @@ import (
 	"time"
 
 	storagev1 "k8s.io/api/storage/v1"
+	storagev1alpha1 "k8s.io/api/storage/v1alpha1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -79,6 +80,30 @@ type driverDefinition struct {
 		FromExistingClassName string
 	}
 
+	// VolumeAttributesClass must be set to enable volume modification tests.
+	// The default is to not run those tests.
+	VolumeAttributesClass struct {
+		// FromName set to true enables the usage of a
+		// VolumeAttributesClass with DriverInfo.Name as
+		// provisioner and no parameters.
+		FromName bool
+
+		// FromFile is used only when FromName is false.  It
+		// loads a storage class from the given .yaml or .json
+		// file. File names are resolved by the
+		// framework.testfiles package, which typically means
+		// that they can be absolute or relative to the test
+		// suite's --repo-root parameter.
+		//
+		// This can be used when the VolumeAttributesClass
+		// is meant to have additional parameters.
+		FromFile string
+
+		// FromExistingClassName specifies the name of a pre-installed
+		// VolumeAttributesClass that will be copied and used for the tests.
+		FromExistingClassName string
+	}
+
 	// SnapshotClass must be set to enable snapshotting tests.
 	// The default is to not run those tests.
 	SnapshotClass struct {
@@ -405,6 +430,43 @@ func (d *driverDefinition) GetSnapshotClass(ctx context.Context, e2econfig *stor
 	return utils.GenerateSnapshotClassSpec(snapshotter, parameters, ns)
 }
 
+func (d *driverDefinition) GetVolumeAttributesClass(ctx context.Context, e2econfig *storageframework.PerTestConfig) *storagev1alpha1.VolumeAttributesClass {
+	if !d.VolumeAttributesClass.FromName && d.VolumeAttributesClass.FromFile == "" && d.VolumeAttributesClass.FromExistingClassName == "" {
+		e2eskipper.Skipf("Driver %q has no configured VolumeAttributesClass - skipping", d.DriverInfo.Name)
+		return nil
+	}
+
+	var (
+		vac *storagev1alpha1.VolumeAttributesClass
+		err error
+	)
+
+	f := e2econfig.Framework
+	switch {
+	case d.VolumeAttributesClass.FromName:
+		vac = &storagev1alpha1.VolumeAttributesClass{DriverName: d.DriverInfo.Name}
+	case d.VolumeAttributesClass.FromExistingClassName != "":
+		vac, err = f.ClientSet.StorageV1alpha1().VolumeAttributesClasses().Get(ctx, d.VolumeAttributesClass.FromExistingClassName, metav1.GetOptions{})
+		framework.ExpectNoError(err, "getting VolumeAttributesClass %s", d.VolumeAttributesClass.FromExistingClassName)
+	case d.VolumeAttributesClass.FromFile != "":
+		var ok bool
+		items, err := utils.LoadFromManifests(d.VolumeAttributesClass.FromFile)
+		framework.ExpectNoError(err, "load VolumeAttributesClass from %s", d.VolumeAttributesClass.FromFile)
+		gomega.Expect(items).To(gomega.HaveLen(1), "exactly one item from %s", d.VolumeAttributesClass.FromFile)
+		err = utils.PatchItems(f, f.Namespace, items...)
+		framework.ExpectNoError(err, "patch VolumeAttributesClass from %s", d.VolumeAttributesClass.FromFile)
+
+		vac, ok = items[0].(*storagev1alpha1.VolumeAttributesClass)
+		if !ok {
+			framework.Failf("cast VolumeAttributesClass from %s", d.VolumeAttributesClass.FromFile)
+		}
+	}
+
+	gomega.Expect(vac).ToNot(gomega.BeNil(), "VolumeAttributesClass is unexpectantly nil")
+
+	return storageframework.CopyVolumeAttributesClass(vac, f.Namespace.Name, "e2e-vac")
+}
+
 func (d *driverDefinition) GetVolume(e2econfig *storageframework.PerTestConfig, volumeNumber int) (map[string]string, bool, bool) {
 	if len(d.InlineVolumes) == 0 {
 		e2eskipper.Skipf("%s does not have any InlineVolumeAttributes defined", d.DriverInfo.Name)

test/e2e/storage/external/testdata/example.yaml

@@ -1,5 +1,7 @@
 StorageClass:
   FromExistingClassName: example
+VolumeAttributesClass:
+  FromExistingClassName: example-vac
 DriverInfo:
   Name: example
   RequiredAccessModes:

test/e2e/storage/framework/driver_operations.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 
 	storagev1 "k8s.io/api/storage/v1"
+	storagev1alpha1 "k8s.io/api/storage/v1alpha1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apiserver/pkg/storage/names"
 	"k8s.io/kubernetes/pkg/volume/util"
@@ -92,3 +93,13 @@ func GetStorageClass(
 		VolumeBindingMode: bindingMode,
 	}
 }
+
+// CopyVolumeAttributesClass constructs a new VolumeAttributesClass instance
+// with a unique name that is based on namespace + suffix
+// using the VolumeAttributesClass passed in as a parameter
+func CopyVolumeAttributesClass(vac *storagev1alpha1.VolumeAttributesClass, ns string, suffix string) *storagev1alpha1.VolumeAttributesClass {
+	copy := vac.DeepCopy()
+	copy.ObjectMeta.Name = names.SimpleNameGenerator.GenerateName(ns + "-" + suffix)
+	copy.ResourceVersion = ""
+	return copy
+}

test/e2e/storage/framework/testdriver.go

@@ -22,6 +22,7 @@ import (
 
 	v1 "k8s.io/api/core/v1"
 	storagev1 "k8s.io/api/storage/v1"
+	storagev1alpha1 "k8s.io/api/storage/v1alpha1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/kubernetes/test/e2e/framework"
@@ -130,6 +131,15 @@ type SnapshottableTestDriver interface {
 	GetSnapshotClass(ctx context.Context, config *PerTestConfig, parameters map[string]string) *unstructured.Unstructured
 }
 
+// VolumeAttributesClassTestDriver represents an interface for a TestDriver that supports
+// creating and modifying volumes via VolumeAttributesClass objects
+type VolumeAttributesClassTestDriver interface {
+	TestDriver
+	// GetVolumeAttributesClass returns a VolumeAttributesClass to create/modify PVCs
+	// It will return nil if the TestDriver does not support VACs
+	GetVolumeAttributesClass(ctx context.Context, config *PerTestConfig) *storagev1alpha1.VolumeAttributesClass
+}
+
 // CustomTimeoutsTestDriver represents an interface fo a TestDriver that supports custom timeouts.
 type CustomTimeoutsTestDriver interface {
 	TestDriver

test/e2e/storage/framework/volume_resource.go

@@ -53,11 +53,19 @@ type VolumeResource struct {
 // CreateVolumeResource constructs a VolumeResource for the current test. It knows how to deal with
 // different test pattern volume types.
 func CreateVolumeResource(ctx context.Context, driver TestDriver, config *PerTestConfig, pattern TestPattern, testVolumeSizeRange e2evolume.SizeRange) *VolumeResource {
-	return CreateVolumeResourceWithAccessModes(ctx, driver, config, pattern, testVolumeSizeRange, driver.GetDriverInfo().RequiredAccessModes)
+	return CreateVolumeResourceWithAccessModes(ctx, driver, config, pattern, testVolumeSizeRange, driver.GetDriverInfo().RequiredAccessModes, nil)
+}
+
+// CreateVolumeResource constructs a VolumeResource for the current test using the specified VAC name.
+func CreateVolumeResourceWithVAC(ctx context.Context, driver TestDriver, config *PerTestConfig, pattern TestPattern, testVolumeSizeRange e2evolume.SizeRange, vacName *string) *VolumeResource {
+	if pattern.VolType != DynamicPV {
+		framework.Failf("Creating volume with VAC only supported on dynamic PV tests")
+	}
+	return CreateVolumeResourceWithAccessModes(ctx, driver, config, pattern, testVolumeSizeRange, driver.GetDriverInfo().RequiredAccessModes, vacName)
 }
 
 // CreateVolumeResourceWithAccessModes constructs a VolumeResource for the current test with the provided access modes.
-func CreateVolumeResourceWithAccessModes(ctx context.Context, driver TestDriver, config *PerTestConfig, pattern TestPattern, testVolumeSizeRange e2evolume.SizeRange, accessModes []v1.PersistentVolumeAccessMode) *VolumeResource {
+func CreateVolumeResourceWithAccessModes(ctx context.Context, driver TestDriver, config *PerTestConfig, pattern TestPattern, testVolumeSizeRange e2evolume.SizeRange, accessModes []v1.PersistentVolumeAccessMode, vacName *string) *VolumeResource {
 	r := VolumeResource{
 		Config:  config,
 		Pattern: pattern,
@@ -107,7 +115,7 @@ func CreateVolumeResourceWithAccessModes(ctx context.Context, driver TestDriver,
 			switch pattern.VolType {
 			case DynamicPV:
 				r.Pv, r.Pvc = createPVCPVFromDynamicProvisionSC(
-					ctx, f, dInfo.Name, claimSize, r.Sc, pattern.VolMode, accessModes)
+					ctx, f, dInfo.Name, claimSize, r.Sc, pattern.VolMode, accessModes, vacName)
 				r.VolSource = storageutils.CreateVolumeSource(r.Pvc.Name, false /* readOnly */)
 			case GenericEphemeralVolume:
 				driverVolumeSizeRange := dDriver.GetDriverInfo().SupportedSizeRange
@@ -287,17 +295,19 @@ func createPVCPVFromDynamicProvisionSC(
 	sc *storagev1.StorageClass,
 	volMode v1.PersistentVolumeMode,
 	accessModes []v1.PersistentVolumeAccessMode,
+	vacName *string,
 ) (*v1.PersistentVolume, *v1.PersistentVolumeClaim) {
 	cs := f.ClientSet
 	ns := f.Namespace.Name
 
 	ginkgo.By("creating a claim")
 	pvcCfg := e2epv.PersistentVolumeClaimConfig{
-		NamePrefix:       name,
-		ClaimSize:        claimSize,
-		StorageClassName: &(sc.Name),
-		AccessModes:      accessModes,
-		VolumeMode:       &volMode,
+		NamePrefix:                name,
+		ClaimSize:                 claimSize,
+		StorageClassName:          &(sc.Name),
+		VolumeAttributesClassName: vacName,
+		AccessModes:               accessModes,
+		VolumeMode:                &volMode,
 	}
 
 	pvc := e2epv.MakePersistentVolumeClaim(pvcCfg, ns)

test/e2e/storage/testsuites/base.go

@@ -82,6 +82,7 @@ var CSISuites = append(BaseSuites,
 	InitSnapshottableStressTestSuite,
 	InitVolumePerformanceTestSuite,
 	InitReadWriteOncePodTestSuite,
+	InitVolumeModifyTestSuite,
 )
 
 func getVolumeOpsFromMetricsForPlugin(ms testutil.Metrics, pluginName string) opCounts {

test/e2e/storage/testsuites/disruptive.go

@@ -116,7 +116,8 @@ func (s *disruptiveTestSuite) DefineTests(driver storageframework.TestDriver, pa
 				l.config,
 				pattern,
 				testVolumeSizeRange,
-				accessModes)
+				accessModes,
+				nil)
 		}
 	}