Earlier  
Posted Nick Remark
#openstack-nova - 2021-10-01
15:22:32 dansmith artom: you're asking if you deserialize, tweak and re-serialize, if you'll serialize the 1.4 version that came from the original thing?
15:23:03 artom dansmith, aye
15:23:26 artom Tweak specifically by adding the field that was added in the 1.5 version
15:23:53 bauzas artom: then the remote object won't get it
15:24:08 dansmith no, when you deserialize an object, it becomes a 1.5, which is why you have to be compatible with your object changes
15:24:36 dansmith the new field will be unset because it came from a 1.4, but when you re-serialize it, you'll use the current version unless you ask for an older one (like conductor does when it backports changes for you)
15:24:49 priteau I've just looked at a production system with pinned-instances launched on Train, the system now being on Victoria. The InstanceNUMACell objects are actually showing "nova_object.version": "1.6"
15:25:14 priteau So it looks OK
15:25:23 bauzas wait
15:25:25 bauzas you lost me
15:25:38 artom priteau, cool, so you confirmed in a real system the mechanism that dansmith explained \o/
15:25:43 bauzas who's on 1.5 (or later) and who's on 1.4 ?
15:25:53 dansmith bauzas: Who is on first
15:26:43 bauzas dansmith: that's what I'm asking
15:26:58 bauzas but it looks it works, so meh
15:27:27 priteau 1.4 are pre-Victoria objects. 1.5, actually none because 1.6 was also introduced in Victoria. Any instance launched or restarted on Victoria gets 1.6 (but only for cpu_policy=dedicated, which is the problem)
15:27:46 dansmith artom: I might actually be wrong here, I'
15:27:47 priteau Anyway, there is no problem with the version number, I just wasn't aware of the nova magic
15:27:50 dansmith I'm looking hang on
15:28:17 bauzas priteau: so, you're asking about what would happen with an old compute and a Victoria conductor or compute, I guess
15:28:47 bauzas dansmith explained it very pretty well
15:28:59 bauzas when you serialize, you add the object version
15:29:45 bauzas so any reader getting the serialized object would know the version and creates a new object based on the compatibility methods we provide
15:30:30 bauzas keeping in mind that obj 1.X doesn't know what to do with 1.y if y > x
15:30:32 dansmith soooo,
15:30:52 dansmith I'm actually not sure this behaves the way I thought.. I don't actually know that it matters, but I think it might be different
15:31:21 dansmith it looks like we _will_ preserve the inbound version that we got from the serialized form when we re-serialize,
15:31:45 dansmith which I think we do for the purposes of not up-leveling objects if we're newer than others in the cluster
15:32:05 dansmith i.e. to avoid a new conductor up-leveling an old version that an api sent to it, knowing an old compute will need the old version
15:32:27 dansmith and also I guess to avoid upleveling something in the database when we load/save it if we're not ready to roll everyone forward yet
15:32:52 dansmith but, the behavior you notice is not the version, it's whether or not the field sticks, which is what I was focusing on, and I think it will, even though that doesn't match the version
15:32:54 dansmith meaningm
15:33:11 artom dansmith, wouldn't upleveling not be a problem because we have code to make it compatible to old versions?
15:33:25 dansmith if you store the object with the new field, it will serialize the new field even if the version is wrong, and a newer node will still pull it out because it doesn't know which version the field belongs to
15:33:30 artom So if the conductor uplevels something, then an old compute loads, it will automatically downlevel for that compute?
15:34:04 dansmith artom: well, I picked conductor because it was easy, but imagine two computes talking
15:35:43 dansmith so I'm not actually sure I can explain priteau's observation of the new versions
15:36:02 dansmith that's how I recalled it working, but I can't back that up with references to the code purely in the deserialization/serialization stuff
15:36:12 priteau These instances were live migrated after the Victoria upgrade, could it explain it?
15:36:22 dansmith do we recreate the numa objects as part of another operation? yeah, like live migration
15:38:17 dansmith this is the test that proves that we don't automatically uplevel it: https://github.com/openstack/oslo.versionedobjects/blob/e7b6d52aa4b1b40e68a21a122c09b968d5959b0e/oslo_versionedobjects/tests/test_objects.py#L778-L785
15:38:31 dansmith which isn't how I remembered it, but re-reading that test jogs my memory about why
15:40:33 dansmith so I think technically that operation should do something like object.VERSION = ObjectClass.VERSION when it goes to save it out
15:46:47 artom So the upleveling happens at serialization then?
15:46:58 artom Or some other operation like live migration, as you've said
15:47:00 dansmith it doesn't
15:47:04 artom But not at de-serialization, at any rate
15:47:20 dansmith I'm thinking live migrations create new numa stuff for the new host and that's how those got updated
15:48:18 dansmith in the past, we've done things like "load and save the object to update it" which would run routines to do updates, but they are probably saving the objects back with the old version but including the updates :/
15:48:28 dansmith I can't find any VERSION updating going on in the tree right now
15:48:42 dansmith like for online data migrations I mean
15:49:08 dansmith technically it probably hasn't mattered, because who (besides artom and priteau) would notice that the version was wrong if the data was right? :)
15:49:54 artom Yeah, maybe it's an academic debate - if it does the right thing...
15:50:00 artom Which it appears to do
15:50:22 priteau I just wanted to check it wouldn't break anything if the version number was not updated
15:50:23 dansmith well, we shouldn't store the version incorrectly just because that's going to be confusing, but yeah the object would still load with the data we wanted so in practice it hasn't mattered
15:50:41 dansmith priteau: right, but we should fix it to update the version though
15:52:02 priteau I will leave that to you if you don't mind :D
15:52:42 dansmith artom: do you want me to do that or will you?
15:53:28 artom dansmith, do what? What priteau said, "fix it to update the version though"?
15:53:38 artom Wouldn't that be within priteau own's patch?
15:53:53 artom Well, maybe not, since the on-load migration is pre-existing...
15:53:54 dansmith artom: I said it, but no, the code that claims to migrate is already in the tree
15:53:55 artom He's just fixin it
15:53:55 dansmith right
15:53:58 dansmith right
15:54:21 dansmith looks like there are several _migrate_* cases in instance_numa.py
15:54:28 artom And probably elsewhere...
15:54:33 dansmith maybe, not sure
15:54:57 dansmith if you do it I can review/+2 but ... :)
15:55:46 artom I mean, the hint of a +2 from dansmith on a PS1 is pretty alluring...
15:55:58 dansmith well, if you get it right in PS1.. :P
15:56:13 artom Crazier things have happened
15:56:14 dansmith let me write you a diff to start, hang on
15:58:13 dansmith I think this should be unified instead of done in multiple places like it is, so something like this: https://termbin.com/iawkf
15:59:12 dansmith oh, you know what,
15:59:22 dansmith the other migrate routine is for non-ovo migrations,
15:59:40 dansmith which is different, so probably no need to break out that helper function anyway
16:00:07 dansmith so just the one line VERSION thing plus an assert in whatever tests this and you're probably good
16:02:13 artom Oh I see (I think), your initial thought was "have any on-load migration call to a helper that updates version to latest"
16:02:49 dansmith yeah, because I thought there were at least two,
16:02:53 dansmith but that's because I didn't read
16:04:19 artom Yeah makes sense
16:04:34 artom My stomach is requiring food, I think lunch first, patch after
16:04:53 dansmith I +2 that ordering
16:05:32 artom \o/
16:48:52 melwitt gmann: no I was not aware of the novnc fail, I will look at it
16:52:32 gmann melwitt: thanks.
20:00:38 melwitt gmann: just to update you, I know what is wrong but I don't find how it's happening yet. tl;dr is the "path" in the vnc proxy is wrong and doesn't contain the token (so it's not even validating a token). according to all of the config files I looked at, it should be working. so there is something I'm missing so far
20:01:01 melwitt this is an example of how the path is supposed to look: https://zuul.opendev.org/t/openstack/build/2a183ce6e15f4ab08d31190d4d286a7b/log/controller/logs/screen-n-novnc-cell1.txt#11
20:01:14 melwitt and this is the bad one that's failing: https://zuul.opendev.org/t/openstack/build/9234f33acf1247cdb995416ca9389240/log/controller/logs/screen-n-novnc-cell1.txt#13
20:04:14 melwitt this is where it would pick up the "path" variable https://github.com/novnc/noVNC/blob/v0.4/vnc_auto.html#L106
22:45:16 opendevreview melanie witt proposed openstack/nova master: Add logic to enforce local api and db limits https://review.opendev.org/c/openstack/nova/+/712139
22:45:17 opendevreview melanie witt proposed openstack/nova master: Update quota_class APIs for db and api limits https://review.opendev.org/c/openstack/nova/+/712143
22:45:17 opendevreview melanie witt proposed openstack/nova master: Enforce api and db limits https://review.opendev.org/c/openstack/nova/+/712142
22:45:18 opendevreview melanie witt proposed openstack/nova master: Update quota sets APIs https://review.opendev.org/c/openstack/nova/+/712749
22:45:18 opendevreview melanie witt proposed openstack/nova master: Update limit APIs https://review.opendev.org/c/openstack/nova/+/712707
22:45:19 opendevreview melanie witt proposed openstack/nova master: Enforce resource limits using oslo.limit https://review.opendev.org/c/openstack/nova/+/615180
22:45:19 opendevreview melanie witt proposed openstack/nova master: Tell oslo.limit how to count nova resources https://review.opendev.org/c/openstack/nova/+/713301
22:45:20 opendevreview melanie witt proposed openstack/nova master: Add legacy limits and usage to placement unified limits https://review.opendev.org/c/openstack/nova/+/713498

Earlier   Later