compute: add ext_specs to flavor #2561
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thank you for submitting your first PR! Be sure that we will be looking at it but keep in mind
this sometimes takes a while.
Please let the maintainers know if your PR has not got enough attention after a few days.
If any doubt, please consult our PR tutorial.
| client, err := clients.NewComputeV2Client() | ||
| th.AssertNoErr(t, err) | ||
|
|
||
| // Microversion 2.55 is required to add extra_specs to flavor |
There was a problem hiding this comment.
Bad copy&paste?
The comment should say Microversion 2.61 (first micro-version to support extra_specs according to the docs)
There was a problem hiding this comment.
Sorry, I forget to change this, thank you for the reviewing
There was a problem hiding this comment.
Hi, I have updated the code based on the advices, thank you for the reviewing
| // pairs. This will only be included if the user is allowed by policy to | ||
| // index flavor extra_specs | ||
| // New in version 2.61 | ||
| Properties map[string]string `json:"extra_specs"` |
There was a problem hiding this comment.
Shouldn't it match the json tag?
| Properties map[string]string `json:"extra_specs"` | |
| ExtraSpecs map[string]string `json:"extra_specs"` |
There was a problem hiding this comment.
yeap, I am agree with you. But when I use "openstack flavor show" in the command line, the extra_specs was displayed as properties. I think properties is much easier to understand but I am not sure whether to use it
There was a problem hiding this comment.
We don't care about openstackclient in this case. We care about the API output and in JSON it returns:
{
"extra_specs": {
"hw:cpu_policy": "dedicated",
"hw:mem_page_size": "large"
}
}Let's use ExtraSpecs, I agree with Martin.
There was a problem hiding this comment.
Hi, I have updated the code based on the advices, thank you for the reviewing
|
@mandre Do you think we still need this, given what you pointed out in the issue?
(Sorry @gxxxh that I completely missed that one) |
|
I'd say it's necessary, just noticed this had been done after duplicating the effort, pretty much exactly! If you check out the conversation in #2691 the performance gains can be huge, as well as making client code cleaner, simpler and less bug prone. Just my two cents... this interface was added to OpenStack for a good reason evidently. |
|
We've more or less sorted our branch management - |
Fixes #2497
Openstack API Ref:
flavor get
nova code Ref:
https://github.com/openstack/nova/blob/5c32d5efe1e1aece48b680474617113c61a248d5/nova/objects/flavor.py#L220
As there is no extra_spec in flavor create's parameter,for acceptance test, I write a test which create extra_spec for a already existed flavor, then compare the get result's extra_spec. nova create flavor code.