GET /servers?status=BALONEY returns 400, should return 200 with empty list

Bug #1061712 reported by Jay Pipes
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
Fix Released
Undecided
Davanum Srinivas (DIMS)

Bug Description

This code in Tempest:

    def test_list_servers_status_is_invalid(self):
        """Return an empty list when invalid status is specified"""
        non_existing_status = 'BALONEY'
        resp, body = self.client.list_servers(dict(status=non_existing_status))
        servers = body['servers']
        self.assertEqual('200', resp['status'])
        self.assertEqual([], servers)

Currently fails with:

======================================================================
ERROR: Return an empty list when invalid status is specified
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/jpipes/repos/tempest/tempest/tests/compute/test_list_servers_negative.py", line 137, in test_list_servers_status_is_invalid
    resp, body = self.client.list_servers(dict(status=non_existing_status))
  File "/home/jpipes/repos/tempest/tempest/services/nova/json/servers_client.py", line 114, in list_servers
    resp, body = self.get(url)
  File "/home/jpipes/repos/tempest/tempest/common/rest_client.py", line 166, in get
    return self.request('GET', url, headers)
  File "/home/jpipes/repos/tempest/tempest/common/rest_client.py", line 208, in request
    raise exceptions.BadRequest(resp_body)
BadRequest: Bad request
Details: Bad request
Details: {u'badRequest': {u'message': u'Invalid server status: BALONEY', u'code': 400}}
-------------------- >> begin captured logging << --------------------
tempest.common.rest_client: ERROR: Request URL: http://127.0.0.1:8774/v2/4e0d6c1233154da0afb23f19e780befa/servers?status=BALONEY&
tempest.common.rest_client: ERROR: Request Body: None
tempest.common.rest_client: ERROR: Response Headers: {'date': 'Thu, 04 Oct 2012 15:51:21 GMT', 'status': '400', 'content-length': '74', 'content-type': 'application/json; charset=UTF-8', 'x-compute-request-id': 'req-4ca6978f-f67f-4111-b21c-c6e21541015d'}
tempest.common.rest_client: ERROR: Response Body: {u'badRequest': {u'message': u'Invalid server status: BALONEY', u'code': 400}}
--------------------- >> end captured logging << ---------------------

The return should be an empty list and 200 OK to make this consistent with other non-existing filters like flavour ID or changes-since dates in the future that return an empty list.

The fact is that the API indicates the status parameter is a string, but no more details. As such, status=BALONEY is a valid filter that should return no records since no records match that status.

-jay

Revision history for this message
Dan Prince (dan-prince) wrote :

I would say this may be a gray area where the spec doesn't explicitly state what should happen when an invalid status is specified.

In my opinion the Nova code is currently doing the correct thing by throwing a 400 if a bad status is specified. Here is the relevant code:

        # Verify search by 'status' contains a valid status.
        # Convert it to filter by vm_state for compute_api.
        status = search_opts.pop('status', None)
        if status is not None:
            state = common.vm_state_from_status(status)
            if state is None:
                msg = _('Invalid server status: %(status)s') % locals()
                raise exc.HTTPBadRequest(explanation=msg)
            search_opts['vm_state'] = state

Changed in nova:
status: Confirmed → Invalid
Revision history for this message
Jay Pipes (jaypipes) wrote :

Yes, Dan, I'm aware of what the code does. I'm saying the spec says "string", not "these specific strings", and therefore the 400 is wrong. It should return an empty list... I chatted with Waldon about this on IRC today and he said to expect disagreement.

Disagreement is fine, but I think I've laid out my case, you've laid out yours, and we should have the debate... not just set the bug to Invalid.

Changed in nova:
status: Invalid → Incomplete
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to nova (master)

Fix proposed to branch: master
Review: https://review.openstack.org/14765

Changed in nova:
assignee: nobody → Davanum Srinivas (dims-v)
status: Incomplete → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to nova (master)

Reviewed: https://review.openstack.org/14765
Committed: http://github.com/openstack/nova/commit/e142d21b47e8895a013c468936e69ba0e7e66041
Submitter: Jenkins
Branch: master

commit e142d21b47e8895a013c468936e69ba0e7e66041
Author: Davanum Srinivas <email address hidden>
Date: Wed Oct 24 10:15:39 2012 -0400

    Return empty list when listing servers with bad status value

    API indicates the status parameter is a string, hence status=BALONEY is a valid
    filter that should return zero records since no records match that status. The
    return should be an empty list and 200 OK.

    Fixes bug 1061712

    Change-Id: If796110a18dd75ab5c5d4ba41e9c3b58180341f8

Changed in nova:
status: In Progress → Fix Committed
Thierry Carrez (ttx)
Changed in nova:
milestone: none → grizzly-1
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in nova:
milestone: grizzly-1 → 2013.1
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to nova (stable/folsom)

Fix proposed to branch: stable/folsom
Review: https://review.openstack.org/34490

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to nova (stable/folsom)

Reviewed: https://review.openstack.org/34490
Committed: http://github.com/openstack/nova/commit/ad48866070bf1734be03f26c89b5af18bc1ad616
Submitter: Jenkins
Branch: stable/folsom

commit ad48866070bf1734be03f26c89b5af18bc1ad616
Author: Davanum Srinivas <email address hidden>
Date: Wed Oct 24 10:15:39 2012 -0400

    Return empty list when listing servers with bad status value

    API indicates the status parameter is a string, hence status=BALONEY is a valid
    filter that should return zero records since no records match that status. The
    return should be an empty list and 200 OK.

    Fixes bug 1061712

    Change-Id: If796110a18dd75ab5c5d4ba41e9c3b58180341f8
    (cherry picked from commit e142d21b47e8895a013c468936e69ba0e7e66041)

tags: added: in-stable-folsom
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.