image not updated on image member change

[Brian Rosmaita]

When an image member is added to/deleted from an image, the updated_at time of the image does not change.

This is misleading because image membership affects image behavior, in particular, whether an image is visible to (can be booted by) a particular user. Even though the image member-list is not part of the GET v2/images/{image_id} response, *something* about the image has changed, and the updated_at timestamp should reflect this fact.

(This is causing problems for Searchlight, it's not a purely academic discussion.)

[OpenStack Infra (hudson-openstack)]

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

[Kairat Kushaev (kkushaev)]

Brian,
could you please provide some info about problems for Searchlight? It would help devs to understand the motivation of the change.

[Steve McLellan (sjmc7)]

The problem is that searchlight uses the updated_at time to avoid race conditions with notifications received in quick succession or processed out of order. Searchlight doesn't treat image members as a separate entity as glance does, since logically they're part of the access control for an image. Because the timestamp on the image doesn't change as membership is altered, it's hard to determine whether or not we should apply a given notification (it looks like the indexed image is already up to date).

[OpenStack Infra (hudson-openstack)]

Change abandoned by Nikhil Komawar (<email address hidden>) on branch: master
Review: https://review.openstack.org/283013
Reason: I see no movement on this patch and/or associated bug. Please reopen as/when you find appropriate.

[Nikhil Komawar (nikhil-komawar)]

The patch associated has been abandoned due to lack of movement.

[Nikhil Komawar (nikhil-komawar)]

Is the person assigned to this bug still working on it? Today I saw a comment from abashmak on #openstack-searchlight asking if anyone was still interested in it. Possible he may pick up if no one else is.

Indicate the intent to work on this bug by Sept 18th. We may reassign or un-assign on Sept 19th. Thanks for your co-operation.

[Steve McLellan (sjmc7)]

Yes, we're still interested. I was out on vacation this week.

[Steve McLellan (sjmc7)]

Although given we're at the RC stage maybe not in newton; i don't know if we'll have to make changes to it. I understand the sharing model is being changed in ocata anyway so maybe as part of that?

[Nikhil Komawar (nikhil-komawar)]

looking at this after a break, it makes me think we need a lite spec for this.

[Brian Rosmaita (brian-rosmaita)]

@Nikhil, you are probably right. Plus, I don't think Alex has yet had the pleasure of putting together a lite spec, so it's also a learning opportunity!

[Steve Lewis (steve-lewis)]

This change as described in the bug is coercing the Glance API to reflect assumptions from Searchlight and advancing a behavior which contradicts RESTful convention around the common created_at and updated_at fields.

If the resource doesn't change, the updated_at field shouldn't. Ideally in my mind the Glance API would encourage discoverability of the visibility/membership related resources and Searchlight would follow that clue to address this need but that is well beyond what I expect can be achieved in Glance V2 so seems too much to ask.

[Brian Rosmaita (brian-rosmaita)]

I don't see how this is violating RESTful convention. The visibility and member-list are properties of the image, so the object being referred to has actually changed. A similar thing would be if show_image_locations is off, but an admin updates a location, I'd expect the updated_at to be the only different value in the image-show response. But I may be misunderstanding your point here.

[OpenStack Infra (hudson-openstack)]

Change abandoned by Brian Rosmaita (<email address hidden>) on branch: master
Review: https://review.openstack.org/283013
Reason: Discussed this with Alex, he's going to have to take the implementation in a different direction. I've asked him to put a comment on the new patch linking back to this patch for historical context.

[OpenStack Infra (hudson-openstack)]

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

[Steve Lewis (steve-lewis)]

It would be a distraction to debate the restful convention here but resource representations should be sovereign and not be subject to invisible side effects and for better or worse Glance does not include access controls in the image resource representation.

As an aside, changes to Metadata and Image Properties don't appear to change the Image's updated_at. Are we going to need to do the same for those or did Searchlight design this differently from image access controls?

[Brian Rosmaita (brian-rosmaita)]

I think we do change the updated_at for image properties, at least for the sqlalchemy api:
http://git.openstack.org/cgit/openstack/glance/tree/glance/db/sqlalchemy/api.py?h=stable/newton#n819

As a side note, it's possible to configure property protections so that some properties can't be read by normal end users. If an admin updated one of these properties, the updated_at of the image would change even though everything else in the image would appear to be unchanged to a normal end user. So for better or worse, we have the invisible side effect situation already.

[Brian Rosmaita (brian-rosmaita)]

Alexander ran into some technical problems working on the implementation, so we reconsidered. Here's the correspondence with the Searchlight PTL about this bug:

On 10/17/16, 5:59 PM, "Brian Rosmaita" wrote:

Hi Steve,

We have a discussion going on around the proposed change to updated_at in
the image response [0]. Steve (Lewis) feels (not un-rightly) that
changing updated_at when the representation itself hasn't changed is
un-RESTful. I thought I had a decent argument to the contrary until Alex
pointed out that we aren't modifying the updated_at when there's a change
in member_status, and while I thought it was OK to change updated_at on
the image when the member_list changed, when we get down to the member
level it seems a bit icky even though consistency would compel me to say,
yes, we should change it. (Additionally, there is a technical problem
related to the context in which a member_status changes that will make it
difficult to update the image anyway.)

So, my questions for you are:

(1) does Searchlight need the updated_at to change when member_status
changes (from 'pending' to 'accepted', for instance)?

(2) is there another way to achieve the result Searchlight needs without
modifying the image's updated_at?

We need to explore the situation a bit more.

thanks,
brian

[0] https://bugs.launchpad.net/glance/+bug/1547079

On 10/18/16, 3:21 PM, "McLellan, Steven" wrote:

Hi Brian,

TLDR: if it's contentious, we can live with it.

My feeling on 'the representation hasn't changed' is that the member
list IS part of the representation in the sense that the member list is
absolutely an attribute of "an image", and the fact that essentially
because the member list is a separate SQL table it became a separate
REST endpoint is an implementation detail.

That said, I can see the Steve's point of view that since it IS a
separate entity in a RESTy sense, it shouldn't be diddling with image
properties.

Our workaround is to retrieve the image (and members) from glance and
do an unconditional save (ignoring timestamps). We only consider 'accepted'
image members since we use it primarily for RBAC, so image member state
changes have no impact. The vast likelihood is that it takes longer to
get the image from Glance than the timeframe in which race conditions
can occur, and since it's a relatively uncommon operation there's
probably no performance impact.

HTH!

Steve

[OpenStack Infra (hudson-openstack)]

Change abandoned by Alexander Bashmakov (<email address hidden>) on branch: master
Review: https://review.openstack.org/386688
Reason: Bug marked as Won't Fix