Merge lp:~jaypipes/glance/bug713126 into lp:~glance-coresec/glance/cactus-trunk

[Jay Pipes]

Adds ability for Swift to be used as a full-fledged backend.
Adds POST/PUT capabilities to the SwiftBackend
Adds lots of unit tests for both FilesystemBackend and SwiftBackend
Removes now-unused tests.unit.fakeswifthttp module

[Wayne A. Walls (wayne-walls)]

Checking out this patch right now.

cheers!

[Armando Migliaccio (armando-migliaccio)]

To me this branch was long overdue :)

At a first _glance_, I couldn't find anything wrong with it...only two small niggley bits:

- maybe if tests don't find swift they should say 'skipped' rather than 'ok', but I don't know if nosetest can do that easily. The effect is the same but it just draws attention to the user
- the default swift store options are a bit weak: DEFAULT_SWIFT_ACCOUNT = 'glance' and DEFAULT_SWIFT_CONTAINER = 'glance' do not seem very unique to me :)

Other than that, I'd like to give this branch a whirl on a live swift deployment we got here before giving a full 'approve' from my side :)

[Jay Pipes (jaypipes)]

> To me this branch was long overdue :)

:) Yes, I know.

> At a first _glance_, I couldn't find anything wrong with it...only two small
> niggley bits:
>
> - maybe if tests don't find swift they should say 'skipped' rather than 'ok',
> but I don't know if nosetest can do that easily. The effect is the same but it
> just draws attention to the user

Yes, I thought about this. I had originally used unittest2's @skipIf decorator, but I had proposed an earlier branch and people didn't want to require dependency on unittest2. nose has some ability to do skipping; I could check into this.

> - the default swift store options are a bit weak: DEFAULT_SWIFT_ACCOUNT =
> 'glance' and DEFAULT_SWIFT_CONTAINER = 'glance' do not seem very unique to me
> :)

Well, the account and container are more general-purpose than anything to do with authenticating (user and key are for that purpose). What do you think I should set account and container to?

> Other than that, I'd like to give this branch a whirl on a live swift
> deployment we got here before giving a full 'approve' from my side :)

Can't wait for feedback! Thanks, Armando!

-jay

[Armando Migliaccio (armando-migliaccio)]

I guess that as long as we use an account that cannot be used by anyone else, then there will never be a risk of having image files mixed up with user data, so in that respect it's safe.

As for the swift_store_auth_address I wonder if we need to append /v1.0 at the end of it.

When I use ST (the tool that comes with swift) without it, I get the following error:

__main__.ClientException: Auth GET failed: https://192.168.1.199:11000 400 Bad Request

[Armando Migliaccio (armando-migliaccio)]

Jay, this may sound silly, but can you spot why I am getting this exception?

2011-02-28 14:30:49 ERROR [glance.store.swift] auch
Traceback (most recent call last):
  File "/usr/lib/python2.6/site-packages/eventlet/wsgi.py", line 336, in handle_one_response
    result = self.application(self.environ, start_response)
  File "build/bdist.linux-i686/egg/webob/dec.py", line 159, in __call__
    return resp(environ, start_response)
  File "build/bdist.linux-i686/egg/routes/middleware.py", line 131, in __call__
    response = self.app(environ, start_response)
  File "build/bdist.linux-i686/egg/webob/dec.py", line 159, in __call__
    return resp(environ, start_response)
  File "build/bdist.linux-i686/egg/webob/dec.py", line 147, in __call__
    resp = self.call_func(req, *args, **self.kwargs)
  File "build/bdist.linux-i686/egg/webob/dec.py", line 208, in call_func
    return self.func(req, *args, **kwargs)
  File "/usr/lib/python2.6/site-packages/glance/common/wsgi.py", line 215, in __call__
    result = method(**arg_dict)
  File "/usr/lib/python2.6/site-packages/glance/server.py", line 352, in create
    self._upload_and_activate(req, image_meta)
  File "/usr/lib/python2.6/site-packages/glance/server.py", line 315, in _upload_and_activate
    raise e
ImportError: No module named common.client

I have swift installed in my site-packages, and I can import swift.common.client.Connection using the python interpreter. This exception occurs every time I try to upload. As the problem is here:

272 + connection_class = get_connection_class(conn_class)

Thanks,
Armando

[Armando Migliaccio (armando-migliaccio)]

I'd like to see this in trunk asap :)

review: Approve

[Rick Harris (rconradharris)]

Excellent work Jay. I got it working with my CloudFiles account with some small-fixes:

> 237 + account = options.get('swift_store_account',
> 238 + DEFAULT_SWIFT_ACCOUNT)

Account is referenced but not used. AFAIK, there isn't a distinction between
account and username, so, I think we can remove all references to account.

> 143 -from __future__ import absolute_import

Was this removed for a reason? The `swift` adapter is shadowing the `swift`
module so `import swift.common.client` won't do the right thing without this.

> 215 + def add(cls, id, data, options, conn_class=None):

We probably shouldn't be using `id`, but, we can save that for a cleanup-patch
where we fix that everywhere, all-at-once.

> 308 + "location %(location)s" % locals())

One space to the right :)

> 334 + except Exception, e:

This should probably be `except swift.common.client.ClientException, e` in
order to guarantee that it has the `http_status` attribute.

Since, you'll need swift.common.client in the local-namespace to do this, you
can either:

* Use separate imports in each function that has to use something from the
  swift module

* Use a lazy-import pattern: define a global SWIFT global variable (initialized to None) and then lazily populate it. Then use that to access sub-modules. I believe this is done in Nova.

> 281 + obj_etag = swift_conn.put_object(container, id, data)

Needs to be `str(id)` since the `quote` method used by swift.Client expects a
string.

Since this is used repeatedly (in the head_object call too), probably best to
do something like:

    obj_name = str(id)

at the top of each method.

Currently we're assuming that the container already exists (if it doesn't, the
PUT will fail with a 404). I would vote that we automatically create the
container if it doesn't exist.

Perhaps this behavior should be governed by a configuration option, something
like `swift_store_create_container_on_put`

review: Needs Fixing

[Jay Pipes (jaypipes)]

> Jay, this may sound silly, but can you spot why I am getting this exception?
>
> 2011-02-28 14:30:49 ERROR [glance.store.swift] auch
> Traceback (most recent call last):
> File "/usr/lib/python2.6/site-packages/eventlet/wsgi.py", line 336, in
> handle_one_response
> result = self.application(self.environ, start_response)
> File "build/bdist.linux-i686/egg/webob/dec.py", line 159, in __call__
> return resp(environ, start_response)
> File "build/bdist.linux-i686/egg/routes/middleware.py", line 131, in
> __call__
> response = self.app(environ, start_response)
> File "build/bdist.linux-i686/egg/webob/dec.py", line 159, in __call__
> return resp(environ, start_response)
> File "build/bdist.linux-i686/egg/webob/dec.py", line 147, in __call__
> resp = self.call_func(req, *args, **self.kwargs)
> File "build/bdist.linux-i686/egg/webob/dec.py", line 208, in call_func
> return self.func(req, *args, **kwargs)
> File "/usr/lib/python2.6/site-packages/glance/common/wsgi.py", line 215, in
> __call__
> result = method(**arg_dict)
> File "/usr/lib/python2.6/site-packages/glance/server.py", line 352, in
> create
> self._upload_and_activate(req, image_meta)
> File "/usr/lib/python2.6/site-packages/glance/server.py", line 315, in
> _upload_and_activate
> raise e
> ImportError: No module named common.client
>
> I have swift installed in my site-packages, and I can import
> swift.common.client.Connection using the python interpreter. This exception
> occurs every time I try to upload. As the problem is here:
>
> 272 + connection_class = get_connection_class(conn_class)

It's because there is a module glance.store.swift and a module swift.common.client. Local namespacing is confusing Python into thinking it should search for "common.client" at a module path of glance.store.swift.common.client. It's a bug, and there are a couple solutions to it. Rick refers to a couple below. Working on it...

Thanks!
jay

[Jay Pipes (jaypipes)]

Thanks Rick, working on fixes now. Should push a branch shortly... gah, jury duty is a pain in the business schedule :)

[Jay Pipes (jaypipes)]

> Excellent work Jay. I got it working with my CloudFiles account with some
> small-fixes:
>
> > 237 + account = options.get('swift_store_account',
> > 238 + DEFAULT_SWIFT_ACCOUNT)
>
> Account is referenced but not used. AFAIK, there isn't a distinction between
> account and username, so, I think we can remove all references to account.

account definitely != username in Swift. username/key is used for auth'ing against an authentication service, but account is used to determine which containers a user has access to, at least if I'm understanding the docs correctly. I believe that the auth middleware inserts the X-User-Account header into the environ? Not quite sure...

Hmm, so I was a little unsure about this and was hoping the Swift guys could shine some light. As far as I have read, I guess Swift objects are always in an account and a container, with the URI structure:

auth_url/account/container/objectname

Our previous code was ignoring account so I ignored it too, but I think I will add it now? This relates to LP bug #717431

> > 143 -from __future__ import absolute_import
>
> Was this removed for a reason? The `swift` adapter is shadowing the `swift`
> module so `import swift.common.client` won't do the right thing without this.

I removed it because absolute_import wasn't being used. :) Though, might need to use it per your answer below ;)

> > 215 + def add(cls, id, data, options, conn_class=None):
>
> We probably shouldn't be using `id`, but, we can save that for a cleanup-patch
> where we fix that everywhere, all-at-once.

Ya, cleanup patch.

> > 308 + "location %(location)s" %
> locals())
>
> One space to the right :)

Done.

> > 334 + except Exception, e:
>
> This should probably be `except swift.common.client.ClientException, e` in
> order to guarantee that it has the `http_status` attribute.
>
> Since, you'll need swift.common.client in the local-namespace to do this, you
> can either:
>
> * Use separate imports in each function that has to use something from the
> swift module
>
> * Use a lazy-import pattern: define a global SWIFT global variable
> (initialized to None) and then lazily populate it. Then use that to access
> sub-modules. I believe this is done in Nova.

The main problem is not having the import of glance.store.swift in tests/unit/test_swift_store.py blow up when Swift isn't installed. I could try the Nova solution, but it always seems hacky compared to just wrapping an import swift.common.client in a try: except ImportError: block...

I could solve the problem using nose's skip stuff, too, I think?

> > 281 + obj_etag = swift_conn.put_object(container, id, data)
>
> Needs to be `str(id)` since the `quote` method used by swift.Client expects a
> string.
>
> Since this is used repeatedly (in the head_object call too), probably best to
> do something like:
>
> obj_name = str(id)
>
> at the top of each method.

Done.

> Currently we're assuming that the container already exists (if it doesn't, the
> PUT will fail with a 404). I would vote that we automatically create the
> container if i...

[Rick Harris (rconradharris)]

>Our previous code was ignoring account so I ignored it too, but I think I will add it now? This relates to LP bug #717431

Ah ok. So, yeah, probably need some clarification, but `account` should be
used in crafting the URI.

> 283 + try:
> 284 + swift_conn.head_container(container)
> 285 + except Exception, e:
> 286 + if e.http_status == httplib.NOT_FOUND:
> 287 + add_container = config.get_option(options,
> 288 + 'swift_store_create_container_on_put',
> 289 + type='bool', default=False)
> 290 + if add_container:
> 291 + try:
> 292 + swift_conn.put_container(container)
> 293 + except Exception, e:
> 294 + msg = ("Failed to add container to Swift.\n"
> 295 + "Got error from Swift: %(e)s" % locals())
> 296 + raise glance.store.BackendException(msg)
> 297 + else:
> 298 + msg = ("The container %(container)s does not exist in "
> 299 + "Swift. Please set the "
> 300 + "swift_store_create_container_on_put option"
> 301 + "to add container to Swift automatically."
> 302 + % locals())
> 303 + raise glance.store.BackendException(msg)

The `add()` method is starting to get a bit long. I'd vote that we break this
code out into a separate method called something like
`_create_container_if_needed`.

> 245 + user = options.get('swift_store_user')
...
> 287 + add_container = config.get_option(options,
> 288 + 'swift_store_create_container_on_put',
> 289 + type='bool', default=False)

We're getting options two--really three if you could CLI options--different
ways. One way is to use paste and the other--the new way-- is to use the
ConfigParser api directly.

I'd vote to continue to use the `paste` method for now, and, if we want to
refactor config options, do that as another, config-options-improvement patch.

> 263 + if not user:
> 264 + logger.error(msg)
> 265 + msg = ("Could not find swift_store_user in configuration "
> 266 + "options.")
> 267 + raise glance.store.BackendException(msg)

There is some repeated code similar to this. Wonder if we'd benefit from a
`_get_required_option` function that returns the value if present, and logs
and raises if not.

> 157 +DEFAULT_SWIFT_ACCOUNT = 'glance'
> 158 +DEFAULT_SWIFT_CONTAINER = 'glance'

I don't see a reason to have these constants. We already have the config-file,
we can just treat the `account` and `container` config options as required. Seems
less confusing to me, IMHO.

[Jay Pipes (jaypipes)]

On Tue, Mar 1, 2011 at 2:20 AM, Rick Harris <email address hidden> wrote:
>> 245   +        user = options.get('swift_store_user')
> ...
>> 287   +                add_container = config.get_option(options,
>> 288   +                                    'swift_store_create_container_on_put',
>> 289   +                                    type='bool', default=False)
>
> We're getting options two--really three if you could CLI options--different
> ways. One way is to use paste and the other--the new way-- is to use the
> ConfigParser api directly.
>
> I'd vote to continue to use the `paste` method for now, and, if we want to
> refactor config options, do that as another, config-options-improvement patch.

Sorry, I'm lost. The above is the glance.common.config.get_option()
function you added in a recent patch that I specifically argued
against using... I'm only following the convention you set in
/glance/registry/db/api.py.

-jay

[Armando Migliaccio (armando-migliaccio)]

Does line:

279 + logger.debug("Adding image object to Swift using "
280 + "(auth_address=%(auth_address)s, user=%(user)s, "
281 + "key=%(key)s)")

need % locals() ?

review: Needs Fixing

[Armando Migliaccio (armando-migliaccio)]

Rick,

did you specify the version for swift_store_auth_address? like this:

swift_store_auth_address=127.0.0.1:11000/v1.0

If I don't do it, I get this error:

BackendException: Failed to add object to Swift.
Got error from Swift: Auth GET failed: https://127.0.0.1:11000 400 Bad Request

If the version is required, we might want to tweak the default otherwise it will raise a lot of false positives from swift newbies :)

Many thanks,
Armando

[Rick Harris (rconradharris)]

Jay:

> I'm only following the convention you set in
/glance/registry/db/api.py

You're right. Disregard comment, acquire sanity. :)

Sorry about the mix-up, I'm just generally confused by config-options at this point. Something we can revisit down the road, I guess.

Armando:

> did you specify the version for swift_store_auth_address?

Yep.

> If the version is required, we might want to tweak the default otherwise it will raise a lot of false positives from swift newbies

Yeah I agree, it's a bit confusing. We can use use a BASE_URI which encapsulates both scheme, host, port, and version number like:

https://auth.api.rackspacecloud.com/v1.0/

Or, we can use a separate config for each part:

swift_store_port = 80
swift_store_host = auth.api.rackspacecloud.com
swift_store_api_version = 1.0

I lean towards the separate-config approach since it seems a little simpler.

[Wayne A. Walls (wayne-walls)]

Greetings, everyone!

So, I am getting VMs to boot, and for all intents and purposes this is 'working.' One thing that I feel is odd, when I jump on my swift proxy, and check the containers...they are empty.

##nova-compute log##
2011-03-01 10:28:02,909 INFO nova.virt.xenapi.vmops [-] Spawning VM instance-00000009 created OpaqueRef:8d097152-7213-607d-a8a0-89acf8b265d6.
2011-03-01 10:28:03,242 INFO nova.virt.xenapi [-] Task [Async.host.call_plugin] OpaqueRef:b0af1d35-2a7a-39be-cecd-377d5f021df5 status: success <value>null</value>
2011-03-01 10:28:03,452 INFO nova.virt.xenapi.vm_utils [-] (VM_UTILS) xenserver vm state -> |Running|
2011-03-01 10:28:03,452 INFO nova.virt.xenapi.vm_utils [-] (VM_UTILS) xenapi power_state -> |1|
2011-03-01 10:28:03,502 DEBUG nova.virt.xenapi.vmops [-] Instance instance-00000009: booted from (pid=964) _wait_for_boot /root/openstack/nova/nova/virt/xenapi/vmops.py:150
2011-03-01 10:28:03,506 INFO nova.virt.xenapi.vm_utils [-] (VM_UTILS) xenserver vm state -> |Running|
2011-03-01 10:28:03,506 INFO nova.virt.xenapi.vm_utils [-] (VM_UTILS) xenapi power_state -> |1|

##on domU##
+----+--------+--------+-----------+----------------+
| ID | Name | Status | Public IP | Private IP |
+----+--------+--------+-----------+----------------+
| 7 | test01 | active | | 184.106.17.204 |
| 8 | test01 | active | | 184.106.17.205 |
| 9 | test01 | active | | 184.106.17.206 |
+----+--------+--------+-----------+----------------+

##On swift proxy##
root@colo01:~# st -A https://184.106.17.135:8080/auth/v1.0 -U system:root -K XXXXX stat
   Account: AUTH_XXXXXXXX
Containers: 0
   Objects: 0
     Bytes: 0

Maybe I am setting up something wrong in my glance.conf? Here it is for your review:

# ============ Swift Store Options =============================

# Address where the Swift authentication service lives
swift_store_auth_address = 184.106.17.135:8080/auth/v1.0

# User to authenticate against the Swift authentication service
swift_store_user = root

# Auth key for the user authenticating against the
# Swift authentication service
swift_store_key = AUTH_XXXXXXXXXXXXXXXXXXXXXX

# Account to use for the user:key Swift auth combination
# for storing images in Swift
swift_store_account = root

# Container within the account that the account should use
# for storing images in Swift
swift_store_container = glance

# Do we create the container if it does not exist?
swift_store_create_container_on_put = True

Any ideas? What else are you folks running into?

Cheers!

[Wayne A. Walls (wayne-walls)]

Oh, and I'm using swauth (bind to 8080) for swift, not devauth.

thanks!

[Rick Harris (rconradharris)]

Wayne:

I tested Jay's code against production CloudFiles. You could sanity check your setup, by first hitting CloudFiles, then, if that works, repointing to your local instance of Swift.

[Armando Migliaccio (armando-migliaccio)]

Wayne,
should your swift_store_account option in glance.conf be system rather than root?

Just guessing here...

> Greetings, everyone!
>
> So, I am getting VMs to boot, and for all intents and purposes this is
> 'working.' One thing that I feel is odd, when I jump on my swift proxy, and
> check the containers...they are empty.
>
> ##nova-compute log##
> 2011-03-01 10:28:02,909 INFO nova.virt.xenapi.vmops [-] Spawning VM
> instance-00000009 created OpaqueRef:8d097152-7213-607d-a8a0-89acf8b265d6.
> 2011-03-01 10:28:03,242 INFO nova.virt.xenapi [-] Task
> [Async.host.call_plugin] OpaqueRef:b0af1d35-2a7a-39be-cecd-377d5f021df5
> status: success <value>null</value>
> 2011-03-01 10:28:03,452 INFO nova.virt.xenapi.vm_utils [-] (VM_UTILS)
> xenserver vm state -> |Running|
> 2011-03-01 10:28:03,452 INFO nova.virt.xenapi.vm_utils [-] (VM_UTILS) xenapi
> power_state -> |1|
> 2011-03-01 10:28:03,502 DEBUG nova.virt.xenapi.vmops [-] Instance
> instance-00000009: booted from (pid=964) _wait_for_boot
> /root/openstack/nova/nova/virt/xenapi/vmops.py:150
> 2011-03-01 10:28:03,506 INFO nova.virt.xenapi.vm_utils [-] (VM_UTILS)
> xenserver vm state -> |Running|
> 2011-03-01 10:28:03,506 INFO nova.virt.xenapi.vm_utils [-] (VM_UTILS) xenapi
> power_state -> |1|
>
> ##on domU##
> +----+--------+--------+-----------+----------------+
> | ID | Name | Status | Public IP | Private IP |
> +----+--------+--------+-----------+----------------+
> | 7 | test01 | active | | 184.106.17.204 |
> | 8 | test01 | active | | 184.106.17.205 |
> | 9 | test01 | active | | 184.106.17.206 |
> +----+--------+--------+-----------+----------------+
>
> ##On swift proxy##
> root@colo01:~# st -A https://184.106.17.135:8080/auth/v1.0 -U system:root -K
> XXXXX stat
> Account: AUTH_XXXXXXXX
> Containers: 0
> Objects: 0
> Bytes: 0
>
> Maybe I am setting up something wrong in my glance.conf? Here it is for your
> review:
>
> # ============ Swift Store Options =============================
>
> # Address where the Swift authentication service lives
> swift_store_auth_address = 184.106.17.135:8080/auth/v1.0
>
> # User to authenticate against the Swift authentication service
> swift_store_user = root
>
> # Auth key for the user authenticating against the
> # Swift authentication service
> swift_store_key = AUTH_XXXXXXXXXXXXXXXXXXXXXX
>
> # Account to use for the user:key Swift auth combination
> # for storing images in Swift
> swift_store_account = root
>
> # Container within the account that the account should use
> # for storing images in Swift
> swift_store_container = glance
>
> # Do we create the container if it does not exist?
> swift_store_create_container_on_put = True
>
>
>
> Any ideas? What else are you folks running into?
>
> Cheers!

[Wayne A. Walls (wayne-walls)]

@Rick, alright I'll give that a try, good idea.

@Armando, I tried 'root' 'system:root' 'system', and a variety of other names. When using cyberduck, I have to use system:root and point to the right auth path "/auth/v1.0" This worked before I changed to swauth, now it's back to giving me errors...shrug

[Wayne A. Walls (wayne-walls)]

@Rick -- Well, same result. VMs are booting, but no containers are being made on my CloudFiles account. I'm not even sure /where/ these VMs are going... I'll continue to check this out...

[Jay Pipes (jaypipes)]

On Tue, Mar 1, 2011 at 3:11 PM, Wayne A. Walls <email address hidden> wrote:
> @Armando, I tried 'root' 'system:root' 'system', and a variety of other names.  When using cyberduck, I have to use system:root and point to the right auth path "/auth/v1.0" This worked before I changed to swauth, now it's back to giving me errors...shrug

The account isn't even being used. In fact, in the swift client code
(/swift/common/client.py in Swift) there's no mention of account
anywhere. We are still trying to figure out how to even specify the
thing or even what it is or if it's different between Cloud Files and
Swift. Some people say that this is the URI structure:

http://user:key@authurl/account/container/objname

Others say this is:

http://user:account:key@authurl/container/objname

Frankly, after going through the Swift docs, I'm at a loss as to what
the account is and how it's different from the swift auth user... if
someone could explain this, we could code it up.

-jay

[Jay Pipes (jaypipes)]

> Does line:
>
> 279 + logger.debug("Adding image object to Swift using "
> 280 + "(auth_address=%(auth_address)s, user=%(user)s, "
> 281 + "key=%(key)s)")
>
> need % locals() ?

Yes it does :) Good catch!

[Jay Pipes (jaypipes)]

Added fixes from reviews. Please do re-review. Thanks :)

[Rick Harris (rconradharris)]

Looks good, unit-tests pass, and I confirmed it works functionally.

Just a couple of femto-nits:

> 189 + conn_class=None):

Looks like several methods are taking the `conn_class` kwarg that is never
used.

> import urllib

Not directly related, but it looks like this import isn't used anymore.

> 377 + except ClientException, e:
> 378 + if e.http_status == httplib.NOT_FOUND:
> 379 + add_container = config.get_option(options,
> 380 + 'swift_store_create_container_on_put',
> 381 + type='bool', default=False)
> 382 + if add_container:
> 383 + try:
> 384 + swift_conn.put_container(container)
> 385 + except ClientException, e:
> 386 + msg = ("Failed to add container to Swift.\n"
> 387 + "Got error from Swift: %(e)s" % locals())
> 388 + raise glance.store.BackendException(msg)
> 389 + else:
> 390 + msg = ("The container %(container)s does not exist in "
> 391 + "Swift. Please set the "
> 392 + "swift_store_create_container_on_put option"
> 393 + "to add container to Swift automatically."
> 394 + % locals())
> 395 + raise glance.store.BackendException(msg)

In the case where e.http_status != HTTP_FOUND, we probably want to re-raise
the current exception.

review: Needs Fixing

[Jay Pipes (jaypipes)]

Yep, all good catches. Fixing now.

[Devin Carlen (devcamcar)]

210 + location = "swift://%s:%s@%s/%s/%s" % (user, key, authurl,
211 + container, obj)

305 + location = "swift://%(user)s:%(key)s@%(auth_address)s/"\
306 + "%(container)s/%(obj_name)s" % locals()

319 + location = "swift://%s:%s@%s/%s/%s" % (user, key, auth_address,
320 + container, id)

350 + location = "swift://%s:%s@%s/%s/%s" % (user, key, authurl,
351 + container, obj)

Can you add a helper method to make this cleaner?

review: Needs Fixing

[Jay Pipes (jaypipes)]

Devin and Rick, pushed fixes from your reviews. Please re-check :) Thanks again!

[Armando Migliaccio (armando-migliaccio)]

Jay,

this looks good!

However, I am still a bit confused about the use of the account bit. Were you able to clarify the point below?

> On Tue, Mar 1, 2011 at 3:11 PM, Wayne A. Walls <email address hidden> wrote:
> The account isn't even being used. In fact, in the swift client code
> (/swift/common/client.py in Swift) there's no mention of account
> anywhere. We are still trying to figure out how to even specify the
> thing or even what it is or if it's different between Cloud Files and
> Swift. Some people say that this is the URI structure:
>
> http://user:key@authurl/account/container/objname
>
> Others say this is:
>
> http://user:account:key@authurl/container/objname
>
> Frankly, after going through the Swift docs, I'm at a loss as to what
> the account is and how it's different from the swift auth user... if
> someone could explain this, we could code it up.
>
> -jay

Moreover:

1) It looks that the variable 'account' is still not used in the 'add' method
2) In the glance.conf.sample it's specified:

+# Address where the Swift authentication service lives
+# The auth address should be in the form:
+# <DOMAIN>[:<PORT>]/<VERSION>/<ACCOUNT>
+swift_store_auth_address = 127.0.0.1:8080/v1.0/glance-account

so one would understand that swift_store_account is 'glance-account', but then the code specifies:

+DEFAULT_SWIFT_ACCOUNT = 'glance'

3) swift_store_account is a missing option in the glance.conf.sample
4) swift_store_create_container_on_put = False (I'd rather see True as a default option)

All of these add up to the confusion :)

I would like to see glance.conf.sample with a set of sensible defaults so that one who does not have a great deal of experience with glance/swift can get up and running without troubles. The above mentioned points make me feel we are not quite there yet :)

Hope this help!
Cheers,
Armando

review: Needs Information

[Jay Pipes (jaypipes)]

On Sun, Mar 6, 2011 at 1:14 PM, Armando Migliaccio
<email address hidden> wrote:
> 1) It looks that the variable 'account' is still not used in the 'add' method
> 2) In the glance.conf.sample it's specified:
>
> +# Address where the Swift authentication service lives
> +# The auth address should be in the form:
> +# <DOMAIN>[:<PORT>]/<VERSION>/<ACCOUNT>
> +swift_store_auth_address = 127.0.0.1:8080/v1.0/glance-account
>
> so one would understand that swift_store_account is 'glance-account', but then the code specifies:
>
> +DEFAULT_SWIFT_ACCOUNT = 'glance'

I meant to remove this. The account should be specified in the
swift_store_auth_address option, and nothing else.

> 3) swift_store_account is a missing option in the glance.conf.sample

I removed this option.

> 4) swift_store_create_container_on_put = False (I'd rather see True as a default option)

I didn't feel comfortable setting it to True by default since that
would change data in Swift (add the container), but I could go either
way I suppose... this is an easy fix obviously.

> All of these add up to the confusion :)
>
> I would like to see glance.conf.sample with a set of sensible defaults so that one who does not have a great deal of experience with glance/swift can get up and running without troubles. The above mentioned points make me feel we are not quite there yet :)

So would I. :) Unfortunately, its not exactly simple to setup Swift
and get everything working without, frankly, knowing what you are
doing and knowing how Swift authentication works (some "auth service"
with a URL like http://localhost:8080/v1.0/account/ authenticates a
user/key combination for an account and returns a storage management
URL that subsequent requests use as their base request URL...

Anyway, I've tried my best to make it sensible to configure *if* you
know how to setup Swift, but I was tentative to try anything more
complex.

-jay

[Devin Carlen (devcamcar)]

lgtm!

review: Approve

[Armando Migliaccio (armando-migliaccio)]

> On Sun, Mar 6, 2011 at 1:14 PM, Armando Migliaccio
> <email address hidden> wrote:
> > 1) It looks that the variable 'account' is still not used in the 'add'
> method
> > 2) In the glance.conf.sample it's specified:
> >
> > +# Address where the Swift authentication service lives
> > +# The auth address should be in the form:
> > +# <DOMAIN>[:<PORT>]/<VERSION>/<ACCOUNT>
> > +swift_store_auth_address = 127.0.0.1:8080/v1.0/glance-account
> >
> > so one would understand that swift_store_account is 'glance-account', but
> then the code specifies:
> >
> > +DEFAULT_SWIFT_ACCOUNT = 'glance'
>
> I meant to remove this. The account should be specified in the
> swift_store_auth_address option, and nothing else.
>
> > 3) swift_store_account is a missing option in the glance.conf.sample
>
> I removed this option.
>
> > 4) swift_store_create_container_on_put = False (I'd rather see True as a
> default option)
>
> I didn't feel comfortable setting it to True by default since that
> would change data in Swift (add the container), but I could go either
> way I suppose... this is an easy fix obviously.
>
> > All of these add up to the confusion :)
> >
> > I would like to see glance.conf.sample with a set of sensible defaults so
> that one who does not have a great deal of experience with glance/swift can
> get up and running without troubles. The above mentioned points make me feel
> we are not quite there yet :)
>
> So would I. :) Unfortunately, its not exactly simple to setup Swift
> and get everything working without, frankly, knowing what you are
> doing and knowing how Swift authentication works (some "auth service"
> with a URL like http://localhost:8080/v1.0/account/ authenticates a
> user/key combination for an account and returns a storage management
> URL that subsequent requests use as their base request URL...
>

fair enough!

> Anyway, I've tried my best to make it sensible to configure *if* you
> know how to setup Swift, but I was tentative to try anything more
> complex.
>
> -jay

I think you did great, as I was able to get this up and running fairly quickly. Apart the niggley bit of

> > +DEFAULT_SWIFT_ACCOUNT = 'glance'

lying around in the code, this branch has both thumbs up from my side

review: Approve

[Rick Harris (rconradharris)]

Agree with Armando, we should probably remove this bit of vestigial code:

257 + account = options.get('swift_store_account',
258 + DEFAULT_SWIFT_ACCOUNT)

170 +DEFAULT_SWIFT_ACCOUNT = 'glance'

review: Needs Fixing

[Jay Pipes (jaypipes)]

Vestigial account stuff gone! :) Please set to approved if that's the final fix...

[Rick Harris (rconradharris)]

lgtm

review: Approve

[OpenStack Infra (hudson-openstack)]

The attempt to merge lp:~jaypipes/glance/bug713126 into lp:glance failed. Below is the output from the failed tests.

running test
running egg_info
creating glance.egg-info
writing glance.egg-info/PKG-INFO
writing top-level names to glance.egg-info/top_level.txt
writing dependency_links to glance.egg-info/dependency_links.txt
writing manifest file 'glance.egg-info/SOURCES.txt'
reading manifest file 'glance.egg-info/SOURCES.txt'
reading manifest template 'MANIFEST.in'
warning: no files found matching 'LICENSE'
warning: no files found matching 'ChangeLog'
warning: no files found matching 'tests/test_data.py'
writing manifest file 'glance.egg-info/SOURCES.txt'
running build_ext

Tests raises BadRequest for invalid store header ... ERROR
Tests to add a basic image in the file store ... ERROR
Tests creates a queued image for no body and no loc header ... ok
test_bad_container_format (tests.unit.test_api.TestGlanceAPI) ... ok
test_bad_disk_format (tests.unit.test_api.TestGlanceAPI) ... ok
test_delete_image (tests.unit.test_api.TestGlanceAPI) ... ERROR
test_delete_non_exists_image (tests.unit.test_api.TestGlanceAPI) ... ok
Test for HEAD /images/<ID> ... ok
test_show_image_basic (tests.unit.test_api.TestGlanceAPI) ... ERROR
test_show_non_exists_image (tests.unit.test_api.TestGlanceAPI) ... ok
Tests that the /images POST registry API creates the image ... ok
Tests proper exception is raised if a bad disk_format is set ... ok
Tests proper exception is raised if a bad disk_format is set ... ok
Tests proper exception is raised if a bad status is set ... ok
Tests that exception raised for bad matching disk and container ... ok
Tests that the /images DELETE registry API deletes the image ... ok
Tests proper exception is raised if attempt to delete non-existing ... ok
Tests that the /images/detail registry API returns ... ok
Tests that the /images registry API returns list of ... ok
Tests that the root registry API returns "index", ... ok
Tests that the /images PUT registry API updates the image ... ok
Tests proper exception is raised if attempt to update non-existing ... ok
Tests that exception raised trying to set a bad container_format ... ok
Tests that exception raised trying to set a bad disk_format ... ok
Tests that exception raised trying to set a bad status ... ok
Tests that exception raised for bad matching disk and container ... ok
Test ClientConnectionError raised ... ok
Tests proper exception is raised if image with ID already exists ... ok
Tests that we can add image metadata and returns the new id ... ok
Tests a bad status is set to a proper one by server ... ok
Tests BadRequest raised when supplying bad store name in meta ... ERROR
Tests can add image by passing image data as file ... ERROR
Tests can add image by passing image data as string ... ERROR
Tests add image by passing image data as string w/ no size attr ... ERROR
Tests that we can add image metadata with properties ... ok
Tests client returns image as queued ... ok
Tests that image metadata is deleted properly ... ERROR
Tests cannot delete non-existing image ... ok
Test a simple file backend retrieval works as expected ... ERROR
Tests that the detailed info about public images retu...

=== modified file 'doc/source/installing.rst'
--- doc/source/installing.rst	2011-02-04 23:59:52 +0000
+++ doc/source/installing.rst	2011-03-07 20:15:11 +0000
@@ -73,7 +73,7 @@
 
 1. Install Bazaar and build dependencies::
 
-   $> sudo apt-get install bzr python-eventlet python-routes python-greenlet
+   $> sudo apt-get install bzr python-eventlet python-routes python-greenlet swift
    $> sudo apt-get install python-argparse python-sqlalchemy python-wsgiref python-pastedeploy
 
 .. note::

=== modified file 'etc/glance.conf.sample'
--- etc/glance.conf.sample	2011-02-11 00:12:51 +0000
+++ etc/glance.conf.sample	2011-03-07 20:15:11 +0000
@@ -8,10 +8,6 @@
 [app:glance-api]
 paste.app_factory = glance.server:app_factory
 
-# Directory that the Filesystem backend store
-# writes image data to
-filesystem_store_datadir=/var/lib/glance/images/
-
 # Which backend store should Glance use by default is not specified
 # in a request to add a new image to Glance? Default: 'file'
 # Available choices are 'file', 'swift', and 's3'
@@ -29,6 +25,33 @@
 # Port the registry server is listening on
 registry_port = 9191
 
+# ============ Filesystem Store Options ========================
+
+# Directory that the Filesystem backend store
+# writes image data to
+filesystem_store_datadir=/var/lib/glance/images/
+
+# ============ Swift Store Options =============================
+
+# Address where the Swift authentication service lives
+# The auth address should be in the form:
+#   <DOMAIN>[:<PORT>]/<VERSION>/<ACCOUNT>
+swift_store_auth_address = 127.0.0.1:8080/v1.0/glance-account
+
+# User to authenticate against the Swift authentication service
+swift_store_user = jdoe
+
+# Auth key for the user authenticating against the
+# Swift authentication service
+swift_store_key = a86850deb2742ec3cb41518e26aa2d89
+
+# Container within the account that the account should use
+# for storing images in Swift
+swift_store_container = glance
+
+# Do we create the container if it does not exist?
+swift_store_create_container_on_put = False
+
 [app:glance-registry]
 paste.app_factory = glance.registry.server:app_factory
 

=== modified file 'glance/server.py'
--- glance/server.py	2011-03-05 16:04:32 +0000
+++ glance/server.py	2011-03-07 20:15:11 +0000
@@ -154,7 +154,8 @@
 
         def image_iterator():
             chunks = get_from_backend(image['location'],
-                                      expected_size=image['size'])
+                                      expected_size=image['size'],
+                                      options=self.options)
 
             for chunk in chunks:
                 yield chunk

=== modified file 'glance/store/filesystem.py'
--- glance/store/filesystem.py	2011-01-28 21:54:34 +0000
+++ glance/store/filesystem.py	2011-03-07 20:15:11 +0000
@@ -19,12 +19,15 @@
 A simple filesystem-backed store
 """
 
+import logging
 import os
 import urlparse
 
 from glance.common import exception
 import glance.store
 
+logger = logging.getLogger('glance.store.filesystem')
+
 
 class ChunkedFile(object):
 
@@ -60,8 +63,7 @@
 
 class FilesystemBackend(glance.store.Backend):
     @classmethod
-    def get(cls, parsed_uri, opener=lambda p: open(p, "rb"),
-            expected_size=None):
+    def get(cls, parsed_uri, expected_size=None, options=None):
         """ Filesystem-based backend
 
         file:///path/to/file.tar.gz.0
@@ -71,6 +73,8 @@
         if not os.path.exists(filepath):
             raise exception.NotFound("Image file %s not found" % filepath)
         else:
+            logger.debug("Found image at %s. Returning in ChunkedFile.",
+                         filepath)
             return ChunkedFile(filepath)
 
     @classmethod
@@ -87,6 +91,7 @@
         fn = parsed_uri.path
         if os.path.exists(fn):
             try:
+                logger.debug("Deleting image at %s", fn)
                 os.unlink(fn)
             except OSError:
                 raise exception.NotAuthorized("You cannot delete file %s" % fn)
@@ -112,6 +117,8 @@
         datadir = options['filesystem_store_datadir']
 
         if not os.path.exists(datadir):
+            logger.info("Directory to write image files does not exist "
+                        "(%s). Creating.", datadir)
             os.makedirs(datadir)
 
         filepath = os.path.join(datadir, str(id))
@@ -129,6 +136,8 @@
                 bytes_written += len(buf)
                 f.write(buf)
 
+        logger.debug("Wrote %(bytes_written)d bytes to %(filepath)s"
+                     % locals())
         return ('file://%s' % filepath, bytes_written)
 
     @classmethod

=== modified file 'glance/store/swift.py'
--- glance/store/swift.py	2011-01-27 04:19:13 +0000
+++ glance/store/swift.py	2011-03-07 20:15:11 +0000
@@ -1,6 +1,6 @@
 # vim: tabstop=4 shiftwidth=4 softtabstop=4
 
-# Copyright 2010 OpenStack, LLC
+# Copyright 2010-2011 OpenStack, LLC
 # All Rights Reserved.
 #
 #    Licensed under the Apache License, Version 2.0 (the "License"); you may
@@ -15,107 +15,264 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+"""Storage backend for SWIFT"""
+
 from __future__ import absolute_import
+
+import httplib
+import logging
+
+from swift.common.client import Connection, ClientException
+
+from glance.common import config
+from glance.common import exception
 import glance.store
 
+DEFAULT_SWIFT_CONTAINER = 'glance'
+
+logger = logging.getLogger('glance.store.swift')
+
 
 class SwiftBackend(glance.store.Backend):
     """
     An implementation of the swift backend adapter.
     """
-    EXAMPLE_URL = "swift://user:password@auth_url/container/file.gz.0"
+    EXAMPLE_URL = "swift://<USER>:<KEY>@<AUTH_ADDRESS>/<CONTAINER>/<FILE>"
 
     CHUNKSIZE = 65536
 
     @classmethod
-    def get(cls, parsed_uri, expected_size, conn_class=None):
+    def get(cls, parsed_uri, expected_size=None, options=None):
         """
         Takes a parsed_uri in the format of:
         swift://user:password@auth_url/container/file.gz.0, connects to the
         swift instance at auth_url and downloads the file. Returns the
         generator resp_body provided by get_object.
         """
-        (user, key, authurl, container, obj) = \
-            cls._parse_swift_tokens(parsed_uri)
+        (user, key, authurl, container, obj) = parse_swift_tokens(parsed_uri)
 
         # TODO(sirp): snet=False for now, however, if the instance of
         # swift we're talking to is within our same region, we should set
         # snet=True
-        connection_class = get_connection_class(conn_class)
-
-        swift_conn = conn_class(
+        swift_conn = Connection(
             authurl=authurl, user=user, key=key, snet=False)
 
-        (resp_headers, resp_body) = swift_conn.get_object(
-            container=container, obj=obj, resp_chunk_size=cls.CHUNKSIZE)
+        try:
+            (resp_headers, resp_body) = swift_conn.get_object(
+                container=container, obj=obj, resp_chunk_size=cls.CHUNKSIZE)
+        except ClientException, e:
+            if e.http_status == httplib.NOT_FOUND:
+                location = format_swift_location(user, key, authurl,
+                                                 container, obj)
+                raise exception.NotFound("Swift could not find image at "
+                                         "location %(location)s" % locals())
 
-        obj_size = int(resp_headers['content-length'])
-        if  obj_size != expected_size:
-            raise glance.store.BackendException(
-                "Expected %s byte file, Swift has %s bytes" %
-                (expected_size, obj_size))
+        if expected_size:
+            obj_size = int(resp_headers['content-length'])
+            if  obj_size != expected_size:
+                raise glance.store.BackendException(
+                    "Expected %s byte file, Swift has %s bytes" %
+                    (expected_size, obj_size))
 
         return resp_body
 
     @classmethod
-    def delete(cls, parsed_uri, conn_class=None):
+    def add(cls, id, data, options):
+        """
+        Stores image data to Swift and returns a location that the image was
+        written to.
+
+        Swift writes the image data using the scheme:
+            ``swift://<USER>:<KEY>@<AUTH_ADDRESS>/<CONTAINER>/<ID>`
+        where:
+            <USER> = ``swift_store_user``
+            <KEY> = ``swift_store_key``
+            <AUTH_ADDRESS> = ``swift_store_auth_address``
+            <CONTAINER> = ``swift_store_container``
+            <ID> = The id of the image being added
+
+        :param id: The opaque image identifier
+        :param data: The image data to write, as a file-like object
+        :param options: Conf mapping
+
+        :retval Tuple with (location, size)
+                The location that was written,
+                and the size in bytes of the data written
+        """
+        container = options.get('swift_store_container',
+                                DEFAULT_SWIFT_CONTAINER)
+        auth_address = options.get('swift_store_auth_address')
+        user = options.get('swift_store_user')
+        key = options.get('swift_store_key')
+
+        # TODO(jaypipes): This needs to be checked every time
+        # because of the decision to make glance.store.Backend's
+        # interface all @classmethods. This is inefficient. Backend
+        # should be a stateful object with options parsed once in
+        # a constructor.
+        if not auth_address:
+            logger.error(msg)
+            msg = ("Could not find swift_store_auth_address in configuration "
+                   "options.")
+            raise glance.store.BackendException(msg)
+        else:
+            full_auth_address = auth_address
+            if not full_auth_address.startswith('http'):
+                full_auth_address = 'https://' + full_auth_address
+
+        if not user:
+            logger.error(msg)
+            msg = ("Could not find swift_store_user in configuration "
+                   "options.")
+            raise glance.store.BackendException(msg)
+
+        if not key:
+            logger.error(msg)
+            msg = ("Could not find swift_store_key in configuration "
+                   "options.")
+            raise glance.store.BackendException(msg)
+
+        swift_conn = Connection(authurl=full_auth_address, user=user,
+                                      key=key, snet=False)
+
+        logger.debug("Adding image object to Swift using "
+                     "(auth_address=%(auth_address)s, user=%(user)s, "
+                     "key=%(key)s)" % locals())
+
+        create_container_if_missing(container, swift_conn, options)
+
+        obj_name = str(id)
+        location = format_swift_location(user, key, auth_address,
+                                         container, obj_name)
+        try:
+            obj_etag = swift_conn.put_object(container, obj_name, data)
+
+            # NOTE: We return the user and key here! Have to because
+            # location is used by the API server to return the actual
+            # image data. We *really* should consider NOT returning
+            # the location attribute from GET /images/<ID> and
+            # GET /images/details
+
+            # We do a HEAD on the newly-added image to determine the size
+            # of the image. A bit slow, but better than taking the word
+            # of the user adding the image with size attribute in the metadata
+            resp_headers = swift_conn.head_object(container, obj_name)
+            size = 0
+            # header keys are lowercased by Swift
+            if 'content-length' in resp_headers:
+                size = int(resp_headers['content-length'])
+            return (location, size)
+        except ClientException, e:
+            if e.http_status == httplib.CONFLICT:
+                raise exception.Duplicate("Swift already has an image at "
+                                          "location %(location)s" % locals())
+            msg = ("Failed to add object to Swift.\n"
+                   "Got error from Swift: %(e)s" % locals())
+            raise glance.store.BackendException(msg)
+
+    @classmethod
+    def delete(cls, parsed_uri):
         """
         Deletes the swift object(s) at the parsed_uri location
         """
-        (user, key, authurl, container, obj) = \
-            cls._parse_swift_tokens(parsed_uri)
+        (user, key, authurl, container, obj) = parse_swift_tokens(parsed_uri)
 
         # TODO(sirp): snet=False for now, however, if the instance of
         # swift we're talking to is within our same region, we should set
         # snet=True
-        connection_class = get_connection_class(conn_class)
-
-        swift_conn = conn_class(
+        swift_conn = Connection(
             authurl=authurl, user=user, key=key, snet=False)
 
-        (resp_headers, resp_body) = swift_conn.delete_object(
-            container=container, obj=obj)
-
-        # TODO(jaypipes): What to return here?  After reading the docs
-        # at swift.common.client, I'm not sure what to check for...
-
-    @classmethod
-    def _parse_swift_tokens(cls, parsed_uri):
-        """
-        Parsing the swift uri is three phases:
-            1) urlparse to split the tokens
-            2) use RE to split on @ and /
-            3) reassemble authurl
-        """
-        path = parsed_uri.path.lstrip('//')
-        netloc = parsed_uri.netloc
-
-        try:
-            try:
-                creds, netloc = netloc.split('@')
-                path = '/'.join([netloc, path])
-            except ValueError:
-                # Python 2.6.1 compat
-                # see lp659445 and Python issue7904
-                creds, path = path.split('@')
-
-            user, key = creds.split(':')
-            path_parts = path.split('/')
-            obj = path_parts.pop()
-            container = path_parts.pop()
-        except (ValueError, IndexError):
-            raise glance.store.BackendException(
-                 "Expected four values to unpack in: swift:%s. "
-                 "Should have received something like: %s."
-                 % (parsed_uri.path, cls.EXAMPLE_URL))
-
-        authurl = "https://%s" % '/'.join(path_parts)
-
-        return user, key, authurl, container, obj
-
-
-def get_connection_class(conn_class):
-    if not conn_class:
-        import swift.common.client
-        conn_class = swift.common.client.Connection
-    return conn_class
+        try:
+            swift_conn.delete_object(container, obj)
+        except ClientException, e:
+            if e.http_status == httplib.NOT_FOUND:
+                location = format_swift_location(user, key, authurl,
+                                                 container, obj)
+                raise exception.NotFound("Swift could not find image at "
+                                         "location %(location)s" % locals())
+            else:
+                raise
+
+
+def parse_swift_tokens(parsed_uri):
+    """
+    Return the various tokens used by Swift.
+
+    :param parsed_uri: The pieces of a URI returned by urlparse
+    :retval A tuple of (user, key, auth_address, container, obj_name)
+    """
+    path = parsed_uri.path.lstrip('//')
+    netloc = parsed_uri.netloc
+
+    try:
+        try:
+            creds, netloc = netloc.split('@')
+            path = '/'.join([netloc, path])
+        except ValueError:
+            # Python 2.6.1 compat
+            # see lp659445 and Python issue7904
+            creds, path = path.split('@')
+
+        user, key = creds.split(':')
+        path_parts = path.split('/')
+        obj = path_parts.pop()
+        container = path_parts.pop()
+    except (ValueError, IndexError):
+        raise glance.store.BackendException(
+             "Expected four values to unpack in: swift:%s. "
+             "Should have received something like: %s."
+             % (parsed_uri.path, SwiftBackend.EXAMPLE_URL))
+
+    authurl = "https://%s" % '/'.join(path_parts)
+
+    return user, key, authurl, container, obj
+
+
+def format_swift_location(user, key, auth_address, container, obj_name):
+    """
+    Returns the swift URI in the format:
+        swift://<USER>:<KEY>@<AUTH_ADDRESS>/<CONTAINER>/<OBJNAME>
+
+    :param user: The swift user to authenticate with
+    :param key: The auth key for the authenticating user
+    :param auth_address: The base URL for the authentication service
+    :param container: The name of the container
+    :param obj_name: The name of the object
+    """
+    return "swift://%(user)s:%(key)s@%(auth_address)s/"\
+           "%(container)s/%(obj_name)s" % locals()
+
+
+def create_container_if_missing(container, swift_conn, options):
+    """
+    Creates a missing container in Swift if the
+    ``swift_store_create_container_on_put`` option is set.
+
+    :param container: Name of container to create
+    :param swift_conn: Connection to Swift
+    :param options: Option mapping
+    """
+    try:
+        swift_conn.head_container(container)
+    except ClientException, e:
+        if e.http_status == httplib.NOT_FOUND:
+            add_container = config.get_option(options,
+                                'swift_store_create_container_on_put',
+                                type='bool', default=False)
+            if add_container:
+                try:
+                    swift_conn.put_container(container)
+                except ClientException, e:
+                    msg = ("Failed to add container to Swift.\n"
+                           "Got error from Swift: %(e)s" % locals())
+                    raise glance.store.BackendException(msg)
+            else:
+                msg = ("The container %(container)s does not exist in "
+                       "Swift. Please set the "
+                       "swift_store_create_container_on_put option"
+                       "to add container to Swift automatically."
+                       % locals())
+                raise glance.store.BackendException(msg)
+        else:
+            raise

=== modified file 'tests/stubs.py'
--- tests/stubs.py	2011-02-25 14:55:26 +0000
+++ tests/stubs.py	2011-03-07 20:15:11 +0000
@@ -33,7 +33,6 @@
 import glance.store
 import glance.store.filesystem
 import glance.store.http
-import glance.store.swift
 import glance.registry.db.api
 
 
@@ -109,7 +108,7 @@
 def stub_out_s3_backend(stubs):
     """ Stubs out the S3 Backend with fake data and calls.
 
-    The stubbed swift backend provides back an iterator over
+    The stubbed s3 backend provides back an iterator over
     the data ""
 
     :param stubs: Set of stubout stubs
@@ -139,78 +138,9 @@
                     yield cls.DATA[i:i + cls.CHUNK_SIZE]
             return chunk_it()
 
-    fake_swift_backend = FakeS3Backend()
+    fake_s3_backend = FakeS3Backend()
     stubs.Set(glance.store.s3.S3Backend, 'get',
-              fake_swift_backend.get)
-
-
-def stub_out_swift_backend(stubs):
-    """Stubs out the Swift Glance backend with fake data
-    and calls.
-
-    The stubbed swift backend provides back an iterator over
-    the data "I am a teapot, short and stout\n"
-
-    :param stubs: Set of stubout stubs
-
-    """
-    class FakeSwiftAuth(object):
-        pass
-
-    class FakeSwiftConnection(object):
-        pass
-
-    class FakeSwiftBackend(object):
-
-        CHUNK_SIZE = 2
-        DATA = 'I am a teapot, short and stout\n'
-
-        @classmethod
-        def get(cls, parsed_uri, expected_size, conn_class=None):
-            SwiftBackend = glance.store.swift.SwiftBackend
-
-            # raise BackendException if URI is bad.
-            (user, key, authurl, container, obj) = \
-                SwiftBackend._parse_swift_tokens(parsed_uri)
-
-            def chunk_it():
-                for i in xrange(0, len(cls.DATA), cls.CHUNK_SIZE):
-                    yield cls.DATA[i:i + cls.CHUNK_SIZE]
-
-            return chunk_it()
-
-    fake_swift_backend = FakeSwiftBackend()
-    stubs.Set(glance.store.swift.SwiftBackend, 'get',
-              fake_swift_backend.get)
-
-
-def stub_out_registry(stubs):
-    """Stubs out the Registry registry with fake data returns.
-
-    The stubbed Registry always returns the following fixture::
-
-        {'files': [
-          {'location': 'file:///chunk0', 'size': 12345},
-          {'location': 'file:///chunk1', 'size': 1235}
-        ]}
-
-    :param stubs: Set of stubout stubs
-
-    """
-    class FakeRegistry(object):
-
-        DATA = \
-            {'files': [
-              {'location': 'file:///chunk0', 'size': 12345},
-              {'location': 'file:///chunk1', 'size': 1235}]}
-
-        @classmethod
-        def lookup(cls, _parsed_uri):
-            return cls.DATA
-
-    fake_registry_registry = FakeRegistry()
-    stubs.Set(glance.store.registries.Registry, 'lookup',
-              fake_registry_registry.lookup)
+              fake_s3_backend.get)
 
 
 def stub_out_registry_and_store_server(stubs):

=== removed file 'tests/unit/swiftfakehttp.py'
--- tests/unit/swiftfakehttp.py	2011-01-27 04:19:13 +0000
+++ tests/unit/swiftfakehttp.py	1970-01-01 00:00:00 +0000
@@ -1,294 +0,0 @@
-# vim: tabstop=4 shiftwidth=4 softtabstop=4
-
-# Copyright 2010 OpenStack, LLC
-# All Rights Reserved.
-#
-#    Licensed under the Apache License, Version 2.0 (the "License"); you may
-#    not use this file except in compliance with the License. You may obtain
-#    a copy of the License at
-#
-#         http://www.apache.org/licenses/LICENSE-2.0
-#
-#    Unless required by applicable law or agreed to in writing, software
-#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
-#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
-#    License for the specific language governing permissions and limitations
-#    under the License.
-
-"""
-fakehttp/socket implementation
-
-- TrackerSocket: an object which masquerades as a socket and responds to
-  requests in a manner consistent with a *very* stupid CloudFS tracker.
-
-- CustomHTTPConnection: an object which subclasses httplib.HTTPConnection
-  in order to replace it's socket with a TrackerSocket instance.
-
-The unittests each have setup methods which create freerange connection
-instances that have had their HTTPConnection instances replaced by
-intances of CustomHTTPConnection.
-"""
-
-from httplib import HTTPConnection as connbase
-import StringIO
-
-
-class FakeSocket(object):
-    def __init__(self):
-        self._rbuffer = StringIO.StringIO()
-        self._wbuffer = StringIO.StringIO()
-
-    def close(self):
-        pass
-
-    def send(self, data, flags=0):
-        self._rbuffer.write(data)
-    sendall = send
-
-    def recv(self, len=1024, flags=0):
-        return self._wbuffer(len)
-
-    def connect(self):
-        pass
-
-    def makefile(self, mode, flags):
-        return self._wbuffer
-
-
-class TrackerSocket(FakeSocket):
-    def write(self, data):
-        self._wbuffer.write(data)
-
-    def read(self, length=-1):
-        return self._rbuffer.read(length)
-
-    def _create_GET_account_content(self, path, args):
-        if 'format' in args and args['format'] == 'json':
-            containers = []
-            containers.append('[\n')
-            containers.append('{"name":"container1","count":2,"bytes":78},\n')
-            containers.append('{"name":"container2","count":1,"bytes":39},\n')
-            containers.append('{"name":"container3","count":3,"bytes":117}\n')
-            containers.append(']\n')
-        elif 'format' in args and args['format'] == 'xml':
-            containers = []
-            containers.append('<?xml version="1.0" encoding="UTF-8"?>\n')
-            containers.append('<account name="FakeAccount">\n')
-            containers.append('<container><name>container1</name>'
-                              '<count>2</count>'
-                              '<bytes>78</bytes></container>\n')
-            containers.append('<container><name>container2</name>'
-                              '<count>1</count>'
-                              '<bytes>39</bytes></container>\n')
-            containers.append('<container><name>container3</name>'
-                              '<count>3</count>'
-                              '<bytes>117</bytes></container>\n')
-            containers.append('</account>\n')
-        else:
-            containers = ['container%s\n' % i for i in range(1, 4)]
-        return ''.join(containers)
-
-    def _create_GET_container_content(self, path, args):
-        left = 0
-        right = 9
-        if 'offset' in args:
-            left = int(args['offset'])
-        if 'limit' in args:
-            right = left + int(args['limit'])
-
-        if 'format' in args and args['format'] == 'json':
-            objects = []
-            objects.append('{"name":"object1",'
-                           '"hash":"4281c348eaf83e70ddce0e07221c3d28",'
-                           '"bytes":14,'
-                           '"content_type":"application\/octet-stream",'
-                           '"last_modified":"2007-03-04 20:32:17"}')
-            objects.append('{"name":"object2",'
-                           '"hash":"b039efe731ad111bc1b0ef221c3849d0",'
-                           '"bytes":64,'
-                           '"content_type":"application\/octet-stream",'
-                           '"last_modified":"2007-03-04 20:32:17"}')
-            objects.append('{"name":"object3",'
-                           '"hash":"4281c348eaf83e70ddce0e07221c3d28",'
-                           '"bytes":14,'
-                           '"content_type":"application\/octet-stream",'
-                           '"last_modified":"2007-03-04 20:32:17"}')
-            objects.append('{"name":"object4",'
-                           '"hash":"b039efe731ad111bc1b0ef221c3849d0",'
-                           '"bytes":64,'
-                           '"content_type":"application\/octet-stream",'
-                           '"last_modified":"2007-03-04 20:32:17"}')
-            objects.append('{"name":"object5",'
-                           '"hash":"4281c348eaf83e70ddce0e07221c3d28",'
-                           '"bytes":14,'
-                           '"content_type":"application\/octet-stream",'
-                           '"last_modified":"2007-03-04 20:32:17"}')
-            objects.append('{"name":"object6",'
-                           '"hash":"b039efe731ad111bc1b0ef221c3849d0",'
-                           '"bytes":64,'
-                           '"content_type":"application\/octet-stream",'
-                           '"last_modified":"2007-03-04 20:32:17"}')
-            objects.append('{"name":"object7",'
-                           '"hash":"4281c348eaf83e70ddce0e07221c3d28",'
-                           '"bytes":14,'
-                           '"content_type":"application\/octet-stream",'
-                           '"last_modified":"2007-03-04 20:32:17"}')
-            objects.append('{"name":"object8",'
-                           '"hash":"b039efe731ad111bc1b0ef221c3849d0",'
-                           '"bytes":64,'
-                           '"content_type":"application\/octet-stream",'
-                           '"last_modified":"2007-03-04 20:32:17"}')
-            output = '[\n%s\n]\n' % (',\n'.join(objects[left:right]))
-        elif 'format' in args and args['format'] == 'xml':
-            objects = []
-            objects.append('<object><name>object1</name>'
-                       '<hash>4281c348eaf83e70ddce0e07221c3d28</hash>'
-                       '<bytes>14</bytes>'
-                       '<content_type>application/octet-stream</content_type>'
-                       '<last_modified>2007-03-04 20:32:17</last_modified>'
-                       '</object>\n')
-            objects.append('<object><name>object2</name>'
-                       '<hash>b039efe731ad111bc1b0ef221c3849d0</hash>'
-                       '<bytes>64</bytes>'
-                       '<content_type>application/octet-stream</content_type>'
-                       '<last_modified>2007-03-04 20:32:17</last_modified>'
-                       '</object>\n')
-            objects.append('<object><name>object3</name>'
-                       '<hash>4281c348eaf83e70ddce0e07221c3d28</hash>'
-                       '<bytes>14</bytes>'
-                       '<content_type>application/octet-stream</content_type>'
-                       '<last_modified>2007-03-04 20:32:17</last_modified>'
-                       '</object>\n')
-            objects.append('<object><name>object4</name>'
-                       '<hash>b039efe731ad111bc1b0ef221c3849d0</hash>'
-                       '<bytes>64</bytes>'
-                       '<content_type>application/octet-stream</content_type>'
-                       '<last_modified>2007-03-04 20:32:17</last_modified>'
-                       '</object>\n')
-            objects.append('<object><name>object5</name>'
-                       '<hash>4281c348eaf83e70ddce0e07221c3d28</hash>'
-                       '<bytes>14</bytes>'
-                       '<content_type>application/octet-stream</content_type>'
-                       '<last_modified>2007-03-04 20:32:17</last_modified>'
-                       '</object>\n')
-            objects.append('<object><name>object6</name>'
-                       '<hash>b039efe731ad111bc1b0ef221c3849d0</hash>'
-                       '<bytes>64</bytes>'
-                       '<content_type>application/octet-stream</content_type>'
-                       '<last_modified>2007-03-04 20:32:17</last_modified>'
-                       '</object>\n')
-            objects.append('<object><name>object7</name>'
-                       '<hash>4281c348eaf83e70ddce0e07221c3d28</hash>'
-                       '<bytes>14</bytes>'
-                       '<content_type>application/octet-stream</content_type>'
-                       '<last_modified>2007-03-04 20:32:17</last_modified>'
-                       '</object>\n')
-            objects.append('<object><name>object8</name>'
-                       '<hash>b039efe731ad111bc1b0ef221c3849d0</hash>'
-                       '<bytes>64</bytes>'
-                       '<content_type>application/octet-stream</content_type>'
-                       '<last_modified>2007-03-04 20:32:17</last_modified>'
-                       '</object>\n')
-            objects = objects[left:right]
-            objects.insert(0, '<?xml version="1.0" encoding="UTF-8"?>\n')
-            objects.insert(1, '<container name="test_container_1"\n')
-            objects.append('</container>\n')
-            output = ''.join(objects)
-        else:
-            objects = ['object%s\n' % i for i in range(1, 9)]
-            objects = objects[left:right]
-            output = ''.join(objects)
-
-        # prefix/path don't make much sense given our test data
-        if 'prefix' in args or 'path' in args:
-            pass
-        return output
-
-    def render_GET(self, path, args):
-        # Special path that returns 404 Not Found
-        if (len(path) == 4) and (path[3] == 'bogus'):
-            self.write('HTTP/1.1 404 Not Found\n')
-            self.write('Content-Type: text/plain\n')
-            self.write('Content-Length: 0\n')
-            self.write('Connection: close\n\n')
-            return
-
-        self.write('HTTP/1.1 200 Ok\n')
-        self.write('Content-Type: text/plain\n')
-        if len(path) == 2:
-            content = self._create_GET_account_content(path, args)
-        elif len(path) == 3:
-            content = self._create_GET_container_content(path, args)
-        # Object
-        elif len(path) == 4:
-            content = 'I am a teapot, short and stout\n'
-        self.write('Content-Length: %d\n' % len(content))
-        self.write('Connection: close\n\n')
-        self.write(content)
-
-    def render_HEAD(self, path, args):
-        # Account
-        if len(path) == 2:
-            self.write('HTTP/1.1 204 No Content\n')
-            self.write('Content-Type: text/plain\n')
-            self.write('Connection: close\n')
-            self.write('X-Account-Container-Count: 3\n')
-            self.write('X-Account-Bytes-Used: 234\n\n')
-        else:
-            self.write('HTTP/1.1 200 Ok\n')
-            self.write('Content-Type: text/plain\n')
-            self.write('ETag: d5c7f3babf6c602a8da902fb301a9f27\n')
-            self.write('Content-Length: 21\n')
-            self.write('Connection: close\n\n')
-
-    def render_POST(self, path, args):
-        self.write('HTTP/1.1 202 Ok\n')
-        self.write('Connection: close\n\n')
-
-    def render_PUT(self, path, args):
-        self.write('HTTP/1.1 200 Ok\n')
-        self.write('Content-Type: text/plain\n')
-        self.write('Connection: close\n\n')
-    render_DELETE = render_PUT
-
-    def render(self, method, uri):
-        if '?' in uri:
-            parts = uri.split('?')
-            query = parts[1].strip('&').split('&')
-            args = dict([tuple(i.split('=', 1)) for i in query])
-            path = parts[0].strip('/').split('/')
-        else:
-            args = {}
-            path = uri.strip('/').split('/')
-
-        if hasattr(self, 'render_%s' % method):
-            getattr(self, 'render_%s' % method)(path, args)
-        else:
-            self.write('HTTP/1.1 406 Not Acceptable\n')
-            self.write('Content-Type: text/plain\n')
-            self.write('Connection: close\n')
-
-    def makefile(self, mode, flags):
-        self._rbuffer.seek(0)
-        lines = self.read().splitlines()
-        (method, uri, version) = lines[0].split()
-
-        self.render(method, uri)
-
-        self._wbuffer.seek(0)
-        return self._wbuffer
-
-
-class CustomHTTPConnection(connbase):
-    def connect(self):
-        self.sock = TrackerSocket()
-
-
-if __name__ == '__main__':
-    conn = CustomHTTPConnection('localhost', 8000)
-    conn.request('HEAD', '/v1/account/container/object')
-    response = conn.getresponse()
-    print "Status:", response.status, response.reason
-    for (key, value) in response.getheaders():
-        print "%s: %s" % (key, value)
-    print response.read()

=== added file 'tests/unit/test_filesystem_store.py'
--- tests/unit/test_filesystem_store.py	1970-01-01 00:00:00 +0000
+++ tests/unit/test_filesystem_store.py	2011-03-07 20:15:11 +0000
@@ -0,0 +1,135 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2011 OpenStack, LLC
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+"""Tests the filesystem backend store"""
+
+import StringIO
+import unittest
+import urlparse
+
+import stubout
+
+from glance.common import exception
+from glance.store.filesystem import FilesystemBackend, ChunkedFile
+from tests import stubs
+
+
+class TestFilesystemBackend(unittest.TestCase):
+
+    def setUp(self):
+        """Establish a clean test environment"""
+        self.stubs = stubout.StubOutForTesting()
+        stubs.stub_out_filesystem_backend()
+        self.orig_chunksize = ChunkedFile.CHUNKSIZE
+        ChunkedFile.CHUNKSIZE = 10
+
+    def tearDown(self):
+        """Clear the test environment"""
+        stubs.clean_out_fake_filesystem_backend()
+        self.stubs.UnsetAll()
+        ChunkedFile.CHUNKSIZE = self.orig_chunksize
+
+    def test_get(self):
+        """Test a "normal" retrieval of an image in chunks"""
+        url_pieces = urlparse.urlparse("file:///tmp/glance-tests/2")
+        image_file = FilesystemBackend.get(url_pieces)
+
+        expected_data = "chunk00000remainder"
+        expected_num_chunks = 2
+        data = ""
+        num_chunks = 0
+
+        for chunk in image_file:
+            num_chunks += 1
+            data += chunk
+        self.assertEqual(expected_data, data)
+        self.assertEqual(expected_num_chunks, num_chunks)
+
+    def test_get_non_existing(self):
+        """
+        Test that trying to retrieve a file that doesn't exist
+        raises an error
+        """
+        url_pieces = urlparse.urlparse("file:///tmp/glance-tests/non-existing")
+        self.assertRaises(exception.NotFound,
+                          FilesystemBackend.get,
+                          url_pieces)
+
+    def test_add(self):
+        """Test that we can add an image via the filesystem backend"""
+        ChunkedFile.CHUNKSIZE = 1024
+        expected_image_id = 42
+        expected_file_size = 1024 * 5  # 5K
+        expected_file_contents = "*" * expected_file_size
+        expected_location = "file://%s/%s" % (stubs.FAKE_FILESYSTEM_ROOTDIR,
+                                              expected_image_id)
+        image_file = StringIO.StringIO(expected_file_contents)
+        options = {'verbose': True,
+                   'debug': True,
+                   'filesystem_store_datadir': stubs.FAKE_FILESYSTEM_ROOTDIR}
+
+        location, size = FilesystemBackend.add(42, image_file, options)
+
+        self.assertEquals(expected_location, location)
+        self.assertEquals(expected_file_size, size)
+
+        url_pieces = urlparse.urlparse("file:///tmp/glance-tests/42")
+        new_image_file = FilesystemBackend.get(url_pieces)
+        new_image_contents = ""
+        new_image_file_size = 0
+
+        for chunk in new_image_file:
+            new_image_file_size += len(chunk)
+            new_image_contents += chunk
+
+        self.assertEquals(expected_file_contents, new_image_contents)
+        self.assertEquals(expected_file_size, new_image_file_size)
+
+    def test_add_already_existing(self):
+        """
+        Tests that adding an image with an existing identifier
+        raises an appropriate exception
+        """
+        image_file = StringIO.StringIO("nevergonnamakeit")
+        options = {'verbose': True,
+                   'debug': True,
+                   'filesystem_store_datadir': stubs.FAKE_FILESYSTEM_ROOTDIR}
+        self.assertRaises(exception.Duplicate,
+                          FilesystemBackend.add,
+                          2, image_file, options)
+
+    def test_delete(self):
+        """
+        Test we can delete an existing image in the filesystem store
+        """
+        url_pieces = urlparse.urlparse("file:///tmp/glance-tests/2")
+
+        FilesystemBackend.delete(url_pieces)
+
+        self.assertRaises(exception.NotFound,
+                          FilesystemBackend.get,
+                          url_pieces)
+
+    def test_delete_non_existing(self):
+        """
+        Test that trying to delete a file that doesn't exist
+        raises an error
+        """
+        url_pieces = urlparse.urlparse("file:///tmp/glance-tests/non-existing")
+        self.assertRaises(exception.NotFound,
+                          FilesystemBackend.delete,
+                          url_pieces)

=== modified file 'tests/unit/test_stores.py'
--- tests/unit/test_stores.py	2011-01-26 20:47:01 +0000
+++ tests/unit/test_stores.py	2011-03-07 20:15:11 +0000
@@ -22,7 +22,6 @@
 import urlparse
 
 from glance.store.s3 import S3Backend
-from glance.store.swift import SwiftBackend
 from glance.store import Backend, BackendException, get_from_backend
 from tests import stubs
 
@@ -40,27 +39,6 @@
         self.stubs.UnsetAll()
 
 
-class TestFilesystemBackend(TestBackend):
-
-    def setUp(self):
-        """Establish a clean test environment"""
-        stubs.stub_out_filesystem_backend()
-
-    def tearDown(self):
-        """Clear the test environment"""
-        stubs.clean_out_fake_filesystem_backend()
-
-    def test_get(self):
-
-        fetcher = get_from_backend("file:///tmp/glance-tests/2",
-                                   expected_size=19)
-
-        data = ""
-        for chunk in fetcher:
-            data += chunk
-        self.assertEqual(data, "chunk00000remainder")
-
-
 class TestHTTPBackend(TestBackend):
 
     def setUp(self):
@@ -104,45 +82,3 @@
 
         chunks = [c for c in fetcher]
         self.assertEqual(chunks, expected_returns)
-
-
-class TestSwiftBackend(TestBackend):
-    def setUp(self):
-        super(TestSwiftBackend, self).setUp()
-        stubs.stub_out_swift_backend(self.stubs)
-
-    def test_get(self):
-
-        swift_uri = "swift://user:pass@localhost/container1/file.tar.gz"
-        expected_returns = ['I ', 'am', ' a', ' t', 'ea', 'po', 't,', ' s',
-                            'ho', 'rt', ' a', 'nd', ' s', 'to', 'ut', '\n']
-
-        fetcher = get_from_backend(swift_uri,
-                                   expected_size=21,
-                                   conn_class=SwiftBackend)
-
-        chunks = [c for c in fetcher]
-
-        self.assertEqual(chunks, expected_returns)
-
-    def test_get_bad_uri(self):
-
-        swift_url = "swift://localhost/container1/file.tar.gz"
-
-        self.assertRaises(BackendException, get_from_backend,
-                          swift_url, expected_size=21)
-
-    def test_url_parsing(self):
-
-        swift_uri = "swift://user:pass@localhost/v1.0/container1/file.tar.gz"
-
-        parsed_uri = urlparse.urlparse(swift_uri)
-
-        (user, key, authurl, container, obj) = \
-            SwiftBackend._parse_swift_tokens(parsed_uri)
-
-        self.assertEqual(user, 'user')
-        self.assertEqual(key, 'pass')
-        self.assertEqual(authurl, 'https://localhost/v1.0')
-        self.assertEqual(container, 'container1')
-        self.assertEqual(obj, 'file.tar.gz')

=== added file 'tests/unit/test_swift_store.py'
--- tests/unit/test_swift_store.py	1970-01-01 00:00:00 +0000
+++ tests/unit/test_swift_store.py	2011-03-07 20:15:11 +0000
@@ -0,0 +1,300 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2011 OpenStack, LLC
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this swift except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+"""Tests the Swift backend store"""
+
+import StringIO
+import hashlib
+import httplib
+import sys
+import unittest
+import urlparse
+
+import stubout
+import swift.common.client
+
+from glance.common import exception
+from glance.store import BackendException
+from glance.store.swift import SwiftBackend, format_swift_location
+
+FIVE_KB = (5 * 1024)
+SWIFT_OPTIONS = {'verbose': True,
+                 'debug': True,
+                 'swift_store_user': 'user',
+                 'swift_store_key': 'key',
+                 'swift_store_auth_address': 'localhost:8080',
+                 'swift_store_container': 'glance'}
+
+
+# We stub out as little as possible to ensure that the code paths
+# between glance.store.swift and swift.common.client are tested
+# thoroughly
+def stub_out_swift_common_client(stubs):
+
+    fixture_containers = ['glance']
+    fixture_headers = {'glance/2':
+                {'content-length': FIVE_KB,
+                 'etag': 'c2e5db72bd7fd153f53ede5da5a06de3'}}
+    fixture_objects = {'glance/2':
+                       StringIO.StringIO("*" * FIVE_KB)}
+
+    def fake_head_container(url, token, container, **kwargs):
+        if container not in fixture_containers:
+            msg = "No container %s found" % container
+            raise swift.common.client.ClientException(msg,
+                        http_status=httplib.NOT_FOUND)
+
+    def fake_put_container(url, token, container, **kwargs):
+        fixture_containers.append(container)
+
+    def fake_put_object(url, token, container, name, contents, **kwargs):
+        # PUT returns the ETag header for the newly-added object
+        fixture_key = "%s/%s" % (container, name)
+        if not fixture_key in fixture_headers.keys():
+            if hasattr(contents, 'read'):
+                fixture_object = StringIO.StringIO()
+                chunk = contents.read(SwiftBackend.CHUNKSIZE)
+                while chunk:
+                    fixture_object.write(chunk)
+                    chunk = contents.read(SwiftBackend.CHUNKSIZE)
+            else:
+                fixture_object = StringIO.StringIO(contents)
+            fixture_objects[fixture_key] = fixture_object
+            fixture_headers[fixture_key] = {
+                'content-length': fixture_object.len,
+                'etag': hashlib.md5(fixture_object.read()).hexdigest()}
+            return fixture_headers[fixture_key]['etag']
+        else:
+            msg = ("Object PUT failed - Object with key %s already exists"
+                   % fixture_key)
+            raise swift.common.client.ClientException(msg,
+                        http_status=httplib.CONFLICT)
+
+    def fake_get_object(url, token, container, name, **kwargs):
+        # GET returns the tuple (list of headers, file object)
+        try:
+            fixture_key = "%s/%s" % (container, name)
+            return fixture_headers[fixture_key], fixture_objects[fixture_key]
+        except KeyError:
+            msg = "Object GET failed"
+            raise swift.common.client.ClientException(msg,
+                        http_status=httplib.NOT_FOUND)
+
+    def fake_head_object(url, token, container, name, **kwargs):
+        # HEAD returns the list of headers for an object
+        try:
+            fixture_key = "%s/%s" % (container, name)
+            return fixture_headers[fixture_key]
+        except KeyError:
+            msg = "Object HEAD failed - Object does not exist"
+            raise swift.common.client.ClientException(msg,
+                        http_status=httplib.NOT_FOUND)
+
+    def fake_delete_object(url, token, container, name, **kwargs):
+        # DELETE returns nothing
+        fixture_key = "%s/%s" % (container, name)
+        if fixture_key not in fixture_headers.keys():
+            msg = "Object DELETE failed - Object does not exist"
+            raise swift.common.client.ClientException(msg,
+                        http_status=httplib.NOT_FOUND)
+        else:
+            del fixture_headers[fixture_key]
+            del fixture_objects[fixture_key]
+
+    def fake_http_connection(*args, **kwargs):
+        return None
+
+    def fake_get_auth(*args, **kwargs):
+        return None, None
+
+    stubs.Set(swift.common.client,
+              'head_container', fake_head_container)
+    stubs.Set(swift.common.client,
+              'put_container', fake_put_container)
+    stubs.Set(swift.common.client,
+              'put_object', fake_put_object)
+    stubs.Set(swift.common.client,
+              'delete_object', fake_delete_object)
+    stubs.Set(swift.common.client,
+              'head_object', fake_head_object)
+    stubs.Set(swift.common.client,
+              'get_object', fake_get_object)
+    stubs.Set(swift.common.client,
+              'get_auth', fake_get_auth)
+    stubs.Set(swift.common.client,
+              'http_connection', fake_http_connection)
+
+
+class TestSwiftBackend(unittest.TestCase):
+
+    def setUp(self):
+        """Establish a clean test environment"""
+        self.stubs = stubout.StubOutForTesting()
+        stub_out_swift_common_client(self.stubs)
+
+    def tearDown(self):
+        """Clear the test environment"""
+        self.stubs.UnsetAll()
+
+    def test_get(self):
+        """Test a "normal" retrieval of an image in chunks"""
+        url_pieces = urlparse.urlparse(
+            "swift://user:key@auth_address/glance/2")
+        image_swift = SwiftBackend.get(url_pieces)
+
+        expected_data = "*" * FIVE_KB
+        data = ""
+
+        for chunk in image_swift:
+            data += chunk
+        self.assertEqual(expected_data, data)
+
+    def test_get_mismatched_expected_size(self):
+        """
+        Test retrieval of an image with wrong expected_size param
+        raises an exception
+        """
+        url_pieces = urlparse.urlparse(
+            "swift://user:key@auth_address/glance/2")
+        self.assertRaises(BackendException,
+                          SwiftBackend.get,
+                          url_pieces,
+                          {'expected_size': 42})
+
+    def test_get_non_existing(self):
+        """
+        Test that trying to retrieve a swift that doesn't exist
+        raises an error
+        """
+        url_pieces = urlparse.urlparse(
+            "swift://user:key@auth_address/noexist")
+        self.assertRaises(exception.NotFound,
+                          SwiftBackend.get,
+                          url_pieces)
+
+    def test_add(self):
+        """Test that we can add an image via the swift backend"""
+        expected_image_id = 42
+        expected_swift_size = 1024 * 5  # 5K
+        expected_swift_contents = "*" * expected_swift_size
+        expected_location = format_swift_location(
+            SWIFT_OPTIONS['swift_store_user'],
+            SWIFT_OPTIONS['swift_store_key'],
+            SWIFT_OPTIONS['swift_store_auth_address'],
+            SWIFT_OPTIONS['swift_store_container'],
+            expected_image_id)
+        image_swift = StringIO.StringIO(expected_swift_contents)
+
+        location, size = SwiftBackend.add(42, image_swift, SWIFT_OPTIONS)
+
+        self.assertEquals(expected_location, location)
+        self.assertEquals(expected_swift_size, size)
+
+        url_pieces = urlparse.urlparse(expected_location)
+        new_image_swift = SwiftBackend.get(url_pieces)
+        new_image_contents = new_image_swift.getvalue()
+        new_image_swift_size = new_image_swift.len
+
+        self.assertEquals(expected_swift_contents, new_image_contents)
+        self.assertEquals(expected_swift_size, new_image_swift_size)
+
+    def test_add_no_container_no_create(self):
+        """
+        Tests that adding an image with a non-existing container
+        raises an appropriate exception
+        """
+        options = SWIFT_OPTIONS.copy()
+        options['swift_store_create_container_on_put'] = 'False'
+        options['swift_store_container'] = 'noexist'
+        image_swift = StringIO.StringIO("nevergonnamakeit")
+
+        # We check the exception text to ensure the container
+        # missing text is found in it, otherwise, we would have
+        # simply used self.assertRaises here
+        exception_caught = False
+        try:
+            SwiftBackend.add(3, image_swift, options)
+        except BackendException, e:
+            exception_caught = True
+            self.assertTrue("container noexist does not exist "
+                            "in Swift" in str(e))
+        self.assertTrue(exception_caught)
+
+    def test_add_no_container_and_create(self):
+        """
+        Tests that adding an image with a non-existing container
+        creates the container automatically if flag is set
+        """
+        options = SWIFT_OPTIONS.copy()
+        options['swift_store_create_container_on_put'] = 'True'
+        options['swift_store_container'] = 'noexist'
+        expected_image_id = 42
+        expected_swift_size = 1024 * 5  # 5K
+        expected_swift_contents = "*" * expected_swift_size
+        expected_location = format_swift_location(
+            options['swift_store_user'],
+            options['swift_store_key'],
+            options['swift_store_auth_address'],
+            options['swift_store_container'],
+            expected_image_id)
+        image_swift = StringIO.StringIO(expected_swift_contents)
+
+        location, size = SwiftBackend.add(42, image_swift, options)
+
+        self.assertEquals(expected_location, location)
+        self.assertEquals(expected_swift_size, size)
+
+        url_pieces = urlparse.urlparse(expected_location)
+        new_image_swift = SwiftBackend.get(url_pieces)
+        new_image_contents = new_image_swift.getvalue()
+        new_image_swift_size = new_image_swift.len
+
+        self.assertEquals(expected_swift_contents, new_image_contents)
+        self.assertEquals(expected_swift_size, new_image_swift_size)
+
+    def test_add_already_existing(self):
+        """
+        Tests that adding an image with an existing identifier
+        raises an appropriate exception
+        """
+        image_swift = StringIO.StringIO("nevergonnamakeit")
+        self.assertRaises(exception.Duplicate,
+                          SwiftBackend.add,
+                          2, image_swift, SWIFT_OPTIONS)
+
+    def test_delete(self):
+        """
+        Test we can delete an existing image in the swift store
+        """
+        url_pieces = urlparse.urlparse(
+            "swift://user:key@auth_address/glance/2")
+
+        SwiftBackend.delete(url_pieces)
+
+        self.assertRaises(exception.NotFound,
+                          SwiftBackend.get,
+                          url_pieces)
+
+    def test_delete_non_existing(self):
+        """
+        Test that trying to delete a swift that doesn't exist
+        raises an error
+        """
+        url_pieces = urlparse.urlparse("swift://user:key@auth_address/noexist")
+        self.assertRaises(exception.NotFound,
+                          SwiftBackend.delete,
+                          url_pieces)

=== modified file 'tools/pip-requires'
--- tools/pip-requires	2011-02-05 00:05:23 +0000
+++ tools/pip-requires	2011-03-07 20:15:11 +0000
@@ -12,5 +12,6 @@
 sphinx
 argparse
 mox==0.5.0
+swift
 -f http://pymox.googlecode.com/files/mox-0.5.0.tar.gz
 sqlalchemy-migrate>=0.6