aggregate-set-metadata return succesful on incorrect usage

[Santiago Baldassin]

aggregate-set-metadata <aggregate> "This is the aggregate metadata" return a "successfully updated" message even when no metadata was added to the aggregate due to the incorrect use of the API

[melanie witt (melwitt)]

Could you give an example of the incorrect API use that results in this behavior?

[Santiago Baldassin (santiago-b-baldassin)]

yes....it's right there in the description:

aggregate-set-metadata <aggregate> "This is the aggregate metadata"

[Santiago Baldassin (santiago-b-baldassin)]

Basically you are not forced to follow the key=value

[Christopher Yeoh (cyeoh-0)]

Yes we should do much better input validation here. Need to keep in mind backwards compatibility for V2 though

[Neetu Jain (nutshi)]

I can take it up ..i think the bug stems up from this line
https://github.com/openstack/nova/blob/0afdcedc3897301ff14c6eefb7355964825c1c12/nova/objects/aggregate.py#L109

I am able to reproduce this problem

this will be my first fix .. some pointers will help ..
I understand the problem i think but i think i can not find a lace where the input validation needs to be added

[Santiago Baldassin (santiago-b-baldassin)]

Hi Neetu Jain, you should submit a patch with the fix so that people can comment on it give you feedback :)

[Santiago Baldassin (santiago-b-baldassin)]

yes. #openstack-nova is the irc channel you are looking for

[Neetu Jain (nutshi)]

ok so i understand this bug a little better i think .. and it seems like there is currently not much information for do_seggregate_set_metadata to know if the deletion of addition of metadata failed in here.

https://github.com/openstack/python-novaclient/blob/076c1a004ab1c861bd02ef7b020e1cbce8a934cc/novaclient/v1_1/shell.py#L2776

because update_metadata from here
https://github.com/openstack/nova/blob/eb19b011eb29245ba17389d21dce02ea89a8a756/nova/objects/aggregate.py#L111
does not return an error if a there is an attempt to delete non-existent key

so what happens in this usecase is

aggregate-set-metadata <aggregate> "This is the aggregate metadata"

the key is "This is the aggregate metadata"

and the function calls tried to delete is ( since there is not value of key provided) .. since it does not return an error or raise an exception the calls succeeds and hence the message. so technically its is correct .. the entry has been updated as per user's choice ( the key has been deleted but since it was never there .. no action took place)

now we could improve the messaging if we introduce some logic in
https://github.com/openstack/nova/blob/eb19b011eb29245ba17389d21dce02ea89a8a756/nova/objects/aggregate.py#L111

1. we read the metadata of that aggregate-ID and determine what keys are present
2. if the user has given a key-value pair we determine if already existent key-value match to the given by user or not .. if they do .. we report and error "stating key=value" already present if they dont match then we go ahead and make the call to the server
3. simillar when only key is present ..then we check if already present keys match the given key .. if the user-key does not match any keys in teh aggregate ..then deletion is not possible so we raise an error " given key is not present hence can not be deleted "

to do this i am trying to understand how to read the meteadata on teh client side ..

[Neetu Jain (nutshi)]

i have the fix now .. for covering corner cases but i am unabel to commit it for review

because of some wierd isseu

https://ask.openstack.org/en/question/30423/please-help-can-nto-submit-my-first-review/

[Neetu Jain (nutshi)]

i am not sure if my review gets posted here or not .. so linking it here

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

the patch solves these 2 problems (and tackles cause of the bug mentioned here)

1. if the user has given a key-value pair we determine if already existent key-value match to the given by user or not .. if they do .. we report and error "stating key=value" already present if they dont match then we go ahead and make the call to the server
2. simillar when only key is present ..then we check if already present keys match the given key .. if the user-key does not match any keys in teh aggregate ..then deletion is not possible so we raise an error " given key is not present hence can not be deleted "

[Neetu Jain (nutshi)]

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