From 3e4c2cf5e7881ed931f50ac524feb26c0f3fe35a Mon Sep 17 00:00:00 2001 From: Connor Catlett Date: Wed, 29 Nov 2023 21:37:05 +0000 Subject: [PATCH 1/2] Fix controller.CreateSnapshot to create new err variable instead of overwriting existing with nil Signed-off-by: Connor Catlett --- pkg/driver/controller.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go index 09d508208f..d0236369c8 100644 --- a/pkg/driver/controller.go +++ b/pkg/driver/controller.go @@ -758,8 +758,8 @@ func (d *controllerService) CreateSnapshot(ctx context.Context, req *csi.CreateS if len(fsrAvailabilityZones) > 0 { _, err := d.cloud.EnableFastSnapshotRestores(ctx, fsrAvailabilityZones, snapshot.SnapshotID) if err != nil { - if _, err = d.cloud.DeleteSnapshot(ctx, snapshot.SnapshotID); err != nil { - return nil, status.Errorf(codes.Internal, "Could not delete snapshot ID %q: %v", snapshotName, err) + if _, deleteErr := d.cloud.DeleteSnapshot(ctx, snapshot.SnapshotID); deleteErr != nil { + return nil, status.Errorf(codes.Internal, "Could not delete snapshot ID %q: %v", snapshotName, deleteErr) } return nil, status.Errorf(codes.Internal, "Failed to create Fast Snapshot Restores for snapshot ID %q: %v", snapshotName, err) } From 46a2f03803103623c0281b334309d692f97a98cc Mon Sep 17 00:00:00 2001 From: Connor Catlett Date: Wed, 29 Nov 2023 21:37:22 +0000 Subject: [PATCH 2/2] Add e2e snapshot and FSR tests --- hack/kops-patch.yaml | 3 +- tests/e2e/driver/driver.go | 5 +- tests/e2e/driver/ebs_csi_driver.go | 4 +- tests/e2e/requires_aws_api.go | 242 ++++++++++++++++++ ...namically_provisioned_cmd_volume_tester.go | 9 +- ...ally_provisioned_volume_snapshot_tester.go | 15 +- tests/e2e/testsuites/e2e_utils.go | 10 + .../pre_provisioned_snapshot_volume_tester.go | 2 +- tests/e2e/testsuites/specs.go | 4 +- 9 files changed, 280 insertions(+), 14 deletions(-) create mode 100644 tests/e2e/requires_aws_api.go diff --git a/hack/kops-patch.yaml b/hack/kops-patch.yaml index 5608976d57..4c846f09c0 100644 --- a/hack/kops-patch.yaml +++ b/hack/kops-patch.yaml @@ -14,7 +14,8 @@ spec: "ec2:DescribeSnapshots", "ec2:DescribeTags", "ec2:DescribeVolumes", - "ec2:DescribeVolumesModifications" + "ec2:DescribeVolumesModifications", + "ec2:EnableFastSnapshotRestores" ], "Resource": "*" }, diff --git a/tests/e2e/driver/driver.go b/tests/e2e/driver/driver.go index fab1624483..b3faa749c4 100644 --- a/tests/e2e/driver/driver.go +++ b/tests/e2e/driver/driver.go @@ -45,7 +45,7 @@ type PreProvisionedVolumeTestDriver interface { } type VolumeSnapshotTestDriver interface { - GetVolumeSnapshotClass(namespace string) *volumesnapshotv1.VolumeSnapshotClass + GetVolumeSnapshotClass(namespace string, parameters map[string]string) *volumesnapshotv1.VolumeSnapshotClass } func getStorageClass( @@ -80,7 +80,7 @@ func getStorageClass( } } -func getVolumeSnapshotClass(generateName string, provisioner string) *volumesnapshotv1.VolumeSnapshotClass { +func getVolumeSnapshotClass(generateName string, provisioner string, parameters map[string]string) *volumesnapshotv1.VolumeSnapshotClass { return &volumesnapshotv1.VolumeSnapshotClass{ TypeMeta: metav1.TypeMeta{ Kind: VolumeSnapshotClassKind, @@ -91,5 +91,6 @@ func getVolumeSnapshotClass(generateName string, provisioner string) *volumesnap }, Driver: provisioner, DeletionPolicy: volumesnapshotv1.VolumeSnapshotContentDelete, + Parameters: parameters, } } diff --git a/tests/e2e/driver/ebs_csi_driver.go b/tests/e2e/driver/ebs_csi_driver.go index 39a459436d..5f65ef27d1 100644 --- a/tests/e2e/driver/ebs_csi_driver.go +++ b/tests/e2e/driver/ebs_csi_driver.go @@ -62,10 +62,10 @@ func (d *ebsCSIDriver) GetDynamicProvisionStorageClass(parameters map[string]str return getStorageClass(generateName, provisioner, parameters, mountOptions, reclaimPolicy, volumeExpansion, bindingMode, allowedTopologies) } -func (d *ebsCSIDriver) GetVolumeSnapshotClass(namespace string) *volumesnapshotv1.VolumeSnapshotClass { +func (d *ebsCSIDriver) GetVolumeSnapshotClass(namespace string, parameters map[string]string) *volumesnapshotv1.VolumeSnapshotClass { provisioner := d.driverName generateName := fmt.Sprintf("%s-%s-dynamic-sc-", namespace, provisioner) - return getVolumeSnapshotClass(generateName, provisioner) + return getVolumeSnapshotClass(generateName, provisioner, parameters) } func (d *ebsCSIDriver) GetPersistentVolume(volumeID string, fsType string, size string, reclaimPolicy *v1.PersistentVolumeReclaimPolicy, namespace string, accessMode v1.PersistentVolumeAccessMode, volumeMode v1.PersistentVolumeMode) *v1.PersistentVolume { diff --git a/tests/e2e/requires_aws_api.go b/tests/e2e/requires_aws_api.go new file mode 100644 index 0000000000..0407c55a07 --- /dev/null +++ b/tests/e2e/requires_aws_api.go @@ -0,0 +1,242 @@ +/* +Copyright 2023 The Kubernetes Authors. + +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 e2e + +import ( + "fmt" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/session" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/aws/aws-sdk-go/service/ec2/ec2iface" + "github.com/kubernetes-sigs/aws-ebs-csi-driver/tests/e2e/driver" + "github.com/kubernetes-sigs/aws-ebs-csi-driver/tests/e2e/testsuites" + "k8s.io/kubernetes/test/e2e/framework" + + volumesnapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v4/apis/volumesnapshot/v1" + . "github.com/onsi/ginkgo/v2" + v1 "k8s.io/api/core/v1" + clientset "k8s.io/client-go/kubernetes" + restclientset "k8s.io/client-go/rest" + admissionapi "k8s.io/pod-security-admission/api" + + awscloud "github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/cloud" + ebscsidriver "github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/driver" +) + +const testTagName = "testTag" +const testTagValue = "3.1415926" + +func validateEc2Snapshot(ec2Client ec2iface.EC2API, input *ec2.DescribeSnapshotsInput) *ec2.DescribeSnapshotsOutput { + describeResult, err := ec2Client.DescribeSnapshots(input) + if err != nil { + Fail(fmt.Sprintf("failed to describe snapshot: %v", err)) + } + + if len(describeResult.Snapshots) != 1 { + Fail(fmt.Sprintf("expected 1 snapshot, got %d", len(describeResult.Snapshots))) + } + + return describeResult +} + +var _ = Describe("[ebs-csi-e2e] [single-az] [requires-aws-api] Dynamic Provisioning", func() { + f := framework.NewDefaultFramework("ebs") + f.NamespacePodSecurityEnforceLevel = admissionapi.LevelPrivileged + + var ( + cs clientset.Interface + snapshotrcs restclientset.Interface + ns *v1.Namespace + ebsDriver driver.PVTestDriver + ) + + BeforeEach(func() { + cs = f.ClientSet + var err error + snapshotrcs, err = restClient(testsuites.SnapshotAPIGroup, testsuites.APIVersionv1) + if err != nil { + Fail(fmt.Sprintf("could not get rest clientset: %v", err)) + } + ns = f.Namespace + ebsDriver = driver.InitEbsCSIDriver() + }) + + // Tests that require that the e2e runner has access to the AWS API + ec2Client := ec2.New(session.Must(session.NewSession())) + + It("should create a volume with additional tags", func() { + pods := []testsuites.PodDetails{ + { + Cmd: testsuites.PodCmdWriteToVolume("/mnt/test-1"), + Volumes: []testsuites.VolumeDetails{ + { + CreateVolumeParameters: map[string]string{ + ebscsidriver.VolumeTypeKey: awscloud.VolumeTypeGP3, + ebscsidriver.FSTypeKey: ebscsidriver.FSTypeExt4, + ebscsidriver.TagKeyPrefix: fmt.Sprintf("%s=%s", testTagName, testTagValue), + }, + ClaimSize: driver.MinimumSizeForVolumeType(awscloud.VolumeTypeGP3), + VolumeMount: testsuites.DefaultGeneratedVolumeMount, + }, + }, + }, + } + test := testsuites.DynamicallyProvisionedCmdVolumeTest{ + CSIDriver: ebsDriver, + Pods: pods, + ValidateFunc: func() { + result, err := ec2Client.DescribeVolumes(&ec2.DescribeVolumesInput{ + Filters: []*ec2.Filter{ + { + Name: aws.String("tag:" + testTagName), + Values: []*string{aws.String(testTagValue)}, + }, + }, + }) + if err != nil { + Fail(fmt.Sprintf("failed to describe volume: %v", err)) + } + + if len(result.Volumes) != 1 { + Fail(fmt.Sprintf("expected 1 volume, got %d", len(result.Volumes))) + } + }, + } + test.Run(cs, ns) + }) + + It("should create a snapshot with additional tags", func() { + pod := testsuites.PodDetails{ + Cmd: testsuites.PodCmdWriteToVolume("/mnt/test-1"), + Volumes: []testsuites.VolumeDetails{ + { + CreateVolumeParameters: map[string]string{ + ebscsidriver.VolumeTypeKey: awscloud.VolumeTypeGP3, + ebscsidriver.FSTypeKey: ebscsidriver.FSTypeExt4, + }, + ClaimSize: driver.MinimumSizeForVolumeType(awscloud.VolumeTypeGP3), + VolumeMount: testsuites.DefaultGeneratedVolumeMount, + }, + }, + } + restoredPod := testsuites.PodDetails{ + Cmd: testsuites.PodCmdGrepVolumeData("/mnt/test-1"), + Volumes: []testsuites.VolumeDetails{ + { + CreateVolumeParameters: map[string]string{ + ebscsidriver.VolumeTypeKey: awscloud.VolumeTypeGP3, + ebscsidriver.FSTypeKey: ebscsidriver.FSTypeExt4, + }, + ClaimSize: driver.MinimumSizeForVolumeType(awscloud.VolumeTypeGP3), + VolumeMount: testsuites.DefaultGeneratedVolumeMount, + }, + }, + } + test := testsuites.DynamicallyProvisionedVolumeSnapshotTest{ + CSIDriver: ebsDriver, + Pod: pod, + RestoredPod: restoredPod, + Parameters: map[string]string{ + ebscsidriver.TagKeyPrefix: fmt.Sprintf("%s=%s", testTagName, testTagValue), + }, + ValidateFunc: func(_ *volumesnapshotv1.VolumeSnapshot) { + validateEc2Snapshot(ec2Client, &ec2.DescribeSnapshotsInput{ + Filters: []*ec2.Filter{ + { + Name: aws.String("tag:" + testTagName), + Values: []*string{aws.String(testTagValue)}, + }, + }, + }) + }, + } + test.Run(cs, snapshotrcs, ns) + }) + + It("should create a snapshot with FSR enabled", func() { + azList, err := ec2Client.DescribeAvailabilityZones(&ec2.DescribeAvailabilityZonesInput{}) + if err != nil { + Fail(fmt.Sprintf("failed to list AZs: %v", err)) + } + fsrAvailabilityZone := *azList.AvailabilityZones[0].ZoneName + + pod := testsuites.PodDetails{ + Cmd: testsuites.PodCmdWriteToVolume("/mnt/test-1"), + Volumes: []testsuites.VolumeDetails{ + { + CreateVolumeParameters: map[string]string{ + ebscsidriver.VolumeTypeKey: awscloud.VolumeTypeGP3, + ebscsidriver.FSTypeKey: ebscsidriver.FSTypeExt4, + }, + ClaimSize: driver.MinimumSizeForVolumeType(awscloud.VolumeTypeGP3), + VolumeMount: testsuites.DefaultGeneratedVolumeMount, + }, + }, + } + restoredPod := testsuites.PodDetails{ + Cmd: testsuites.PodCmdGrepVolumeData("/mnt/test-1"), + Volumes: []testsuites.VolumeDetails{ + { + CreateVolumeParameters: map[string]string{ + ebscsidriver.VolumeTypeKey: awscloud.VolumeTypeGP3, + ebscsidriver.FSTypeKey: ebscsidriver.FSTypeExt4, + }, + ClaimSize: driver.MinimumSizeForVolumeType(awscloud.VolumeTypeGP3), + VolumeMount: testsuites.DefaultGeneratedVolumeMount, + }, + }, + } + test := testsuites.DynamicallyProvisionedVolumeSnapshotTest{ + CSIDriver: ebsDriver, + Pod: pod, + RestoredPod: restoredPod, + Parameters: map[string]string{ + ebscsidriver.FastSnapshotRestoreAvailabilityZones: fsrAvailabilityZone, + }, + ValidateFunc: func(snapshot *volumesnapshotv1.VolumeSnapshot) { + describeResult := validateEc2Snapshot(ec2Client, &ec2.DescribeSnapshotsInput{ + Filters: []*ec2.Filter{ + { + Name: aws.String("tag:" + awscloud.SnapshotNameTagKey), + Values: []*string{aws.String("snapshot-" + string(snapshot.UID))}, + }, + }, + }) + + result, err := ec2Client.DescribeFastSnapshotRestores(&ec2.DescribeFastSnapshotRestoresInput{ + Filters: []*ec2.Filter{ + { + Name: aws.String("snapshot-id"), + Values: []*string{describeResult.Snapshots[0].SnapshotId}, + }, + }, + }) + if err != nil { + Fail(fmt.Sprintf("failed to list AZs: %v", err)) + } + + if len(result.FastSnapshotRestores) != 1 { + Fail(fmt.Sprintf("expected 1 FSR, got %d", len(result.FastSnapshotRestores))) + } + + if *result.FastSnapshotRestores[0].AvailabilityZone != fsrAvailabilityZone { + Fail(fmt.Sprintf("expected FSR to be enabled for %s, got %s", fsrAvailabilityZone, *result.FastSnapshotRestores[0].AvailabilityZone)) + } + }, + } + test.Run(cs, snapshotrcs, ns) + }) +}) diff --git a/tests/e2e/testsuites/dynamically_provisioned_cmd_volume_tester.go b/tests/e2e/testsuites/dynamically_provisioned_cmd_volume_tester.go index c72e56bd64..49f69ed004 100644 --- a/tests/e2e/testsuites/dynamically_provisioned_cmd_volume_tester.go +++ b/tests/e2e/testsuites/dynamically_provisioned_cmd_volume_tester.go @@ -26,8 +26,9 @@ import ( // Waiting for the PV provisioner to create a new PV // Testing if the Pod(s) Cmd is run with a 0 exit code type DynamicallyProvisionedCmdVolumeTest struct { - CSIDriver driver.DynamicPVTestDriver - Pods []PodDetails + CSIDriver driver.DynamicPVTestDriver + Pods []PodDetails + ValidateFunc func() } func (t *DynamicallyProvisionedCmdVolumeTest) Run(client clientset.Interface, namespace *v1.Namespace) { @@ -44,4 +45,8 @@ func (t *DynamicallyProvisionedCmdVolumeTest) Run(client clientset.Interface, na By("checking that the pods command exits with no error") tpod.WaitForSuccess() } + + if t.ValidateFunc != nil { + t.ValidateFunc() + } } diff --git a/tests/e2e/testsuites/dynamically_provisioned_volume_snapshot_tester.go b/tests/e2e/testsuites/dynamically_provisioned_volume_snapshot_tester.go index df6615ca36..ef2b3f7c5a 100644 --- a/tests/e2e/testsuites/dynamically_provisioned_volume_snapshot_tester.go +++ b/tests/e2e/testsuites/dynamically_provisioned_volume_snapshot_tester.go @@ -17,6 +17,7 @@ package testsuites import ( "github.com/kubernetes-sigs/aws-ebs-csi-driver/tests/e2e/driver" + volumesnapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v4/apis/volumesnapshot/v1" v1 "k8s.io/api/core/v1" clientset "k8s.io/client-go/kubernetes" restclientset "k8s.io/client-go/rest" @@ -31,9 +32,11 @@ import ( // And finally delete the snapshot // This test only supports a single volume type DynamicallyProvisionedVolumeSnapshotTest struct { - CSIDriver driver.PVTestDriver - Pod PodDetails - RestoredPod PodDetails + CSIDriver driver.PVTestDriver + Pod PodDetails + RestoredPod PodDetails + Parameters map[string]string + ValidateFunc func(*volumesnapshotv1.VolumeSnapshot) } func (t *DynamicallyProvisionedVolumeSnapshotTest) Run(client clientset.Interface, restclient restclientset.Interface, namespace *v1.Namespace) { @@ -52,7 +55,7 @@ func (t *DynamicallyProvisionedVolumeSnapshotTest) Run(client clientset.Interfac tpod.WaitForSuccess() By("taking snapshots") - tvsc, cleanup := CreateVolumeSnapshotClass(restclient, namespace, t.CSIDriver) + tvsc, cleanup := CreateVolumeSnapshotClass(restclient, namespace, t.CSIDriver, t.Parameters) defer cleanup() snapshot := tvsc.CreateSnapshot(tpvc.persistentVolumeClaim) @@ -73,4 +76,8 @@ func (t *DynamicallyProvisionedVolumeSnapshotTest) Run(client clientset.Interfac defer trpod.Cleanup() By("checking that the pods command exits with no error") trpod.WaitForSuccess() + + if t.ValidateFunc != nil { + t.ValidateFunc(snapshot) + } } diff --git a/tests/e2e/testsuites/e2e_utils.go b/tests/e2e/testsuites/e2e_utils.go index 4fbf7792f9..eb121e5b22 100644 --- a/tests/e2e/testsuites/e2e_utils.go +++ b/tests/e2e/testsuites/e2e_utils.go @@ -43,6 +43,11 @@ const ( AnnotationVolumeType = "ebs.csi.aws.com/volumeType" ) +var DefaultGeneratedVolumeMount = VolumeMountDetails{ + NameGenerate: "test-volume-", + MountPathGenerate: "/mnt/test-", +} + // PodCmdWriteToVolume returns pod command that would write to mounted volume func PodCmdWriteToVolume(volumeMountPath string) string { return fmt.Sprintf("echo 'hello world' >> %s/data && grep 'hello world' %s/data && sync", volumeMountPath, volumeMountPath) @@ -53,6 +58,11 @@ func PodCmdContinuousWrite(volumeMountPath string) string { return fmt.Sprintf("while true; do echo \"$(date -u)\" >> /%s/out.txt; sleep 5; done", volumeMountPath) } +// PodCmdGrepVolumeData returns pod command that would check that a volume was written to by PodCmdWriteToVolume +func PodCmdGrepVolumeData(volumeMountPath string) string { + return fmt.Sprintf("grep 'hello world' %s/data", volumeMountPath) +} + // IncreasePvcObjectStorage increases `storage` of a K8s PVC object by specified Gigabytes func IncreasePvcObjectStorage(pvc *v1.PersistentVolumeClaim, sizeIncreaseGi int64) resource.Quantity { pvcSize := pvc.Spec.Resources.Requests["storage"] diff --git a/tests/e2e/testsuites/pre_provisioned_snapshot_volume_tester.go b/tests/e2e/testsuites/pre_provisioned_snapshot_volume_tester.go index b4d2af45f2..81f10cd738 100644 --- a/tests/e2e/testsuites/pre_provisioned_snapshot_volume_tester.go +++ b/tests/e2e/testsuites/pre_provisioned_snapshot_volume_tester.go @@ -35,7 +35,7 @@ type PreProvisionedVolumeSnapshotTest struct { func (t *PreProvisionedVolumeSnapshotTest) Run(client clientset.Interface, restclient k8srestclient.Interface, namespace *v1.Namespace, snapshotId string) { By("taking snapshots") - tvsc, cleanup := CreateVolumeSnapshotClass(restclient, namespace, t.CSIDriver) + tvsc, cleanup := CreateVolumeSnapshotClass(restclient, namespace, t.CSIDriver, nil) defer cleanup() tvolumeSnapshotContent := tvsc.CreateStaticVolumeSnapshotContent(snapshotId) diff --git a/tests/e2e/testsuites/specs.go b/tests/e2e/testsuites/specs.go index 57d5021d1f..20a7d679a0 100644 --- a/tests/e2e/testsuites/specs.go +++ b/tests/e2e/testsuites/specs.go @@ -187,9 +187,9 @@ func (volume *VolumeDetails) SetupPreProvisionedPersistentVolumeClaim(client cli return tpvc, cleanupFuncs } -func CreateVolumeSnapshotClass(client restclientset.Interface, namespace *v1.Namespace, csiDriver driver.VolumeSnapshotTestDriver) (*TestVolumeSnapshotClass, func()) { +func CreateVolumeSnapshotClass(client restclientset.Interface, namespace *v1.Namespace, csiDriver driver.VolumeSnapshotTestDriver, vscParameters map[string]string) (*TestVolumeSnapshotClass, func()) { By("setting up the VolumeSnapshotClass") - volumeSnapshotClass := csiDriver.GetVolumeSnapshotClass(namespace.Name) + volumeSnapshotClass := csiDriver.GetVolumeSnapshotClass(namespace.Name, vscParameters) tvsc := NewTestVolumeSnapshotClass(client, namespace, volumeSnapshotClass) tvsc.Create()