| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-08-17 | |||
| 12:35:16 | sean-k-mooney | as is whereter we abort or force complete when the timeout expires | |
| 12:35:19 | gibi_ | sean-k-mooney: ack, I got it now that aarents' goal is to save the VM running state instead of avoiding the DB inconsistency in case of RPC/DB issue | |
| 12:36:35 | gibi_ | sean-k-mooney, aarents: I'm fine with the patch with addition of the "not postcopy_enabled" condition as sean-k-mooney suggests | |
| 12:37:13 | sean-k-mooney | gibi_: i think i would be oke with it with that added also | |
| 12:37:19 | gibi_ | cool | |
| 12:37:37 | aarents | gibi_: sean-k-mooney yep this "not postcopy_enabled" condition make sense | |
| 12:38:30 | aarents | I will add that, thanks | |
| 12:39:19 | lyarwood | https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainAbortJob FWIW | |
| 12:39:26 | lyarwood | In case the job is a migration in a post-copy mode, virDomainAbortJob will report an error (see virDomainMigrateStartPostCopy for more details). | |
| 12:40:20 | lyarwood | https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainMigrateStartPostCopy has some more context | |
| 12:40:29 | lyarwood | On the other hand once the guest is running on the destination host, the migration can no longer be rolled back because none of the hosts has complete state. If this happens, libvirt will leave the domain paused on both hosts with VIR_DOMAIN_PAUSED_POSTCOPY_FAILED reason. It's up to the upper layer to decide what to do in such case. Because of this, libvirt will refuse to cancel post-copy migration via virDomainAbortJob. | |
| 12:41:38 | lyarwood | so tbh I don't think we need to check anything | |
| 12:42:42 | lyarwood | oh wait I missed that live_migration_abort is raising the error back, sigh | |
| 12:45:25 | sean-k-mooney | ya although we are catching and ignoring that with a log in aarents patch | |
| 12:45:44 | sean-k-mooney | i guess we could rely on that behavior but i would prefer to have a comment to that effect honestly | |
| 12:45:49 | sean-k-mooney | jsut to not forget that | |
| 12:46:13 | sean-k-mooney | as the next time i see the abbort ill get suspicios about post copy again. | |
| 12:49:01 | gibi_ | aarents: are you seeing this ^^ :) | |
| 12:50:17 | aarents | gibi_: Yes so I will add a comment that say that abort may not work in case of post copy ? | |
| 12:50:51 | gibi_ | aarents: I guess you need to catch the error returned from abort and ignore it | |
| 12:52:03 | aarents | So I drop the warning | |
| 12:52:25 | aarents | ? | |
| 12:53:22 | gibi_ | aarents: sorry, so you already catching the error from abort, that is OK | |
| 12:53:27 | gibi_ | keep the warning too | |
| 12:53:32 | gibi_ | just add a note as sean-k-mooney requested | |
| 12:53:32 | aarents | And is there a concensus about sean-k-mooney suggestion to change except Exception with except libvirt.libvirtError: ? | |
| 12:53:38 | aarents | gibi_: ok | |
| 12:53:55 | gibi_ | yepp go with libvirtError | |
| 12:55:00 | aarents | ok cool | |
| 13:05:41 | opendevreview | Stephen Finucane proposed openstack/nova master: docs: Add documentation on database migrations https://review.opendev.org/c/openstack/nova/+/800078 | |
| 13:05:41 | opendevreview | Stephen Finucane proposed openstack/nova master: db: Final cleanups https://review.opendev.org/c/openstack/nova/+/800484 | |
| 13:05:42 | opendevreview | Stephen Finucane proposed openstack/nova master: tests: Enable SADeprecationWarning warnings https://review.opendev.org/c/openstack/nova/+/804708 | |
| 13:05:42 | opendevreview | Stephen Finucane proposed openstack/nova master: WIP tests: Enable SQLAlchemy 2.0 deprecation warnings https://review.opendev.org/c/openstack/nova/+/804709 | |
| 13:05:47 | stephenfin | gibi_: ^ | |
| 13:07:02 | opendevreview | Lee Yarwood proposed openstack/nova master: api: Introduce microversion 2.89 adding attachment_id to responses https://review.opendev.org/c/openstack/nova/+/804275 | |
| 13:09:15 | gibi | stephenfin: ack | |
| 13:10:52 | gibi | stephenfin: thanks, now I'm +2 all the way | |
| 13:53:17 | opendevreview | Stephen Finucane proposed openstack/nova master: tests: Enable SQLAlchemy 2.0 deprecation warnings https://review.opendev.org/c/openstack/nova/+/804709 | |
| 13:53:18 | opendevreview | Stephen Finucane proposed openstack/nova master: Replace use of Engine.scalar(), Engine.execute() https://review.opendev.org/c/openstack/nova/+/804878 | |
| 13:55:29 | opendevreview | Elod Illes proposed openstack/nova stable/rocky: [stable-only] Fix lower-constraints job https://review.opendev.org/c/openstack/nova/+/769910 | |
| 14:00:34 | opendevreview | Alexandre arents proposed openstack/nova master: libvirt: Abort live-migration job when monitoring fails https://review.opendev.org/c/openstack/nova/+/764435 | |
| 14:28:08 | opendevreview | Elod Illes proposed openstack/nova stable/rocky: [stable-only] Fix lower-constraints job https://review.opendev.org/c/openstack/nova/+/769910 | |
| 14:50:29 | gibi | melwitt: hi! I left feedback in https://review.opendev.org/c/openstack/nova/+/713301 the most concerning for me is the dependency on an oslo.limit patch as non client libraries are going to feature freeze this week | |
| 15:49:10 | ganso | melwitt, gibi hi! if you have a few minutes could please take a look at this 1-liner fix https://review.opendev.org/c/openstack/nova/+/804303 ? Thanks in advance | |
| 15:50:32 | gibi | ganso: looking | |
| 15:52:40 | spatel | sean-k-mooney hey! i am upgrading minor version of victoria and during upgrade at this step i hit this issue - https://paste.opendev.org/show/808150/ | |
| 15:54:48 | gibi | FYI, nova meeting starts in 5 minutes here in the channel | |
| 15:56:13 | gibi | ganso: does the 1 vcpu + multiqueue case works for other than vif_type=tap? | |
| 15:57:33 | ganso | gibi that code path is only reached when vif_type=tap. If using openvswitch, it wasn't impacted by the previous patch (that introduced the regression), neither this. I am not sure if the same problem happens with ovs, but since the original problem didn't, I believe this one also doesn't | |
| 15:59:48 | gibi | ganso: ok, let me try with ovs | |
| 15:59:55 | sean-k-mooney | vif_type=tap is not currently used with ovs | |
| 16:00:03 | sean-k-mooney | it was added tempoery and then removed | |
| 16:00:18 | sean-k-mooney | i belive the only thing that uses vif_type=tap today is calico | |
| 16:00:28 | gibi | sean-k-mooney: yepp the bug mentions calico | |
| 16:00:35 | gibi | sean-k-mooney: https://bugs.launchpad.net/nova/+bug/1939604 | |
| 16:00:41 | melwitt | gibi: ah, right. it's not a hard dep, so we can untie it. it's nice-to-have since it will cache limit for a repeated try N, N-1, N-2, etc for a multi create | |
| 16:01:02 | sean-k-mooney | gibi: ah ok ill look at it after the meeting | |
| 16:01:23 | gibi | melwitt: OK, it is easier then. lets see if the oslo patch lands before the deadline and if not then just remove the depends-on | |
| 16:01:33 | gibi | but now lets have a meeting | |
| 16:01:38 | gibi | #startmeeting nova | |
| 16:01:38 | opendevmeet | Meeting started Tue Aug 17 16:01:38 2021 UTC and is due to finish in 60 minutes. The chair is gibi. Information about MeetBot at http://wiki.debian.org/MeetBot. | |
| 16:01:38 | opendevmeet | Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. | |
| 16:01:38 | opendevmeet | The meeting name has been set to 'nova' | |
| 16:01:38 | melwitt | gibi: ++ thanks | |
| 16:01:55 | gibi | sean-k-mooney: thanks | |
| 16:02:45 | gibi | #topic Bugs (stuck/critical) | |
| 16:02:52 | gibi | no critical bug open | |
| 16:02:56 | gibi | #link 15 new untriaged bugs (+4 since the last meeting): #link https://bugs.launchpad.net/nova/+bugs?search=Search&field.status=New | |
| 16:03:22 | gibi | is there any specific bug to discuss today? | |
| 16:04:16 | gibi | I see ganso has one in the open discussion, lets bring that up here | |
| 16:04:24 | gibi | (ganso): bug "Compute node deletes itself if rebooted without DNS": https://bugs.launchpad.net/nova/+bug/1939920 | |
| 16:04:29 | gibi | was this a design choice? acceptable solutions discussion | |
| 16:04:33 | gibi | EOM | |
| 16:04:37 | ganso | gibi thanks | |
| 16:05:00 | ganso | so, IMO this is a critical bug, and after reading the code and the way it works it kinda feels like a design choice | |
| 16:05:29 | ganso | because seems like it was intentionally implemented for it to scan for "orphan compute nodes" and delete them, clear the allocations and RP, etc | |
| 16:05:42 | gibi | yes that was intentional | |
| 16:05:51 | ganso | but it is producing this effect which is very undesirable | |
| 16:06:33 | gibi | so in your infra the compute host can change hostname and that causing the issue | |
| 16:06:33 | ganso | as I suggested in the bug, a possible solution I see if to compare the host field in the nova.compute_nodes table | |
| 16:06:43 | ganso | if it is the same, then we would skip this | |
| 16:06:57 | ganso | gibi: it is not that it "can" change the hostname. But it happens due to external reasons | |
| 16:07:09 | ganso | like, lack of connectivity when it boots, a DNS outage, etc | |
| 16:07:26 | melwitt | it will recover in that it will create a new compute node etc. the main thing that is "unique" is the hostname, that's what's stored in the instance.host and a whole lot of other places. so changing the name you break all the associations and in reality you essentially have a new/different service and compute node | |
| 16:08:39 | melwitt | if the associations were done using UUID it would be a different story. but unfortunately it is what it is and would take a large work to change it IMHO | |
| 16:08:39 | ganso | melwitt: right, so the instance.host captures the entire FQDN, and the FQDN is what is changing, therefore when that changes, running instances are no longer identifiable as running in that node | |
| 16:08:58 | melwitt | right | |
| 16:09:26 | ganso | melwitt: so that is another side-effect of that FQDN changing problem, but I am not proposing changing that. I am just proposing to skip this "deletion" step if the compute_nodes.host field does not change | |
| 16:09:33 | ganso | if will avoid part of the issues | |
| 16:10:00 | ganso | melwitt it will not avoid the issue you described, but 1 issue is better than 2 I think | |
| 16:10:24 | dansmith | I'm missing the distinction I think | |
| 16:11:22 | gibi | but compute_nodes.host comes from the DB isn't it? so it won't ever change | |
| 16:11:24 | sean-k-mooney | ganso: right so nova does not support compute hosts changing hostname today | |
| 16:11:52 | ganso | gibi: doesn't it derive from the FQDN it reads from the system? | |
| 16:11:57 | sean-k-mooney | so if it is changing for external reasons that is not expected to work out of the box | |
| 16:12:34 | dansmith | sean-k-mooney: ++ | |
| 16:12:42 | ganso | sean-k-mooney: right, but I'm not proposing that it does support, but just stop doing what it is doing today. That thing about orphan compute nodes isn't supposed to address changing hostnames either | |
| 16:13:05 | gibi | ganso: when the ComputeNode is created then yes, it is coming from the hostname reported by libvirt, but never changes after | |
| 16:13:35 | sean-k-mooney | ganso: even if we did not clean up the orpah compute nodes teh instnace.host is used to make rpc calls to the host that the instance is on | |
| 16:14:00 | sean-k-mooney | so unless you hardcode the chost paramter in the nova.conf so it does not change | |