Re: [openstack-dev] [Heat] Dealing with nonexistent resources during resource-list / stack-delete

[Johannes Grassler]

On 03/08/2016 06:26 PM, Zane Bitter wrote:

On 08/03/16 08:03, Johannes Grassler wrote:
I think you're being a little too pessimistic. I was going to explain one
approach, but it turned out to be easier to just submit a patch:

https://review.openstack.org/290027


Ah, that one looks good, thanks :-)


I believe this satisfies requirements 1, 2, and 4. I agree that there's no
way we can really tackle 3 as far as third-party plugins go, but at least all
explicit dependencies are guaranteed to be added. For the implicit ones, it's
up to plugin authors not to screw it up.


That's fair enough and makes for a good warning to include in the docs.


BTW I created https://bugs.launchpad.net/heat/+bug/1554625 to cover this
larger issue (as opposed to the specific problem you are fixing in bug
1442121); that would probably be a good one to reference in your docs
changes.


Ok, thanks. That link will prevent a lot of unwieldy problem explanation people
are unlikely to read anyway :-)

Cheers,

Johannes

--
Johannes Grassler, Cloud Developer
SUSE Linux GmbH, HRB 21284 (AG Nürnberg)
GF: Felix Imendörffer, Jane Smithard, Graham Norton
Maxfeldstr. 5, 90409 Nürnberg, Germany

__
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] Dealing with nonexistent resources during resource-list / stack-delete

[Steve Baker]

On 09/03/16 05:41, Johannes Grassler wrote:

Hello,

On 03/08/2016 04:57 PM, Zane Bitter wrote:

On 08/03/16 10:40, Johannes Grassler wrote:

On 03/07/2016 04:48 PM, Zane Bitter wrote:

On 04/03/16 04:35, Johannes Grassler wrote:

[Uncaught client exceptions in resource plugins' add_dependencies()
methods]

In the meantime, we need to find and squash every instance of this
problem
wherever we can like you said.


It might also be a good idea to caution against unchecked API client
invocations in

http://docs.openstack.org/developer/heat/developing_guides/pluginguide.html 


[...]

It's best if they *do* omit it entirely. The only reason we override 
it in
the Neutron resources is that the Neutron API is terrible for 
orchestration

purposes[1]. It adds a bunch of invisible, fragile magic that breaks in
subtle ways when e.g. resources are moved into nested stacks.


I never saw the Neutron API that way before (I guess I just got used 
to the

unintuitive bits), but seeing it spelled out in your rant makes it very
obvious, yes. I didn't know that was the root cause for overriding
add_dependencies() and that very ignorance of mine...

The default implementation provides everything that we *ought* to 
need, so if
we document anything I think it should be that plugin developers 
should not

touch add_dependencies() at all.


...suggests mentioning that much is probably a good idea (lest 
somebody pick
one of the Neutron plugins as a template to base their own resource 
plugin

on).


Definitely not big enough to require a spec IMO.


Yes, I can see that, given how it's not something plugin writers 
should do
anyway. Then I'll just write a little paragraph cautioning against 
overriding

add_dependcies() and add a Related-Bug: line.

Thanks for that. The paragraph could say that it is preferable to not 
override add_dependencies, but if they do then they should *never* make 
REST API calls inside it. This is for the reason you've discovered, and 
also because it will kill the performance of some stack operations.


From what I can see you've discovered the only 2 places where REST API 
calls are made from add_dependencies, I've added comments to the 
review[1] to suggest how they can be removed.


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

__
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] Dealing with nonexistent resources during resource-list / stack-delete

[Zane Bitter]

On 08/03/16 08:03, Johannes Grassler wrote:

Hello,

On 03/07/2016 04:48 PM, Zane Bitter wrote:

On 04/03/16 04:35, Johannes Grassler wrote:

[Uncaught client exceptions in resource plugins' add_dependencies()
methods]

Yes, you're right and this sucks. That's not the only problem we've
had in
this area recently - for example there was also:

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

The fact that we have to have these hacked in implicit dependencies at
all is
terrible, but we really need to make sure they can't break basic
operations
like loading a stack from the DB so we can show or delete it. The
ideal would
be:

* We can guarantee to catch all (non-exit) exceptions, no matter what
kind of
crazy stuff people write in add_dependencies() * The plugin developer
doesn't
have to do anything special to get this behaviour


Just these two are a tall order already. One could have the client plugins
default to ignoring 404s (maybe by adding a `ignore_defaults` keyword
argument
to the client_plugin() method that defaults to True).

That will break third-party plugins that need to handle this exception
themselves for one reason or another. Are there such plugins, or could
there
conceivably be such plugins? I can't think of a likely scenario off the
top of
my head, but since I'm not familiar with any third party plugins that
may not
mean much.

Also, and probably more serious, it may require a potentially largish
number of
adaptations to core heat code in places where this behaviour is undesirable
(and that might in turn cause new bugs).


* The code remains backwards compatible with any third-party resource
plugins
circulating out there


...and that rules out handling the exception at a higher level (i.e.
whereever
add_dependencies() is called). Doing that creates a lot of possibilities
to end
up with a messy dependency graph.


* We always add as many dependencies as possible (i.e. all
non-exception-raising dependencies are added)


That pretty much means code in the add_dependcies() method, and the only
feasible way I see of influencing this code is finding a way to
selectively get
the client plugins to default to ignoring dependencies.


* Genuine dependency problems (e.g. non-existent target of
get_resource/get_attr) are still surfaced, preferably on CREATE only


Ignoring the 404s at a higher level may be feasible in the DELETE case,
but I'm
not sure how bad a problem a possibly incomplete dependency graph
creates for
stack-delete: is it a complete show stopper or just a matter of issuing
multiple stack-delete requests until all resources are gone?


I think you're being a little too pessimistic. I was going to explain 
one approach, but it turned out to be easier to just submit a patch:


https://review.openstack.org/290027

I believe this satisfies requirements 1, 2, and 4. I agree that there's 
no way we can really tackle 3 as far as third-party plugins go, but at 
least all explicit dependencies are guaranteed to be added. For the 
implicit ones, it's up to plugin authors not to screw it up.


BTW I created https://bugs.launchpad.net/heat/+bug/1554625 to cover this 
larger issue (as opposed to the specific problem you are fixing in bug 
1442121); that would probably be a good one to reference in your docs 
changes.


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] Dealing with nonexistent resources during resource-list / stack-delete

[Johannes Grassler]

Hello,

On 03/08/2016 04:57 PM, Zane Bitter wrote:

On 08/03/16 10:40, Johannes Grassler wrote:

On 03/07/2016 04:48 PM, Zane Bitter wrote:

On 04/03/16 04:35, Johannes Grassler wrote:

[Uncaught client exceptions in resource plugins' add_dependencies()
methods]

In the meantime, we need to find and squash every instance of this
problem
wherever we can like you said.


It might also be a good idea to caution against unchecked API client
invocations in

http://docs.openstack.org/developer/heat/developing_guides/pluginguide.html

[...]


It's best if they *do* omit it entirely. The only reason we override it in
the Neutron resources is that the Neutron API is terrible for orchestration
purposes[1]. It adds a bunch of invisible, fragile magic that breaks in
subtle ways when e.g. resources are moved into nested stacks.


I never saw the Neutron API that way before (I guess I just got used to the
unintuitive bits), but seeing it spelled out in your rant makes it very
obvious, yes. I didn't know that was the root cause for overriding
add_dependencies() and that very ignorance of mine...


The default implementation provides everything that we *ought* to need, so if
we document anything I think it should be that plugin developers should not
touch add_dependencies() at all.


...suggests mentioning that much is probably a good idea (lest somebody pick
one of the Neutron plugins as a template to base their own resource plugin
on).


Definitely not big enough to require a spec IMO.


Yes, I can see that, given how it's not something plugin writers should do
anyway. Then I'll just write a little paragraph cautioning against overriding
add_dependcies() and add a Related-Bug: line.

Cheers,

Johannes

--
Johannes Grassler, Cloud Developer
SUSE Linux GmbH, HRB 21284 (AG Nürnberg)
GF: Felix Imendörffer, Jane Smithard, Graham Norton
Maxfeldstr. 5, 90409 Nürnberg, Germany

__
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] Dealing with nonexistent resources during resource-list / stack-delete

[Zane Bitter]

On 08/03/16 10:40, Johannes Grassler wrote:

Hello,

On 03/07/2016 04:48 PM, Zane Bitter wrote:

On 04/03/16 04:35, Johannes Grassler wrote:

[Uncaught client exceptions in resource plugins' add_dependencies()
methods]

In the meantime, we need to find and squash every instance of this
problem
wherever we can like you said.


It might also be a good idea to caution against unchecked API client
invocations in

http://docs.openstack.org/developer/heat/developing_guides/pluginguide.html

until a real fix for the problem is in place. Also, that document does
not even
mention the add_dependencies() method, leaving plugin developers that
much more
room to come up with dodgy code that does weird stuff in
add_dependencies() or
even omits it entirely.


It's best if they *do* omit it entirely. The only reason we override it 
in the Neutron resources is that the Neutron API is terrible for 
orchestration purposes[1]. It adds a bunch of invisible, fragile magic 
that breaks in subtle ways when e.g. resources are moved into nested 
stacks. The default implementation provides everything that we *ought* 
to need, so if we document anything I think it should be that plugin 
developers should not touch add_dependencies() at all.


If they must, though, then the relevant guidelines should be to ensure 
that any exceptions raised:


1) are deterministic; and
2) should prevent the stack from being created.


[1] Obligatory rant: 
http://lists.openstack.org/pipermail/openstack-dev/2014-April/032098.html



I haven't written any plugins so far, but I
suppose I
could update that part of the documentation - I've become rather
familiar this part of the code while fixing
.

If I wanted to add a section on add_dependencies() and its pitfalls to
pluginguide.rst, how would I go about it? Just submit a change with a
"Partial-Bug: #1442121" or is that big enough to require a spec?


Definitely not big enough to require a spec IMO.

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] Dealing with nonexistent resources during resource-list / stack-delete

[Johannes Grassler]

Hello,

On 03/07/2016 04:48 PM, Zane Bitter wrote:

On 04/03/16 04:35, Johannes Grassler wrote:

[Uncaught client exceptions in resource plugins' add_dependencies() methods]

In the meantime, we need to find and squash every instance of this problem
wherever we can like you said.


It might also be a good idea to caution against unchecked API client
invocations in

http://docs.openstack.org/developer/heat/developing_guides/pluginguide.html

until a real fix for the problem is in place. Also, that document does not even
mention the add_dependencies() method, leaving plugin developers that much more
room to come up with dodgy code that does weird stuff in add_dependencies() or
even omits it entirely. I haven't written any plugins so far, but I suppose I
could update that part of the documentation - I've become rather familiar this part 
of the code while fixing .

If I wanted to add a section on add_dependencies() and its pitfalls to
pluginguide.rst, how would I go about it? Just submit a change with a
"Partial-Bug: #1442121" or is that big enough to require a spec?

Cheers,

Johannes

--
Johannes Grassler, Cloud Developer
SUSE Linux GmbH, HRB 21284 (AG Nürnberg)
GF: Felix Imendörffer, Jane Smithard, Graham Norton
Maxfeldstr. 5, 90409 Nürnberg, Germany

__
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] Dealing with nonexistent resources during resource-list / stack-delete

[Johannes Grassler]

Hello,

On 03/07/2016 04:48 PM, Zane Bitter wrote:

On 04/03/16 04:35, Johannes Grassler wrote:

[Uncaught client exceptions in resource plugins' add_dependencies() methods]

Yes, you're right and this sucks. That's not the only problem we've had in
this area recently - for example there was also:

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

The fact that we have to have these hacked in implicit dependencies at all is
terrible, but we really need to make sure they can't break basic operations
like loading a stack from the DB so we can show or delete it. The ideal would
be:

* We can guarantee to catch all (non-exit) exceptions, no matter what kind of
crazy stuff people write in add_dependencies() * The plugin developer doesn't
have to do anything special to get this behaviour


Just these two are a tall order already. One could have the client plugins
default to ignoring 404s (maybe by adding a `ignore_defaults` keyword argument
to the client_plugin() method that defaults to True).

That will break third-party plugins that need to handle this exception
themselves for one reason or another. Are there such plugins, or could there
conceivably be such plugins? I can't think of a likely scenario off the top of
my head, but since I'm not familiar with any third party plugins that may not
mean much.

Also, and probably more serious, it may require a potentially largish number of
adaptations to core heat code in places where this behaviour is undesirable
(and that might in turn cause new bugs).


* The code remains backwards compatible with any third-party resource plugins
circulating out there


...and that rules out handling the exception at a higher level (i.e. whereever
add_dependencies() is called). Doing that creates a lot of possibilities to end
up with a messy dependency graph.


* We always add as many dependencies as possible (i.e. all
non-exception-raising dependencies are added)


That pretty much means code in the add_dependcies() method, and the only
feasible way I see of influencing this code is finding a way to selectively get
the client plugins to default to ignoring dependencies.


* Genuine dependency problems (e.g. non-existent target of
get_resource/get_attr) are still surfaced, preferably on CREATE only


Ignoring the 404s at a higher level may be feasible in the DELETE case, but I'm
not sure how bad a problem a possibly incomplete dependency graph creates for
stack-delete: is it a complete show stopper or just a matter of issuing
multiple stack-delete requests until all resources are gone?

Cheers,

Johannes

--
Johannes Grassler, Cloud Developer
SUSE Linux GmbH, HRB 21284 (AG Nürnberg)
GF: Felix Imendörffer, Jane Smithard, Graham Norton
Maxfeldstr. 5, 90409 Nürnberg, Germany

__
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] Dealing with nonexistent resources during resource-list / stack-delete

[Zane Bitter]

On 04/03/16 04:35, Johannes Grassler wrote:

Hello,

I am currently looking into https://bugs.launchpad.net/heat/+bug/1442121
and
not quite sure how to tackle it, since the problematic code is used for
lots of
things[0]: the root cause of the problem are API clients in resource
plugins
that do not anticipate a resource with an entry in Heat's database
having been
deleted in the implementing service's database[1]. Here's an example:

https://github.com/openstack/heat/blob/e4dc942ce1a8c79b450345c7afae326c80d8a5d6/heat/engine/resources/openstack/neutron/floatingip.py#L179


If that happens[1] an uncaught exception will be thrown that among other
things
breaks the very operations one would need for cleaning up the mess.

As far as I can see it, the cleanest way would be to go through all
resources
with a fine comb and add exception handling to the API calls in the
add_dependencies() method where it is missing (just return False for any
resource that no longer exists). Or is there a better way?


Yes, you're right and this sucks. That's not the only problem we've had 
in this area recently - for example there was also:


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

The fact that we have to have these hacked in implicit dependencies at 
all is terrible, but we really need to make sure they can't break basic 
operations like loading a stack from the DB so we can show or delete it. 
The ideal would be:


* We can guarantee to catch all (non-exit) exceptions, no matter what 
kind of crazy stuff people write in add_dependencies()
* The plugin developer doesn't have to do anything special to get this 
behaviour
* The code remains backwards compatible with any third-party resource 
plugins circulating out there
* We always add as many dependencies as possible (i.e. all 
non-exception-raising dependencies are added)
* Genuine dependency problems (e.g. non-existent target of 
get_resource/get_attr) are still surfaced, preferably on CREATE only


I'm pretty sure getting all of those is impossible. I'd be very 
interested in evaluating different tradeoffs we could make among them 
though.


In the meantime, we need to find and squash every instance of this 
problem wherever we can like you said.


cheers,
Zane.



Cheers,

Johannes

Footnotes:

[0] Whenever a stack's resources are being listed using
 heat.engine.service.list_stack_resources(). resource-list and
stack-delete,
 all invoke list_stack_resources()). stack-abandon does so
indirectly (it
 appears to trigger stack-delete judging by the log, but it yields the
 desired output, at least in Liberty). These are just the ones I
tested,
 there are probably more.

[1] It can happen for a number of reasons, either due to resource
dependency
 problems upon stack-delete as it happened in the original bug
report or due
 to an operator accidently deleting resources that are managed by Heat.





__
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] Dealing with nonexistent resources during resource-list / stack-delete

[Johannes Grassler]

Hello,

I am currently looking into https://bugs.launchpad.net/heat/+bug/1442121 and
not quite sure how to tackle it, since the problematic code is used for lots of
things[0]: the root cause of the problem are API clients in resource plugins
that do not anticipate a resource with an entry in Heat's database having been
deleted in the implementing service's database[1]. Here's an example:

https://github.com/openstack/heat/blob/e4dc942ce1a8c79b450345c7afae326c80d8a5d6/heat/engine/resources/openstack/neutron/floatingip.py#L179

If that happens[1] an uncaught exception will be thrown that among other things
breaks the very operations one would need for cleaning up the mess.

As far as I can see it, the cleanest way would be to go through all resources
with a fine comb and add exception handling to the API calls in the
add_dependencies() method where it is missing (just return False for any
resource that no longer exists). Or is there a better way?

Cheers,

Johannes

Footnotes:

[0] Whenever a stack's resources are being listed using
heat.engine.service.list_stack_resources(). resource-list and stack-delete,
all invoke list_stack_resources()). stack-abandon does so indirectly (it
appears to trigger stack-delete judging by the log, but it yields the
desired output, at least in Liberty). These are just the ones I tested,
there are probably more.

[1] It can happen for a number of reasons, either due to resource dependency
problems upon stack-delete as it happened in the original bug report or due
to an operator accidently deleting resources that are managed by Heat.


--
Johannes Grassler, Cloud Developer
SUSE Linux GmbH, HRB 21284 (AG Nürnberg)
GF: Felix Imendörffer, Jane Smithard, Graham Norton
Maxfeldstr. 5, 90409 Nürnberg, Germany

__
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