Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Azure Gateway to azure-storage-blob SDK #8537

Merged
merged 11 commits into from Dec 2, 2019

Conversation

@c-w
Copy link
Contributor

c-w commented Nov 19, 2019

Description

This change updates the MinIO Azure Gateway to use the new Azure Storage SDK: azure-storage-blob-go/azblob.

Motivation and Context

The currently used Azure Storage SDK azure-sdk-for-go/storage has been in maintenance-only mode since February 2018 and will be deprecated in the future.

Additionally, there are several limitations with the SDK, including lack of support for containerized storage deployments like Azurite or Azure IoT Edge storage. These limitations currently make it hard to work with the MinIO Azure Gateway in disconnected scenarios, e.g. for local development or local integration testing.

Besides migrating away from an SDK that is no longer actively developed and adding support for containerized storage deployment backends, this change also enables us to in many cases combine previously separate PutBlob, SetMetadata and SetProperties requests into a single call which should slightly improve performance.

How to test this PR?

First, build the code from this change into a Docker image, e.g. via TAG=minio/minio make docker. Then, execute the mint tests against the updated Azure gateway using Azurite as the backend storage implementation and observe that all the tests pass:

MINIO_ACCESS_KEY="devstoreaccount1"
MINIO_SECRET_KEY="Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw=="

docker network create minio

docker run -d --rm --network minio --name azurite mcr.microsoft.com/azure-storage/azurite azurite-blob --blobHost 0.0.0.0

docker run -d --rm --network minio --name minio -e MINIO_ACCESS_KEY=$MINIO_ACCESS_KEY -e MINIO_SECRET_KEY=$MINIO_SECRET_KEY minio/minio gateway azure http://azurite:10000

docker run --network minio -e SERVER_ENDPOINT=minio:9000 -e ACCESS_KEY=$MINIO_ACCESS_KEY -e SECRET_KEY=$MINIO_SECRET_KEY -e ENABLE_HTTPS=0 minio/mint

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • Fixes a regression
  • Documentation needed
  • Unit tests needed
  • Functional tests needed
@kannappanr kannappanr requested review from Praveenrajmani and vadmeste Nov 19, 2019
@harshavardhana

This comment has been minimized.

Copy link
Member

harshavardhana commented Nov 21, 2019

Can you resolve the conflicts @c-w ? thanks

@c-w

This comment has been minimized.

Copy link
Contributor Author

c-w commented Nov 21, 2019

@harshavardhana: rebased onto master and fixed the merge conflicts.

@harshavardhana

This comment has been minimized.

Copy link
Member

harshavardhana commented Nov 22, 2019

Copy link
Member

vadmeste left a comment

Thanks for the PR!

Some comments before extensive testing.

cmd/gateway/azure/gateway-azure.go Outdated Show resolved Hide resolved
cmd/gateway/azure/gateway-azure.go Outdated Show resolved Hide resolved
cmd/gateway/azure/gateway-azure.go Show resolved Hide resolved
cmd/gateway/azure/gateway-azure.go Show resolved Hide resolved
c-w added 3 commits Nov 15, 2019
The azure-sdk-for-go/storage package has been in maintenance-only mode
since February 2018 (see [1]) and will be deprecated in the future.
Additionally, there are several limitations with the storage SDK,
including lack of support for containerized storage deployments like
Azurite [2] or Azure IoT Edge storage [3]. These limitations currently
make it hard to work with the MinIO Azure Gateway in disconnected
scenarios, e.g. for local development or local integration testing.

As such, this change updates the MinIO Azure Gateway to use the new and
recommended Azure Storage SDK: azure-storage-blob-go/azblob [4]. Besides
migrating away from a package that is no longer actively developed and
adding support for containerized storage deployment backends, this new
SDK also enables us to in many cases combine previously separate
PutBlob, SetMetadata and SetProperties requests into a single call which
should slightly improve performance.

[1] Azure/azure-sdk-for-go@fa0a4d1
[2] https://github.com/Azure/Azurite
[3] https://docs.microsoft.com/en-us/azure/iot-edge/how-to-store-data-blob
[4] https://github.com/Azure/azure-storage-blob-go
@harshavardhana

This comment has been minimized.

Copy link
Member

harshavardhana commented Nov 22, 2019

To validate all these changes please also run docker run minio/mint:edge documentation is here https://github.com/minio/minio/tree/master/mint

@c-w

This comment has been minimized.

Copy link
Contributor Author

c-w commented Nov 22, 2019

@harshavardhana Thanks for letting me know that I should run mint with the edge tag. I previously only ran mint:latest and those tests passed (see screenshot I linked from the pull request description).

I noticed that when I run mint:edge against the latest published minio docker image I get a failure in the aws-cli tests:

docker rmi minio/minio:latest
docker pull minio/minio:latest

MINIO_ACCESS_KEY=changeme

az group create -l eastus -n $MINIO_ACCESS_KEY
az storage account create -l eastus --sku Standard_LRS --kind StorageV2 -n $MINIO_ACCESS_KEY -g $MINIO_ACCESS_KEY
MINIO_SECRET_KEY=$(az storage account keys list -n $MINIO_ACCESS_KEY -o tsv --query [0].value)

docker network create minio

docker run -d --rm --network minio --name minio -e MINIO_ACCESS_KEY=$MINIO_ACCESS_KEY -e MINIO_SECRET_KEY=$MINIO_SECRET_KEY minio/minio:latest gateway azure

docker run --network minio -e SERVER_ENDPOINT=minio:9000 -e ACCESS_KEY=$MINIO_ACCESS_KEY -e SECRET_KEY=$MINIO_SECRET_KEY minio/mint:edge

Screenshot showing minio/mint:edge failure with minio/minio:latest

The docker logs show the following:

Screenshot showing docker logs for aws-cli test failure

Can you reproduce this failure on your end or is there something that I'm doing wrong?

@c-w c-w requested review from vadmeste and harshavardhana Nov 22, 2019
@harshavardhana

This comment has been minimized.

Copy link
Member

harshavardhana commented Nov 22, 2019

Also, make sure to set MINT_MODE=full to run more / bigger tests @c-w

@harshavardhana

This comment has been minimized.

Copy link
Member

harshavardhana commented Nov 22, 2019

The docker logs show the following:

Screenshot showing docker logs for aws-cli test failure

Can you reproduce this failure on your end or is there something that I'm doing wrong?

This looks like PutObjectPart code needs to be fixed, its missing content-length header @c-w

@c-w

This comment has been minimized.

Copy link
Contributor Author

c-w commented Nov 25, 2019

@harshavardhana Thanks for the reply. In my previous comment I mentioned that the error I posted is happening for the latest published MinIO image, not my branch. It looks like this is an existing issue with MinIO. To double check and confirm, I executed the tests on a completely clean VM pulling only minio/minio:latest and minio/mint:edge and was able to reproduce the issue:

Screenshot showing content-length error on latest published MinIO version

I suspect the cause of the issue is this line in master which allows for zero-length blocks to be uploaded (this gets rejected by the Azure API). I have fixed this issue in my branch in 5a6f830, however, I'd hypothesize that this existing error reduces the usefulness of the mint tests in evaluating this pull request since we don't seem to have a stable test suite to compare against. Please advise how you'd like to proceed.

@vadmeste

This comment has been minimized.

Copy link
Member

vadmeste commented Nov 26, 2019

@harshavardhana Thanks for the reply. In my previous comment I mentioned that the error I posted is happening for the latest published MinIO image, not my branch. It looks like this is an existing issue with MinIO. To double check and confirm, I executed the tests on a completely clean VM pulling only minio/minio:latest and minio/mint:edge and was able to reproduce the issue:

Screenshot showing content-length error on latest published MinIO version

I suspect the cause of the issue is this line in master which allows for zero-length blocks to be uploaded (this gets rejected by the Azure API). I have fixed this issue in my branch in 5a6f830, however, I'd hypothesize that this existing error reduces the usefulness of the mint tests in evaluating this pull request since we don't seem to have a stable test suite to compare against. Please advise how you'd like to proceed.

Thanks @c-w, I will take a look with mint fails with the standard MinIO image.

Also I noticed something weird while testing your PR. If you pass wrong credentials, some operations like, creating bucket, putting object won't fail with 403 as it used to be.

$ MINIO_ACCESS_KEY=foobar MINIO_SECRET_KEY=$(echo "foobar12345" | base64) minio gateway azure

Then run this in another terminal (after configuring mc alias)

$ mc mb myazure/testbucket
$ mc cp file myazure/testbucket
@vadmeste

This comment has been minimized.

Copy link
Member

vadmeste commented Nov 26, 2019

@c-w I am not able to reproduce the mint error, can you tell us how did you start your MinIO server container ?

@c-w

This comment has been minimized.

Copy link
Contributor Author

c-w commented Nov 26, 2019

@vadmeste I used the bash snippet in #8537 (comment) to run MinIO and Mint.

@c-w

This comment has been minimized.

Copy link
Contributor Author

c-w commented Nov 26, 2019

Also I noticed something weird while testing your PR. If you pass wrong credentials, some operations like, creating bucket, putting object won't fail with 403 as it used to be.

@vadmeste Great catch. Turns out the case where we get an Azure error from the SDK but don't convert it to a MinIO error wasn't covered by the unit tests so the refactor introduced a regression. I fixed the behavior and also added a test to cover the scenario in 32e94e2.

Screenshot showing 403 error with mc client

Copy link
Member

vadmeste left a comment

New comment

cmd/gateway/azure/gateway-azure.go Outdated Show resolved Hide resolved
@c-w c-w requested a review from vadmeste Dec 1, 2019
@minio-ops

This comment has been minimized.

Copy link

minio-ops commented Dec 1, 2019

Mint Automation

Test Result
mint-xl.sh ✔️
mint-compression-fs.sh ✔️
mint-worm.sh ✔️
mint-fs.sh ✔️
mint-dist-xl.sh ✔️
mint-gateway-nas.sh ✔️
mint-compression-xl.sh more...
mint-large-bucket.sh more...
mint-compression-dist-xl.sh more...

8537-7289ccb/mint-compression-xl.sh.log:

Running with
SERVER_ENDPOINT:      72.28.97.53:30775
ACCESS_KEY:           minio
SECRET_KEY:           ***REDACTED***
ENABLE_HTTPS:         0
SERVER_REGION:        us-east-1
MINT_DATA_DIR:        /mint/data
MINT_MODE:            full
ENABLE_VIRTUAL_STYLE: 0

To get logs, run 'docker cp 56c63d7746df:/mint/log /tmp/mint-logs'

(1/14) Running aws-sdk-go tests ... FAILED in 6 minutes and 6 seconds
{
  "alert": "",
  "args": {
    "bucketName": "aws-sdk-go-test-9yx2akybv5o9lw",
    "expiry": 60000000000,
    "objectName": "presignedTest"
  },
  "duration": 366474,
  "error": "XMinioServerNotInitialized: Server not initialized, please try again.\n\tstatus code: 503, request id: 15DC1C301739EF7F, host id: ",
  "function": "PresignedPut",
  "message": "AWS SDK Go CreateBucket Failed",
  "name": "aws-sdk-go",
  "status": "FAIL"
}

Executed 0 out of 14 tests successfully.

8537-7289ccb/mint-large-bucket.sh.log:

Running with
SERVER_ENDPOINT:      72.28.97.58:30404
ACCESS_KEY:           minio
SECRET_KEY:           ***REDACTED***
ENABLE_HTTPS:         0
SERVER_REGION:        us-east-1
MINT_DATA_DIR:        /mint/data
MINT_MODE:            full
ENABLE_VIRTUAL_STYLE: 0

To get logs, run 'docker cp 65839c4a66f4:/mint/log /tmp/mint-logs'

(1/14) Running aws-sdk-go tests ... done in 1 seconds
(2/14) Running aws-sdk-java tests ... done in 1 seconds
(3/14) Running aws-sdk-php tests ... done in 53 seconds
(4/14) Running aws-sdk-ruby tests ... done in 6 seconds
(5/14) Running awscli tests ... done in 1 minutes and 31 seconds
(6/14) Running healthcheck tests ... done in 0 seconds
(7/14) Running mc tests ... done in 1 minutes and 26 seconds
(8/14) Running minio-dotnet tests ... done in 1 minutes and 4 seconds
(9/14) Running minio-go tests ... FAILED in 4 minutes and 48 seconds
{
  "args": {},
  "duration": 97842,
  "error": "Please reduce your request",
  "function": "testStorageClassMetadataCopyObject()",
  "message": "CopyObject failed on RRS",
  "name": "minio-go: testStorageClassMetadataCopyObject",
  "status": "FAIL"
}

Executed 8 out of 14 tests successfully.

8537-7289ccb/mint-compression-dist-xl.sh.log:

Running with
SERVER_ENDPOINT:      72.28.97.54:30707
ACCESS_KEY:           minio
SECRET_KEY:           ***REDACTED***
ENABLE_HTTPS:         0
SERVER_REGION:        us-east-1
MINT_DATA_DIR:        /mint/data
MINT_MODE:            full
ENABLE_VIRTUAL_STYLE: 0

To get logs, run 'docker cp 9525dcd4b9dc:/mint/log /tmp/mint-logs'

(1/14) Running aws-sdk-go tests ... done in 4 seconds
(2/14) Running aws-sdk-java tests ... done in 1 seconds
(3/14) Running aws-sdk-php tests ... done in 46 seconds
(4/14) Running aws-sdk-ruby tests ... done in 3 seconds
(5/14) Running awscli tests ... done in 1 minutes and 59 seconds
(6/14) Running healthcheck tests ... done in 0 seconds
(7/14) Running mc tests ... done in 56 seconds
(8/14) Running minio-dotnet tests ... done in 42 seconds
(9/14) Running minio-go tests ... done in 1 minutes and 38 seconds
(10/14) Running minio-java tests ... FAILED in 7 seconds
{
  "name": "minio-java",
  "function": "listObjects(final String bucketName, final String prefix, final boolean recursive)",
  "args": "prefix :minio, recursive: true",
  "duration": 404,
  "status": "FAIL",
  "error": "error occurred\nErrorResponse(code=SlowDown, message=Please reduce your request, bucketName=minio-java-test-h8s7ts, objectName=minio-java-test-246e0is, resource=/minio-java-test-h8s7ts/minio-java-test-246e0is, requestId=15DC1C369FC25F91, hostId=bfa3c58e-3653-401f-9418-f8de26394c5c)\nrequest={method=PUT, url=http://72.28.97.54:30707/minio-java-test-h8s7ts/minio-java-test-246e0is, headers=Content-Type: application/octet-stream\nContent-Encoding: aws-chunked\nx-amz-decoded-content-length: 1\nHost: 72.28.97.54:30707\nUser-Agent: MinIO (amd64; amd64) minio-java/dev\nx-amz-content-sha256: STREAMING-AWS4-HMAC-SHA256-PAYLOAD\nx-amz-date: 20191201T014932Z\nAuthorization: AWS4-HMAC-SHA256 Credential=*REDACTED*/20191201/us-east-1/s3/aws4_request, SignedHeaders=content-encoding;host;x-amz-content-sha256;x-amz-date;x-amz-decoded-content-length, Signature=*REDACTED*\n}\nresponse={code=503, headers=Accept-Ranges: bytes\nContent-Length: 361\nContent-Security-Policy: block-all-mixed-content\nContent-Type: application/xml\nRetry-After: 120\nServer: MinIO/DEVELOPMENT.2019-12-01T01-24-27Z\nVary: Origin\nX-Amz-Request-Id: 15DC1C369FC25F91\nX-Xss-Protection: 1; mode=block\nDate: Sun, 01 Dec 2019 01:49:50 GMT\n}\n >>> [io.minio.MinioClient.executeReq(MinioClient.java:1204), io.minio.MinioClient.execute(MinioClient.java:1066), io.minio.MinioClient.executePut(MinioClient.java:1430), io.minio.MinioClient.executePut(MinioClient.java:1452), io.minio.MinioClient.putObject(MinioClient.java:4511), io.minio.MinioClient.putObject(MinioClient.java:4564), io.minio.MinioClient.putObject(MinioClient.java:4478), FunctionalTest.listObject_test3(FunctionalTest.java:1413), FunctionalTest.runTests(FunctionalTest.java:3008), FunctionalTest.main(FunctionalTest.java:3148)]"
}

Executed 9 out of 14 tests successfully.

Error: No such image: minio/minio:8537-7289ccb

Copy link
Contributor

Praveenrajmani left a comment

LGTM

@harshavardhana

This comment has been minimized.

Copy link
Member

harshavardhana commented Dec 1, 2019

PTAL @vadmeste

Copy link
Member

vadmeste left a comment

LGTM & tested

@harshavardhana harshavardhana merged commit 947bc8c into minio:master Dec 2, 2019
2 checks passed
2 checks passed
Simple CI Minimalist CI System
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.