Instances filters for server name, flavor and status no longer work

Confirmed in local testing.

So, it turns out that name search parameters are not passed to exact match column filters any more, but instead are passed to Regex filters :( And it is this that is incorrectly filtering the wrong instances from the output. I added some debug output into nova-api in order to identify the problem...

2012-07-24 11:46:28 DEBUG nova.db.sqlalchemy.api [req-7ecfafb0-3619-49ef-b9f8-dc55eea647d1 ListServerFiltersTest-user ListServerFiltersTest-tenant] *** BEFORE REGEX *** [{'vm_state': u'active', 'availability_zone': None, 'terminated_at': None, 'ramdisk_id': u'a529e5e8-451f-4049-bad1-2d884111d712', 'instance_type_id': 2L, 'updated_at': datetime.datetime(2012, 7, 24, 15, 46, 26), 'vm_mode': None, 'deleted_at': None, 'reservation_id': u'r-tt0b5sbj', 'id': 13L, 'disable_terminate': False, 'root_device_name': u'/dev/vda', 'user_id': u'c04b37baeb164a9689e8f0a0195a2e32', 'uuid': u'4576e6d3-d257-4e28-b68b-30e3efe50d52', 'server_name': None, 'default_swap_device': None, 'hostname': u'server63679872311', 'launched_on': u'uberbox', 'display_description': u'server63679872311', 'key_data': None, 'kernel_id': u'00c2b5f2-75ce-46f2-81d9-775ea5a8682b', 'power_state': 1L, 'default_ephemeral_device': None, 'progress': 0L, 'project_id': u'ea7414d9efe0467f96a8bbf29d0bcd60', 'launched_at': datetime.datetime(2012, 7, 24, 15, 46, 26), 'scheduled_at': datetime.datetime(2012, 7, 24, 15, 46), 'ephemeral_gb': 0L, 'access_ip_v6': None, 'access_ip_v4': None, 'deleted': False, 'key_name': None, 'user_data': u'', 'host': u'uberbox', 'display_name': u'server63679872311', 'task_state': None, 'shutdown_terminate': False, 'root_gb': 0L, 'locked': False, 'created_at': datetime.datetime(2012, 7, 24, 15, 46), 'launch_index': 0L, 'memory_mb': 512L, 'vcpus': 1L, 'image_ref': u'7f697b5a-65a2-4130-a891-a9d6d4c856ed', 'architecture': None, 'auto_disk_config': None, 'os_type': None, 'config_drive': u''}, {'vm_state': u'active', 'availability_zone': None, 'terminated_at': None, 'ramdisk_id': u'a529e5e8-451f-4049-bad1-2d884111d712', 'instance_type_id': 2L, 'updated_at': datetime.datetime(2012, 7, 24, 15, 46, 24), 'vm_mode': None, 'deleted_at': None, 'reservation_id': u'r-bpytbf2j', 'id': 14L, 'disable_terminate': False, 'root_device_name': u'/dev/vda', 'user_id': u'c04b37baeb164a9689e8f0a0195a2e32', 'uuid': u'd14f038b-d258-4391-858d-e4bf300e7a99', 'server_name': None, 'default_swap_device': None, 'hostname': u'server952716972', 'launched_on': u'uberbox', 'display_description': u'server952716972', 'key_data': None, 'kernel_id': u'00c2b5f2-75ce-46f2-81d9-775ea5a8682b', 'power_state': 1L, 'default_ephemeral_device': None, 'progress': 0L, 'project_id': u'ea7414d9efe0467f96a8bbf29d0bcd60', 'launched_at': datetime.datetime(2012, 7, 24, 15, 46, 24), 'scheduled_at': datetime.datetime(2012, 7, 24, 15, 46), 'ephemeral_gb': 0L, 'access_ip_v6': None, 'access_ip_v4': None, 'deleted': False, 'key_name': None, 'user_data': u'', 'host': u'uberbox', 'display_name': u'server952716972', 'task_state': None, 'shutdown_terminate': False, 'root_gb': 0L, 'locked': False, 'created_at': datetime.datetime(2012, 7, 24, 15, 46), 'launch_index': 0L, 'memory_mb': 512L, 'vcpus': 1L, 'image_ref': u'7f697b5a-65a2-4130-a891-a9...

From examining the returned dicts, it seems like some column names were changed (display_name -> server_name). Investigating further...

FINALLY figured out what is going on here...

Turns out that the instance dict returned from POST /servers recently changed. The key_name key was added to this return.

However, the instance dicts returned in a call to GET /servers DO NOT contain this key_name key. This is what caused the line:

self.assertTrue(self.s1 in servers)

to fail, because the dicts were not equal, as shown in this Python shell output:

>>> print sorted(d.keys())
[u'OS-DCF:diskConfig', u'OS-EXT-STS:power_state', u'OS-EXT-STS:task_state', u'OS-EXT-STS:vm_state', u'accessIPv4', u'accessIPv6', u'addresses', u'config_drive', u'created', u'flavor', u'hostId', u'id', u'image', u'key_name', u'links', u'metadata', u'name', u'progress', u'status', u'tenant_id', u'updated', u'user_id']
>>> print sorted(i1[0].keys())
[u'OS-DCF:diskConfig', u'OS-EXT-STS:power_state', u'OS-EXT-STS:task_state', u'OS-EXT-STS:vm_state', u'accessIPv4', u'accessIPv6', u'addresses', u'config_drive', u'created', u'flavor', u'hostId', u'id', u'image', u'links', u'metadata', u'name', u'progress', u'status', u'tenant_id', u'updated', u'user_id']

The top dict is what is returned from POST /servers and the bottom dict is what is returned from GET /servers.

Bottom line solution is really to fix the fragile test in Tempest. That said, however, it is a good warning when API results change in Nova! :)

Hmm. In the etherpad http://etherpad.openstack.org/Hn8rKP7XgB I said:

Generally considered OK:

1. The change is the only way to fix a security bug
2. Adding a new response header or value in a returned dictionary.
3. Change obviously buggy error behavior to a reasonable success result
4. Adding an optional keyword argument, if the existing API was coded to
ignore unexpected
     keywords.

*NOTE*: Should (4) be allowed? It would not break programs but has the
problem that there is no way for the user of an API to know which
arguments are supported.

This was an example of (2). I still think we should allow it but perhaps
there should be an asterisk around this one.

BTW, how do I change the name of the etherpad to something meaningful?
Searched for how but didn't find it.

  -David

On 7/24/2012 3:25 PM, Jay Pipes wrote:
> FINALLY figured out what is going on here...
>
> Turns out that the instance dict returned from POST /servers recently
> changed. The key_name key was added to this return.
>
> However, the instance dicts returned in a call to GET /servers DO NOT
> contain this key_name key. This is what caused the line:
>
> self.assertTrue(self.s1 in servers)
>
> to fail, because the dicts were not equal, as shown in this Python shell
> output:
>
>>>> print sorted(d.keys())
> [u'OS-DCF:diskConfig', u'OS-EXT-STS:power_state', u'OS-EXT-STS:task_state', u'OS-EXT-STS:vm_state', u'accessIPv4', u'accessIPv6', u'addresses', u'config_drive', u'created', u'flavor', u'hostId', u'id', u'image', u'key_name', u'links', u'metadata', u'name', u'progress', u'status', u'tenant_id', u'updated', u'user_id']
>>>> print sorted(i1[0].keys())
> [u'OS-DCF:diskConfig', u'OS-EXT-STS:power_state', u'OS-EXT-STS:task_state', u'OS-EXT-STS:vm_state', u'accessIPv4', u'accessIPv6', u'addresses', u'config_drive', u'created', u'flavor', u'hostId', u'id', u'image', u'links', u'metadata', u'name', u'progress', u'status', u'tenant_id', u'updated', u'user_id']
>
> The top dict is what is returned from POST /servers and the bottom dict
> is what is returned from GET /servers.
>
> Bottom line solution is really to fix the fragile test in Tempest. That
> said, however, it is a good warning when API results change in Nova! :)
>
> ** Changed in: tempest
> Status: Confirmed => Triaged
>
> ** Changed in: nova
> Status: In Progress => Opinion
>

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

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

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

Reviewed: https://review.openstack.org/10255
Committed: http://github.com/openstack/tempest/commit/a6aa5f21074b78f1bf3f44111c9f6fc7d740f2de
Submitter: Jenkins
Branch: master

commit a6aa5f21074b78f1bf3f44111c9f6fc7d740f2de
Author: Jay Pipes <email address hidden>
Date: Tue Jul 24 19:40:29 2012 -0400

    Fix checks in server listing only lok for an ID

    The list server filter details checks were checking
    entire mappings against the list of mappings returned
    from a call to GET /servers/detail. Unfortunately, the
    mappings returned by the GET /servers/detail and the
    mapping returned by POST /servers are slightly different,
    resulting in the comparison failing. We really only are
    checking that the server instance we expect to be in the
    returned list of instances is there, so checking for the
    server ID in the returned list is fine.

    Change-Id: If9680f56a1ea06ffe93393d06b23bd231ccddd01
    Fixes: LP Bug 1028465pep8 build for tempest.