| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-07-15 | |||
| 12:04:00 | gibi | yes, that would be a piece of work :) | |
| 12:04:15 | gibi | and yes, I got you, I try to figure out something better... | |
| 12:05:00 | artom | Sorry for not bringing any better solution :P | |
| 12:05:11 | gibi | basically the mock and the vnic_type change need to be coupled somehow... | |
| 12:05:43 | gibi | artom: no worries, you have a valid point, and it was a good excersise to talk it through | |
| 12:20:20 | gibi | artom: interestingly the hard reboot did not fail | |
| 12:20:57 | gibi | it created a proper macvtap interface and passed to the instance | |
| 12:21:28 | gibi | it is probably works because hard reboot unplugs the vif -> the VF will be freed the plug can lookup the netdev | |
| 12:22:10 | gibi | and the accounting in nova side is actually correct as the macvtap dev needs to keep the parent VF allocated to the instance (and that does not change during the reboot) | |
| 12:22:21 | gibi | so probably a direct -> direct-phyical change would not work | |
| 12:29:31 | artom | Ah, right, because it's the host OS accounting | |
| 12:29:36 | artom | So as long as we unplug first, we're fine | |
| 12:31:14 | gibi | in direct -> macvtap yes we are fine both on the OS level an in the nova PCI tracking | |
| 12:31:36 | gibi | in case of direct -> direct-phyical the PCI tracking in nova will be inconsistent | |
| 12:31:59 | gibi | as the instance will have a VF allocated but consuming a PF instead (probably) | |
| 12:58:33 | gibi | artom: I think I see a way to make the test more logical. We can check in the get_ifname_by_pci_address mock if the device being looked up is actaully consumed as a VF by the instance | |
| 13:00:58 | opendevreview | Balazs Gibizer proposed openstack/nova master: Reproduce bug 1981813 in func env https://review.opendev.org/c/openstack/nova/+/849985 | |
| 13:01:02 | gibi | artom: ^^ | |
| 13:17:30 | artom | Oooo, nice | |
| 13:18:50 | gibi | now if you move the port update before the boot then the mock will not raise the exception and the test will fail | |
| 13:19:23 | gibi | we have the external state but it is stored the domain in the fake libvirt connectioin | |
| 13:19:34 | gibi | * stored in the domain | |
| 13:23:44 | opendevreview | Balazs Gibizer proposed openstack/nova master: Gracefully ERROR in _init_instance vnic_type changed https://review.opendev.org/c/openstack/nova/+/850003 | |
| 13:23:58 | gibi | and here is the "fix" (two error logs and a skip) | |
| 13:24:26 | gibi | I have to leave early today so I will add unit test and reno to the patch on monday | |
| 13:29:07 | artom | Sure | |
| 13:43:16 | sean-k-mooney | gibi: ack ill review that shortly i just got back form doctors appoinent they were runing 90 mins behind so took longer then planned | |
| 13:44:54 | gibi | thanks | |
| 13:45:04 | gibi | no rush, I will probably look at it only on Monday | |
| 13:45:44 | sean-k-mooney | ack | |
| 13:56:31 | sean-k-mooney | gibi: i have set the public security flag on https://bugs.launchpad.net/nova/+bug/1981813 by the way just to highlight it to the security team | |
| 13:57:15 | sean-k-mooney | it was a public bug downstream so that ship had already sailed when we triaged it | |
| 14:12:21 | opendevreview | Stephen Finucane proposed openstack/nova master: Fix compatibility with jsonschema 4.x https://review.opendev.org/c/openstack/nova/+/849867 | |
| 14:12:21 | opendevreview | Stephen Finucane proposed openstack/nova master: Remove unused requirement https://review.opendev.org/c/openstack/nova/+/850006 | |
| 14:12:37 | stephenfin | sean-k-mooney: Sorry, I missed your ping earlier. Respun with an arbitrary version now ^ | |
| 14:14:30 | sean-k-mooney | cool | |
| 14:14:39 | sean-k-mooney | will look soon | |
| 15:23:45 | opendevreview | Stephen Finucane proposed openstack/nova master: Bump jsonschema minimum to 4.0.0 https://review.opendev.org/c/openstack/nova/+/850021 | |
| 15:55:41 | sean-k-mooney | artom: you are aware that what you sugesed in https://review.opendev.org/c/openstack/nova/+/849104 is not allowed or what you are ment to do right | |
| 15:56:13 | artom | sean-k-mooney, what's not allowed? | |
| 15:56:37 | sean-k-mooney | using _IntegratedTestBase in new regression tests | |
| 15:56:55 | artom | o_O | |
| 15:57:00 | artom | Is that a new thing? | |
| 15:57:06 | sean-k-mooney | the regression tests are explicatly not ment to use the full test infra and we are not ment to use _IntegratedTestBase for new test in genera | |
| 15:57:19 | artom | ... | |
| 15:57:30 | sean-k-mooney | artom: yes its docuemnted in the readme for the regression tests i linked it in the commend inline | |
| 15:57:38 | artom | [artom@zoe nova]$ ag _IntegratedTestBase nova/tests/functional/regressions/ | wc -l | |
| 15:57:38 | artom | 19 | |
| 15:57:49 | sean-k-mooney | yep there are cases where its used | |
| 15:57:59 | sean-k-mooney | but we are trying not to do ti any more | |
| 15:58:16 | artom | Are we? Why? Because backports? | |
| 15:58:42 | sean-k-mooney | because regression tests are ment to be independing of the rest fo the fucntest so that they are not impacted by refacortings | |
| 15:59:10 | sean-k-mooney | it also help with backprots but that is not the main reason | |
| 15:59:33 | artom | I mean https://review.opendev.org/c/openstack/nova/+/812126 | |
| 15:59:45 | artom | That's at the beginning of this year | |
| 15:59:56 | sean-k-mooney | yep i know that should not have used that | |
| 16:00:31 | artom | Like, we can discuss this, and others can correct me if I'm wrong, but this is the first I hear of it, and I wasn't aware it was an established thing that all core reviewers had agreed on. | |
| 16:00:46 | sean-k-mooney | its in the readme | |
| 16:00:52 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/tests/functional/regressions/README.rst#writing-regression-tests= | |
| 16:01:09 | artom | And clearly at least *some* people have not read it :) | |
| 16:01:54 | sean-k-mooney | right this is a very long standing policy for that subdirectory | |
| 16:02:03 | artom | Of not reading it? :D | |
| 16:02:08 | sean-k-mooney | it was intoduced when it was created https://github.com/openstack/nova/commit/5fe04c2ee8b721ecdaea4d06de54b4998a0df395 | |
| 16:02:43 | sean-k-mooney | i had condiered asking gibi to rewrite there repoducer to follow it a few minute ago | |
| 16:03:01 | sean-k-mooney | but we dont require all repoduced to be added as freestandign regressions | |
| 16:03:02 | sean-k-mooney | so i did not | |
| 16:03:15 | sean-k-mooney | but if they are in that folder they should be freestanding like that | |
| 16:06:15 | artom | Genuinely news to me. | |
| 16:06:32 | artom | If we want to make that a thing I won't stand in anyone's ways, but I disagree that it's already a thing. | |
| 16:06:44 | sean-k-mooney | it reallly is | |
| 16:06:57 | artom | I mean in that case let's add a hacking rule | |
| 16:07:16 | artom | If it finds _IntegratedTestCase in the regressions folder, pep8 -1 | |
| 16:07:27 | artom | If we're *actually* going to be strict about it | |
| 16:07:34 | sean-k-mooney | artom: the other thing to keep in mind is that stephen had been trying to remvoe _IntegratedTestCase at one point | |
| 16:07:48 | artom | stephenfin's been trying to remove *everything* at one point ;) | |
| 16:07:49 | sean-k-mooney | so we should not be directlly adding new tests that use it as a base | |
| 16:08:02 | sean-k-mooney | right but _IntegratedTestCase is considerd private | |
| 16:08:12 | sean-k-mooney | and new code should not inherit form it directly | |
| 16:08:17 | artom | I feel like facts on the ground contradict that :) | |
| 16:08:20 | sean-k-mooney | its subclasses are fine but not that | |
| 16:09:25 | sean-k-mooney | i think lee used it for conveicne not because it was the most correct way to repoduce | |
| 16:11:06 | sean-k-mooney | we generally try to be pragmatic about this but i disagree that amit shoudl rewrite the test they created | |
| 16:13:01 | sean-k-mooney | ProviderUsageBaseTestCase would be prefible if you require someing other then the mixins for new tests | |
| 16:14:28 | sean-k-mooney | or LibvirtProviderUsageBaseTestCase if you need libvirt | |
| 17:03:21 | opendevreview | Artom Lifshitz proposed openstack/nova master: hacking: forbid _IntegratedTestBase in regression tests https://review.opendev.org/c/openstack/nova/+/850053 | |
| 17:03:34 | artom | sean-k-mooney ^^ Yes, I'm part-trolling :) | |
| 17:03:46 | artom | Also, it doens't actually work, which... is surprisingly annoying? | |
| 17:03:56 | artom | Like, I thought I'd be able to hack it up quicker, so the ego takes a hit | |
| 17:04:26 | sean-k-mooney | its kind fo hard to do sicne its not about _IntegratedTestBase sepcificaly | |
| 17:04:37 | sean-k-mooney | we are alowed to use the mixins | |
| 17:06:00 | artom | Oh, like those will never get refactored :P | |
| 17:06:34 | sean-k-mooney | well they should not the can have methods added but they idaaly would be addditive changes only or mainly | |
| 17:12:49 | sean-k-mooney | im going to call it a day chat to you monday o/ | |
| 17:26:27 | artom | Oh crap that remind me, I'm off next week | |
| #openstack-nova - 2022-07-18 | |||
| 04:28:00 | opendevreview | Merged openstack/nova stable/wallaby: Add missing condition https://review.opendev.org/c/openstack/nova/+/847011 | |
| 05:00:37 | opendevreview | Merged openstack/nova master: Update the file for IPv4-only or IPv6-only network https://review.opendev.org/c/openstack/nova/+/465891 | |
| 05:12:10 | opendevreview | Merged openstack/nova master: etc: Highlight absence of packages from config gen https://review.opendev.org/c/openstack/nova/+/849796 | |
| 07:02:52 | opendevreview | Merged openstack/nova stable/victoria: libvirt: make mdev types name attribute be optional https://review.opendev.org/c/openstack/nova/+/754401 | |
| 07:16:50 | bauzas | good Monday everyone | |
| 07:18:43 | sean-k-mooney | bauzas: o/ | |