glanceclient 2.4.0 breaks update() for tags

[Dean Troyer]

glanceclient 2.3.0 makes the following REST call via images.update():
curl -g -i -X PATCH -H 'Accept-Encoding: gzip, deflate' -H 'Accept: */*' -H 'User-Agent: python-glanceclient' -H 'Connection: keep-alive' -H 'X-Auth-Token: {SHA1}b03066fc16493ac35f5442d7cc5b616defeb9c5c' -H 'Content-Type: application/openstack-images-v2.1-json-patch' -d '[{"path": "/tags", "value": ["02", "04", "01"], "op": "replace"}]' http://192.168.5.41:9292/v2/images/126d4cd0-cf0f-45d0-a841-ce5dd3c7cad7

"PATCH /v2/images/126d4cd0-cf0f-45d0-a841-ce5dd3c7cad7 HTTP/1.1" 400 71

release 2.4.0 make sthe following (different) call under the same code (OSC):
curl -g -i -X PATCH -H 'Accept-Encoding: gzip, deflate' -H 'Accept: */*' -H 'User-Agent: python-glanceclient' -H 'Connection: keep-alive' -H 'X-Auth-Token: {SHA1}d2be018aa10d3db55c054319a0566afa667b9a65' -H 'Content-Type: application/openstack-images-v2.1-json-patch' -d '[{"path": "/tags/1", "value": "03", "op": "add"}]' http://192.168.5.41:9292/v2/images/126d4cd0-cf0f-45d0-a841-ce5dd3c7cad7

"PATCH /v2/images/126d4cd0-cf0f-45d0-a841-ce5dd3c7cad7 HTTP/1.1" 400 71

Since 2.3.0 works, the likely candidate for this break is in https://review.openstack.org/#/c/324046/. Reverting that commit fixes the problem for me locally.

https://review.openstack.org/#/c/357624/ has been proposed to revert this change.

Replication: using current release (2.6.0) or current master of OpenStackClient, run an image set --tag command:

openstack image set --tag 01 cirros-0.3.4-x86_64-uec

[Steve Martinelli (stevemar)]

To clarify, in OSC, we call the following:

   kwargs['tags'] = list(set(image.tags).union(set(parsed_args.tags)))
   try:
       image = image_client.images.update(image.id, **kwargs)
   ....

That seems to be enough to cause the regression

https://github.com/openstack/python-openstackclient/blob/master/openstackclient/image/v2/image.py#L833-L838

[Nikhil Komawar (nikhil-komawar)]

I am not sure how Mike's commit [1] for not updating tags every time for bug [2] will affect the calls.

The commit has relatively simple logic which does:

if 'tags' in new and 'tags' not in original:
   <then update tags else not>

Is there a possibility of this commit affecting osc in any way https://github.com/openstack/python-glanceclient/commit/96927a6c2b4b17c3cc85d27a7af848cfe3e90f64 ? It may not be used in glanceclient but I am not too familiar with osc atm to say it does or doesn't affect.

These are some initial findings but we will keep digging.

[1] https://github.com/openstack/python-glanceclient/commit/e77322c17931810f4029ef339a791f702f2f4580
[2] https://bugs.launchpad.net/python-glanceclient/+bug/1587999

glanceclient/openstack/common/apiclient/client.py

[Nikhil Komawar (nikhil-komawar)]

OTOH, the commits to help osc are

* https://review.openstack.org/357955
* https://review.openstack.org/357937

These blocks to requirements help osc in short term but we still need to determine underlying issue why osc fails on that commit (why is revert working https://review.openstack.org/#/c/357624/ ) to avoid future regressions.

Please Note: The update to tags on every call is very expensive operation and the DB is hit hard if tags are plenty for an image. The bug needs fixing somehow https://bugs.launchpad.net/python-glanceclient/+bug/1587999

[Nikhil Komawar (nikhil-komawar)]

medium because there's a workaround short term fix

[Dean Troyer (dtroyer)]

I suspect it isn't Mike's patch directly that is the bug, it just uncovered it. The REST call changes from a PATCH to /tags with an op of 'replace' to a PATCH to /tags/<N> with an op of 'add'. I think I see how the patch causes that, but the add operation is where the fail is, whether in the client or server, I don't know.

[Nikhil Komawar (nikhil-komawar)]

Indeed, the pointer to the tags is being generated and we will have to figure out where.

Glance api currently doesn't support pointers for patch requests.

[Nikhil Komawar (nikhil-komawar)]

So, as Dean & Brian rightly pointed out about the pointers. Here's the source of the issue:

https://github.com/openstack/python-glanceclient/blob/e77322c17931810f4029ef339a791f702f2f4580/glanceclient/v2/schemas.py#L26-L30

https://github.com/openstack/python-glanceclient/blob/e77322c17931810f4029ef339a791f702f2f4580/glanceclient/v2/schemas.py#L39

The op needs to be 'replace' instead of 'add'. However, this is something expected for core properties and somehow it's being applicable to tags.

[Nikhil Komawar (nikhil-komawar)]

(some more info) Latest findings from rosmaita are here:

http://eavesdrop.openstack.org/irclogs/%23openstack-glance/%23openstack-glance.2016-08-19.log.html#t2016-08-19T19:45:09

[Nikhil Komawar (nikhil-komawar)]

Bug https://bugs.launchpad.net/python-glanceclient/+bug/1587999 has been moved back to open status.

Please address both bugs 1587999 and (this) 1614971 in the same patch.

[Ian Cordasco (icordasc)]

The upper constraint for glanceclient is presently at 2.6.0. It seems safe to assume this was fixed and released. If I'm incorrect in that assumption, I'll happily re-prioritize this issue and target it to the correct series. Otherwise, I'm marking this as fix released