| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-09-04 | |||
| 12:50:00 | openstackgerrit | Luyao Zhong proposed openstack/nova master: Add functional tests for virtual persistent memory https://review.opendev.org/678470 | |
| 12:51:31 | brinzhang | sean-k-mooney: Good experience, got it. | |
| 13:09:46 | mriedem | bauzas: can you push this simple refactor through? https://review.opendev.org/#/c/679651/ | |
| 13:10:18 | bauzas | mriedem: sure thing | |
| 13:10:58 | luyao | dansmith: Are you around? | |
| 13:17:20 | bauzas | mriedem: gibi: I was paying attention to the qos-bandwith series but I'm a bit lost with it | |
| 13:17:26 | bauzas | gibi: what's the top change ? | |
| 13:17:29 | bauzas | https://review.opendev.org/#/c/679653/ ? | |
| 13:17:44 | bauzas | because I don't see yet changes for cold migrate | |
| 13:17:55 | openstackgerrit | sean mooney proposed openstack/nova master: multi numa nfv testing job https://review.opendev.org/679656 | |
| 13:17:56 | openstackgerrit | sean mooney proposed openstack/nova master: [DNM] test migration with pinning https://review.opendev.org/679754 | |
| 13:18:08 | mriedem | gibi: replied in https://review.opendev.org/#/c/656422/18/nova/compute/manager.py@2122 | |
| 13:18:30 | bauzas | nah nevermind https://review.opendev.org/#/c/679019/4 | |
| 13:18:39 | bauzas | gerrit fucks up with the series | |
| 13:18:48 | bauzas | there are some branches | |
| 13:18:51 | gibi | bauzas: this is the top https://review.opendev.org/#/c/679019/4 | |
| 13:19:07 | gibi | bauzas: but I will respin the whole soonish to fix mriedem's comments | |
| 13:19:11 | gibi | mriedem: looking | |
| 13:19:20 | bauzas | gibi: okay i can wait then | |
| 13:19:46 | mriedem | gibi: tl;dr i think you need the service version check in conductor after scheduler picks a dest and also a check in compute in case the compute rpc api versions are pinned in config | |
| 13:20:00 | mriedem | i.e. the compute services can be running train but configured to only send stein level messages during an upgrade | |
| 13:20:04 | mriedem | artom ran into this yesterday | |
| 13:20:36 | mriedem | both are temporary and can be removed in U | |
| 13:21:22 | brinzhang | mriedem: https://review.opendev.org/#/c/673133/14/nova/compute/api.py@4028, I was updated that comment, found the volume_bdm.save() report an "ObjectActionError: Object action obj_load_attr failed because: attribute id not lazy-loadable" | |
| 13:21:34 | brinzhang | mriedem: That why? | |
| 13:21:40 | gibi | mriedem: ack. I will put the checks in the conductor. Good point about that the service version is no enough | |
| 13:21:53 | mriedem | brinzhang: in unit tests or a tempest run? | |
| 13:22:08 | mriedem | brinzhang: if unit tests, you need to mock the BlockDeviceMapping.save call | |
| 13:22:08 | artom | *me meerkats up* | |
| 13:22:13 | brinzhang | mriedem: functioanl test failed | |
| 13:22:44 | openstackgerrit | sean mooney proposed openstack/nova master: [DNM] test migration with pinning https://review.opendev.org/679754 | |
| 13:22:44 | openstackgerrit | sean mooney proposed openstack/nova master: multi numa nfv testing job https://review.opendev.org/679656 | |
| 13:22:54 | mriedem | brinzhang: ok i can look in a bit | |
| 13:23:25 | gmann | brinzhang: mriedem just commented, it should be passed in via reserve_block_device_name() | |
| 13:23:50 | artom | mriedem, I don't think they can be removed in U - dansmith can explain in more detail when he comes online because I'm not sure I understand it fully, but you can't remove a check until the next major version bump | |
| 13:24:04 | artom | Or maybe I'm misunderstanding it | |
| 13:24:28 | mriedem | gmann: no it shouldn't | |
| 13:24:39 | gmann | we can directly save ? | |
| 13:24:42 | mriedem | gmann: see my earlier comments on that very thing - i specifically don't want to do that b/c it adds a lot of unnecessary complexity | |
| 13:24:57 | mriedem | i have to take my kid to the bus stop, will look into this later | |
| 13:25:09 | gmann | ohk, did not see the previous comment | |
| 13:25:41 | brinzhang | gmann: that will be change the rpc version, in PS13 do that | |
| 13:26:05 | gibi | bauzas: It might take more time to update the bandwidth series than I first aniticipated. I will ping you when the new version is up | |
| 13:26:13 | gmann | i did not see that, checking.. | |
| 13:28:10 | bauzas | gibi: no worries, I'm still working on updating the placement audit command | |
| 13:35:26 | dansmith | artom: to what are you referring? | |
| 13:36:11 | artom | dansmith, all those checks I have about either making sure we got a param that appears in RPC 5.3, or that we can send RPC 5.3, those can only be removed in the next major version bump, right? | |
| 13:36:29 | artom | dansmith, if so, mind explaining why, exactly? It's still not clear in my head why they can't just go away in U | |
| 13:37:05 | dansmith | well, it's because there are two sets of "rules" at play here: | |
| 13:37:25 | dansmith | The first is that we don't allow RPC upgrades from N-2 to N in terms of releases | |
| 13:38:00 | dansmith | we do that not just because of strict rpc compatibility, but because of stuff buried deeper that expects fields to be filled out two releases from now, data migrations to have run, etc | |
| 13:38:13 | artom | dansmith, this means that we don't support Train anything talking to Rocky anything, right? Only Stein | |
| 13:39:18 | dansmith | the second is that the RPC versioning rules say that within a major version, you must be backwards compatible, able to accept anything back to 5.0 and able to send anything back to 5.0 if asked (by the version pin). If we always bumped versions on every release, then you could always remove stuff two releases from when you added it (since each release would have to bridge between two majors, you could never remove it in the next one) | |
| 13:39:59 | dansmith | artom: that's what it means, but since the rpc version is all we have, it's really a soft statement of support that we can't validate or quantify, | |
| 13:40:04 | dansmith | especially since people backport stuff | |
| 13:40:31 | artom | People backport RPC stuff? | |
| 13:40:56 | dansmith | the RPC versioning scheme that we have is why, in many cases, you actually can run N-2 talking to N when things haven't shifted much, and what would save our bacon if we had to start supporting such a thing | |
| 13:41:04 | dansmith | upstream doesn't, but distros have in the past for sure | |
| 13:41:05 | dansmith | however, | |
| 13:41:29 | dansmith | it's easy to backport something that doesn't technically affect the rpc interface directly, but definitely changes behavior or semantics | |
| 13:41:41 | dansmith | like something that causes a field to be filled out that wouldn't have been if the client was really as old as it is, etc | |
| 13:42:04 | dansmith | like some distro that wanted to backport an api feature that now sends something newer, but doesn't technically need an rpc change to make it work | |
| 13:42:25 | artom | So the reason the first rule doesn't invalidate the second rule is that even if we only allow a mix of N and N-1, there's nothing stopping an older RPC pin from being set? | |
| 13:42:51 | dansmith | yes, but let me state it slightly differently: | |
| 13:43:19 | dansmith | the first rule is our support policy and the second is the actual mechanical way we make sure our services can sanely talk to each other across versions | |
| 13:43:37 | artom | Ah, I see | |
| 13:43:56 | dansmith | so theoretically the mechanical bit should be able to support a wider envelope (rule 2) than we officially say we support (rule 1) | |
| 13:44:06 | artom | IOW, we don't have mechanics in place to enforce our support policy, short of bumping major on every release | |
| 13:44:14 | dansmith | because the N->N+2 thing has lots of other implications as well, like database migrations, things that need to check service versions to enable api features, etc, etc | |
| 13:44:23 | dansmith | that's a true statement | |
| 13:45:11 | artom | Aha, so the support policy is not just for RPC, it's for other things as well (that list you just said) | |
| 13:45:33 | dansmith | yup | |
| 13:45:53 | dansmith | because the other things are buried deep and don't necessarily even have a direct impact on rpc, but can definitely have an indirect one | |
| 13:46:23 | dansmith | alright, so I've passed the torch of knowledge, now you can be the enforcer right? | |
| 13:46:36 | artom | dansmith, not sure about that, but I'll write it up in https://docs.openstack.org/nova/stein/reference/index.html | |
| 13:46:51 | mriedem | it's already written up in dan's blog... | |
| 13:47:04 | mriedem | if you wanted to condense that, sure | |
| 13:47:06 | dansmith | but writing it up in our docs would be good | |
| 13:47:24 | mriedem | what i was thinking about was https://review.opendev.org/#/c/634606/73/nova/compute/manager.py@6574 specifically is only needed for train, | |
| 13:47:30 | mriedem | since we don't support Stein -> U | |
| 13:47:34 | dansmith | if my blog is part of the official nova docs, I'm going to throw some ads up there :) | |
| 13:47:47 | artom | dansmith, for AWS :D | |
| 13:48:08 | dansmith | mriedem: that's only the case because of the rule 1 above | |
| 13:48:29 | mriedem | sure, | |
| 13:48:33 | dansmith | mriedem: they can still set the pin lower than train in U and would fail to make that call.. it'd be an unsupported situation, but it's probably good if we just leave it in place | |
| 13:48:40 | mriedem | and law of nova #35 says we'll forget to remove the compat code for 3 releases anyway | |
| 13:48:49 | dansmith | heh | |
| 13:49:08 | dansmith | I can tell you what rule #1 is | |
| 13:49:27 | mriedem | there is a rule somewhere that you must have at least 3 different types of BlockDevice(Mapping) objects | |
| 13:49:37 | dansmith | #17 iirc | |
| 13:49:59 | artom | dansmith, you left us hanging on #1 there | |
| 13:50:08 | dansmith | artom: don't talk about nova, obvi | |
| 13:50:32 | artom | OK, I lied. | |
| 13:50:58 | artom | dansmith, mriedem, just so I'm clear, are we waiting on anything from me and/or CI for the NUMA LM series? | |
| 13:50:58 | alex_xu | dansmith: let me know I understand this rule correct or not https://review.opendev.org/#/c/678447/7/nova/db/sqlalchemy/migrate_repo/versions/401_add_resources.py@32 :) | |
| 13:51:32 | dansmith | artom: nope, I had it up to be looking at when I saw your ping | |
| 13:51:34 | mriedem | i've been online for ~45 minutes and already have a full tab queue, so no | |
| 13:51:42 | luyao | dansmith: Hi, I have a question about https://review.opendev.org/#/c/678447/8 . Do you mean the script can run out of the nova code, the script will just update DB, then db column is deleted while the updates on nova code have not taken effect.I don't understand how I should do. | |
| 13:52:02 | alex_xu | luyao: i just asked :) | |
| 13:52:28 | artom | mriedem, yep, fully away of your overcommitment, as just making sure I understood where we stood :) | |