[openstack-dev] [python-openstacksdk][shade] Reviewing the merge-shade patches

Monty Taylor mordred at inaugust.com
Mon Nov 13 16:30:10 UTC 2017


On 11/13/2017 09:06 AM, David Shrewsbury wrote:
> Hi!
> 
> On Mon, Nov 13, 2017 at 9:54 AM, Monty Taylor <mordred at inaugust.com 
> <mailto:mordred at inaugust.com>> wrote:
> 
>     Hey everybody,
> 
>     You may have noticed a giant patch series up:
> 
>     https://review.openstack.org/#/q/topic:merge-shade
>     <https://review.openstack.org/#/q/topic:merge-shade>
> 
> <snip>
> 
> One thing I don't see covered here is the current set of Ansible module 
> tests. Unless I've missed it,
> I don't see any migration of those, nor any reference of the plan for 
> them in the future. I know that
> we were waiting for Zuulv3 to do some cool github integration things. 
> And since the modules import
> shade, not this code, we won't be breaking them. But, what's the plan 
> for that?

That's a great question. I actually did add the ansible functional tests 
to the "add zuulv3 jobs" patch - but as you say, those aren't really 
testing anything yet since the ansible modules do "import shade"

That brings up a few really good things that should be pointed out:

1) The intent is to turn the code in the shade repo into a thin compat 
shim that imports python-openstacksdk and subclasses/wraps the sdk 
object. That'll let us fix some of the interface mistakes we made in 
shade but are stuck with for compat reasons in the sdk version of the 
code ... but can still keep the old defaults/behavior in the shade code. 
For instance ... we've learned that fetching extra_specs on flavors in 
shade was ... stupid. We can have the sdk version NOT fetch by default, 
then in shade do:

     def list_flavors(self, fetch_extra_specs=True):
         return super(OpenStackCloud, self).list_flavors(
             fetch_extra_specs=fetch_extra_specs)

With this in place, it should mean that shade users can continue on 
without being broken.

2) Once we have a release of openstacksdk that has the shade code in a 
place we're happy with, we should update the ansible modules to do 
import openstack instead of import shade

3) Cross-testing shade, os-client-config and openstacksdk is essential, 
so that we can make sure we're not breaking anything as we work on 
compat shims. The same goes with python-openstackclient, but current v3 
and the v4 branch dean is working on. We should probably add a shared 
change queue to the zuul v3 config for each of them. We can't add 
python-openstackclient to that shared queue since I'm pretty sure it's 
in the integrated-gate change queue. We COULD add shade, sdk and occ to 
the integrated-gate queue ... but that might slow us down at the moment, 
so maybe once it's all tied in together like we want it ...

4) openstack.cloud is the wrong home for the shade code. It's just there 
for expedience sake for now (let's not change TOO many things all at 
once) I'm *pretty* sure the shade methods want to just live on the 
Connection object, so that we wind up with:

   conn = openstack.connection.Connection(cloud='example')
   conn.list_servers() # shade version
   conn.compute.servers() # sdk version
   conn.compute.get('/servers') # REST version

We could alternately put it as a sub-resource, but since the shade 
methods are intended to be the 'easy' methods, doing this:

   conn = openstack.connection.Connection(cloud='example')
   conn.cloud.list_servers() # shade version
   conn.compute.servers() # sdk version
   conn.compute.get('/servers') # REST version

feels wrong. Maybe keeping the code in openstack/cloud is fine and just 
making it a mixin class (kind of like how we do in shade today with the 
normalize methods) would allow for some better source-code organization.

I also think that the OpenStackCloud/OperatorCloud split from shade 
wound up being more confusing than helpful.

Finally - while I'm rambling here ... after we finish the Resource -> 
Resource2 migration, I'd love to ponder whether or not we can make 
Resource2 be a subclass of munch.Munch. shade needs to be able to return 
objects that are directly json serializable (so that the ansible layer 
works easily/well) - it would be awesome if we didn't have to shove a 
to_dict() into every return on that layer. I haven't dug in to the magic 
guts of Resource2 fully, so I'm not 100% sure doing that will work. 
Since Resource2 is already doing data model definition via object 
parameters the munch part may not be super useful. We'll have to think 
about how we can pass through something so that shade users get things 
that are isinstance(munch.Munch) though - which is maybe a metaclass if 
we can't do it directly at the Resource layer.

Monty



More information about the OpenStack-dev mailing list