Open Stack

On Thu, 2022-11-24 at 17:57 +0000, Sean Mooney wrote:
> On Thu, 2022-11-24 at 16:15 +0000, Stephen Finucane wrote:
> > On Thu, 2022-11-24 at 13:36 +0000, Sean Mooney wrote:
> > > On Thu, 2022-11-24 at 12:17 +0000, Stephen Finucane wrote:
> > > > On Wed, 2022-11-23 at 14:44 +0000, Sean Mooney wrote:
> > > > > On Wed, 2022-11-23 at 14:11 +0000, Stephen Finucane wrote:
> > > > > > 👋
> > > > > > 
> > > > > > tl;dr: $subject
> > > > > > 
> > > > > > I reviewed a patch against openstackclient (OSC) today [1] and left a rather
> > > > > > lengthy comment that I thought worthy of bringing up to a wider audience. The
> > > > > > patch itself doesn't matter so much as what is was trying to achieve, namely
> > > > > > modifying an existing OSC command to better match the structure of the
> > > > > > equivalent legacy client command. The review provides more detail than I do here
> > > > > > but the tl;dr: is that this is a big no-no and OSC will and must maintain
> > > > > > consistency between OSC commands over consistency with legacy clients. As I
> > > > > > noted in the review, consistency is one of the biggest advantages of OSC over
> > > > > > the legacy clients: if you know the name of the resource type you wish to work
> > > > > > with, you can pretty accurately guess the command and its structure. This is a
> > > > > > thing that operators have consistently said they love about OSC and its one of
> > > > > > the key reasons we're trying to get every command to provide full current API
> > > > > > implementations in OSC (and SDK).
> > > > > > 
> > > > > > Now I get that the way some of these consistent commands have been implemented
> > > > > > has been the cause of contention in the past. I don't imagine it remains any
> > > > > > less contentious today. However, these patterns are well-understood, well-known
> > > > > > patterns that have for the most part worked just fine for close to a decade now.
> > > > > > The kind of patterns I'm thinking about include:
> > > > > > 
> > > > > >  * The command to create a new resource should always take the format
> > > > > >  '<resource> create <human-readable identifer>
> > > > > >  * The command to modify some property of a resource should always take the
> > > > > >  format '<resource> set --property=value <name-or-identifier>'
> > > > > >  * The command to list, fetch or delete resources should always take the format
> > > > > >  '<resource> list', '<resource> get <name-or-identifier>', and '<resource>
> > > > > >  delete <name-or-identifier>', respectively.
> > > > > you have listed '<resource> get <name-or-identifier>' to fetch a resouce but in my experince
> > > > > "show" is the more common action 
> > > > > 
> > > > > openstack server show <name-or-identifier>
> > > > > openstack image show <name-or-identifier>
> > > > > openstack volume show <name-or-identifier>
> > > > > 
> > > > > also network and port and subnet baiscaly all the resouce form the core services
> > > > > 
> > > > > get does not really seam to be used.
> > > > 
> > > > Whoops, typo. '<resource> show <name-or-identifier>' is what I meant.
> > > > 
> > > > > >  * Boolean options should always take the form of flags with an alternate
> > > > > >  negative option like '--flag' and '--no-flag', rather than '--
> > > > > >  flag=<true|false>'
> > > > > i personally dont like this but i agree with being consitant. i strongly
> > > > > prefer the 
> > > > > '--flag=<true|false>' approch as something that is more readble 
> > > > > but its not the pattern in use in osc. i would prefer to keep things consitent
> > > > > then change this at this point.
> > > > > 
> > > > > >  * And a couple of other things that we tend to highlight in reviews.
> > > > > > 
> > > > > > We want to preserve this behavior, lest OSC lose this huge USP and devolve
> > > > > > into
> > > > > > a muddle mess of different ideas and various individuals'/teams'
> > > > > > preferences.
> > > > > > I'm more than happy to discuss and debate this stuff with anyone who's
> > > > > > interested and we'll continue reviewing each patch on its merit and
> > > > > > providing
> > > > > > exceptions to these rules where they make sense, but it will remain an
> > > > > > ongoing
> > > > > > goal and it's something we'd like people to consider when working on OSC
> > > > > > itself
> > > > > > or any of its plugins.
> > > > > 
> > > > > i agree with what you said in general but there is one digerance already that
> > > > > we might need to reconsider.
> > > > > 
> > > > > i hate that i bring this up but one of the design guidlines of OSC was
> > > > > commands must not auto negociagte the latest micorverion.
> > > > > that again was for consitency so that command would work the same across
> > > > > different clouds with different api versions.
> > > > > many plugins have broken this design requirement btu the core osc client still
> > > > > maintains its orginal design.
> > > > > 
> > > > > to level set osc intentionally does not support microverion negocaitation, it
> > > > > was a desgin choice not an oversight.
> > > > 
> > > > Like gtema, I'm not aware of any such design decision in OSC. Looking through
> > > > the docs and git logs, I'm also unable to find any references to it. I _suspect_
> > > > that you might be confusing OSC with the legacy clients, where this behavior was
> > > > very much a design choice. OSC has traditionally inherited this behavior
> > > > owing to its use of the API bindings from the legacy clients but this wasn't
> > > > intentional. 
> > > > 
> > > that is not the case if you want to understand the history dean has captured it here https://youtu.be/D-4Avtxjby0?t=310
> > > i was stitting in the room at the time. the commitment ot provideing a stable comandline interface for puppet ansible and other
> > > scripting was intoduced in the 1.0 release.
> > > 
> > > this was one of the big departure form the project cleint that do not provide a stabel command line gurantee for scripting.
> > > we try not to break people intentually but osc was ment to be the stable client that had consitenty behavior.#
> > 
> > This is a different thing. What Dean is talking about there is the command
> > structure itself. We still do this. For example, you can create a volume based
> > on an existing volume snapshot. Older versions of OSC did this like so:
> > 
> >   openstack volume create --snapshot-id <snapshot> ... <name>
> > 
> > However, we allow users to specify either a name or ID for the snapshot so this
> > name was misleading. As a result, at some point this option was renamed and
> > you'd now create a volume from a snapshot like so:
> > 
> >   openstack volume create --snapshot <snapshot> ... <name>
> > 
> > Crucially though, we did not remove '--snapshot-id'. It's no longer emitted in
> > the help message (we do this using 'help=argparse.SUPPRESS') but if you pass
> > this, OSC will continue to honour it.
> > 
> > There have been exceptions to this. The old '--live <host>' parameter for
> > 'server migrate' jumps to mind, but that was removed because it was actively
> > harmful: users almost never want to bypass the scheduler when live migrating.
> > They are exceptions though.
> > 
> > > > We must remember that OSC is designed for humans first and
> > > > foremost, while machines should use SDK or the clients directly.
> > > > 
> > > again that is wrong machine parsable output is a core part of the openstack client phiosophy
> > > https://youtu.be/D-4Avtxjby0?t=911
> > > https://www.youtube.com/watch?v=EMy9IsRHY-o&t=1528s
> > 
> > I'm not suggesting that machine parseable output isn't a concern and I've
> > invested time in fixing bugs with the machine readable formats. However, the
> > human-readable output is our primary concern since that's what most people that
> > talk to us about OSC care about. I suspect most others are using something like
> > Ansible nowadays...
> > 
> > > >  When auto-
> > > > negotiation is done correctly (i.e. without the bugs that fungi highlighted), it
> > > > allows a human to get the best possible functionality from their deployment (if
> > > > we are to assume that each new microversion is an improvement on its
> > > > predecessors) which ultimately results in a better user experience.
> > > > 
> > > its a better user experince only if its correct and to have it be correct and consitent 
> > > to be both you would need to use the oldest microversoin that supports the parmaters you passed
> > > using the oldest ensure the behavior is consitent across clouds.
> > 
> > We have to ensure it's correct, yes, but I don't think it has to be consistent.
> > We should strive to provide the best experience to a user and this means using
> > the latest API versions. If a user wants to be consistent then they can
> > explicitly request an API version.
> > 
> > > >  As we
> > > > replace use of these clients with SDK, we are slowly fixing this in core OSC and
> > > > we'd like to eventually see all commands in OSC auto-negotiating microversions,
> > > > where this makes sense for the underlying service.
> > > that will directly break existing users and documentation.
> > > so if you eant to enabel that it needs to be a majory version as it will break the api gurantees of the openstack client.
> > > im not arguring that auto negociation would not be a better ux but we dont get it for free.
> > > the cost will be in consitent behavior across openstack clouds and versiosn.
> > 
> > We're already way down the road and have released multiple major versions since
> > we started using SDK (for glance initially, I think). '--os-{server}-api-
> > version' is an option to.
> sure so theere shoudl be no issue with doing another one if/when we start enabling version negocation in osc
> 
> the sdk is currently working on its 1.0.0 release but it has already broken other consumer liek the ansible openstack collections
> in advance of that release.
> https://github.com/openstack/ansible-collections-openstack#breaking-backward-compatibility-warning
> 
> fortunetly that also means that the ansibel collections shoudl not be affected by osc change since they shoudl already be using the sdk.
> 
> for other integrations liek the openstack puppet moduels for creating opentack flavors
> that is implemtned in ruby and cant use the sdk so they use the openstack client and that module will start having flavor extraspec validation
> enabled by default if we negociate the latest micoversion.
> https://github.com/openstack/puppet-nova/blob/master/lib/puppet/provider/nova_flavor/openstack.rb
> 
> there service list inteface will start using uuid ids instead of int
> https://github.com/openstack/puppet-nova/blob/master/lib/puppet/provider/nova_service/openstack.rb#L17
> 
> i honestly dont really read puppet/ruby and dont really know if any of those changes will break them but if we follow semver and treat
> this as a breaking change in terms of the default behavor at least puppet can pin the osc version and take the same approch ansibel took
> with pinning the sdk version to <0.99.0 to allow them to adapt.
> 
> 
> the api for interacting with glance https://github.com/openstack/puppet-glance/blob/master/lib/puppet/provider/glance_image/openstack.rb via puppet
> seam pretty small and it seams to still default to v2... but hopefully the were not broken the previous changes.
> the did howeer need to adapt to osc v4.0.0 previously https://github.com/openstack/puppet-glance/commit/13653589788c6ebe4f0d129968161216fd53f161
> i would expect they will have to do something simialr if we adopt version autonegocation in general.
> > 
> > > > > since many of the plugins have ignored that and implemnted it anyway i think it would be good to provide a way to opt into the
> > > > > desired behavior. i.e. provide a --latest global flag or change the default for the --os-compute-api ectr command to latest in a major
> > > > > version fo osc.
> > > > 
> > > > As you know, you can manually set API versions in three ways:
> > > > 
> > > >  * Via command-line arguments
> > > >  * Via environment variables
> > > >  * Via clouds.yaml
> > > > 
> > > > Any of these will override the version negotiation done by OSC. For a power user
> > > > like yourself, I suspect this might be what you want and you're free to do it.
> > > > Nothing changes with these. We'll just start doing auto-negotiation by default.
> > > im concerned about the ecosystem of user and docs that have been built based on teh stable command line api gurenetee
> > > that the openstack cleint team commited to in the 1.0 release.
> > > 
> > > i will be fine in any case but you have reniforced my perspective that teh people that gave that guarnteee have left the proejct
> > > and the fact that was a fundemental part fo the desgin has been lost for the knowlage of the current team.
> > > 
> > > we use a lazy concent model in openstack as with may opensource proejct and if we decied to revoke that guarenttee and decied that as a comunity its
> > > better to focuse on human users i am fine with that if its a deliberate decsion. i dont think we should sleep walk into that because
> > > there is no one to advocate for the orginal design commitments. the oringal design has a user centric approch to ensuring that
> > > all command are inutitve and hiding the implmantion detail of which project provide a given logic resouce.
> > > 
> > > i woudl like it if my command just worked too but we need to be very very carful with this type of change.
> > 
> > As noted above, I think we're talking about different things and I don't think
> > we're planning on blindly removing commands themselves. A given command
> > invocation should continue working for a long-time to come.
> 
> coreect and im not talkign about command removal.
> 
> what i dont think shoudl happen without a osc major version release is for the beahivor of an exsiting command to change from
> defaulting to oldest microversion to latest.
> 
> that would break things like rsa keypair generateion which is nolonger supported in the latest microverion
> anythin that used to get  stats form teh hypervior api would break, there are severall other examples.
> 
> so if we do this all im really asking for is a major veriosn of the openstack client to signal to peopel
> and project that there is  semantic api breakage that is not backward compatible and that they shoudl pause before using it.
> there may not be a syntaxtic breakage which woudl happen if a command was remvoed or renamemed but there is a sematic change.

Ah, in that case sure, no problem. We're rather judicious with our major
versions bumps so this will happen naturally even if we don't plan it. For
scripting, I would suggest tools like puppet pass '--os-{service}-api-version'
options to their commands to ensure things remain unchanged.

Stephen

> 
> 
> > 
> > Stephen
> > 
> > > 
> > > > 
> > > > > we can provide a common impelmatiton in osc and the plugins can just reuse that instead of all of them that chose to suport it implemneting it them
> > > > > selves.
> > > > > 
> > > > > again this goes directly against the orginial design intent fo osc to provide a stable comandline interface across
> > > > > clouds with differnt versions of openstack, however since most of the peopel that cared about that have now moved on form openstack
> > > > > and osc and since the comunity seam to have change its mind in providing a stable api expirence we should proably adress this divergance.
> > > > > 
> > > > > i see the fact that some plugins added micorversion negocation in direct breach of this design principal to be more problematic
> > > > > form a consticy point of view then any other divergance.
> > > > 
> > > > See above :)
> > > > 
> > > > Stephen
> > > > 
> > > > > > 
> > > > > > I will now get off my pedestal/ivory tower 🗼
> > > > > > 
> > > > > > Thanks!
> > > > > > Stephen
> > > > > > 
> > > > > > PS: I'm talking here about the command themselves, not their
> > > > > > implementations. We
> > > > > > do somethings extra in OSC that are user helpful, like allowing users to
> > > > > > identify resources by their name in addition to by their UUIDs. We also
> > > > > > currently do things that no so user helpful, like crashing and burning if
> > > > > > the
> > > > > > name lookups fail (I'm thinking about the various Glance-related commands
> > > > > > that
> > > > > > error out if a project name/ID is passed to a command and the user can't
> > > > > > look up
> > > > > > that project). These are things we're more than willing to fix and will
> > > > > > happily
> > > > > > accept patches for :)
> > > > > > 
> > > > > > [1] https://review.opendev.org/c/openstack/python-openstackclient/+/865377
> > > > 
> > > 
> > 
>