Skip to content

Commit

Permalink
Use existing 6MiB file for S3 multipart copy test on Azure ADLS2 (#7609)
Browse files Browse the repository at this point in the history
* Use existing 6MiB file for S3 multipart copy test on Azure ADLS2

Fixes #7485: it just takes too long to upload some _new_ data there.

* [bug] Use correct path for existing 6 MiB object on ADLS gen2
  • Loading branch information
arielshaqed authored Mar 28, 2024
1 parent bf9ff62 commit 0a916a4
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 14 deletions.
1 change: 1 addition & 0 deletions .github/workflows/esti.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -956,6 +956,7 @@ jobs:
LAKEFS_BLOCKSTORE_TYPE: azure
ESTI_BLOCKSTORE_TYPE: azure
ESTI_STORAGE_NAMESPACE: https://esti4hns.blob.core.windows.net/esti-system-testing/${{ github.run_number }}/${{ steps.unique.outputs.value }}
ESTI_LARGE_OBJECT_PATH: https://esti4hns.blob.core.windows.net/esti-system-testing-data/copy-test-data/data.6mib
ESTI_AZURE_STORAGE_ACCOUNT: esti4hns
ESTI_AZURE_STORAGE_ACCESS_KEY: ${{ secrets.LAKEFS_BLOCKSTORE_AZURE_STORAGE_GEN2_ACCESS_KEY }}

Expand Down
1 change: 1 addition & 0 deletions esti/ops/docker-compose-external-db.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ services:
- ESTI_LAKECTL_DIR=/app
- ESTI_GOTEST_FLAGS
- ESTI_FLAGS
- ESTI_LARGE_OBJECT_PATH
- ESTI_FORCE_PATH_STYLE
- ESTI_AZURE_STORAGE_ACCOUNT
- ESTI_AZURE_STORAGE_ACCESS_KEY
Expand Down
58 changes: 44 additions & 14 deletions esti/s3_gateway_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package esti

import (
"bytes"
"context"
"fmt"
"io"
"math/rand"
"net/http"
Expand All @@ -28,7 +30,8 @@ type GetCredentials = func(id, secret, token string) *credentials.Credentials
const (
numUploads = 100
randomDataPathLength = 1020
gatewayTestPrefix = "main/data/"
branch = "main"
gatewayTestPrefix = branch + "/data/"
)

func newMinioClient(t *testing.T, getCredentials GetCredentials) *minio.Client {
Expand Down Expand Up @@ -329,6 +332,40 @@ func TestS3HeadBucket(t *testing.T) {
})
}

// getOrCreatePathToLargeObject returns a configured existing large
// (largeDataContentLength, 6MiB) object, or creates a new one under
// testPrefix.
func getOrCreatePathToLargeObject(t *testing.T, ctx context.Context, s3lakefsClient *minio.Client, repo, branch string) (string, int64) {
t.Helper()

path := "source-file"
s3Path := fmt.Sprintf("%s/source-file", branch)

if physicalAddress := viper.GetString("large_object_path"); physicalAddress != "" {
res, err := client.LinkPhysicalAddressWithResponse(ctx, repo, branch, &apigen.LinkPhysicalAddressParams{Path: path}, apigen.LinkPhysicalAddressJSONRequestBody{
Checksum: "dont-care",
// TODO(ariels): Check actual length of object!
SizeBytes: largeDataContentLength,
Staging: apigen.StagingLocation{
PhysicalAddress: &physicalAddress,
},
})
require.NoError(t, err)
require.NotNil(t, res.JSON200)
return s3Path, largeDataContentLength
}

// content
r := rand.New(rand.NewSource(17))
objContent := testutil.NewRandomReader(r, largeDataContentLength)

// upload data
_, err := s3lakefsClient.PutObject(ctx, repo, s3Path, objContent, largeDataContentLength,
minio.PutObjectOptions{})
require.NoError(t, err)
return s3Path, largeDataContentLength
}

func TestS3CopyObjectMultipart(t *testing.T) {
ctx, _, repo := setupTest(t)
defer tearDownTest(repo)
Expand All @@ -338,17 +375,10 @@ func TestS3CopyObjectMultipart(t *testing.T) {
destRepo := createRepositoryByName(ctx, t, destRepoName)
defer deleteRepositoryIfAskedTo(ctx, destRepoName)

// content
r := rand.New(rand.NewSource(17))
objContent := testutil.NewRandomReader(r, largeDataContentLength)
srcPath := gatewayTestPrefix + "source-file"
destPath := gatewayTestPrefix + "dest-file"

// upload data
s3lakefsClient := newMinioClient(t, credentials.NewStaticV4)
_, err := s3lakefsClient.PutObject(ctx, repo, srcPath, objContent, largeDataContentLength,
minio.PutObjectOptions{})
require.NoError(t, err)

srcPath, objectLength := getOrCreatePathToLargeObject(t, ctx, s3lakefsClient, repo, branch)
destPath := gatewayTestPrefix + "dest-file"

dest := minio.CopyDestOptions{
Bucket: destRepo,
Expand All @@ -367,7 +397,7 @@ func TestS3CopyObjectMultipart(t *testing.T) {
Object: srcPath,
MatchRange: true,
Start: minDataContentLengthForMultipart,
End: largeDataContentLength - 1,
End: objectLength - 1,
},
}

Expand All @@ -376,8 +406,8 @@ func TestS3CopyObjectMultipart(t *testing.T) {
t.Fatalf("minio.Client.ComposeObject from(%+v) to(%+v): %s", srcs, dest, err)
}

if ui.Size != largeDataContentLength {
t.Errorf("Copied %d bytes when expecting %d", ui.Size, largeDataContentLength)
if ui.Size != objectLength {
t.Errorf("Copied %d bytes when expecting %d", ui.Size, objectLength)
}

// Comparing 2 readers is too much work. Instead just hash them.
Expand Down
1 change: 1 addition & 0 deletions pkg/testutil/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ func SetupTestingEnv(params *SetupTestingEnvParams) (logging.Logger, apigen.Clie
viper.SetDefault("lakectl_dir", filepath.Join(currDir, ".."))
viper.SetDefault("azure_storage_account", "")
viper.SetDefault("azure_storage_access_key", "")
viper.SetDefault("large_object_path", "")
err = viper.ReadInConfig()
if err != nil && !errors.As(err, &viper.ConfigFileNotFoundError{}) {
logger.WithError(err).Fatal("Failed to read configuration")
Expand Down

0 comments on commit 0a916a4

Please sign in to comment.