Open Stack

On Thu, 2019-01-31 at 09:00 -0600, Matt Riedemann wrote:
> I'm going to top post and try to summarize where we are on this thread 
> since I have it on today's nova meeting agenda under the "stuck reviews" 
> section.
> 
> * The email started as a proposal to change the proposed image property 
> and flavor extra spec from a confusing boolean to an enum.
> 
> * The question was raised why even have any other option than a unique 
> serial number for all instances based on the instance UUID.
> 
> * Stephen asked Daniel Berrange (danpb) about the history of the 
> [libvirt]/sysinfo_serial configuration option and it sounds like it was 
> mostly added as a way to determine guests running on the same host, 
> which can already be determined using the hostId parameter in the REST 
> API (hostId is the hashed instance.host + instance.project_id so it's 
> not exactly the same since it's unique per host and project, not just 
> host). However, the hostId is not exposed to the guest in the metadata 
> API / config drive - so that could be a regression for applications that 
> used this somehow to calculate affinity within the guest based on the 
> serial (note that mnaser has a patch to expose hostId in the metadata 
> API / config drive [1]).
> 
> * danpb said the system.serial we set today should really be 
> chassis.serial but that's only available in libvirt >= 4.1.0 and our 
> current minimum required version of libvirt is 1.3.1 so setting 
> chassis.serial would have to be conditional on the running version of 
> libvirt (this is common in that driver).
> 
> * Applications that depend on the serial number within the guest were 
> not guaranteed it would be unique or not change because migrating the 
> guest to another host would change the serial number anyway (that's the 
> point of the blueprint - to keep the serial unchanged for each guest), 
> so if we just changed to always using unique serial numbers everywhere 
> it should probably be OK (and tolerated/expected by guest applications).
> 
> * Clearly we would have a release note if we change this behavior but 
> keep in mind that end users are not reading release notes, and none of 
> this is documented today anyway outside of the [libvirt]/sysinfo_serial 
> config option. So a release note would really only help an operator or 
> support personal if they get a ticket due to the change in behavior 
> (which we probably wouldn't hear about upstream for 2+ years given how 
> slow openstack deployments upgrade).
> 
> So where are we? If we want the minimal amount of behavior change as 
> possible then we just add the new image property / flavor extra spec / 
> config option choice, but that arguably adds technical debt and 
> virt-driver specific behavior to the API (again, that's not uncommon 
> though).
> 
> If we want to simplify, we don't add the image property / flavor extra 
> spec. But what do we do about the existing config option?
> 
> Do we add the 'unique' choice, make it the default, and then deprecate 
> the option to at least signal the change is coming in Train?
> 
> Or do we just deprecate the option in Stein and completely ignore it, 
> always setting the unique serial number as the instance.uuid (and set 
> the host serial in chassis.serial if libvirt>=4.1.0)?
 personally i would do ^ assuming we also do v
> 
> In addition, do we expose hostId in the metadata API / config drive via 
> [1] so there is a true alternative *from within the guest* to determine 
> guest affinity on the same host? I'm personally OK with [1] if there is 
> some user documentation around it (as noted in the review).
> 
> If we are not going to add the new image property / extra spec, my 
> personal choice would be to:
> 
> - Add the 'unique' choice to the [libvirt]/sysinfo_serial config option 
> and make it the default for new deployments.
> - Deprecate the sysinfo_serial config option in Stein and remove it in 
> Train. This would give at least some window of time for transition 
> and/or raising a stink if someone thinks we should leave the old 
> per-host behavior.
> - Merge mnaser's patch to expose hostId in the metadata API and config 
> drive so users still have a way within the guest to determine that 
> affinity for servers in the same project on the same host.
> 
> What do others think?
yes i think your personal choice above makes sense too
so i would  be +1 on that too as it gives a cycle for people to
move if they care about the serials.
> 
> [1] https://review.openstack.org/#/c/577933/
> 
> On 1/24/2019 9:09 AM, Matt Riedemann wrote:
> > The proposal from the spec for this feature was to add an image property 
> > (hw_unique_serial), flavor extra spec (hw:unique_serial) and new 
> > "unique" choice to the [libvirt]/sysinfo_serial config option. The image 
> > property and extra spec would be booleans but really only True values 
> > make sense and False would be more or less ignored. There were no plans 
> > to enforce strict checking of a boolean value, e.g. if the image 
> > property was True but the flavor extra spec was False, we would not 
> > raise an exception for incompatible values, we would just use OR logic 
> > and take the image property True value.
> > 
> > The boolean usage proposed is a bit confusing, as can be seen from 
> > comments in the spec [1] and the proposed code change [2].
> > 
> > After thinking about this a bit, I'm now thinking maybe we should just 
> > use a single-value enum for the image property and flavor extra spec:
> > 
> > image: hw_guest_serial=unique
> > flavor: hw:guest_serial=unique
> > 
> > If either are set, then we use a unique serial number for the guest. If 
> > neither are set, then the serial number is based on the host 
> > configuration as it is today.
> > 
> > I think that's more clear usage, do others agree? Alex does. I can't 
> > think of any cases where users would want hw_unique_serial=False, so 
> > this removes that ability and confusion over whether or not to enforce 
> > mismatching booleans.
> > 
> > [1] 
> > https://review.openstack.org/#/c/612531/2/specs/stein/approved/per-instance-libvirt-sysinfo-serial.rst@43 
> > 
> > [2] 
> > https://review.openstack.org/#/c/619953/7/nova/virt/libvirt/driver.py@4894
> 
>