zerovm / zpm

ZeroVM Package Manager
Apache License 2.0
6 stars 11 forks source link

Fix system.map handling #165

Closed larsbutler closed 10 years ago

larsbutler commented 10 years ago

Zapp creation, bundling, and execution was pretty much broken, evidently due to d0325d7ac043a0a3ae62541ec78daccc9cce66ab.

Exceptions like this would get thrown:

~/proj/hello  $ ~/proj/zpm/zpm bundle && ~/proj/zpm/zpm deploy -l debug -V 2.0 foo hello.zapp --execute
created hello.zapp
INFO:zpmlib.commands: deploying hello.zapp
No handlers could be found for logger "keystoneclient.session"
INFO:zpmlib.zpm: Container 'foo' not found. Creating it...
Traceback (most recent call last):
  File "/Users/lars/proj/zpm/zpm", line 29, in <module>
    args.func(args)
  File "/Users/lars/proj/zpm/zpmlib/commands.py", line 94, in inner
    return func(namespace, *args, **kwargs)
  File "/Users/lars/proj/zpm/zpmlib/commands.py", line 236, in deploy
    zpm.deploy_project(args)
  File "/Users/lars/proj/zpm/zpmlib/zpm.py", line 521, in deploy_project
    deploy_index = _deploy_zapp(conn, args.target, args.zapp, auth_opts)
  File "/Users/lars/proj/zpm/zpmlib/zpm.py", line 448, in _deploy_zapp
    for path, data, content_type in uploads:
  File "/Users/lars/proj/zpm/zpmlib/zpm.py", line 465, in _generate_uploads
    job = _prepare_job(tar, zapp_config, swift_url)
  File "/Users/lars/proj/zpm/zpmlib/zpm.py", line 236, in _prepare_job
    fp = tar.extractfile('%s.json' % zapp['meta']['name'])
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/tarfile.py", line 2110, in extractfile
    tarinfo = self.getmember(member)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/tarfile.py", line 1794, in getmember
    raise KeyError("filename %r not found" % name)
KeyError: "filename 'hello.json' not found"
pkit commented 10 years ago

Needed to write tests for that, but it's unclear how to mock zerocloud here...

larsbutler commented 10 years ago

@pkit We could write some integration tests that do everything: new -> bundle -> deploy -> execute. It shouldn't be too hard to do, assuming we just mock out the HTTP requests. Obviously that won't catch errors that might happen in the real world (or if ZeroCloud/Swift change something), but these kind of tests would catch bugs like this.

mgeisler commented 10 years ago

Could we try and review things better? I'm not surprised that renaming a central file like this causes trouble. Also, I have written a zap (the test registry) that relies on finding the JSON file under the old name -- @pkit did you take things like that into account?

larsbutler commented 10 years ago

Made an issue to make better integration tests: https://github.com/zerovm/zpm/issues/166

pkit commented 10 years ago

I did not take it into account, because it goes against image format supported by zerocloud. :) Most of the image-based zerocloud functionality relies on tar having special header, and that header is the boot/system.map file. You can insert any other json file anywhere else, but to be cooperative with server side it's better to have system.map in place

mgeisler commented 10 years ago

Constantine Peresypkin notifications@github.com writes:

I did not take it into account, because it goes against image format supported by zerocloud. :)

As you probably know now, we didn't use the supported format until now. We always uploaded the tarball and POSTed a job description afterwards.

Most of the image-based zerocloud functionality relies on tar having special header, and that header is the boot/system.map file. You can insert any other json file anywhere else, but to be cooperative with server side it's better to have system.map in place

Yes, I see that now and I like the new name. I'm just saying that we need to get into a habit of thinking about these things. The specifics of this case is not super important, but the general mindset is.