SecurityGroups Tests : invalid id must be valid uuid

[Jordan Pittier]

Hi,
When a try to run this test suite : nosetests tempest/tests/compute/security_groups/test_security_group_rules.py the tests test_security_group_rules_create_with_invalid_id and test_security_group_rules_delete_with_invalid_id fails with the following same stack trace :

_StringException: Traceback (most recent call last):
  File "/opt/stack/tempest/tempest/tests/compute/security_groups/test_security_group_rules.py", line 190, in test_security_group_rules_delete_with_invalid_id
    rand_name('999'))
BadRequest: Bad request
Details: {'message': 'Security group id should be uuid', 'code': '400'}

I think it is caused by this check : https://github.com/openstack/nova/blob/master/nova/network/security_group/quantum_driver.py#L137

I am working on a patch.

Jordan

------------------
Tempest master
Quantum v2.0
Openvswitch

[Sean Dague (sdague)]

Adding to nova as well as this is a case where Nova is returning different error codes based on back end for the same input, which seems like a no no.

[OpenStack Infra (hudson-openstack)]

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

[Matt Riedemann (mriedem)]

Mailing list thread on this bug: http://lists.openstack.org/pipermail/openstack-dev/2013-July/011666.html

[Matt Riedemann (mriedem)]

Proposed a patch to skip the tests until this is resolved: https://review.openstack.org/#/c/40015/

[OpenStack Infra (hudson-openstack)]

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

[Matt Riedemann (mriedem)]

Another patch to skip more tests for the same root issue in nova:

https://review.openstack.org/#/c/40669/

[Attila Fazekas (afazekas)]

This is one of the different behaviours when you use neutron instead of nova network.
I wonder why this issue does not have high/medium priority in nova.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/40669
Committed: http://github.com/openstack/tempest/commit/31f28ddc22154375cc6435e7625ded1bad28c1a7
Submitter: Jenkins
Branch: master

commit 31f28ddc22154375cc6435e7625ded1bad28c1a7
Author: Matt Riedemann <email address hidden>
Date: Tue Aug 6 18:20:45 2013 -0700

    Skip more security group tests until bug 1182384 is fixed

    This patch builds on change I3f0d21c23661f556354c321476ba559d4925dccd
    which skipped a couple of tests that are failing in nova if neutron is
    configured. The previous patch missed a couple of other tests that fail
    for the same bug. This patch skips those other failing tests for the
    same bug.

    Related-Bug: #1182384

    Change-Id: I4f191f21232f9e58ad09456261fb6247d64f22b5

[Matt Riedemann (mriedem)]

I've changed the priority since we're skipping the tests in tempest now.

[Miguel Lavalle (minsel)]

mriedem,

I read all the comments in this bug. It is not clear to me what is the next step. Are we going to add uuid's to security groups in nova? I am interested in helping to solve this issue, but I want to undertsnad first how we plan to move forward

Cheers

[Matt Riedemann (mriedem)]

This was the conversation I had with Sean Dague in IRC back on 6/18:

http://paste.openstack.org/show/44543/

When Sean is talking about catching the 400 and translating it to a 404, I think that's tied to the second option in the mailing list thread here:

http://lists.openstack.org/pipermail/openstack-dev/2013-July/011666.html

"> 2) Encapsulate all calls to validate_id() in a try/catch HTTPBadRequest and raise a HTTPNotFound instead (exception translation)"

However, later when I talked to Sean on 7/24 he suggested I could try to fix the nova security group id validation issue by using this patch as a template:

https://review.openstack.org/#/c/34559/

In that patch nova-network and neutron are validating name/description differently (neutron wasn't actually validating name/description), and it tries to move the validation into the base class. I'm not actually sure how that applies to the id validation since that's implemented in both the nova-network and neutron SecurityGroupBase implementations in nova.

So it sounds like Sean thinks we should return 404 in either case when the id is invalid, but I've also seen the proposed tempest patch to fix the nova tests in tempest if neutron is being used, and I didn't understand the problem with those (maybe because the client is forced to know the api implementation in nova?).

Anyway, could probably follow up on the mailing list or try to get a discussion going in #openstack-nova IRC?

[Miguel Lavalle (minsel)]

Today, I spent some time running tempest/tests/compute/security_groups/test_security_group_rules.py (the test case mentioned in the bug description). I ran it in two different devstack instances: one with nova network and the other with neutron. This is the summary of the results:

1) In the instance with nova network: when method test_security_group_rules_create_with_invalid_id attempts to create rules in a security group that doesn't exist and the security group id is a string convertible to an integer, the response is a 404, which is what it expects. When the same method is modified to provide a security group id that is a string not convertible to integer, the response is a 400, and it fails.

2) In the instance with neutron: when method test_security_group_rules_create_with_invalid_id attempts to create rules in a security group that doesn't exist and the security group id is a string convertible to an integer, the response is a 400 and it fails. When the same method is modified to provide a security group id that is a valid uuid, the response is a 404, which is what it expects.

Based on this behavior, I propose that we don't have a bug. When this test provides a security group id in the correct format to the nova api (integer string for nova network, uuid for neutron) the response is the expected 404. Conversely, when this test provides a security group id in an incorrect format, both nova network and neutron return a 400. In my opinion, these are correct behaviors

[Matt Riedemann (mriedem)]

Note that for the floating IPs tests, tempest is doing something like this now (checking for neutron and using a fake UUID):

https://github.com/openstack/tempest/blob/master/tempest/api/compute/floating_ips/test_list_floating_ips.py#L93

https://github.com/openstack/tempest/blob/master/tempest/api/compute/floating_ips/test_floating_ips_actions.py#L52

Which is what Jordan was trying to do here: https://review.openstack.org/#/c/29899/

Looks like some other cores were OK with it here: https://review.openstack.org/#/c/46886/

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

commit b0ede30709729b32c8bca6b0df581c236ac39c32
Author: Matt Riedemann <email address hidden>
Date: Wed Nov 6 12:15:28 2013 -0800

    Remove skips for bug 1182384

    Commit a2ccca0 fixed some negative tests to check for neutron when
    determining the invalid ID to use for nova APIs that behave differently
    when using neutron vs nova-network.

    This patch uses the same idea to remove the skips in place for bug
    1182384 in the compute security group negative tests.

    Closes-Bug: #1182384

    Change-Id: I58ba125012a74ea49311a163e9bf7fd7af33c1fc