Skip to content

Commit

Permalink
Rebased
Browse files Browse the repository at this point in the history
  • Loading branch information
mdzraf committed Aug 6, 2024
2 parents 857d257 + 40f2d17 commit 07f7765
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 37 deletions.
6 changes: 5 additions & 1 deletion pkg/driver/controller_modify_volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,14 +206,18 @@ func parseModifyVolumeParameters(params map[string]string) (*modifyVolumeRequest
} else if strings.HasPrefix(key, ModificationDeleteTag) {
options.modifyTagsOptions.TagsToDelete = append(options.modifyTagsOptions.TagsToDelete, value)
} else {
return nil, status.Errorf(codes.InvalidArgument, "Invalid parameter key: %s", key)
return nil, status.Errorf(codes.InvalidArgument, "Invalid mutable parameter key: %s", key)
}
}
}
addTags, err := template.Evaluate(scTags, tProps, false)
if err != nil {
return nil, status.Errorf(codes.InvalidArgument, "Error interpolating the tag value: %v", err)
}
addTags, err := template.Evaluate(scTags, tProps, false)
if err != nil {
return nil, status.Errorf(codes.InvalidArgument, "Error interpolating the tag value: %v", err)
}
if err := validateExtraTags(addTags, false); err != nil {
return nil, status.Errorf(codes.InvalidArgument, "Invalid tag value: %v", err)
}
Expand Down
8 changes: 8 additions & 0 deletions pkg/driver/controller_modify_volume_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ const (
tagSpecificationWithNoEqual = "key1"
validTagDeletion = "key2"
invalidTagSpecification = "aws:test=TEST"
invalidParameter = "invalid_parameter"
)

func TestParseModifyVolumeParameters(t *testing.T) {
Expand Down Expand Up @@ -129,6 +130,13 @@ func TestParseModifyVolumeParameters(t *testing.T) {
},
expectError: true,
},
{
name: "invalid VAC parameter",
params: map[string]string{
invalidParameter: "20",
},
expectError: true,
},
}

for _, tc := range testCases {
Expand Down
52 changes: 26 additions & 26 deletions tests/e2e/modify_volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,72 +37,72 @@ var (
modifyVolumeTests = map[string]testsuites.ModifyVolumeTest{
"with a new iops annotation": {
CreateVolumeParameters: defaultModifyVolumeTestGp3CreateVolumeParameters,
ModifyVolumeAnnotations: map[string]string{
testsuites.AnnotationIops: "4000",
ModifyVolumeParameters: map[string]string{
testsuites.Iops: "4000",
},
ShouldResizeVolume: false,
ShouldTestInvalidModificationRecovery: false,
},
"with a new io2 volumeType annotation": {
CreateVolumeParameters: defaultModifyVolumeTestGp3CreateVolumeParameters,
ModifyVolumeAnnotations: map[string]string{
testsuites.AnnotationVolumeType: awscloud.VolumeTypeIO2,
testsuites.AnnotationIops: testsuites.DefaultIopsIoVolumes, // As of aws-ebs-csi-driver v1.25.0, parameter iops must be re-specified when modifying volumeType io2 volumes.
ModifyVolumeParameters: map[string]string{
testsuites.VolumeType: awscloud.VolumeTypeIO2,
testsuites.Iops: testsuites.DefaultIopsIoVolumes, // As of aws-ebs-csi-driver v1.25.0, parameter iops must be re-specified when modifying volumeType io2 volumes.
},
ShouldResizeVolume: false,
ShouldTestInvalidModificationRecovery: false,
},
"with a new throughput annotation": {
CreateVolumeParameters: defaultModifyVolumeTestGp3CreateVolumeParameters,
ModifyVolumeAnnotations: map[string]string{
testsuites.AnnotationThroughput: "150",
ModifyVolumeParameters: map[string]string{
testsuites.Throughput: "150",
},
ShouldResizeVolume: false,
ShouldTestInvalidModificationRecovery: false,
},
"with a new tag annotation": {
CreateVolumeParameters: defaultModifyVolumeTestGp3CreateVolumeParameters,
ModifyVolumeAnnotations: map[string]string{
testsuites.AnnotationsTagSpec: "key1=test1",
ModifyVolumeParameters: map[string]string{
testsuites.TagSpec: "key1=test1",
},
ShouldResizeVolume: false,
ShouldTestInvalidModificationRecovery: false,
ExternalResizerOnly: true,
},
"with new throughput, and iops annotations": {
CreateVolumeParameters: defaultModifyVolumeTestGp3CreateVolumeParameters,
ModifyVolumeAnnotations: map[string]string{
testsuites.AnnotationIops: "4000",
testsuites.AnnotationThroughput: "150",
ModifyVolumeParameters: map[string]string{
testsuites.Iops: "4000",
testsuites.Throughput: "150",
},
ShouldResizeVolume: false,
ShouldTestInvalidModificationRecovery: false,
},
"with new throughput, iops, and tag annotations": {
CreateVolumeParameters: defaultModifyVolumeTestGp3CreateVolumeParameters,
ModifyVolumeAnnotations: map[string]string{
testsuites.AnnotationIops: "4000",
testsuites.AnnotationThroughput: "150",
testsuites.AnnotationsTagSpec: "key2=test2",
ModifyVolumeParameters: map[string]string{
testsuites.Iops: "4000",
testsuites.Throughput: "150",
testsuites.TagSpec: "key2=test2",
},
ShouldResizeVolume: false,
ShouldTestInvalidModificationRecovery: false,
ExternalResizerOnly: true,
},
"with a larger size and new throughput and iops annotations": {
CreateVolumeParameters: defaultModifyVolumeTestGp3CreateVolumeParameters,
ModifyVolumeAnnotations: map[string]string{
testsuites.AnnotationIops: "4000",
testsuites.AnnotationThroughput: "150",
ModifyVolumeParameters: map[string]string{
testsuites.Iops: "4000",
testsuites.Throughput: "150",
},
ShouldResizeVolume: true,
ShouldTestInvalidModificationRecovery: false,
},
"with a larger size and new throughput and iops annotations after providing an invalid annotation": {
CreateVolumeParameters: defaultModifyVolumeTestGp3CreateVolumeParameters,
ModifyVolumeAnnotations: map[string]string{
testsuites.AnnotationIops: "4000",
testsuites.AnnotationThroughput: "150",
ModifyVolumeParameters: map[string]string{
testsuites.Iops: "4000",
testsuites.Throughput: "150",
},
ShouldResizeVolume: true,
ShouldTestInvalidModificationRecovery: true,
Expand All @@ -113,10 +113,10 @@ var (
ebscsidriver.FSTypeKey: ebscsidriver.FSTypeExt4,
ebscsidriver.IopsKey: testsuites.DefaultIopsIoVolumes,
},
ModifyVolumeAnnotations: map[string]string{
testsuites.AnnotationVolumeType: awscloud.VolumeTypeGP3,
testsuites.AnnotationIops: "4000",
testsuites.AnnotationThroughput: "150",
ModifyVolumeParameters: map[string]string{
testsuites.VolumeType: awscloud.VolumeTypeGP3,
testsuites.Iops: "4000",
testsuites.Throughput: "150",
},
ShouldResizeVolume: true,
ShouldTestInvalidModificationRecovery: false,
Expand Down
18 changes: 13 additions & 5 deletions tests/e2e/testsuites/e2e_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@ const (
DefaultResizeTimout = 1 * time.Minute
DefaultK8sApiPollingInterval = 5 * time.Second

AnnotationIops = "ebs.csi.aws.com/iops"
AnnotationThroughput = "ebs.csi.aws.com/throughput"
AnnotationVolumeType = "ebs.csi.aws.com/volumeType"
AnnotationsTagSpec = "ebs.csi.aws.com/tagSpecification"
AnnotationTagDel = "ebs.csi.aws.com/tagDeletion"
Iops = "iops"
Throughput = "throughput"
VolumeType = "type"
TagSpec = "tagSpecification"
TagDel = "tagDeletion"
)

var DefaultGeneratedVolumeMount = VolumeMountDetails{
Expand Down Expand Up @@ -173,3 +173,11 @@ func CreateVolumeDetails(createVolumeParameters map[string]string, volumeSize st

return &volume
}

func PrefixAnnotations(prefix string, parameters map[string]string) map[string]string {
result := make(map[string]string)
for key, value := range parameters {
result[prefix+key] = value
}
return result
}
12 changes: 7 additions & 5 deletions tests/e2e/testsuites/modify_volume_tester.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,15 @@ import (
// ModifyVolumeTest will provision pod with attached volume, and test that modifying its pvc will modify the associated pv.
type ModifyVolumeTest struct {
CreateVolumeParameters map[string]string
ModifyVolumeAnnotations map[string]string
ModifyVolumeParameters map[string]string
ShouldResizeVolume bool
ShouldTestInvalidModificationRecovery bool
ExternalResizerOnly bool
}

var (
invalidAnnotations = map[string]string{
AnnotationIops: "1",
Iops: "1",
}
volumeSize = "10Gi" // Different from driver.MinimumSizeForVolumeType to simplify iops, throughput, volumeType modification
)
Expand All @@ -57,6 +57,8 @@ func (modifyVolumeTest *ModifyVolumeTest) Run(c clientset.Interface, ns *v1.Name
testVolume, _ := volumeDetails.SetupDynamicPersistentVolumeClaim(c, ns, ebsDriver)
defer testVolume.Cleanup()

parametersWithPrefix := PrefixAnnotations("ebs.csi.aws.com/", modifyVolumeTest.ModifyVolumeParameters)

By("deploying pod continuously writing to volume")
formatOptionMountPod := createPodWithVolume(c, ns, PodCmdContinuousWrite(DefaultMountPath), testVolume, volumeDetails)
defer formatOptionMountPod.Cleanup()
Expand All @@ -70,15 +72,15 @@ func (modifyVolumeTest *ModifyVolumeTest) Run(c clientset.Interface, ns *v1.Name
By("modifying the pvc")
modifyingPvc, _ := c.CoreV1().PersistentVolumeClaims(ns.Name).Get(context.TODO(), testVolume.persistentVolumeClaim.Name, metav1.GetOptions{})
if testType == VolumeModifierForK8s {
AnnotatePvc(modifyingPvc, modifyVolumeTest.ModifyVolumeAnnotations)
AnnotatePvc(modifyingPvc, parametersWithPrefix)
} else if testType == ExternalResizer {
vac, err := c.StorageV1alpha1().VolumeAttributesClasses().Create(context.Background(), &v1alpha1.VolumeAttributesClass{
ObjectMeta: metav1.ObjectMeta{
Name: formatOptionMountPod.pod.Name,
Namespace: ns.Name,
},
DriverName: "ebs.csi.aws.com",
Parameters: modifyVolumeTest.ModifyVolumeAnnotations,
Parameters: modifyVolumeTest.ModifyVolumeParameters,
}, metav1.CreateOptions{})
framework.ExpectNoError(err)

Expand All @@ -100,7 +102,7 @@ func (modifyVolumeTest *ModifyVolumeTest) Run(c clientset.Interface, ns *v1.Name

By("wait for and confirm pv modification")
if testType == VolumeModifierForK8s {
err = WaitForPvToModify(c, ns, testVolume.persistentVolume.Name, modifyVolumeTest.ModifyVolumeAnnotations, DefaultModificationTimeout, DefaultK8sApiPollingInterval)
err = WaitForPvToModify(c, ns, testVolume.persistentVolume.Name, parametersWithPrefix, DefaultModificationTimeout, DefaultK8sApiPollingInterval)
} else if testType == ExternalResizer {
err = WaitForVacToApplyToPv(c, ns, testVolume.persistentVolume.Name, *modifyingPvc.Spec.VolumeAttributesClassName, DefaultModificationTimeout, DefaultK8sApiPollingInterval)
}
Expand Down

0 comments on commit 07f7765

Please sign in to comment.