[openstack-dev] [TripleO] [Ironic] Let's stop hijacking other projects' OSC namespaces

Dmitry Tantsur dtantsur at redhat.com
Tue Nov 10 16:40:02 UTC 2015


On 11/10/2015 05:18 PM, Ben Nemec wrote:
> +1 to moving anything we can into Ironic itself.
>
> I do want to note that if we rename anything, we can't just rip and
> replace.  We have users of the current commands, and we need to
> deprecate those to give people a chance to move to the new ones.

Definitely. I think I used word deprecation somewhere in this long letter :)

>
> A few more thoughts inline.
>
> On 11/09/2015 06: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.
>
> True, although I feel like an "import from JSON" feature would not be
> inappropriate for inclusion in Ironic.  I can't believe that we're the
> only ones who would be interested in mass importing nodes from a very
> common format like this.

I would warmly welcome such command, but it should not require patching 
os-cloud-config every time we add a new driver or driver property.

>
>>
>> 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)
>
> I thought that was fixed?  It used to try to introspect nodes that were
> in an invalid state (like active), but it shouldn't anymore.

It introspects nodes in "available" state, which is a rude violation of 
the ironic state machine ;)

>
> Unless your objection is that it introspects things in an available
> state, which I think has to do with the state Ironic puts (or used to
> put) nodes in after registration.  In any case, this one likely requires
> some more discussion over how it should work.

Well, if we upgrade baremetal API version we used from 1.6 (essentially 
Kilo) to Liberty one, nodes would appear in a new ENROLL state. I've 
started a patch for it, but I don't have time to finish. Anyone is free 
to overtake: https://review.openstack.org/#/c/235158/

>
>>    c. Defaults to polling
>>
>> 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.
>
> Well, as I understand it we don't intend for it to be Dell-specific,
> it's just that the Dell implementation is the only one that has been
> done so far.
>
> That said, since I think this is just layering some TripleO-specific
> logic on top of the vendor-specific calls Ironic provides I agree that
> it probably doesn't belong in the baremetal namespace.
>
>>
>> 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.
>
> +1
>
>>
>> 2. baremetal show capabilities
>>
>>     We'll have this or similar command in ironic, hopefully this cycle.
>
> \o/
>
>>
>> 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.
>
> The only thing is that because this does the state manipulation, polling
> is essentially the only way it can work, although moving this to the API
> will change that.  Like I said above, I think this one is going to
> require some ongoing discussion as part of the API work.
>
>>
>> 4. overcloud nodes import --dry-run
>>
>>     could be a replacement for "baremetal instackenv validate".
>
> I'm wondering if we should even bother with --dry-run, and instead just
> run this validation as part of the import in the first place.  That's
> essentially what --dry-run would end up looking like anyway.  You'd look
> at the file, do the validation, then depending on whether --dry-run was
> specified either register the nodes or not.  We could provide a --force
> in case someone really, really wants to import a node list that won't
> work for some reason, and that way we don't have to worry about people
> forgetting to run the validation.

+1

>
> __________________________________________________________________________
> 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