clalancette / oz

Automated installation for guest images
GNU Lesser General Public License v2.1
310 stars 129 forks source link

RFE: stable Python API #33

Closed tomassedovic closed 11 years ago

tomassedovic commented 11 years ago

It would be really nice if Oz exposed a reasonably stable API so that Python programs didn't have to shell out to build images.

It's already possible to import oz.TDL and oz.GuestFactory and do the work. In fact, we're doing just that in heat-jeos. However, since this is not documented anywhere, I suspect we're using internals that could change any time.

So, if it's safe and expected to import oz and use it that way, it should probably be explicitly mentioned in the readme or the wiki. If not, would you be opposed to providing such an API? I'll be happy to help.

clalancette commented 11 years ago

On Sun, Oct 7, 2012 at 5:48 PM, Tomas Sedovic notifications@github.comwrote:

It would be really nice if Oz exposed a reasonably stable API so that Python programs didn't have to shell out to build images.

It's already possible to import oz.TDL and oz.GuestFactory and do the work. In fact, we're doing just that in heat-jeoshttps://github.com/heat-api/heat-jeos. However, since this is not documented anywhere, I suspect we're using internals that could change any time.

So, if it's safe and expected to import oz and use it that way, it should probably be explicitly mentioned in the readme or the wiki. If not, would you be opposed to providing such an API? I'll be happy to help.

Hey Tomas, I would absolutely love to get to a point where we have a stable API in Oz. This would help out imagefactory as well as heat, so I think it would be a good direction to go in. As it stands, I did have in mind an external, stable API for Oz when I originally wrote it. Since I didn't really have any users of it, that API is pretty limited and probably not that useful. I would definitely accept patches to a) document the existing "stable" interface, and b) create an more useful API we can use going forward. To start with, what kind of operations are you doing that you think might be using internals? And do you have any ideas for improving the API?

Thanks, Chris

tomassedovic commented 11 years ago

Thanks for the reply, Chris.

Pretty much all we're doing now is creating/updating a TDL XML and building an image from it. We then take the image path, and upload it to OpenStack. So it goes something like this:

tdl = oz.TDL.TDL(tdl_xml)
config = ozutil.parse_config(None)
guest = oz.GuestFactory.guest_factory(tdl, config, None, None)
image_path = guest.diskimage
image_name = guest.name
guest.check_for_guest_conflict()
try:
    force_download = False
    guest.generate_install_media(force_download)
    try:
        guest.generate_diskimage(force=force_download)
        libvirt_xml = guest.install(50000, force_download)
    except:
        guest.cleanup_old_guest()
        raise
finally:
    guest.cleanup_install()

guest.customize(libvirt_xml)
return image_path, image_name

What we mostly need is the path to the built image and a way to build it by passing our own TDL XML. Essentially, running oz-install -t 50000 -u /path/to/tdl using the default config.

So having something like:

client = oz.Client()
client.install(tdl_xml)
print client.libvirt_xml
client.customize()
print client.image_path

would be neat, but mostly we just want to be sure we're not calling something that's can change on us any time. Do you see any issue with what we use now?

clalancette commented 11 years ago

On Tue, Oct 9, 2012 at 11:34 AM, Tomas Sedovic notifications@github.comwrote:

Thanks for the reply, Chris.

Pretty much all we're doing now is creating/updating a TDL XML and building an image from it. We then take the image path, and upload it to OpenStack. So it goes something like this:

tdl = oz.TDL.TDL(tdl_xml) config = ozutil.parse_config(None) guest = oz.GuestFactory.guest_factory(tdl, config, None, None) image_path = guest.diskimage image_name = guest.name guest.check_for_guest_conflict() try: force_download = False guest.generate_install_media(force_download) try: guest.generate_diskimage(force=force_download) libvirt_xml = guest.install(50000, force_download) except: guest.cleanup_old_guest() raise finally: guest.cleanup_install()

guest.customize(libvirt_xml) return image_path, image_name

What we mostly need is the path to the built image and a way to build it by passing our own TDL XML. Essentially, running oz-install -t 50000 -u /path/to/tdl using the default config.

So having something like:

client = oz.Client() client.install(tdl_xml) print client.libvirt_xml client.customize() print client.image_path

would be neat, but mostly we just want to be sure we're not calling something that's can change on us any time. Do you see any issue with what we use now?

Thanks for the detailed explanation. The good news is that most of what you are currently doing is fully supported, and that API is not intended to change. The only exception to this is reaching into the guest object for the guest.diskimage and guest.name, as those are really meant to be internal variables.

As a separate note, I kind of like your proposed API change. The API we currently have is a bit organic and basically grew as I implemented Oz. It works, but it is a bit clunky and it would be nice to hide some of those details from the user. What you propose above would need to be expanded a bit to accommodate all of the features, but I certainly think we can do better than what we have right now.

So I see three things we can do here: 1) Document the current supported API. That would be basically what you are doing above, plus a couple of other calls that you aren't using (but that are supported). 2) Figure out a better way to return the guest.name and guest.diskimage to you. I might suggest explicit methods guest.name() and guest.diskimage(); at least it will be clear when making changes that those need to stay the same (I'm open to other suggestions here). 3) Think about cleaning up the API. This may be slightly challenging, since I don't want to totally break my current users. But if we can figure out a way to keep the current API, deprecated it, and add a new API alongside it, I would be all for that.

Let me know what you think about this plan.

Chris

tomassedovic commented 11 years ago

Thanks a lot. I think this makes a lot of sense. It would be good to have a cleaner API but that's probably going to take a while and in the interim, documenting the current API and adding guest.name() and guest.diskimage() methods would be perfect.

I don't have a very detailed knowledge of Oz but I can definitely start documenting the parts I'm familiar with. What would be the best way to do that? The wiki? The man pages?

clalancette commented 11 years ago

On Wed, Oct 10, 2012 at 10:00 AM, Tomas Sedovic notifications@github.comwrote:

Thanks a lot. I think this makes a lot of sense. It would be good to have a cleaner API but that's probably going to take a while and in the interim, documenting the current API and adding guest.name() and guest.diskimage()methods would be perfect.

I don't have a very detailed knowledge of Oz but I can definitely start documenting the parts I'm familiar with. What would be the best way to do that? The wiki? The man pages?

Definitely update the wiki[1]. All of the methods that you are talking about are documented in the pydoc stuff, but it isn't very clear what is internal and external. It also might be a good idea to shore up the documentation in init.py.

Beyond that, a couple of simple patches to add the methods for guest.name() and guest.diskimage() should do it. Let me know if you have more questions or need help with anything.

Chris

[1] I keep meaning to send a patch to Justin to get rid of the Oz stuff from the Aeolus website and just put a pointer to the github wiki. Maybe I'll get to it this weekend.

clalancette commented 11 years ago

We are done with this for now, so I'll close out this issue.