Open Stack

On 07/28/2015 10:51 PM, Devananda van der Veen wrote:
> On Tue, Jul 28, 2015 at 1:01 AM Dmitry Tantsur <dtantsur at redhat.com
> <mailto:dtantsur at redhat.com>> wrote:
>
>     On 07/27/2015 10:41 PM, Sean Dague wrote:
>      > On 07/27/2015 04:35 PM, Jim Rollenhagen wrote:
>      >> Hi friends.
>      >>
>      >> Ironic implemented API "micro" versions in Kilo. We originally
>     did this
>      >> to allow for breaking changes in the API while allowing users to
>     opt in
>      >> to the breakage.
>      >>
>      >> Since then, we've had a "default version" for our client that we
>     bump to
>      >> something sensible with each release. Currently it is at 1.8.
>      >> Negotiation is done with the server to figure out what is
>     supported and
>      >> adjust accordingly.
>      >>
>      >> Now we've landed a patch[0] with a new version (1.11) that is not
>      >> backward compatible. It causes newly added Node objects to begin
>     life in
>      >> the ENROLL state, rather than AVAILABLE. This is a good thing, and
>      >> people should want this! However, it is a breaking change.
>     Automation
>      >> that adds nodes to Ironic will need to do different things after the
>      >> node-create call.
>      >>
>      >> Our API versioning scheme makes this opt-in (by specifying the API
>      >> version). However, some folks have a problem with releasing this
>     change
>      >> as-is. The logic is that we might release a client that defaults
>     to 1.11
>      >> or higher, or the user may request 1.12 later to get a new
>     feature, thus
>      >> breaking their application that enrolls nodes.
>      >>
>      >> This is clearly backwards. Users should read release notes and
>     be aware
>      >> of what changes between versions in the API. Users need to be
>     aware of
>      >> the fact that our API is versioned, and use that to their advantage.
>      >>
>      >> It seems to me that the goal of the version negotiation in our
>     client
>      >> has been to pretend that our API versions don't exist, from a user
>      >> perspective. We need to stop doing this and force users to think
>     about
>      >> what they are doing when they interact with our API.
>      >>
>      >> It seems to me we have a few options here:
>      >>
>      >> 1) Default the python client and CLI to the earliest supported
>     version.
>      >> This will never break users by default.
>      >>
>      >> 2) Default the python client and CLI to use the special version
>      >> 'latest'. This will always use the latest API version, and always
>      >> break people when a new server version (that is not backwards
>      >> compatible) is deployed.
>      >>
>      >> 3) Do what Nova does[1]. Default CLI to latest and python client to
>      >> earliest. This assumes that CLI is typically used for one-time
>     commands
>      >> (and it isn't a big deal if we break a one-off command once),
>     and the
>      >> python client is used for applications.
>      >
>      > Actually what Nova is doing is slight different than this, the
>     CLI will
>      > default to "latest" on the server. There will be an extra round
>     trip to
>      > figure out what that is. And the CLI will (long term) just not
>     present
>      > commands that aren't available at the server level you are
>     talking to.
>      >
>      > Consider the CLI an auto negotiating microversion application of the
>      > python API client. And, realistically, should solve some of the
>     issues
>      > of people running "nova foo" and getting cryptic errors from the
>     server
>      > when they are hitting an old version of Nova that doesn't know
>     how to foo.
>      >
>      > So the CLI should actually break less often, and will expose the most
>      > functionality you can get out of your cloud.
>
>     What I find weird about this and similar approaches is that we treat CLI
>     and any different from other ways to use API. And I *suspect* this is
>     because we, the developers, use it the most and point newcomers to it.
>     And I agree it's troublesome to explain that to use "manage" provision
>     verb you have to provide --ironic-api-version 1.6. Or was 1.6 for
>     inspect? Hmm, I really don't remember.
>
>     CLI is not different, CLI is not special and CLI is not "an auto
>     negotiating microversion application of the python API client". By
>     saying any of these we just refusing it eat our dog's food. If we
>     consciously believe (I personally don't) that versioning is the right
>     thing to do for people using our API - lets stop dodging it ourselves.
>     Otherwise it looks like we've invented an architecture that people
>     presumably dream of, but we don't know if it's even usable (to say
>     nothing about useful). I tried writing versioning-aware code for our
>     devstack yesterday, and it wasn't that nice and shiny.
>
>     If our versioning requires negotiations, lets have it on API level, so
>     that all users get it. Actually, server has the same level of
>     information as the client. Let's have "X-OpenStack-XXX-Version:
>     negotiate" figure out a proper version for us - or refuse to process a
>     request if it's not possible. And if we think that versioning as it is
>     now is unusable at all, lets rework the whole idea.
>
>     tl;dr my vote if for CLI to strictly follow whatever server API does.
>     That is, default to the lowest version, require explicit version
>     argument to get new features. (it's up to a follow up discussion what to
>     do during the deprecation period).
>
>
> Hi Dmitry,
>
> I appreciate a good rant as much as anyone -- and I post them, too --
> but aside from the last paragraph, it's hard for me to tell what your
> intent here is, so I'm going to take a guess and respond mostly to that
> last paragraph.

Nevermind, looks like we're in agreement on this one actually.

>
> The REST API does require negotiation right now, or else the client will
> get the minimum (1.1) version and its respective behavior.  However, as
> you know, the client defaults to sending a version right now, and that
> seems to be the crux of this debate.
>
> So, what if the client sends *no* version by default, thus getting
> default minimum version from the server? It should then print any needed
> deprecation warnings, and *allow* the user to specify a different
> version -- and I think that's quite reasonable. It reverses the current
> client's behavior, but I agree that it's better in the long run. And
> that is exactly what I proposed a couple weeks ago, here:
>
> https://review.openstack.org/#/c/196320/2/specs/devref/api-version-negotiation.rst,cm

Yes, and I agree with this part.

>
>
> However, it doesn't address my objection to the ENROLL change -- namely,
> that it *does not* require a breaking change. Users should be given the
> ability to opt-in to the new behavior by a means other than an API
> version, and the client should use the API version to discover whether
> that capability is present in the cloud, and give appropriately helpful
> error messages to the user if it isn't.

Then you should have -2ed the spec (or stopped the discussion at the 
summit, or stopped the discussion at the previous summit). A lot of 
users won't opt into a feature that requires some headache without a 
clearly visible bonus. And we'll end up with supporting 2 completely 
different entry points in our state machine, and users that sometimes 
mess them. I'd like to be sure then when I say "create a node in Ironic" 
it will mean one thing at least for one version of API.

And let me remind you again that we first introduced versioning in 
ironic as a means for handling None->AVAILABLE transition. Was it a 
required breakage? No, we could easily live with None. Did we do it 
because it made our state machine saner? Yes, I think this was the 
reason. Did we make it opt-in? Seems like no.

Both AVAILABLE and ENROLL state breakages were our attempts to clean up 
our state machine. Probably we should not have done it.

Now half a year passed. Without a proper discussion we started to do 
feature hiding via versioning (which I still see as largely pointless 
effort), and now we ended up discussing how to avoid a breaking change 
of very similar nature.

Now, if you agree that we should switch to explicitly specifying version 
in client, what's the problem? People will update once they're ready, 
and it's pretty easy to become ready for this change. Enroll change was 
designed so that it reuses already working transition: create -> manage 
-> provide.

Anyway, I don't think I understand what we discuss *right now*, because 
any revert is a real breaking change. The only thing we could do is to 
create API version 1.12 defaulting back to AVAILABLE. But we can't do it 
for the same reasoning as your provide below: then people already using 
1.11 and ENROLL state will see 1.12 as a breaking change.

>
> Why go through all this? Because we can, because it's not that hard to
> implement, and finally, because as a user, I don't want to have to
> update the flow control logic of my code (much like Clint already
> exemplified) just to get the next backwards-compatible feature we roll out.

Yes, this feature hiding is a pain ;) But it's our versioning approach 
that is broken in this case, not the enroll feature. But actually I see 
some sense in it: if you want your code just working, fix some version 
of API. If you want new features - get ready to accept whatever changed 
since then.

By the way, I don't particularly like the API-WG spec on versioning, but 
I don't think we should diverge from it. And it clearly does not 
distinguish between breaking and not breaking changes.

>
> Thanks,
> -Deva
>
>
>
> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>