| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-11-05 | |||
| 10:04:31 | elod | then we just have to wait the gate :] | |
| 10:04:31 | lyarwood | elod: ack thanks | |
| 10:04:47 | lyarwood | I guess we don't test rebuilds in a mixed upgrade state | |
| 10:05:13 | lyarwood | and that's why grenade multinode didn't hit this | |
| 10:05:33 | gibi | bauzas: looking | |
| 10:06:20 | bauzas | lyarwood: stephenfin: gibi: I'm not telling you were bad about reviewing (i also sometimes misses some issues), but maybe it would be nice for you to look at both the fix https://review.opendev.org/#/c/761458/ but also to review https://review.opendev.org/#/c/761452/ to understand how RPC API works | |
| 10:06:46 | bauzas | again, no worries at all | |
| 10:07:24 | bauzas | it's more for providing a knowledge help for you folks about how RPC versions work | |
| 10:07:34 | stephenfin | ah, I knew that and forgot about it :( | |
| 10:07:45 | bauzas | if you knew it, all good then | |
| 10:08:16 | gibi | bauzas: yeah, thanks for the pointers. I'm wondering if we can make some test enhancements to catch these in the future | |
| 10:08:17 | lyarwood | I didn't even review the broken patch here so I'm not sure what you're trying to say | |
| 10:08:21 | lyarwood | .... | |
| 10:08:27 | bauzas | but hopefully the proxy change I'm providing is nice for knowing how to have a major version | |
| 10:09:14 | bauzas | lyarwood: not about any previous reviews, just for helping you to know what to review when you have a change with a RPC modification | |
| 10:09:56 | lyarwood | sure | |
| 10:11:14 | lyarwood | bauzas: look forward to your reference docs patches | |
| 10:14:27 | stephenfin | gibi: We could probably hash the signature or something? | |
| 10:14:41 | bauzas | actually, I could write something in https://docs.openstack.org/nova/latest/contributor/code-review.html | |
| 10:14:55 | bauzas | stephenfin: gibi: testing it is not simple | |
| 10:14:55 | stephenfin | i.e. identify all the position, non-optional arguments and generate/save a hash for those | |
| 10:15:09 | stephenfin | then compare each time, like we do for o.vos | |
| 10:15:14 | bauzas | since the arguments are different between RPC versions | |
| 10:15:14 | gibi | stephenfin: that would be the ovo way yes | |
| 10:15:26 | stephenfin | *positional | |
| 10:15:38 | lyarwood | bauzas: https://docs.openstack.org/nova/latest/reference/rpc.html I was thinking more in here | |
| 10:15:44 | lyarwood | bauzas: but either way | |
| 10:15:52 | bauzas | stephenfin: we have non-positional arguments that are unrelated to RPC versions | |
| 10:16:10 | bauzas | we just keep them optional | |
| 10:16:49 | bauzas | lyarwood: oh, TIL this page was existing | |
| 10:17:02 | bauzas | gibi: we fixed it by code reviews | |
| 10:17:49 | bauzas | ah, this is already documented https://docs.openstack.org/nova/latest/contributor/code-review.html#rpc-api-versions | |
| 10:17:53 | gibi | bauzas: sure, code review is the fallaback, human intelligence is king, but if we can automate it then we could avoid failing humans like me at the original code rview | |
| 10:18:12 | lyarwood | bauzas: ah cool | |
| 10:18:20 | bauzas | but I guess "The manager-side method needs to tolerate older calls as well as newer calls" is maybe too much overall, and we need to explain it more | |
| 10:18:47 | bauzas | gibi: we could enforce owners to propose functional tests | |
| 10:18:53 | bauzas | for testing the RPC pins | |
| 10:19:06 | bauzas | like I did in my regression test | |
| 10:19:29 | bauzas | this would be a simpliest approach | |
| 10:19:52 | gibi | I guess enforce by code review | |
| 10:20:01 | bauzas | that, yeah | |
| 10:20:06 | gibi | I agree | |
| 10:20:13 | bauzas | but from what I've seen, nobody is really doing it | |
| 10:20:16 | gibi | still I want to automate it if possible :D | |
| 10:20:39 | lyarwood | shouldn't we cover mixed compute upgrades in the multinode grenade job? | |
| 10:20:46 | bauzas | gibi: well, we don't really set new versions a lot right? | |
| 10:20:50 | gibi | becuase all are code review rules are as good as the way we enforce them | |
| 10:21:00 | gibi | bauzas: we do it less and less, I agree | |
| 10:21:01 | bauzas | like, we only had one rpc minor bump per release since a while | |
| 10:21:16 | bauzas | gibi: well, we have a code review documentation | |
| 10:21:24 | bauzas | and I expect cores to know it at least | |
| 10:21:39 | bauzas | I mean, that's a breaking change to accept a RPC change | |
| 10:22:43 | bauzas | maybe we rushed over accepting some feature that was long overdue, but maybe considering to require a functest would ensure that we would put the burden on code owners | |
| 10:22:45 | gibi | bauzas: you are correct that we assume that core reviews catch these kind of problems, but they don't as you found | |
| 10:23:16 | gibi | if we do these thing less and less it means that we will easier to forget what to look at in these changes | |
| 10:24:07 | gibi | as we don't excersize this knowledge | |
| 10:26:26 | gibi | so I agree that one thing is to raise awerness for this issue as you did. | |
| 10:27:16 | gibi | but also I will think about some kind of automation as I cannot promise I won't forget this rule again 6 months from now when we bump the next | |
| 10:44:06 | bauzas | gibi: ahah lol, i had to rush off home because I forgot my kids at the school :whoops: | |
| 10:44:38 | bauzas | gibi: fwiw, the change we merged was a bit hairy, so I do understand that it was difficult to find the problem | |
| 10:45:02 | bauzas | gibi: that's why I said we should at least ask to provide a functional test, that's it | |
| 10:45:41 | gibi | yeah, I should not forget to ask a functional test pining to old RPC version when a new RPC version is proposed | |
| 12:05:33 | brinzhang0 | gibi: hi good morning | |
| 12:05:59 | brinzhang0 | gibi: Hope you can review | |
| 12:06:00 | brinzhang0 | Cyborg shelve/unshelve support patch https://review.opendev.org/#/c/729563/ :D | |
| 12:06:33 | gibi | brinzhang0: add to my queue | |
| 12:06:42 | gibi | added | |
| 12:06:51 | brinzhang0 | gibi: thanks | |
| 13:36:01 | bauzas | brinzhang0: gibi: hah, this time the new argument is nullable :p | |
| 13:36:23 | bauzas | but maybe it's time to ask for a functional testclass verifying the RPC API ? :) | |
| 14:11:28 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Bump the lowest eventlet version to 0.26.1 https://review.opendev.org/761427 | |
| 15:12:05 | iurygregory | Hi nova folks, a friend of mine using openstack queens asked me if it's possible to update the config drive of an instance? | |
| 15:12:31 | bauzas | gibi: the next nova meeting is in 45 mins, right? | |
| 15:12:36 | bauzas | tz change | |
| 15:14:35 | gibi | bauzas: yes, the meeting is at 16:00 UTC which is 17:00 CET | |
| 15:14:43 | bauzas | cool cool | |
| 15:14:48 | bauzas | nicer for us :) | |
| 15:15:23 | gibi | :) | |
| 15:18:40 | sean-k-mooney | openstack meetings are alwasy utc and never move | |
| 15:18:46 | sean-k-mooney | its other that do | |
| 15:19:57 | sean-k-mooney | fortunetlly DLS will not be a thing in europe after 2021 | |
| 15:20:27 | dansmith | hopefully not on the west coast either, but it's not set yet | |
| 15:20:56 | sean-k-mooney | it was ment to happen this year but got delayed so this was ment to be the last switch | |
| 15:21:34 | sean-k-mooney | the current plan is contries adopting permenatn summer time will swap for the last time in the spring | |
| 15:21:51 | sean-k-mooney | and the rest will swap for the last time in the fall | |
| 15:32:15 | gibi | stephenfin, bauzas: spent some time thinking automating to catch bugs like https://bugs.launchpad.net/nova/+bug/1902925 . Besides code review (that fails some time like in this case) what we can do is to extend the grenade testing. | |
| 15:32:15 | openstack | Launchpad bug 1902925 in OpenStack Compute (nova) "Upgrades to compute RPC API 5.12 are broken" [Critical,In progress] - Assigned to Sylvain Bauza (sylvain-bauza) | |
| 15:32:36 | gibi | As far as I understand it run livemigration between mixed computes | |
| 15:32:41 | bauzas | sean-k-mooney: I'm against summer time | |
| 15:33:01 | bauzas | gibi: you need then two compute services | |
| 15:33:09 | gibi | bauzas: we have multinode grenade | |
| 15:33:10 | bauzas | and a rolling upgrade scenario | |
| 15:33:33 | bauzas | because the rpc pins will automatically set the version to the oldest compute one | |
| 15:33:42 | bauzas | (if set to 'auto') | |
| 15:34:01 | gibi | I think nova-grenade-multinode does what we need | |
| 15:34:11 | bauzas | and again, tbh, I wonder whether it's just a code review usage | |
| 15:34:46 | gibi | as per https://github.com/openstack/nova/blob/d25bc07d26212408211b64953af7ef6047ca3d9d/playbooks/legacy/nova-grenade-multinode/run.yaml#L47-L50 | |
| 15:34:49 | bauzas | dansmith: your thoughts on it ? tl;dr: automatical uprade testing vs. asking for functional tests that would verify a RPC version minor bump | |
| 15:35:37 | bauzas | gibi: if we run two computes, then okay, we don't need them to be on separate nodes but the other services | |
| 15:35:43 | bauzas | ie. aio+compute | |