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 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-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev



__________________________________________________________________________
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

__________________________________________________________________________
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev



__________________________________________________________________________
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Reply via email to