[openstack-dev] [api] [Nova] [Ironic] [Magnum] Microversion guideline in API-WG
Dmitry Tantsur
dtantsur at redhat.com
Mon Jun 22 07:51:00 UTC 2015
On 06/20/2015 02:14 AM, Devananda van der Veen wrote:
> Almost all of our discussions so far on this topic have left something
> out, which Monty pointed out to me last week. I'm following up now
> because E_TRAVEL...
>
> tldr;
> What we're versioning here are API's, not packages. It's not a question
> of numbering and dependency ordering, but of communicating
> support[ed|able] interfaces between running services. Libtool is more
> relevant than semver.
> http://www.gnu.org/software/libtool/manual/html_node/Updating-version-info.html
>
> Right now we lack a means to express that the API response is
> "compatible-with" a particular previously-released version of the API.
> If we had that, instead of "current-version", I believe we would have
> much happier users (and a happier Dmitry and jroll).
>
>
> Long version...
> Every HTTP response from Ironic today includes three headers: min, max,
> and version. The service can present an older API version, as long as it
> is greater-than-or-equal-to the minimum supported version, even if that
> version is incompatible with the maximum supported version. It does
> this by rewriting responses to match what was expected in the requested
> (older) version.
>
> When the newer version is identical *for all interfaces present* in the
> older version, this can be called compatible. Dmitry's point is that we
> don't need to hide newer interfaces from users who request an older API
> version, because the client won't know or care about things that weren't
> in the version it requested.
+1
>
> However, we *do* need to signal their presence, and we don't have a good
> means for that right now. We also need to signal to the client that the
> response given is "compatible with" a certain "age" of API, even if it's
> not exactly the same. And we don't have any means for that, either.
>
> Time for an example....
>
> Let's say that an incompatible change was made in v1.3. Let's also say
> that a change was made in v1.5 that added a new endpoint. Today, this is
> what the response headers would look like when calling a server running
> v1.5.
>
> a) client requests v1.2, receives headers (min: 1.1, max: 1.5, current:
> 1.2) b) client requests v1.4, receives headers (min: 1.1, max: 1.5,
> current 1.4) c) client requests v1.5, receives headers (min: 1.1, max:
> 1.5, current: 1.5)
>
> What we have implemented today is that in case (b), the service will
> *hide* any changes that we introduced in v1.5. But those changes did not
> affect any functionality of the v1.4 API, so Dmitry objects to this. So
> do I.
>
> The issue at hand is the response in case (b) ... though after spending
> the last several months working on api versioning, I actually think all
> of those are poor responses.
>
> What I believe we should have:
> a) client requests v1.2, receives headers (min: 1.1, max: 1.5,
> compatible-with: 1.1)
> b) client requests v1.4, receives headers (min: 1.1, max: 1.5,
> compatible-with: 1.3)
> b) client requests v1.5, receives headers (min: 1.1, max: 1.5,
> compatible-with: 1.3)
That sounds really like something people actually want from us.
Just one clarifying question: shouldn't b) and c) be compatible-with:
1.4, as 1.4 was a breaking change?
>
> Yes -- (b) and (c) are identical responses.
>
> Discuss.
>
> -Devananda
>
>
> On Tue, Jun 16, 2015 at 7:13 AM Dmitry Tantsur <dtantsur at redhat.com
> <mailto:dtantsur at redhat.com>> wrote:
>
> On 06/16/2015 03:47 PM, Jim Rollenhagen wrote:
> > On Tue, Jun 16, 2015 at 08:56:37AM +0200, Dmitry Tantsur wrote:
> >> On 06/04/2015 08:58 AM, Xu, Hejie wrote:
> >>> Hi, guys,
> >>> I’m working on adding Microversion into the API-WG’s guideline
> which
> >>> make sure we have consistent Microversion behavior in the API
> for user.
> >>> The Nova and Ironic already have Microversion implementation,
> and as I
> >>> know Magnum _https://review.openstack.org/#/c/184975/_ is going to
> >>> implement Microversion also.
> >>> Hope all the projects which support( or plan to) Microversion
> can join
> >>> the review of guideline.
> >>> The Mircoversion specification(this almost copy from nova-specs):
> >>> _https://review.openstack.org/#/c/187112_
> >>> And another guideline for when we should bump Mircoversion
> >>> _https://review.openstack.org/#/c/187896/_
> >>> As I know, there already have a little different between Nova and
> >>> Ironic’s implementation. Ironic return min/max version when the
> requested
> >>> version doesn’t support in server by http-headers. There isn’t such
> >>> thing in nova. But that is something for version negotiation we
> need for
> >>> nova also.
> >>> Sean have pointed out we should use response body instead of http
> >>> headers, the body can includes error message. Really hope
> ironic team
> >>> can take a
> >>> look at if you guys have compelling reason for using http headers.
> >>> And if we think return body instead of http headers, we
> probably need
> >>> think about back-compatible also. Because Microversion itself isn’t
> >>> versioned.
> >>> So I think we should keep those header for a while, does make
> sense?
> >>> Hope we have good guideline for Microversion, because we only
> can change
> >>> Mircoversion itself by back-compatible way.
> >>> Thanks
> >>> Alex Xu
> >>
> >> Hi all!
> >>
> >> I'd like to try put in feedback based on living with
> microversions in Kilo
> >> release of Ironic.
> >
> > And here's my take, based on my experiences. Keep in mind I'm a core
> > reviewer, a developer, and an operator of Ironic.
>
> Thanks Jim, much appreciated!
>
> >
> > From an ops perspective, our team has built our fair share of
> tooling to
> > help us run Ironic. Some of it uses the REST API via python or
> node.js,
> > and of course we all use the CLI client often.
> >
> > We also continuously deploy Ironic, for full transparency. My
> experience
> > is not with how this works every 6 months, but in the day-to-day.
> >
> >>
> >> First of all, after talking to folks off-list, I realized that
> we all, and
> >> the spec itself, confuse 3 aspects of microversion usage:
> >>
> >> 1. protecting from breaking changes.
> >> This is clearly a big win from user's point of view, and it
> allowed us to
> >> conduct painful change with renaming an important node state in
> our state
> >> machine. It will allows us even worse change this cycle: change
> of the
> >> default state.
> >>
> >
> > +1. Good stuff. My tooling doesn't break when I upgrade. Yay.
> >
> >> 2. API discoverability.
> >> While I believe that there maybe be better implementation of
> this idea, I
> >> think I got it now. People want services to report API versions they
> >> support. People want to be able to request a specific version,
> and fail
> >> early if it is not present. Also +1 from me.
> >>
> >
> > I don't tend to personally do this. I usually am aware of what
> version
> > of Ironic I'm running against. However I see how this could be useful
> > for other folks.
> >
> > I do, however, use the versions to say, "Oh, I can now request
> 1.5 which
> > has logical names! That's useful, let's set those to the names in our
> > CMDB." Now my tooling that interacts with the CMDB and Ironic can
> look
> > at the version and decide to use node.name <http://node.name>
> instead of the old hack we
> > used to use.
> >
> >> 3. hiding new features from older clients
> >> This is not directly stated by the spec, but many people imply
> it, and Nova
> >> and Ironic did it in Kilo. I want us to be clear: it is not the
> same as #2.
> >> You can report versions, but still allow new features to be used.
> >>
> >
> > This is still totally useful. If you know what version you are
> running
> > against, you know exactly what features are available.
>
> "You know" is about #2 - that's where confusion is :)
> so if you know, that moving to inspection state is disallowed for your
> tooling (but not for the whole system!), what does it give you?
>
> >
> > I think the disconnect here is that we don't expect users
> (whether those
> > are people or computers) to explicitly request a version. We need to
> > message better that if you are using Ironic or building a tool
> against
> > Ironic's API, you should be pinning the version. We also need to take
> > this comment block[0] and put it in our docs, so users know what each
> > version does.
> >
> > Knowing that I get feature X when I upgrade to version Y is useful.
> >
> >> It is this particular thing that gets -2 from me, after I've
> seen how it
> >> worked in practice, and that's why.
> >>
> >> First of all, I don't believe anyone needs it. Seriously, I
> can't imagine a
> >> user asking "please prevent me from using non-breaking changes".
> And attempt
> >> to implement it was IMO a big failure for the following reasons:
> >>
> >> a) It's hard to do. Even we, the core team, got confused, and
> for non-core
> >> people it took several iteration to do right. It's a big load on
> both
> >> developers and reviewers.
> >>
> >
> > I do agree with this. It's been painful. However, I think we're
> mostly
> > past that pain at this point. Does this patch[1] look like developer
> > pain?
>
> It's not that painful to write. Now. When we have 10-20 version, it
> probably will :)
> anyway, it's hard to explain newcomers how to do it, and it's hard to
> review the result. we failed at it, e.g. with error codes.
>
> >
> >> b) It's incomplete (at least for Ironic). We have several
> API-exposed things
> >> that are just impossible to hide. Good example are node states:
> if node is
> >> in a new state, we can't but expose it to older tooling. Our
> free-form JSON
> >> fields properties, instance_info, driver_info and
> driver_internal_info are
> >> examples as well. It's useless to speak about API contract,
> while we have
> >> those.
> >>
> >
> > I somewhat agree here.
> >
> > With node states, there are cases where we were able to hide it
> > (NOSTATE -> AVAILABLE), and cases where we were not (adding
> MANAGEABLE).
> > However, this list of states is (AIUI) not part of the API contract;
> > rather the verbs available to move between states are.
>
> What's the point in contract, if there are things not covered by it that
> drastically change the system behavior?
>
> >
> > As far as JSON fields, we've never had a contract around what
> keys are
> > available. Only the semantics of working with those fields, and which
> > fields exist.
>
> ditto as above: you can request new features by modifying driver_info.
>
> >
> >> c) It gives additional push back to making (required) breaking
> changes. We
> >> already got suggestions to have ONE MORE feature gating for breaking
> >> changes. Reason: people will need to increase microversions to
> get features,
> >> and your breaking change will prevent it.
> >>
> >
> > This is just silly. If 1.10 breaks a user, and the user wants 1.11,
> > they'll need to fix that breakage.
>
> ++ but not everyone agreed on the summit, when I was talking about
> ENROLL state
>
> >
> >> d) It requires a hard compromise on the CLI tool. You either
> default it to
> >> 1.0 forever, and force all the people to get used to figuring
> out version
> >> numbers and using `ironic --ironic-api-version x.y` every time
> (terrible
> >> user experience), or you default it to some known good version,
> bumping it
> >> from time to time. This, in turn, has 2 more serious problems:
> >>
> >
> > I disagree that pinning a version all the time is a terrible
> experience.
> > We already require a number of options for authentication
> (OS_USERNAME,
> > OS_PASSWORD, etc etc). How many folks do you think type these in
> every
> > time? Solution is simple: add IRONIC_API_VERSION to whatever
> exports the
> > other environment variables.
>
> It's not that bad, especially if devstack/tripleo will provide some
> reasonable default for you.
>
> I remember, however, Devananda didn't like the idea.
>
> And it definitely makes a quick start guide a bit harder to follow. I
> already imagine how many people will forget about this pinning (either
> to do it, or do update when they need new features).
>
> >
> > The version depends on the environment you are running against -
> why not
> > treat it as such?
> >
> >> d.1) you start to break people \o/ that's not a theoretical
> concern: our
> >> downstream tooling did get broken by updating to newer
> ironicclient from git
> >>
> >
> > As I said before, we need to encourage folks to pin client
> versions if
> > they don't want to break. I'm probably alone here, but I would even
> > propose making the version *required*. Force people to think
> about what
> > they are doing. If folks are okay with being broken, they can pass
> > "latest".
>
> Could be a good default for devstack btw
>
> >
> >> d.2) you require complex version negotiations on the client
> side. Otherwise
> >> imaging CLI tool defaulting to 1.6 will issue `node-create` to
> Ironic
> >> supporting only 1.5. Guess what? It will fail despite
> node-create being very
> >> old feature. Again, that's not something theoretical: that's how
> we broke
> >> TripleO CI.
> >>
> >
> > Again, pin it.
> >
> >> e) Every microversion should be fully tested separately. Which
> ended up in
> >> Ironic having 4 versions 1.2-1.5 that were never ever gate
> tested. Even
> >> worse, initially, our gate tested only the oldest version 1.1,
> but we solved
> >> it (though it took time to realize). The only good thing here is
> that these
> >> versions 1.2-1.5 were probably never used by anyone.
> >>
> >
> > Hi. I've used some of these. :)
>
> You didn't tell me last time we talked :) note, that you didn't use
> them, unless you explicitly requested, because IIRC we never defaulted
> our client to any of these. So for most people, even deploying from
> master, it was 1.1 -> 1.6.
>
> >
> > // jim
> >
> > [0]
> https://github.com/openstack/ironic/blob/master/ironic/api/controllers/v1/__init__.py#L59-63
> > [1]
> https://review.openstack.org/#/c/188873/1/ironic/api/controllers/v1/node.py
> >>
> >> To sum this long post up, I'm seeing that hiding new features
> based on
> >> microversions brings much more problems, than it solves (I'm not
> aware of
> >> the latter at all). I'm very opposed to continuing doing it in
> Ironic, and
> >> I'm going to propose patch stopping gating Kilo changes
> (non-breaking
> >> obviously).
> >>
> >> Hope that helps,
> >> Dmitry
> >>
> >>>
>
>
>
> __________________________________________________________________________
> 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
>
More information about the OpenStack-dev
mailing list