Re: [openstack-dev] [heat] Issue with validation and preview due to get_attr==None

[Zane Bitter]

On 27/03/16 22:58, Anant Patil wrote:


On 24-Mar-16 20:26, Sergey Kraynev wrote:

Zane, I like you idea. As example we may discuss some steps for it
during summit session (if it need).

Also I have another question, which probably came in your heads a lot

of times:


A few times, but I usually chased it out again quite quickly ;)


Can we somekind improve our existing approach for validation?
we do validation twice - before create and during it.
The one issue, which I also see is:
first validation is a synchronous operation, and It takes a lot of
time, for huge stacks.


If we want to solve this problem then the first thing to do is to 
profile the code and figure out why it's taking so long in the first 
place. The validation code was written without much regard for premature 
optimisation on the assumption that if we ran into problems we would do 
that. It still shouldn't be super CPU-intensive though, so if it is we 
can fix it.


I suspect that will reveal the problem to be too many ReST API calls to 
validate custom constraints. If that's the case then the solution that 
suggests itself is to stop letting individual constraints validate 
themselves and instead having something walk the template and produce a 
list of constraints that can be validated together. So e.g. you'd have 
one "nova list" call to check that the 50 servers we need exist, rather 
than 50+ "nova show" calls to check the same thing.



May be we need to make separate state like
validation for stacks ? and maybe it also allows to solve our current
issue with build-in functions ?


There's definitely some overlap there, but I suspect not so much that 
implementing either one or the other would get us the second one for free.



I too had the same question since long time. I was thinking about two
possible solutions:

(1) Like you said, as of now the stack validation happens before the
response is given to user. This is not feasible for bigger stacks; the
RPC timesout before the stack template is even validated. I think we
should take the request and respond immediately with a stack-id and then
proceed with template validation and stack creation. If the template is
not correct, the stack creation or update fails with template invalid
error. I have been looking for this since long time, but could never work
on it.


Yeah, this is probably a good idea, as long as we can define some 
minimum level of correctness that we require before writing to the 
database. (Specifically, enough that we can always load the stack to 
delete it again :)



(2) Giving freedom to user to skip the validation, if they are sure
about the template, they might want to skip the validation [1].

[1]  https://review.openstack.org/#/c/182924/


I think that one's pretty problematic in general, but I'm open to some 
more specific formulations of the idea. I commented on the review.


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

Re: [openstack-dev] [heat] Issue with validation and preview due to get_attr==None

[Anant Patil]

On 24-Mar-16 20:26, Sergey Kraynev wrote:
> Zane, I like you idea. As example we may discuss some steps for it
> during summit session (if it need).
>
> Also I have another question, which probably came in your heads a lot
of times:
> Can we somekind improve our existing approach for validation?
> we do validation twice - before create and during it.
> The one issue, which I also see is:
> first validation is a synchronous operation, and It takes a lot of
> time, for huge stacks. May be we need to make separate state like
> validation for stacks ? and maybe it also allows to solve our current
> issue with build-in functions ?

I too had the same question since long time. I was thinking about two
possible solutions:

(1) Like you said, as of now the stack validation happens before the
response is given to user. This is not feasible for bigger stacks; the
RPC timesout before the stack template is even validated. I think we
should take the request and respond immediately with a stack-id and then
proceed with template validation and stack creation. If the template is
not correct, the stack creation or update fails with template invalid
error. I have been looking for this since long time, but could never work
on it.

(2) Giving freedom to user to skip the validation, if they are sure
about the template, they might want to skip the validation [1].

[1]  https://review.openstack.org/#/c/182924/

-- Anant

>
> On 23 March 2016 at 23:11, Zane Bitter  wrote:
>> On 23/03/16 13:14, Steven Hardy wrote:
>>>
>>> Hi all,
>>>
>>> I'm looking for some help and additional input on this bug:
>>>
>>> https://bugs.launchpad.net/heat/+bug/1559807
>>
>>
>> Hmm, I was wondering how this ever worked, but it appears you're making
>> particularly aggressive use of the list_join and map_merge Functions
there -
>> where you're not only getting the elements in the list of things to merge
>> (as presumably originally envisioned) but actually getting the list
itself
>> from an intrinsic function. If we're going to support that then those
>> functions need to handle the fact that the input argument may be
None, just
>> as they do for the list members (see the ensure_string() and ensure_map()
>> functions inside the result() methods of those two Functions).
>>
>>
>>> Basically, we have multiple issues due to the fact that we consider
>>> get_attr to resolve to None at any point before a resource is actually
>>> instantiated.
>>>
>>> It's due to this:
>>>
>>>
>>>
https://github.com/openstack/heat/blob/master/heat/engine/hot/functions.py#L163
>>>
>>> This then causes problems during validation of several intrinsic
>>> functions,
>>> because if they reference get_attr, they have to contain hacks and
>>> special-cases to work around the validate-time None value (or, as
reported
>>> in the bug, fail to validate when all would be fine at runtime).
>>>
>>>
>>>
https://github.com/openstack/heat/blob/master/heat/engine/resource.py#L1333
>>>
>>> I started digging into fixes, and there are probably a few possible
>>> approaches, e.g setting stack.Stack.strict_validate always to False, or
>>> reworking the intrinsic function validation to always work with the
>>> temporary None value.
>>>
>>> However, it's a more widespread issue than just validation - this
affects
>>> any action which happens before the actual stack gets created, so things
>>> like preview updates are also broken, e.g consider this:
>>>
>>> resources:
>>>random:
>>>  type: OS::Heat::RandomString
>>>
>>>config:
>>>  type: OS::Heat::StructuredConfig
>>>  properties:
>>>group: script
>>>config:
>>>  foo: {get_attr: [random, value]}
>>>
>>>deployment:
>>>  type: OS::Heat::StructuredDeployment
>>>  properties:
>>>config:
>>>  get_resource: config
>>>server: "dummy"
>>>
>>> On update, nothing is replaced, but if you do e.g:
>>>
>>>heat stack-update -x --dry-run
>>>
>>> You see this:
>>>
>>> | replaced  | config| OS::Heat::StructuredConfig |
>>>
>>> Which occurs due to the false comparison between the current value of
>>> "random" and the None value we get from get_attr in the temporary stack
>>> used for preview comparison:
>>>
>>>
https://github.com/openstack/heat/blob/master/heat/engine/resource.py#L528
>>>
>>> after_props.get(key) returns None, which makes us falsely declare the
>>> "config" resource gets replaced :(
>>>
>>> I'm looking for ideas on how we solve this - it's clearly a major issue
>>> which completely invalidates the results of validate and preview
>>> operations
>>> in many cases.
>>
>>
>> I've been thinking about this (for about 2 years).
>>
>> My first thought (it seemed like a good idea at the time, 2 years
ago, for
>> some reason) was for Function objects themselves to take on the types of
>> their return values, so e.g. a Function returning a list would have a
>> __getitem__ method and generally act like a list. Don't try this at home,
>> BTW, it 

Re: [openstack-dev] [heat] Issue with validation and preview due to get_attr==None

[Sergey Kraynev]

Zane, I like you idea. As example we may discuss some steps for it
during summit session (if it need).

Also I have another question, which probably came in your heads a lot of times:
Can we somekind improve our existing approach for validation?
we do validation twice - before create and during it.
The one issue, which I also see is:
first validation is a synchronous operation, and It takes a lot of
time, for huge stacks. May be we need to make separate state like
validation for stacks ? and maybe it also allows to solve our current
issue with build-in functions ?

On 23 March 2016 at 23:11, Zane Bitter  wrote:
> On 23/03/16 13:14, Steven Hardy wrote:
>>
>> Hi all,
>>
>> I'm looking for some help and additional input on this bug:
>>
>> https://bugs.launchpad.net/heat/+bug/1559807
>
>
> Hmm, I was wondering how this ever worked, but it appears you're making
> particularly aggressive use of the list_join and map_merge Functions there -
> where you're not only getting the elements in the list of things to merge
> (as presumably originally envisioned) but actually getting the list itself
> from an intrinsic function. If we're going to support that then those
> functions need to handle the fact that the input argument may be None, just
> as they do for the list members (see the ensure_string() and ensure_map()
> functions inside the result() methods of those two Functions).
>
>
>> Basically, we have multiple issues due to the fact that we consider
>> get_attr to resolve to None at any point before a resource is actually
>> instantiated.
>>
>> It's due to this:
>>
>>
>> https://github.com/openstack/heat/blob/master/heat/engine/hot/functions.py#L163
>>
>> This then causes problems during validation of several intrinsic
>> functions,
>> because if they reference get_attr, they have to contain hacks and
>> special-cases to work around the validate-time None value (or, as reported
>> in the bug, fail to validate when all would be fine at runtime).
>>
>>
>> https://github.com/openstack/heat/blob/master/heat/engine/resource.py#L1333
>>
>> I started digging into fixes, and there are probably a few possible
>> approaches, e.g setting stack.Stack.strict_validate always to False, or
>> reworking the intrinsic function validation to always work with the
>> temporary None value.
>>
>> However, it's a more widespread issue than just validation - this affects
>> any action which happens before the actual stack gets created, so things
>> like preview updates are also broken, e.g consider this:
>>
>> resources:
>>random:
>>  type: OS::Heat::RandomString
>>
>>config:
>>  type: OS::Heat::StructuredConfig
>>  properties:
>>group: script
>>config:
>>  foo: {get_attr: [random, value]}
>>
>>deployment:
>>  type: OS::Heat::StructuredDeployment
>>  properties:
>>config:
>>  get_resource: config
>>server: "dummy"
>>
>> On update, nothing is replaced, but if you do e.g:
>>
>>heat stack-update -x --dry-run
>>
>> You see this:
>>
>> | replaced  | config| OS::Heat::StructuredConfig |
>>
>> Which occurs due to the false comparison between the current value of
>> "random" and the None value we get from get_attr in the temporary stack
>> used for preview comparison:
>>
>> https://github.com/openstack/heat/blob/master/heat/engine/resource.py#L528
>>
>> after_props.get(key) returns None, which makes us falsely declare the
>> "config" resource gets replaced :(
>>
>> I'm looking for ideas on how we solve this - it's clearly a major issue
>> which completely invalidates the results of validate and preview
>> operations
>> in many cases.
>
>
> I've been thinking about this (for about 2 years).
>
> My first thought (it seemed like a good idea at the time, 2 years ago, for
> some reason) was for Function objects themselves to take on the types of
> their return values, so e.g. a Function returning a list would have a
> __getitem__ method and generally act like a list. Don't try this at home,
> BTW, it doesn't work.
>
> I now think the right answer is to return some placeholder object (but not
> None). Then the validating code can detect the placeholder and do some
> checks. e.g. we would be able to say that the placeholder for get_resource
> on a Cinder volume would have type 'cinder.volume' and any property with a
> custom constraint would check that type to see if it matches (and fall back
> to accepting any text type if the placeholder doesn't have a type
> associated). get_param would get its type from the parameter schema
> (including any custom constraints). For get_attr we could make it part of
> the attribute schema.
>
> The hard part obviously would be getting this to work with deeply-nested
> trees of data and across nested stacks. We could probably get the easy parts
> going and incrementally improve from there though. Worst case we just return
> None and get the same behaviour as now.
>
> cheers,
> Zane.
>
>
> 

Re: [openstack-dev] [heat] Issue with validation and preview due to get_attr==None

[Zane Bitter]

On 23/03/16 13:14, Steven Hardy wrote:

Hi all,

I'm looking for some help and additional input on this bug:

https://bugs.launchpad.net/heat/+bug/1559807


Hmm, I was wondering how this ever worked, but it appears you're making 
particularly aggressive use of the list_join and map_merge Functions 
there - where you're not only getting the elements in the list of things 
to merge (as presumably originally envisioned) but actually getting the 
list itself from an intrinsic function. If we're going to support that 
then those functions need to handle the fact that the input argument may 
be None, just as they do for the list members (see the ensure_string() 
and ensure_map() functions inside the result() methods of those two 
Functions).



Basically, we have multiple issues due to the fact that we consider
get_attr to resolve to None at any point before a resource is actually
instantiated.

It's due to this:

https://github.com/openstack/heat/blob/master/heat/engine/hot/functions.py#L163

This then causes problems during validation of several intrinsic functions,
because if they reference get_attr, they have to contain hacks and
special-cases to work around the validate-time None value (or, as reported
in the bug, fail to validate when all would be fine at runtime).

https://github.com/openstack/heat/blob/master/heat/engine/resource.py#L1333

I started digging into fixes, and there are probably a few possible
approaches, e.g setting stack.Stack.strict_validate always to False, or
reworking the intrinsic function validation to always work with the
temporary None value.

However, it's a more widespread issue than just validation - this affects
any action which happens before the actual stack gets created, so things
like preview updates are also broken, e.g consider this:

resources:
   random:
 type: OS::Heat::RandomString

   config:
 type: OS::Heat::StructuredConfig
 properties:
   group: script
   config:
 foo: {get_attr: [random, value]}

   deployment:
 type: OS::Heat::StructuredDeployment
 properties:
   config:
 get_resource: config
   server: "dummy"

On update, nothing is replaced, but if you do e.g:

   heat stack-update -x --dry-run

You see this:

| replaced  | config| OS::Heat::StructuredConfig |

Which occurs due to the false comparison between the current value of
"random" and the None value we get from get_attr in the temporary stack
used for preview comparison:

https://github.com/openstack/heat/blob/master/heat/engine/resource.py#L528

after_props.get(key) returns None, which makes us falsely declare the
"config" resource gets replaced :(

I'm looking for ideas on how we solve this - it's clearly a major issue
which completely invalidates the results of validate and preview operations
in many cases.


I've been thinking about this (for about 2 years).

My first thought (it seemed like a good idea at the time, 2 years ago, 
for some reason) was for Function objects themselves to take on the 
types of their return values, so e.g. a Function returning a list would 
have a __getitem__ method and generally act like a list. Don't try this 
at home, BTW, it doesn't work.


I now think the right answer is to return some placeholder object (but 
not None). Then the validating code can detect the placeholder and do 
some checks. e.g. we would be able to say that the placeholder for 
get_resource on a Cinder volume would have type 'cinder.volume' and any 
property with a custom constraint would check that type to see if it 
matches (and fall back to accepting any text type if the placeholder 
doesn't have a type associated). get_param would get its type from the 
parameter schema (including any custom constraints). For get_attr we 
could make it part of the attribute schema.


The hard part obviously would be getting this to work with deeply-nested 
trees of data and across nested stacks. We could probably get the easy 
parts going and incrementally improve from there though. Worst case we 
just return None and get the same behaviour as now.


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

Re: [openstack-dev] [heat] Issue with validation and preview due to get_attr==None

[Jay Dobies]

This is the same issue I ran into a few months ago regarding the nested 
parameter validation. Since it resolves to None at that time, there's no 
hook in our current nested parameters implementation to show that it 
will have a value passed in from the parent template.


Unfortunately, I don't have much to offer in terms of a solution, but 
I'm very interested in where this conversation goes :)


On 3/23/16 1:14 PM, Steven Hardy wrote:

Hi all,

I'm looking for some help and additional input on this bug:

https://bugs.launchpad.net/heat/+bug/1559807

Basically, we have multiple issues due to the fact that we consider
get_attr to resolve to None at any point before a resource is actually
instantiated.

It's due to this:

https://github.com/openstack/heat/blob/master/heat/engine/hot/functions.py#L163

This then causes problems during validation of several intrinsic functions,
because if they reference get_attr, they have to contain hacks and
special-cases to work around the validate-time None value (or, as reported
in the bug, fail to validate when all would be fine at runtime).

https://github.com/openstack/heat/blob/master/heat/engine/resource.py#L1333

I started digging into fixes, and there are probably a few possible
approaches, e.g setting stack.Stack.strict_validate always to False, or
reworking the intrinsic function validation to always work with the
temporary None value.

However, it's a more widespread issue than just validation - this affects
any action which happens before the actual stack gets created, so things
like preview updates are also broken, e.g consider this:

resources:
   random:
 type: OS::Heat::RandomString

   config:
 type: OS::Heat::StructuredConfig
 properties:
   group: script
   config:
 foo: {get_attr: [random, value]}

   deployment:
 type: OS::Heat::StructuredDeployment
 properties:
   config:
 get_resource: config
   server: "dummy"

On update, nothing is replaced, but if you do e.g:

   heat stack-update -x --dry-run

You see this:

| replaced  | config| OS::Heat::StructuredConfig |

Which occurs due to the false comparison between the current value of
"random" and the None value we get from get_attr in the temporary stack
used for preview comparison:

https://github.com/openstack/heat/blob/master/heat/engine/resource.py#L528

after_props.get(key) returns None, which makes us falsely declare the
"config" resource gets replaced :(

I'm looking for ideas on how we solve this - it's clearly a major issue
which completely invalidates the results of validate and preview operations
in many cases.

Steve

__
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



__
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

[openstack-dev] [heat] Issue with validation and preview due to get_attr==None

[Steven Hardy]

Hi all,

I'm looking for some help and additional input on this bug:

https://bugs.launchpad.net/heat/+bug/1559807

Basically, we have multiple issues due to the fact that we consider
get_attr to resolve to None at any point before a resource is actually
instantiated.

It's due to this:

https://github.com/openstack/heat/blob/master/heat/engine/hot/functions.py#L163

This then causes problems during validation of several intrinsic functions,
because if they reference get_attr, they have to contain hacks and
special-cases to work around the validate-time None value (or, as reported
in the bug, fail to validate when all would be fine at runtime).

https://github.com/openstack/heat/blob/master/heat/engine/resource.py#L1333

I started digging into fixes, and there are probably a few possible
approaches, e.g setting stack.Stack.strict_validate always to False, or
reworking the intrinsic function validation to always work with the
temporary None value.

However, it's a more widespread issue than just validation - this affects
any action which happens before the actual stack gets created, so things
like preview updates are also broken, e.g consider this:

resources:
  random:
type: OS::Heat::RandomString

  config:
type: OS::Heat::StructuredConfig
properties:
  group: script
  config:
foo: {get_attr: [random, value]}

  deployment:
type: OS::Heat::StructuredDeployment
properties:
  config:
get_resource: config
  server: "dummy"

On update, nothing is replaced, but if you do e.g:

  heat stack-update -x --dry-run

You see this:

| replaced  | config| OS::Heat::StructuredConfig |

Which occurs due to the false comparison between the current value of
"random" and the None value we get from get_attr in the temporary stack
used for preview comparison:

https://github.com/openstack/heat/blob/master/heat/engine/resource.py#L528

after_props.get(key) returns None, which makes us falsely declare the
"config" resource gets replaced :(

I'm looking for ideas on how we solve this - it's clearly a major issue
which completely invalidates the results of validate and preview operations
in many cases.

Steve

__
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