settingsLogin | Registersettings

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

0 votes

Hey everybody,

You may have noticed a giant patch series up:

https://review.openstack.org/#/q/topic:merge-shade

It's a big beastie, so I thought a synopsis of what's going would be
useful. Also, there's a pile of patches towards the end that we want to
squash before landing but which have been pushed up in smaller patches
because reading it all as one patch would be unpossible - but reworking
the unittest mocks for each step would be wasted energy.

Any of the patches in this email that are green from Zuul are ready for
review/merge.

The easy patches:

These next patches are easy because they're just mechanical - nothing
really will have changed in SDK because of them.

https://review.openstack.org/#/c/518128 - Merge shade and
os-client-config into the tree

You can't see the whole picture in gerrit. This merges in shade and
os-client-config, including their entire git history. Most of that merge
got pushed up to a throwaway branch so that the review in gerrit would
be a single patch containing the merge. It's best to just grab the repo
at that branch and take a look.

It's also important to note here that I'm pretty sure openstack.cloud is
not actually where shade wants to live and that openstack.config needs a
refactor - but we can do that after this stack, as the stack is too big
already.

https://review.openstack.org/#/c/518129/ - Merge in recent fixes from
the shade repo

Merges patches that already landed in shade between the time I made the
first merge commit and proposed it. No need to actually review the
content itself.

https://review.openstack.org/#/c/518130 - Add jobs for Zuul v3

Adds functional zuulv3 native test jobs that match what shade was doing.
These run both shade and sdk's functional tests (they're one set of tests)

https://review.openstack.org/#/c/518946 - Fix magnum functional test

Fixes the previous one.

https://review.openstack.org/#/c/518131 - Handle glance image pagination
links better
https://review.openstack.org/#/c/518132 - Fix regression for
listrouterinterfaces
https://review.openstack.org/#/c/518133 - Support filtering servers in
list_servers using arbitrary parameters

Patches landed in shade in the interim. No need to review content.

Once those are landed we're ready for some fun:

https://review.openstack.org/#/c/518799 - Migrate to testtools for
functional tests

Reworks the functional base class to use the testtools based functional
base class from shade, along with the logger fakes and whatnot. The
biggest change here is the move to using setUp/tearDown instead of
setUpClass/tearDownClass.

This patch is annoyingly large, but it's also important so that we have
one consistent approach to these things across the combined codebase.

https://review.openstack.org/#/c/519029 - Rework config and rest layers

The squahsed/rollup patch. It can be viewed broken out by looking at the
other patches that have https://review.openstack.org/#/c/518799 as a parent.

This is where all of the invasive breaking-changes happen. Most notably
stepping away from Profile, Session and Authenticator in favor of using
openstack.config/os-client-config and the Adapter class from
keystoneauth so that we can rely on ksa for discovery and whatnot. It
also adds a pure-rest interface on each service, removes the plugin
entrypoints structure and and renames 4 of the service objects to match
their official service-type (although it also keeps aliases so that
shouldn't cause too much carnage for people)

Removal of profile/authenticator is the biggest user-visible breaking
change, and it's not ACTUALLY removed in this patch (although it was at
one point) We have to keep Profile around long enough to remove its use
from OSC - including osc3, since OSC is used to set up resources as part
of devstack. It might be worthwhile to consider fleshing out the
backwards compat for allowing profiles to be passed in for a short time
since it's there now.

There's still a little more plumbing to do before we can start working
on using this layer under the shade code - most notably we need to plumb
in the task manager ... but to my knowledge this stack works with Dean's
OSC4 branch so we should be in good shape.

https://review.openstack.org/#/c/501438 - Add notes about moving forward

A document with some thoughts - some of which are a good idea and some
not - about next steps and what we need to get done.

Thanks for the patience in reading these ... they're SUPER long. In many
cases it's about letting the test suites let us know they're ok.

Monty


OpenStack Development Mailing List (not for usage questions)
Unsubscribe: OpenStack-dev-request@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
asked Nov 13, 2017 in openstack-dev by Monty_Taylor (22,780 points)   2 4 6

3 Responses

0 votes

Hi!

On Mon, Nov 13, 2017 at 9:54 AM, Monty Taylor mordred@inaugust.com wrote:

Hey everybody,

You may have noticed a giant patch series up:

https://review.openstack.org/#/q/topic:merge-shade

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?

--
David Shrewsbury (Shrews)


OpenStack Development Mailing List (not for usage questions)
Unsubscribe: OpenStack-dev-request@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
responded Nov 13, 2017 by David_Shrewsbury (1,000 points)   1
0 votes

On 11/13/2017 09:06 AM, David Shrewsbury wrote:
Hi!

On Mon, Nov 13, 2017 at 9:54 AM, Monty Taylor <mordred@inaugust.com
mordred@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>

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


OpenStack Development Mailing List (not for usage questions)
Unsubscribe: OpenStack-dev-request@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
responded Nov 13, 2017 by Monty_Taylor (22,780 points)   2 4 6
0 votes

Hi!

Thanks for the explanations.

On Mon, Nov 13, 2017 at 11:30 AM, Monty Taylor mordred@inaugust.com wrote:

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

Yep. At the time, the classes were doing different auth things, so it at
least made a little sense. Now, not so much.

-Dave
--
David Shrewsbury (Shrews)


OpenStack Development Mailing List (not for usage questions)
Unsubscribe: OpenStack-dev-request@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
responded Nov 13, 2017 by David_Shrewsbury (1,000 points)   1
...