Open Stack

On 08/12/16 08:40, Steven Hardy wrote:
> On Mon, Dec 05, 2016 at 04:27:50PM -0500, Zane Bitter wrote:
>> Any parameter in a Heat template that has a default other than None is
>> considered optional, so the user is not required to pass a value. Otherwise,
>> however, the parameter is required and creating the stack will fail pretty
>> immediately if the user does not pass a value.
>>
>> I've noticed that this presents a giant pain, particularly when trying to
>> create what we used to call provider templates. If you do e.g.
>>
>> `openstack orchestration resource type show -f yaml --template-type hot
>> OS::Nova::Server`
>
>
> Yes, indeed, this is a giant pain - e.g in TripleO where we make very heavy
> use of nested "provider" templates, it's really inconvenient having to work
> around spurious validation issues related to get_attr returning None, and

I think we might be mixing two different ideas here - that's a separate 
issue which we're hoping to address with 
https://review.openstack.org/#/c/392499/

> also figuring out what values are passed as parameters via the parent
> templates.  So yes, *please* lets fix this :)

I'm not sure if this will solve all of your problems in this area, but 
it should definitely help somewhat.

>> then you get back a template with dozens of parameters, most of which don't
>> have defaults (because the corresponding resource properties don't have
>> defaults) and are therefore not optional. I consider that a bug, because in
>> many cases the corresponding resource properties are *not* required
>> (properties have a "required" flag that is independent from the "default"
>> value).
>>
>> The result is that it's effectively impossible for our users to build
>> re-usable child templates; they have to know which properties the parent
>> template does and does not want to specify values for.
>>
>> Using a default that corresponds to the parameter type ("", [], {}, 0,
>> false) doesn't work, I don't think, because there are properties that treat
>> None differently to e.g. an empty dict.
>>
>> The obvious alternative is to use a different sentinel value, other than
>> None, for determining whether a parameter default is provided and then
>> allowing users to pass null as default. We could then adjust the properties
>> code to treat this sentinel as if no value were specified for the property.
>>
>> The difficulty of this is knowing how to handle other places that get_param
>> might be used, especially in arguments to other functions. I guess we have
>> that problem now in some ways, because get_attr often returns None up to the
>> point where the resource it refers to is created. I hoped that we might get
>> away from that with the placeholders spec though :/
>
> Maybe I'm oversimplifying but I was expecting as solution similar to you
> describe above with a per-type sentinel, but with a reworked base-class for
> all intrinsic functions, so that the new sentinel value can be
> transparently passed around instead of evaluated?
>
> I guess the tricky part about this is we'd always need a type for any
> attributes (e.g including outputs which are currently untyped).
>
> We'd possibly also need to modify parameter contraints evaluation to ensure
> the sentinel isn't failed for constraints, but IIRC we disable value
> validation for most non-runtime validation anyway?
>
> More thought required here but for sure I'd love to see this fixed and will
> be happy to help with testing/reviews on TripleO if anyone has any WIP
> patches based on the above ideas :)

Unless I'm misunderstanding, I think what you're talking about is 
basically the placeholders spec I linked above.

The open question for me is: what do we do when the get_param function 
is used, the user hasn't passed a value for the parameter, and the 
default is null.

Right now the answer is easy: that can't happen.

When get_param is used directly to supply a property of a resource, the 
answer is also easy: use the property default (which may also be None) 
if the property is optional, fail if it is required.

However, when get_param is used as an input to another intrinsic 
function, it's trickier. We have to do the Right Thing (and it's not 
even clear what that is) on a case-by-case basis for every function that 
could possibly get None as an input. We're already trying to do that now 
because get_attr can return None at some stages of the process, and it's 
not going well (https://bugs.launchpad.net/heat/+bug/1559807) because 
there's no systematic way of ensuring it's handled correctly.

I'd actually be fine with this just failing in all cases where null 
isn't a valid input to the function. We want it to fail as early as 
possible though, and the currently handling for get_attr is going to get 
in the way of that. (We currently try to delay checking for None as long 
as possible, because get_attr may not start returning a non-None value 
until runtime.) Maybe that means we should tackle this after the 
placeholder stuff is implemented, so that we can enforce earlier 
detection of errors?

cheers,
Zane.