CreateStack non-graceful failure for invalid template-url server

Bug #1072951 reported by Angus Salkeld
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Heat
Fix Released
Medium
Jeff Peeler
Grizzly
Fix Released
Medium
Jeff Peeler

Bug Description

If the server specified in --template-url is valid, but is not running an http server, the API gets stuck waiting for a response resulting in the following non-graceful failure at the client side.

I guess we need some more sanity checks in the API code to avoid this

[root@heatlt heat]# service httpd stop
Stopping httpd (via systemctl): [ OK ]

[root@heatlt heat]# heat -d create wordpress --template-url=http://localhost/badd --parameters="InstanceType=m1.xlarge;DBUsername=${USER};DBPassword=verybadpass;KeyName=${USER}_key"
DEBUG:Debug level logging enabled
Traceback (most recent call last):
  File "/usr/bin/heat", line 5, in <module>
    pkg_resources.run_script('heat==5', 'heat')
  File "/usr/lib/python2.7/site-packages/pkg_resources.py", line 499, in run_script
    self.require(requires)[0].run_script(script_name, ns)
  File "/usr/lib/python2.7/site-packages/pkg_resources.py", line 1235, in run_script
    execfile(script_filename, namespace, namespace)
  File "/usr/lib/python2.7/site-packages/heat-5-py2.7.egg/EGG-INFO/scripts/heat", line 622, in <module>
    main()
  File "/usr/lib/python2.7/site-packages/heat-5-py2.7.egg/EGG-INFO/scripts/heat", line 609, in main
    result = cmd(opts, args)
  File "/usr/lib/python2.7/site-packages/heat-5-py2.7.egg/heat/utils.py", line 39, in wrapper
    ret = func(*arguments, **kwargs)
  File "/usr/lib/python2.7/site-packages/heat-5-py2.7.egg/EGG-INFO/scripts/heat", line 190, in stack_create
    result = c.create_stack(**parameters)
  File "/usr/lib/python2.7/site-packages/heat-5-py2.7.egg/heat/client.py", line 57, in create_stack
    return self.stack_request("CreateStack", "POST", **kwargs)
  File "/usr/lib/python2.7/site-packages/heat-5-py2.7.egg/heat/client.py", line 46, in stack_request
    res = self.do_request(method, "/", params=params)
  File "/usr/lib/python2.7/site-packages/heat-5-py2.7.egg/heat/common/client.py", line 56, in wrapped
    return func(self, *args, **kwargs)
  File "/usr/lib/python2.7/site-packages/heat-5-py2.7.egg/heat/common/client.py", line 418, in do_request
    headers=headers)
  File "/usr/lib/python2.7/site-packages/heat-5-py2.7.egg/heat/common/client.py", line 73, in wrapped
    return func(self, method, url, body, headers)
  File "/usr/lib/python2.7/site-packages/heat-5-py2.7.egg/heat/common/client.py", line 521, in _do_request
    res = c.getresponse()
  File "/usr/lib64/python2.7/httplib.py", line 1030, in getresponse
    response.begin()
  File "/usr/lib64/python2.7/httplib.py", line 407, in begin
    version, status, reason = self._read_status()
  File "/usr/lib64/python2.7/httplib.py", line 371, in _read_status
    raise BadStatusLine(line)
httplib.BadStatusLine: '

shardy
=======
Couple of notes from my initial investigation:

Problem is in api/v1/stacks.py StackController::_get_template
The httplib.HTTPConnection succeeds provided the host exists, regardless of whether the http(s) server is up
Behavior seems slightly different between F16 and F17 - on F16 IIRC the conn.request blocks for a long time causing a null response to the client, whereas IIRC on F17 the request actually does throw an error (which should make it easier as we can just try/catch around it)

Angus Salkeld (asalkeld)
Changed in heat:
status: New → Triaged
importance: Undecided → Medium
Revision history for this message
Jeff Peeler (jpeeler-z) wrote :

I was looking at this and wasn't sure how catching the error in _get_template would be all that useful. Is it alright to just make the traceback a little better? Otherwise, try/except blocks would need to also be put in create_or_update as well as client.py: stack_request. Having a generic except block at such a high level would eliminate potential useful debugging information for unexpected errors, right?

Revision history for this message
Steven Hardy (shardy) wrote :

The problem is that the error which ends up being displayed to the user provides no clue what the actual problem is, what should happen (IMO) is that the API should catch the error internally, and return a HeatInvalidParameterValueError

We already have a try/catch in creat_or_update which will allow this:

try:
            templ = self._get_template(req)
except socket.gaierror:
            msg = _('Invalid Template URL')
            return exception.HeatInvalidParameterValueError(detail=msg)

All we need to do is make _get_template do some additional checks, and raise some sort of error which can be caught in addition to socket.gaierror - then perhaps we can append some useful info to msg which will provide the user with a clue as to what is wrong "Invalid Template URL : Server foo appears to be down or is not responding.." or something.

While I agree we should not catch errors in a way which prevents debugging, we should not ever (ideally) fail in a way which causes the client code to spew unhelpful errors at the users - we really need to ensure that all api errors are caught inside the API, and that these errors are mapped internally to the most appropriate exception in api/aws/exception.py (for the CFN API at least), with detail added through the exception detail string to help the user figure out what went wrong.

Changed in heat:
status: Triaged → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to heat (master)

Reviewed: https://review.openstack.org/16445
Committed: http://github.com/heat-api/heat/commit/90b747061c7bf8efe624eeb7842789ac98a45709
Submitter: Jenkins
Branch: master

commit 90b747061c7bf8efe624eeb7842789ac98a45709
Author: Jeff Peeler <email address hidden>
Date: Mon Nov 19 12:10:12 2012 -0500

    Provide more information with template URL error

    Previously a generic read error was given. Inform the user with proper
    exceptions for: the case when a resource is present but non-responsive
    and when a resource is present but the requested data could not be
    found.

    Fixes bug 1072951

    Change-Id: I35b92cea38358691015f8752e80efc6720e34e48
    Signed-off-by: Jeff Peeler <email address hidden>

Changed in heat:
status: In Progress → Fix Committed
Thierry Carrez (ttx)
Changed in heat:
milestone: none → grizzly-2
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in heat:
milestone: grizzly-2 → 2013.1
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.