Code review comment for lp:~usc-isi/nova/instance_type_extra_specs

Revision history for this message
Lorin Hochstein (lorinh) wrote :

On Jun 1, 2011, at 3:54 PM, Brian Waldon wrote:

> Review: Needs Information
> The code looks rather solid, however I do have a few higher-level questions:
>
> 1) The concept of 'metadata' doesn't fit in very well here. The data you are providing is integral to the provisioning process. Our usage of metadata in other places represents user-defined key/value pairs that our services will never care about. cpu_arch would probably fit better as a core instance type attribute, while we could look at the others as something closer to "optional attributes". An instance could use one of the standard instance types with a don't-care attitude when provisioning towards something like xpu_arch, while you could provide more specific instance types with those optional attributes being explicitly defined.
>

How about the name "InstanceTypeExtraSpecs"? This would keep terminology consistent with the distributed zone scheduler code, which uses the term "specs" to refer to requested attributes that need to match the capabilities of a compute node.

> 2) I'm not completely sold on the idea that a deployer should provide another method for communication of instance types. I would love to integrate this information into the existing flavors resource in the openstack api.
>

I must admit that I'm only familiar with the ec2 API. I'll take a look at the openstack api to try and figure out how to support querying this type of data.

> 3) Am I correct in assuming another merge prop will be coming to fill in the scheduler integration? Otherwise, there is no way to respect whatever values are actually indicated in the metadata.
>

Yes, this will come in a future merge prop, although I may end up adding it in to the nova/scheduler/HostFilterScheduler code in this merge proposal once I've made the additional changes mentioned above.

« Back to merge proposal