object get can't work when fields not valid

Bug #1549669 reported by Lisa Li
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Cinder
Fix Released
High
Lisa Li

Bug Description

In master,
Run following commands to create backup from snapshot,

curl -g -i -X POST http://192.168.2.180:8776/v2/2add7d16f22a45a789ab896805e3bdc4/backups -H "User-Agent: python-cinderclient" -H "Content-Type: application/json" -H "Accept: application/json" -H "X-Auth-Token: 4d52d927501745318d578814cd61cc88" -d '{"backup": {"force": false, "description": null, "incremental": false, "snapshot_id": "3d12f820-853d-406d-920f-01399c94426a", "volume_id": "2e9f2a00-5c13-40ca-95fb-fa7215dc3a38", "container": null, "name": "test1"}}'

It gave following result:

ERROR: The server has either erred or is incapable of performing the requested operation. (HTTP 500) (Request-ID: req-ac12f5a4-638b-4764-a62c-6fb78c151d01)

Check the database, find that the field attach_status is null, availability_zone is null in new created volume.

This leads that all the interface related to get the invalid volume don't work. Cinder api/volume/schedule can't work related to volumes.

Revision history for this message
Lisa Li (lisali) wrote :

This bug is opened to track the change for objects._from_db_object().

Cinder needs to deal with exception raised by the above interface.

And to fix creation problem when backing snapshot, I will use this bug: https://bugs.launchpad.net/cinder/+bug/1549673

Changed in cinder:
importance: Undecided → High
Revision history for this message
Lisa Li (lisali) wrote :

The two fields can't be null:
        'availability_zone': fields.StringField(),
        'attach_status': fields.StringField(),

So if they are null in db, when reading the two objects, it raises exception:

    def _from_db_object(context, volume, db_volume, expected_attrs=None):
        if expected_attrs is None:
            expected_attrs = []
        for name, field in volume.fields.items():
            if name in Volume.OPTIONAL_FIELDS:
                continue
            value = db_volume.get(name)
            if isinstance(field, fields.IntegerField):
                value = value or 0
            volume[name] = value Here it raises exception, I think we need to handle.

Changed in cinder:
status: New → Confirmed
Revision history for this message
Michal Dulko (michal-dulko-f) wrote :

Aww, we should keep these in sync and I don't know why we haven't…

As discussed on IRC the best solution would be fixing nullable field properties on objects to match the statuses from DB and prepare a UT enforcing that. Below are some pointers on how to prepare such test.

<dulek> lixiaoy1_: https://github.com/openstack/cinder/blob/master/cinder/db/sqlalchemy/api.py#L4314-L4325
<lixiaoy1_> dulek: this is for name change between db models and versioned object. any problems?
<dulek> lixiaoy1_: In the UT you can iterate over objects like test_objects.TestObjectVersions.test_versions_history is doing.
<dulek> lixiaoy1_: And in the loop you can use this method to fetch the correct DB model for the Object and compare nullable statuses for fields and columns.
<lixiaoy1_> dulek: yes!
<dulek> lixiaoy1_: I advise you to iterate over DB model columns and check obj fields and not the other way around.
<dulek> lixiaoy1_: Because sometimes there more fields than columns.
<dulek> lixiaoy1_: Yeah, that should work. :)
<lixiaoy1_> dulek: nullable, default value, data type, need to check. how do you think
<dulek> lixiaoy1_: We can start with nullable, maybe default.
<lixiaoy1_> dulek: ok
<dulek> data type is more complicated - UUID on Object is Varchar in the DB. Same for enums.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to cinder (master)

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

Changed in cinder:
status: Confirmed → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to cinder (master)

Reviewed: https://review.openstack.org/284600
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=231f856ece08ebc65ed5fcf201df10b1cec27d0b
Submitter: Jenkins
Branch: master

commit 231f856ece08ebc65ed5fcf201df10b1cec27d0b
Author: lisali <email address hidden>
Date: Thu Feb 25 08:59:42 2016 +0000

    Add necessary fields to volume creation

    This patch is to add the two fields in driver.py when creating
    volumes.

    Without the fix, it leads query related to the volume failed
    with exception.

    Change-Id: I0055b5bf402b0aa7b4ee78b0fb40bedea850af94
    Closes-Bug: #1549673
    Related-Bug: #1549669

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to cinder (master)

Reviewed: https://review.openstack.org/285086
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=7a158ac4488ac86c742e15c714dda7dd76772366
Submitter: Jenkins
Branch: master

commit 7a158ac4488ac86c742e15c714dda7dd76772366
Author: LisaLi <email address hidden>
Date: Fri Feb 26 10:41:19 2016 +0800

    Make nullable of fields in db model and object match

    Currently there are some fields whose nullable in db
    model and object don't match. This may lead object query
    fails.

    For example, for volume, if attach_status is nullable in
    db schema, but is not nullable in object schema.
    When writting data to db using db interface, the attach_status
    can be null. But later when reading data from db to construct
    volume object, it raises exception. As a result, all the
    query related to the volume fails.

    This patch is to make all the fields in corresponding db
    model and object match.

    Change-Id: I84fb16f0a67f9a1f7a09e91bb8ebe6298f3f49c4
    Closes-Bug: #1549669
    Related-Bug: #1549673

Changed in cinder:
status: In Progress → Fix Released
Revision history for this message
Thierry Carrez (ttx) wrote : Fix included in openstack/cinder 8.0.0.0rc1

This issue was fixed in the openstack/cinder 8.0.0.0rc1 release candidate.

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.