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

Storage support openstack swift #493

Merged
merged 41 commits into from
Jul 23, 2015
Merged

Storage support openstack swift #493

merged 41 commits into from
Jul 23, 2015

Conversation

nevermosby
Copy link
Contributor

Fixes: #220

@stevvooe
Copy link
Collaborator

stevvooe commented May 5, 2015

@nevermosby We don't really use the "open-design" process. Just submit your intent as an issue or go even further with a PR with documentation or a minimal implementation. Please see #443 for an example of a proposal that will likely be successful.

Mostly, we are interested in the following:

  1. Will there be a maintainer for at least 1 year?
  2. Can the driver be tested in our CI system for every PR?
  3. Will the integration with the particular driver complicate the build for other users who aren't using that driver?

Glad to have you on board and I'm excited to see an implementation!

@nevermosby
Copy link
Contributor Author

@stevvooe Thank you very much for the information, we will submit an issue or a PR with further description and the feedback to your concerns.

@lebauce
Copy link
Contributor

lebauce commented May 11, 2015

@nevermosby Hi. I implemented an Openstack Swift driver (https://github.com/lebauce/distribution/tree/swift-backend). There still are 2 tests that fail though.

@nevermosby
Copy link
Contributor Author

@lebauce great work! Is there any way we can work together to contribute for docker? Any thoughts are welcome.

@dmp42
Copy link
Contributor

dmp42 commented May 13, 2015

@lebauce @nevermosby this is exciting to see interest in building such a driver!

Addressing @stevvooe questions would definitely help in getting this merged, and indeed combining efforts on this is certainly a good idea.

Looking forward to it!

@ojacques
Copy link
Contributor

@stevvooe, I work with @nevermosby. On your questions:

Will there be a maintainer for at least 1 year?
Yes, we commit to maintain the swift storage driver for at least 1 year. We plan to use it ourselves for more than that.

Can the driver be tested in our CI system for every PR?
The driver will be tested when Openstack env variables are set, as this require an Openstack Swift storage backend. We will be using the same mechanism as S3 storage tests. To actually test a Swift back-end, we could either use Dockenstack or an actual Openstack swift back-end on the HP public cloud. What would you prefer?

Will the integration with the particular driver complicate the build for other users who aren't using that driver?
No. The Go Swift library is fetched at build time. The tests are not executed if Openstack env variables are not set.

@lebauce, how does this sound to you?

@lebauce
Copy link
Contributor

lebauce commented May 13, 2015

@stevvooe Hi !

  1. Regarding the maintenance, it's ok for me. @nevermosby would you be interested in co-maintaining it ?
  2. We definitely should test the driver in your CI. The library that provides the Go bindings to Swift - github.com/ncw/swift - provides a Swift server written in Go for testing purpose. I'll try to start the Swift server in the testsuite.
    That said, we should also test the driver against a real Swift server. This server could be a VM (https://github.com/swiftstack/vagrant-swift-all-in-one) or a Docker container. I would be happy to help in setting this up.
  3. I don't think it complicates the build for other users. The Swift driver is pretty similar to the S3 one, it just implements the StorageDriver API and make use of a Go library to talk to Swift.

@lebauce
Copy link
Contributor

lebauce commented May 13, 2015

@ojacques Sorry, I didn't see your answer before starting mine (see above).

This sounds really good to me ! I think we agree on every point.

@nevermosby
Copy link
Contributor Author

@lebauce I am very glad to do the maintaining.

@ojacques
Copy link
Contributor

@lebauce, @nevermosby would merging both branches in this PR would make most sense to avoid duplicate effort? Obviously @lebauce, your implementation was further along, so let us know how you would rather proceed.

@lebauce
Copy link
Contributor

lebauce commented May 13, 2015

@ojacques @nevermosby It surely makes sense. It seems the PR was mostly about describing the proposal. The points where our 2 PR's differ is on the attribute names in the config file, and the environment variables for the test suite. Could you please take a look at the docs/configuration.md, docs/storage-drivers/swift.md and registry/storage/driver/swift/swift_test.go in particular and tell me if it's ok for you ?

By the way, I also added the code to start a Swift server for testing if you do not have a real Swift server running.

@nevermosby
Copy link
Contributor Author

@lebauce the two documentation file is ok for me.
And I tried your swift_test.go, unfortunately, I didn't get the pass result either with my local installed Swift server or with the Swift server started in the testsuite.

For my local installed Swift server, I got the result with timeout error and did see the two containers had been created but with a mess of objects populated.

For the Swift server started in the testsuite, I got some compile error:

./swift_test.go:54: swiftServer.AuthURL undefined (type *swifttest.SwiftServer has no field or method AuthURL)

which might be caused by SwiftServer does not public the property/function AuthURL(PS: I also tried url but failed, maybe this one need to be hard-coded).

I believe the test file may not be your final version, please let me know if I can help with that.

@nevermosby nevermosby closed this May 14, 2015
@nevermosby nevermosby reopened this May 14, 2015
@lebauce
Copy link
Contributor

lebauce commented May 15, 2015

@nevermosby I made a few modifications to the ncw/swift library, I therefore updated the Godeps.json file. A 'go deps' should fix the 'AuthURL' issue.

Regarding the errors with your local Swift server : are you running the test suite with 'go test -short' ? If not, could you try it ? One of the test uploads a 5Go file :-/

@nevermosby
Copy link
Contributor Author

@lebauce the 'AuthURL' issue has been fixed. One minor issue is the import path for swiftest lib in swift_test.go should be changed to your repo.

'go test -short' passed all the necessary test cases for local Swift server and the one started in the testsuite.

However, normal 'go test' for two kinds of Swift server got timeout error, the local Swift server might be caused by low bandwidth of my local network.
Is that acceptable for docker CI validation?

@lebauce
Copy link
Contributor

lebauce commented May 15, 2015

@nevermosby My bad. I forgot to modify the incluse, sorry.
I had the same issue than you. I haven't been able to the testsuite without the '-short' flag because of the large file uploads. @stevvooe, what's your opinion on this ?

@stevvooe
Copy link
Collaborator

@lebauce I am not up to date on the full conversation, but the following must happen:

  1. The circleci file should be modified to be able to run the tests. It is okay if it is a remote service. Let us (@dmp42) know if there are secrets that must be configured and we can help set that up.
  2. The driver must pass the full test suite.

Unfortunately, the tests are a little aggressive with the amount of data they write. How long are they running against the swift server?

@nevermosby
Copy link
Contributor Author

@stevvooe @lebauce The test suite for swift driver has been run for several times with our local swift server built on a destop PC, the best result we have is it costs 3580s to pass the almost whole test cases without the one "TestConcurrentFileStreams", which always caused timeout error.

We will try it again with a swift server built on HP public cloud to see whether the result would be better.

@lebauce
Copy link
Contributor

lebauce commented May 19, 2015

@nevermosby Can you run the testsuite with "go test -timeout 9999999s" ?

On my setup, the full suite takes a bit less than 30 minutes

@nevermosby
Copy link
Contributor Author

@lebauce yes, I tried it before. It turns out that it throws the timeout error with message:
408, "Timeout when reading or writing data"
What's ur test environment for a 30-minutes full test run?

Besides, let's start the code merge work, this is my proposal to proceed:

  1. I will fork ur repo with swift backend: https://github.com/lebauce/distribution/tree/swift-backend
  2. I will create a pull request to ur repo with the results of merged work
  3. We work on that pull request until you accept it.
  4. You can do a pull request against this pull request: Storage support openstack swift #493 with our merged work
  5. I will accept ur pull request and continue from there.
    What's ur option on this?

@stevvooe
Copy link
Collaborator

@nevermosby @lebauce Any insight into the performance issues? If the performance is really that bad with the swift driver, the implementation would be unusable. Is this do to configuration or perhaps IO on the CI host?

@lebauce
Copy link
Contributor

lebauce commented May 20, 2015

@stevvooe With the -short option, on my machine :

OK: 32 passed, 2 skipped
PASS
ok      github.com/docker/distribution/registry/storage/driver/swift    96.892s

The full test suite takes around 45 minutes :

OK: 34 passed
PASS
ok      github.com/docker/distribution/registry/storage/driver/swift    2554.795s

The test TestWriteReadLargeStreams alone - that uploads a 5G file and then reads it - takes 25 minutes on my setup when ran against a public cloud. It sounds reasonable to me, considering my bandwidth.

Does the testsuite run in "full" mode in your CI, or in "short" mode ?

@lebauce
Copy link
Contributor

lebauce commented May 20, 2015

@nevermosby I increased the timeout - it was only 60 seconds in my previous push :-/
Could you please give a try to my latest version ?

(I also added a patch to the Swift bindings because of a bug in net/http : ncw/swift#41)

Regarding the workflow you proposed, it sounds ok to me but maybe we can do something easier. Instead of you forking my repository and adding your work to it, you could just rebase your storage-support-openstack-swift branch on my swift-backend branch and we just keep this one pull request to track all the changes. What do you think ?

@nevermosby
Copy link
Contributor Author

@lebauce Sounds great to me about your proposal, I will follow it up.
For the full testsuite, I will try the latest version of ur code with a public cloud and let u know the result.

@stevvooe
Copy link
Collaborator

@lebauce Interesting. The CI tests run the full suite. Mostly, we are ensuring that even after moving a large amount of data under different circumstances, no corruptions occurs. Given the performance you're seeing, it seems likely related to bandwidth. Let's see if we can get these running in the CI system used for this project and we'll work from there.

Please ping @dmp42 if you need help getting setup.

lebauce and others added 14 commits July 21, 2015 23:55
Signed-off-by: Sylvain Baubeau <sbaubeau@redhat.com>
Signed-off-by: Sylvain Baubeau <sbaubeau@redhat.com>
Signed-off-by: Sylvain Baubeau <sbaubeau@redhat.com>
Signed-off-by: Sylvain Baubeau <sbaubeau@redhat.com>
Signed-off-by: Sylvain Baubeau <sbaubeau@redhat.com>
Signed-off-by: Sylvain Baubeau <sbaubeau@redhat.com>
Signed-off-by: Sylvain Baubeau <sbaubeau@redhat.com>
Signed-off-by: Sylvain Baubeau <sbaubeau@redhat.com>
Signed-off-by: Sylvain Baubeau <sbaubeau@redhat.com>
Signed-off-by: Sylvain Baubeau <sbaubeau@redhat.com>
Signed-off-by: Sylvain Baubeau <sbaubeau@redhat.com>
Signed-off-by: Olivier Jacques <olivier.jacques@hp.com>
Signed-off-by: Li Wenquan <wenquan.li@hp.com>
…ty v1.0 example

Signed-off-by: Li Wenquan <wenquan.li@hp.com>
@stevvooe
Copy link
Collaborator

@snayakie BLOB_UNKNOWN is a standard error return for the API. We log it as an error. We may change the way this works so there is a single log message, either error or non-error, per request, which would be info level.

Either way, these look like content checks, since they are HEAD requests. These could be perfectly normal for the request flow.

@snayakie
Copy link

@lebauce I did try "make test", no issues from that. But that doesn't test a running registry against my softlayer account. If there is any other test suite I can run against the registry, please let me know how. Unfortunately I can't give you any SoftLayer account with objectstorage myself, I just have my account, which there isn't any easy way to share.

@stevvooe from your description, sounds like this is a non-issue.

@lebauce
Copy link
Contributor

lebauce commented Jul 22, 2015

@snayakie Go to registry/storage/driver/swift and run the go test -short from there.
You need to have export the SWIFT_USERNAME, SWIFT_PASSWORD, SWIFT_TENANT_NAME and SWIFT_CONTAINER_NAME environment variables set, otherwise the testsuite will run but with a local Swift test server

@snayakie
Copy link

@lebauce The test result is "ok". Since this is a v1 authentication, I set the SWIFT_USERNAME, SWIFT_PASSWORD, SWIFT_AUTH_URL and SWIFT_CONTAINER_NAME. Then results are:

$ export GOPATH=godep path:$GOPATH
$ go test -short
OK: 31 passed, 2 skipped
PASS
ok github.com/docker/distribution/registry/storage/driver/swift 322.278s

So it looks all good from this test.

@stevvooe
Copy link
Collaborator

@snayakie @lebauce Are we good to go here?

@lebauce
Copy link
Contributor

lebauce commented Jul 23, 2015

@stevvooe Yes, I think we are

stevvooe added a commit that referenced this pull request Jul 23, 2015
@stevvooe stevvooe merged commit 34e5b18 into distribution:master Jul 23, 2015
@stevvooe
Copy link
Collaborator

Grats, all!

@snayakie
Copy link

Thanks @stevvooe , all!

@nevermosby
Copy link
Contributor Author

@lebauce @stevvooe @ojacques Wow, greatThx all

thaJeztah pushed a commit to thaJeztah/distribution that referenced this pull request Apr 22, 2021
…penstack-swift

Storage support openstack swift
thaJeztah pushed a commit to thaJeztah/distribution that referenced this pull request Jan 19, 2022
…penstack-swift

Storage support openstack swift
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.

Swift storage driver