Skip to content

Commit

Permalink
Move testing S3 client to its own package (#294)
Browse files Browse the repository at this point in the history
This PR moves S3 client used in tests to its own package for making it
reusable. It also changes the logic for generating testing bucket names.
Previously we were using cluster name as prefix, and also we had a logic
to truncate bucket name to make space for `--azid--x-s3` suffix. That
was causing unique hash in the bucket name to be truncated and different
clusters to use the same name, and it was causing spurious failures such
as
https://github.com/awslabs/mountpoint-s3-csi-driver/actions/runs/11835794931/job/32979499659#step:10:2599.

---

By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice.

---------

Signed-off-by: Burak Varlı <[email protected]>
  • Loading branch information
unexge authored Nov 15, 2024
1 parent fe0917c commit e158baf
Show file tree
Hide file tree
Showing 3 changed files with 186 additions and 87 deletions.
3 changes: 3 additions & 0 deletions tests/e2e-kubernetes/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"flag"
"testing"

"github.com/awslabs/aws-s3-csi-driver/tests/e2e-kubernetes/s3client"
custom_testsuites "github.com/awslabs/aws-s3-csi-driver/tests/e2e-kubernetes/testsuites"

ginkgo "github.com/onsi/ginkgo/v2"
Expand All @@ -25,6 +26,8 @@ func init() {
flag.StringVar(&BucketPrefix, "bucket-prefix", "local", "prefix for temporary buckets")
flag.BoolVar(&Performance, "performance", false, "run performance tests")
flag.Parse()

s3client.DefaultRegion = BucketRegion
}

func TestE2E(t *testing.T) {
Expand Down
165 changes: 165 additions & 0 deletions tests/e2e-kubernetes/s3client/client.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
// Package s3client provides an Amazon S3 client to be used in tests for creating and deleting Amazon S3 buckets.
package s3client

import (
"context"
"fmt"
"strings"

"github.com/aws/aws-sdk-go-v2/aws"
"github.com/aws/aws-sdk-go-v2/config"
"github.com/aws/aws-sdk-go-v2/service/s3"
"github.com/aws/aws-sdk-go-v2/service/s3/types"
utilrand "k8s.io/apimachinery/pkg/util/rand"
"k8s.io/kubernetes/test/e2e/framework"
)

// DefaultRegion is the default AWS region to use if unspecified.
// It is public in order to be modified from test binary which receives region to use as a flag.
var DefaultRegion string

// See https://docs.aws.amazon.com/AmazonS3/latest/userguide/s3-express-networking.html#s3-express-endpoints
var expressAZs = map[string]string{
"us-east-1": "use1-az4",
"us-west-2": "usw2-az1",
"eu-north-1": "eun1-az1",
}

const s3BucketNameMaxLength = 63
const s3BucketNamePrefix = "s3-csi-k8s-e2e-"

// DeleteBucketFunc is a cleanup function thats returned as a result of "Create*Bucket" calls.
// It clears the content of the bucket if not empty, and then deletes the bucket.
type DeleteBucketFunc func(context.Context) error

// A Client is an S3 client for creating an deleting S3 buckets.
type Client struct {
region string
client *s3.Client
}

// New returns a new client with "DefaultRegion".
func New() *Client {
return NewWithRegion(DefaultRegion)
}

// NewWithRegion returns a new client with the given `region`.
func NewWithRegion(region string) *Client {
cfg, err := config.LoadDefaultConfig(context.Background(),
config.WithRegion(region),
)
framework.ExpectNoError(err)
return &Client{region: region, client: s3.NewFromConfig(cfg)}
}

// CreateStandardBucket creates a new standard S3 bucket with a random name,
// and returns the bucket name and a clean up function.
func (c *Client) CreateStandardBucket(ctx context.Context) (string, DeleteBucketFunc) {
bucketName := c.randomBucketName("")

input := &s3.CreateBucketInput{
Bucket: aws.String(bucketName),
}

if c.region != "us-east-1" {
input.CreateBucketConfiguration = &types.CreateBucketConfiguration{
LocationConstraint: types.BucketLocationConstraint(c.region),
}
}

return bucketName, c.create(ctx, input)
}

// CreateDirectoryBucket creates a new directory S3 bucket with a random name (by following
// "Directory bucket naming rules") and returns the bucket name and a clean up function.
func (c *Client) CreateDirectoryBucket(ctx context.Context) (string, DeleteBucketFunc) {
regionAz := expressAZs[c.region]
if regionAz == "" {
framework.Failf("Unknown S3 Express region %s\n", c.region)
}

// refer to s3 express bucket naming conventions
// https://docs.aws.amazon.com/AmazonS3/latest/userguide/directory-bucket-naming-rules.html
suffix := fmt.Sprintf("--%s--x-s3", regionAz)
bucketName := c.randomBucketName(suffix)

// s3 express doesn't allow non-virtually routable names
bucketName = strings.Replace(bucketName, ".", "", -1)

return bucketName, c.create(ctx, &s3.CreateBucketInput{
Bucket: aws.String(bucketName),
CreateBucketConfiguration: &types.CreateBucketConfiguration{
Location: &types.LocationInfo{
Name: aws.String(regionAz),
Type: types.LocationTypeAvailabilityZone,
},
Bucket: &types.BucketInfo{
DataRedundancy: types.DataRedundancySingleAvailabilityZone,
Type: types.BucketTypeDirectory,
},
},
})
}

func (c *Client) create(ctx context.Context, input *s3.CreateBucketInput) DeleteBucketFunc {
bucketName := *input.Bucket

_, err := c.client.CreateBucket(ctx, input)
framework.ExpectNoError(err, "Failed to create S3 bucket")
if err == nil {
framework.Logf("S3 Bucket %s created", bucketName)
}

return func(ctx context.Context) error {
return c.delete(ctx, bucketName)
}
}

func (c *Client) delete(ctx context.Context, bucketName string) error {
framework.Logf("Deleting S3 Bucket %s...", bucketName)

objects, err := c.client.ListObjectsV2(ctx, &s3.ListObjectsV2Input{
Bucket: aws.String(bucketName),
})
if err != nil {
return err
}

var objectIds []types.ObjectIdentifier
// get all object keys in the s3 bucket
for _, obj := range objects.Contents {
objectIds = append(objectIds, types.ObjectIdentifier{Key: obj.Key})
}

// delete all objects from the bucket
if len(objectIds) > 0 {
_, err = c.client.DeleteObjects(ctx, &s3.DeleteObjectsInput{
Bucket: aws.String(bucketName),
Delete: &types.Delete{Objects: objectIds},
})
if err != nil {
return err
}
}

// finally delete the bucket
_, err = c.client.DeleteBucket(ctx, &s3.DeleteBucketInput{
Bucket: aws.String(bucketName),
})
if err != nil {
return err
}

framework.Logf("S3 Bucket %s deleted", bucketName)

return nil
}

// randomBucketName generates a random bucket name by using prefix (`s3BucketNamePrefix`) and `suffix`
// and generating random string for the remaining space according to S3's limit (63 as of today).
func (c *Client) randomBucketName(suffix string) string {
prefixLen := len(s3BucketNamePrefix)
suffixLen := len(suffix)
rand := utilrand.String(s3BucketNameMaxLength - prefixLen - suffixLen)
return s3BucketNamePrefix + rand + suffix
}
105 changes: 18 additions & 87 deletions tests/e2e-kubernetes/testdriver.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,17 @@ package e2e

import (
"context"
"fmt"
"strings"

"github.com/aws/aws-sdk-go-v2/aws"
"github.com/aws/aws-sdk-go-v2/config"
"github.com/aws/aws-sdk-go-v2/service/s3"
"github.com/aws/aws-sdk-go-v2/service/s3/types"
"github.com/awslabs/aws-s3-csi-driver/tests/e2e-kubernetes/s3client"
custom_testsuites "github.com/awslabs/aws-s3-csi-driver/tests/e2e-kubernetes/testsuites"

v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apiserver/pkg/storage/names"
f "k8s.io/kubernetes/test/e2e/framework"
e2eskipper "k8s.io/kubernetes/test/e2e/framework/skipper"
"k8s.io/kubernetes/test/e2e/storage/framework"
)

const (
maxS3ExpressBucketNameLength = 63
)

var (
CommitId string
BucketRegion string // assumed to be the same as k8s cluster's region
Expand All @@ -31,11 +21,13 @@ var (
)

type s3Driver struct {
client *s3client.Client
driverInfo framework.DriverInfo
}

type s3Volume struct {
bucketName string
deleteBucket s3client.DeleteBucketFunc
authenticationSource string
}

Expand All @@ -45,6 +37,7 @@ var _ framework.PreprovisionedPVTestDriver = &s3Driver{}

func initS3Driver() *s3Driver {
return &s3Driver{
client: s3client.New(),
driverInfo: framework.DriverInfo{
Name: "s3.csi.aws.com",
MaxFileSize: framework.FileSizeLarge,
Expand Down Expand Up @@ -86,59 +79,28 @@ func (d *s3Driver) CreateVolume(ctx context.Context, config *framework.PerTestCo
if volumeType != framework.PreprovisionedPV {
f.Failf("Unsupported volType: %v is specified", volumeType)
}
bucketName := names.SimpleNameGenerator.GenerateName(fmt.Sprintf("%s-e2e-kubernetes-%s-", BucketPrefix, CommitId))
input := &s3.CreateBucketInput{
Bucket: aws.String(bucketName),
}
if BucketRegion != "us-east-1" {
input.CreateBucketConfiguration = &types.CreateBucketConfiguration{
LocationConstraint: types.BucketLocationConstraint(BucketRegion),
}
}

var bucketName string
var deleteBucket s3client.DeleteBucketFunc
if config.Prefix == custom_testsuites.S3ExpressTestIdentifier {
// assume us-east-1 since that's where our integration tests currently do their work
// https://docs.aws.amazon.com/AmazonS3/latest/userguide/s3-express-networking.html
regionAz := "use1-az4"
if BucketRegion == "us-west-2" {
regionAz = "usw2-az1"
}
// refer to s3 express bucket naming conventions
// https://docs.aws.amazon.com/AmazonS3/latest/userguide/directory-bucket-naming-rules.html
suffix := fmt.Sprintf("--%s--x-s3", regionAz)
// s3 express doesn't allow non-virtually routable names
bucketName = strings.Replace(bucketName, ".", "", -1)
if len(bucketName)+len(suffix) > maxS3ExpressBucketNameLength {
bucketName = strings.TrimRight(bucketName[:maxS3ExpressBucketNameLength-len(suffix)], "-")
}
bucketName = fmt.Sprintf("%s%s", bucketName, suffix)
input = &s3.CreateBucketInput{
Bucket: aws.String(bucketName),
CreateBucketConfiguration: &types.CreateBucketConfiguration{
Location: &types.LocationInfo{
Name: aws.String(regionAz),
Type: types.LocationTypeAvailabilityZone,
},
Bucket: &types.BucketInfo{
DataRedundancy: types.DataRedundancySingleAvailabilityZone,
Type: types.BucketTypeDirectory,
},
},
}
bucketName, deleteBucket = d.client.CreateDirectoryBucket(ctx)
} else {
bucketName, deleteBucket = d.client.CreateStandardBucket(ctx)
}

return &s3Volume{
bucketName: bucketName,
deleteBucket: deleteBucket,
authenticationSource: custom_testsuites.AuthenticationSourceFromContext(ctx),
}
f.Logf("Attempting to create bucket: %s", bucketName)
_, err := newS3Client().CreateBucket(ctx, input)
f.ExpectNoError(err)
f.Logf("Created bucket: %s", bucketName)
return &s3Volume{bucketName: bucketName, authenticationSource: custom_testsuites.AuthenticationSourceFromContext(ctx)}
}

func (d *s3Driver) GetPersistentVolumeSource(readOnly bool, fsType string, testVolume framework.TestVolume) (*v1.PersistentVolumeSource, *v1.VolumeNodeAffinity) {
volume, _ := testVolume.(*s3Volume)

volumeAttributes := map[string]string{"bucketName": volume.bucketName}
if volume.authenticationSource != "" {
f.Logf("Using authencation source %s for volume", volume.authenticationSource)
f.Logf("Using authentication source %s for volume", volume.authenticationSource)
volumeAttributes["authenticationSource"] = volume.authenticationSource
}

Expand All @@ -152,37 +114,6 @@ func (d *s3Driver) GetPersistentVolumeSource(readOnly bool, fsType string, testV
}

func (v *s3Volume) DeleteVolume(ctx context.Context) {
s3Client := newS3Client()
objects, err := s3Client.ListObjectsV2(context.TODO(), &s3.ListObjectsV2Input{
Bucket: aws.String(v.bucketName),
})
f.ExpectNoError(err)
var objectIds []types.ObjectIdentifier
// get all object keys in the s3 bucket
for _, obj := range objects.Contents {
objectIds = append(objectIds, types.ObjectIdentifier{Key: obj.Key})
}
// delete all objects from the bucket
if len(objectIds) > 0 {
_, err = s3Client.DeleteObjects(context.TODO(), &s3.DeleteObjectsInput{
Bucket: aws.String(v.bucketName),
Delete: &types.Delete{Objects: objectIds},
})
f.ExpectNoError(err)
}
// finally delete the bucket
input := &s3.DeleteBucketInput{
Bucket: aws.String(v.bucketName),
}
_, err = s3Client.DeleteBucket(context.TODO(), input)
f.ExpectNoError(err)
f.Logf("Deleted bucket: %s", v.bucketName)
}

func newS3Client() *s3.Client {
cfg, err := config.LoadDefaultConfig(context.TODO(),
config.WithRegion(BucketRegion),
)
f.ExpectNoError(err)
return s3.NewFromConfig(cfg)
err := v.deleteBucket(ctx)
f.ExpectNoError(err, "Failed to delete S3 Bucket: %s", v.bucketName)
}

0 comments on commit e158baf

Please sign in to comment.