On 11/10/2015 06:08 PM, Ben Nemec wrote:
On 11/10/2015 10:28 AM, John Trowbridge wrote:


On 11/10/2015 10:43 AM, Ben Nemec wrote:
On 11/10/2015 05:26 AM, John Trowbridge wrote:


On 11/09/2015 07:44 AM, Dmitry Tantsur wrote:
Hi OOO'ers, hopefully the subject caught your attentions :)

Currently, tripleoclient exposes several commands in "openstack
baremetal" and "openstack baremetal introspection" namespaces belonging
to ironic and ironic-inspector accordingly. TL;DR of this email is to
deprecate them and move to TripleO-specific namespaces. Read on to know
why.

Problem
=======

I realized that we're doing a wrong thing when people started asking me
why "baremetal introspection start" and "baremetal introspection bulk
start" behave so differently (the former is from ironic-inspector, the
latter is from tripleoclient). The problem with TripleO commands is that
they're highly opinionated workflows commands, but there's no way a user
can distinguish them from general-purpose ironic/ironic-inspector
commands. The way some of them work is not generic enough ("baremetal
import"), or uses different defaults from an upstream project
("configure boot"), or does something completely unacceptable upstream
(e.g. the way "introspection bulk start" deals with node states).

So, here are commands that tripleoclient exposes with my comments:

1. baremetal instackenv validate

  This command assumes there's an "baremetal instackenv" object, while
instackenv is a tripleo-specific file format.

2. baremetal import

  This command supports a limited subset of ironic drivers and driver
properties, only those known to os-cloud-config.

3. baremetal introspection bulk start

  This command does several bad (IMO) things:
  a. Messes with ironic node states
  b. Operates implicitly on all nodes (in a wrong state)
  c. Defaults to polling


I have considered this whole command as a bug for a while now. I
understand what we were trying to do and why, but it is pretty bad to
hijack another project's namespace with a command that would get a firm
-2 there.

4. baremetal show capabilities

  This is the only commands that is generic enough and could actually
make it to ironicclient itself.

5. baremetal introspection bulk status

  See "bulk start" above.

6. baremetal configure ready state

  First of all, this and the next command use "baremetal configure"
prefix. I would not promise we'll never start using it in ironic,
breaking the whole TripleO.

  Seconds, it's actually DELL-specific.

7. baremetal configure boot

  This one is nearly ok, but it defaults to local boot, which is not an
upstream default. Default values for images may not work outside of
TripleO as well.

Proposal
========

As we already have "openstack undercloud" and "openstack overcloud"
prefixes for TripleO, I suggest we move these commands under "openstack
overcloud nodes" namespace. So we end up with:

  overcloud nodes import
  overcloud nodes configure ready state --drac
  overcloud nodes configure boot

As you see, I require an explicit --drac argument for "ready state"
command. As to the remaining commands:

1. baremetal introspection status --all

   This is fine to move to inspector-client, as inspector knows which
nodes are/were on introspection. We'll need a new API though.

2. baremetal show capabilities

   We'll have this or similar command in ironic, hopefully this cycle.

3. overcloud nodes introspect --poll --allow-available

   I believe that we need to make 2 things explicit in this replacement
for "introspection bulk status": polling and operating on "available"
nodes.

I am not totally convinced that we gain a huge amount by hiding the
state manipulation in this command. We need to move that logic to
tripleo-common anyways, so I think it is worth considering splitting it
from the introspect command.

Dmitry and I discussed briefly at summit having the ability to pass a
list of nodes to the inspector client for introspection as well. So if
we separated out the bulk state manipulation bit, we could just use that.

I get that this is going in the opposite direction of the original
intention of lowering the amount of commands needed to get a functional
deployment. However, I think that goal is better solved elsewhere
(tripleo.sh, some ansible playbooks, etc.). Instead it would be nice if
the tripleoclient was more transparent.

-2.  This is exactly the thing that got us to a place where our GUI was
unusable.  Business logic (and state management around Ironic node
inspection is just that) has to live in the API so all consumers can
take advantage of it.  Otherwise everyone has to reimplement it
themselves and anything but the developer-used CLI interfaces (like
tripleo.sh) fall behind, and we end up right back where we are today.

It's not simply about reducing the number of commands to deploy a cloud,
it's also (and more importantly) about codifying the process behind an
API that can be used by the CLI, GUI, and any other future clients
someone wants to write.

I think you misunderstand my suggestion. The basic premise is that we
have to move the state manipulation logic into tripleo-common along with
the inspection logic, so that is consumable via the REST API. At that
point, what is gained by having it mixed together?

It is the mixing of these two bits of logic that I object to. We are
consuming two totally separate service APIs (ironic and inspector) in
the same command in a way that is not transparent to end users.

More so, it limits our ability to take better implementations from
upstream as they become available. The ability to do bulk inspection
will definitely land before the ability to do bulk state manipulation in
Ironic.

So we are in total agreement on the fact that the process needs to be
codified behind an API. My suggestion is just that it does not need to
be a SINGLE API call for these two very different operations. Otherwise,
why not just have `openstack baremetal go` that just does all the steps
we currently have in the baremetal namespace. It is this part (the
sequencing of commands or API calls into a workflow with no logic) that
I think is better handled by
tripleo.sh/ansible/insert_workflow_management_tool_of_choice.

Actually, I would love an openstack baremetal go command. :-)

In fact, I'm proposing that we coalesce some of our existing baremetal
commands because (for example) it's silly for us to register nodes and
not configure them to be able to boot:
https://review.openstack.org/#/c/238192/  I'm not sure this fits the
same pattern because introspection is optional, but I wouldn't hate the
idea of a register/configure/introspect call that just took your list of
nodes as input and ended with all of the nodes ready for use.

In any case, my main argument is that we aren't doing two very different
operations here.  The user is asking us to introspect all of their
nodes, and we're doing it.  Yes, from an Ironic/Inspector perspective
there's a bunch of stuff going on here, but from a user perspective
there's one: introspection of all nodes.  The user shouldn't have to
care about the node state song and dance that we're doing to make it
happen IMNSHO.

Well, users will care when they figure out that our shortcuts break promises made by ironic. E.g. introspection nodes in available state means that we can start introspection on nodes that nova is going to schedule on.

Actually, all our scripts assume that they'll always be run sequentially as written in the documentation. It is not true, as we know now.

Finally, we'll soon have no choice. Latest versions of Ironic API make ENROLL the default state, so you have to care about it.


This is getting kind of off-topic for the CLI namespace question though,
so we might want to move this discussion to an API spec.



Thanks Dmitry for starting this discussion.

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



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



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



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

Reply via email to