Getting instances by ID when admin only returns deleted instances

[Ed Leafe]

In nova/db/sqlalchemy/api.py, the instance_get_by_id() method checks for admin context. If not an admin, it limits queries to non-deleted records. That's correct behavior.

If it is called with admin rights, it should allow you to see both deleted and non-deleted records if 'can_read_deleted(context)' returns True. However, the code as written filters the results on 'deleted = can_read_deleted(context)'. This means that if you are allowed to access deleted records, all queries will add 'deleted=True' to the filter, which means that you cannot access non-deleted records.

[Ed Leafe (ed-leafe)]

Upon further investigation, every method in this script that accesses can_read_deleted() does it incorrectly. I will fix all of them, not just the instance_get_by_id().

[Vish Ishaya (vishvananda)]

I think perhaps there is a misunderstanding of the way this works. The current behavior is expected. You must explicitly set the context to read_deleted=True to access deleted records and you only get deleted records if you do so. There currently isn't a way to access deleted records and non-deleted records in the same query. It may be better for read_deleted to allow access to all records, but the current behavior was intentional.

[Ed Leafe (ed-leafe)]

On Jan 10, 2011, at 1:49 PM, Vish Ishaya wrote:

> I think perhaps there is a misunderstanding of the way this works. The
> current behavior is expected. You must explicitly set the context to
> read_deleted=True to access deleted records and you only get deleted
> records if you do so. There currently isn't a way to access deleted
> records and non-deleted records in the same query. It may be better for
> read_deleted to allow access to all records, but the current behavior
> was intentional.

 I'm creating the context using context.get_admin_context(). The default for read_deleted is True. Is that expected behavior?

-- Ed Leafe

[Ed Leafe (ed-leafe)]

Correction: the default is actually False. I was confused because it was not returning the Instance even though the ID is present in the instances table. Looking in MySQL, I see:

mysql> select id, admin_pass, deleted from instances;
+----+------------+---------+
| id | admin_pass | deleted |
+----+------------+---------+
| 1 | NULL | 0 |
+----+------------+---------+
1 row in set (0.00 sec)

But if I set a break in the code, and execute the call directly, I get the following:

In [1]: result = session.query(models.Instance).filter_by(id=1).filter_by(deleted=False).first()

In [2]: result

In [3]: result = session.query(models.Instance).filter_by(id=1).filter_by(deleted=True).first()

In [4]: result
Out[4]: <nova.db.sqlalchemy.models.Instance object at 0x4195a90>

In [5]: result.id, result.deleted
Out[5]: (1, True)

So it seems that the original bug I reported is not valid. However, there is a different problem: why is sqlalchemy returning deleted=True for this record?

[Vish Ishaya (vishvananda)]

that does seem very odd. Are you sure you aren't hitting two different databases? There was a recently fixed bug that was causing the workers to access a local db instead of the normal one.

Vish

On Jan 10, 2011, at 12:05 PM, Ed Leafe wrote:

> Correction: the default is actually False. I was confused because it was
> not returning the Instance even though the ID is present in the
> instances table. Looking in MySQL, I see:
>
>
> mysql> select id, admin_pass, deleted from instances;
> +----+------------+---------+
> | id | admin_pass | deleted |
> +----+------------+---------+
> | 1 | NULL | 0 |
> +----+------------+---------+
> 1 row in set (0.00 sec)
>
> But if I set a break in the code, and execute the call directly, I get
> the following:
>
> In [1]: result =
> session.query(models.Instance).filter_by(id=1).filter_by(deleted=False).first()
>
> In [2]: result
>
> In [3]: result =
> session.query(models.Instance).filter_by(id=1).filter_by(deleted=True).first()
>
> In [4]: result
> Out[4]: <nova.db.sqlalchemy.models.Instance object at 0x4195a90>
>
> In [5]: result.id, result.deleted
> Out[5]: (1, True)
>
> So it seems that the original bug I reported is not valid. However,
> there is a different problem: why is sqlalchemy returning deleted=True
> for this record?
>
>
> ** Changed in: nova
> Status: New => Invalid
>
> --
> You received this bug notification because you are a member of Nova Bug
> Team, which is subscribed to OpenStack Compute (nova).
> https://bugs.launchpad.net/bugs/701121
>
> Title:
> Getting instances by ID when admin only returns deleted instances
>
> Status in OpenStack Compute (Nova):
> Invalid
>
> Bug description:
> In nova/db/sqlalchemy/api.py, the instance_get_by_id() method checks for admin context. If not an admin, it limits queries to non-deleted records. That's correct behavior.
>
> If it is called with admin rights, it should allow you to see both deleted and non-deleted records if 'can_read_deleted(context)' returns True. However, the code as written filters the results on 'deleted = can_read_deleted(context)'. This means that if you are allowed to access deleted records, all queries will add 'deleted=True' to the filter, which means that you cannot access non-deleted records.
>
>

[Ed Leafe (ed-leafe)]

vish - you nailed it!

I have my nova.conf set for sql_connection=mysql://root:nova@127.0.0.1/nova, but it was somehow getting set to: sqlite:////home/ed/openstack/xs-password-reset/nova/..//nova.sqlite. In that database, those two records are indeed deleted.

Thanks for your help!