On 01/04/16 12:31, Steven Hardy wrote:
On Fri, Apr 01, 2016 at 04:15:30PM +0200, Thomas Herve wrote:
On Fri, Apr 1, 2016 at 3:21 AM, Zane Bitter <zbit...@redhat.com> wrote:
On 31/03/16 18:10, Zane Bitter wrote:


I'm in favour of some sort of variable-based implementation for a few
reasons. One is that (5) seems to come up fairly regularly in a complex
deployment like TripleO. Another is that Fn::If feels awkward compared
to get_variable.


I actually have to revise this last part after reviewing the patches.
get_variable can't replace Fn::If, because we'd still need to handle stuff
of the form:

     some_property: {if: [{get_variable: some_var},
                          {get_resource: res1},
                          {get_resource: res2}]

where the alternatives can't come from a variable because they contain
resource references and we have said we'd constrain variables to be static.

In fact the intrinsic functions that could be allowed in the first argument
to the {if: } function would have to constrained in the same way as the
constraint field in the resource, because we should only validate and obtain
dependencies from _one_ of the alternates, so we need to be able to
determine which one statically and not have to wait until the actual value
is resolved. This is possibly the strongest argument for keeping on the cfn
implementation course.

We talked about another possibilities on IRC: instead of having a new
section, create a new resource OS::Heat::Value which can hold some
data. It would look like that:

resources:
     is_prod:
         type: OS::Heat::Value
         properties:
             value: {equals: {get_param, env}, prod}}

     my_resource:
         condition: {get_attr: [is_prod, value}}

Another alternative (which maybe you discussed, sorry I missed the IRC
chat) would be just to use parameters, as that's already conceptually where
we obtain values that are input to resources.

E.g:

parameters:
   env:
    type: string
    default: prod

   is_prod:
     type: boolean
     default: {equals: {get_param, env}}

 From an interface standpoint this seems much cleaner and more intuitive than
the other solutions discussed IMHO, but I suspect it's potentially harder to
implement.

Yeah, the problems to solve here would be:

1) Need to define some subset of functions that are valid in templates (for HOT, probably all except get_resource and get_attr).

- Easy enough, just add a separate method called param_functions or something to the Template class. We'd have to do the same for the "conditions" section anyway.

2) Need to prevent circular references between parameters.

- Hard. Since currently no templates support parsing parameters for functions, we can add a separate implementation without breaking the third-party Template API though, so that helps. Lots of edge cases though.

3) Need to make clear to users that only a parameter is a valid input to a resource condition.

  - Could be done with something like:

 my_resource:
   condition_parameter: is_prod

    instead of:

 my_resource:
   condition: {get_param: is_prod}

    and something similar for {if: [is_prod, <if_true>, <if_false>]}

4) Need to implement the conditions section in CloudFormation templates.

  - Could have some sort of hacky pseudo-parameter thing.

Your original example gets much cleaner too if we allow all intrinsic function
(except get_attr) in the scope of parameters:

parameters:
   host:
     type: string
   port:
     type: string
   endpoint:
     type: string
     default:
       str_replace:
         template:
            http://HOSTORT/
         params:
            HOST: {get_param: host}
            PORT: {get_param: port}

So the alternative is:

  parameters:
    host:
      type: string
    port:
      type: string

  resources:
    endpoint:
      type: OS::Heat::Value
      properties:
        type: string
        value:
          str_replace:
            template:
              http://HOST:PORT/
            params:
              HOST: {get_param: host}
              PORT: {get_param: port}

which is known to be trivial to implement, and has the advantage that it can also combine in data that is only available at runtime (e.g. it's easy to imagine that HOST might come from within the template, but that you'd still need to use the endpoint URL in enough places that it's not worth copying and pasting the whole str_replace function). Also, every template format would get it for free, which is nice.

The only downside is that you can't replace the whole value definition with a parameter value passed in from outside. Given that it would have to be a complete override including the substitute values (i.e. we can't parse functions from template _values_, only defaults), I'm not sure that is a big loss.

In my view it is almost a toss-up, but I am marginally in favour of a conditions section (per the current proposed implementation) plus a OS::Heat::Value resource over computable parameter defaults, mainly because the implementation would be simpler.

cheers,
Zane.

__________________________________________________________________________
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