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

cache: support Alibaba Cloud oss backend cache #4502

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

njucjc
Copy link
Contributor

@njucjc njucjc commented Dec 20, 2023

Fix #4487

@njucjc njucjc force-pushed the oss_cache_support branch 2 times, most recently from b3e2e4b to bcafb0a Compare December 20, 2023 03:34
Signed-off-by: njucjc <njucjc@gmail.com>
@crazy-max
Copy link
Member

Can we have integration tests similar to

buildkit/client/client_test.go

Lines 5224 to 5260 in fc9de56

func testBasicAzblobCacheImportExport(t *testing.T, sb integration.Sandbox) {
integration.SkipOnPlatform(t, "windows")
workers.CheckFeatureCompat(t, sb,
workers.FeatureCacheExport,
workers.FeatureCacheImport,
workers.FeatureCacheBackendAzblob,
)
opts := helpers.AzuriteOpts{
AccountName: "azblobcacheaccount",
AccountKey: base64.StdEncoding.EncodeToString([]byte("azblobcacheaccountkey")),
}
azAddr, cleanup, err := helpers.NewAzuriteServer(t, sb, opts)
require.NoError(t, err)
defer cleanup()
im := CacheOptionsEntry{
Type: "azblob",
Attrs: map[string]string{
"account_url": azAddr,
"account_name": opts.AccountName,
"secret_access_key": opts.AccountKey,
"container": "cachecontainer",
},
}
ex := CacheOptionsEntry{
Type: "azblob",
Attrs: map[string]string{
"account_url": azAddr,
"account_name": opts.AccountName,
"secret_access_key": opts.AccountKey,
"container": "cachecontainer",
},
}
testBasicCacheImportExport(t, sb, []CacheOptionsEntry{im}, []CacheOptionsEntry{ex})
}
?

I see in the docs that we could use the S3 SDK to access OSS. Can we consider it to reduce dependencies overhead as we support S3 already?: https://www.alibabacloud.com/help/en/oss/developer-reference/use-amazon-s3-sdks-to-access-oss#section-sp0-q7h-dvj

This way we could use the same integration tests as s3 one using minio.

Comment on lines +37 to +43
bucket, ok := attrs[attrBucket]
if !ok {
bucket, ok = os.LookupEnv("ALIBABA_CLOUD_OSS_BUCKET")
if !ok {
return &Config{}, errors.Errorf("bucket ($ALIBABA_CLOUD_OSS_BUCKET) not set for oss cache")
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like s3 should we also consider having an env var to set the region? It seems the region is inferred from endpoint url so maybe not needed?: https://www.alibabacloud.com/help/en/oss/developer-reference/go-configure-access-credentials

// Create an OSSClient instance.
// Specify the endpoint of the region in which the bucket is located. For example, if the bucket is located in the China (Hangzhou) region, set the endpoint to https://oss-cn-hangzhou.aliyuncs.com. Specify your actual endpoint.

Just asking because I see the SDK having a method to set it: https://github.com/aliyun/aliyun-oss-go-sdk/blob/2bc58741fa507528ff70b501e7e881a92f88ce3e/oss/client.go#L2891-L2895

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the region env var is not needed for OSS SDK

@tonistiigi
Copy link
Member

per discussion in #4487 I think we need to figure out some external plugin model for this instead as adding lots of builtin code for every possible vendor does not scale.

Isn't using s3 option for this case? There seems to be some compatibility.

@njucjc
Copy link
Contributor Author

njucjc commented Dec 21, 2023

per discussion in #4487 I think we need to figure out some external plugin model for this instead as adding lots of builtin code for every possible vendor does not scale.

Isn't using s3 option for this case? There seems to be some compatibility.

Yes, there are some compatibility between S3 and OSS driver, but in S3 cache code,it does not use concurrent multi-part upload when uploading blobs, so there may be some performance issues if the cached layer is large.

_, err := s3Client.Upload(ctx, input)

I'm not sure if S3 can also implement concurrent multi-part uploads.

I agree we need to figure out some external plugin model for cache driver, I'd like to have a try, but it will be a long term case.

* `secret_access_key`: Security Token (optional)

`--export-cache` options:
* `type=oss`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be called "alibaba-oss" to avoid confusion

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also wonder if this should be an external plugin that would talk to buildkitd over grpc or something

@AkihiroSuda AkihiroSuda changed the title cache: support oss backend cache cache: support Alibaba Cloud oss backend cache Dec 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Is there any plan to support to export/import image cache to/from Alibaba Cloud OSS ?
4 participants