| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-09-01 | |||
| 08:57:03 | bauzas | sean-k-mooney: dansmith: auniyal: looks like https://review.opendev.org/c/openstack/nova/+/882284 created a gate issue | |
| 08:57:04 | frickler | a reno referencing "this change" is weird wording btw | |
| 08:57:44 | bauzas | frickler: we can modify this | |
| 08:57:53 | auniyal | bauzas, ack | |
| 08:58:04 | bauzas | in general we say "Now, Nova does X or Y" | |
| 08:58:26 | bauzas | or "With this new feature named "blah", now we have" | |
| 09:02:15 | bauzas | auniyal: I'm working on a patch that will conditionnally check the volumes if only BDMs exist | |
| 09:02:51 | auniyal | ack bauzas, any thing I can help with | |
| 09:03:43 | bauzas | we may need core reviewers | |
| 09:04:36 | frickler | elodilles: thx, now just someone needs to retrigger the W+1 I think | |
| 09:09:03 | elodilles | frickler: i did it just before you wrote :) | |
| 09:10:09 | bauzas | auniyal: actually, we can't avoid a Cinder API call for getting the volumes | |
| 09:10:53 | bauzas | volume attachments* | |
| 09:11:02 | auniyal | yeah, I was about to say | |
| 09:11:11 | auniyal | saw the comment on bug report | |
| 09:11:56 | auniyal | cinder calls are separate from bdms what nova have | |
| 09:12:25 | auniyal | can we increase reboot time check ? | |
| 09:14:41 | bauzas | oh wait | |
| 09:15:09 | bauzas | this isn't calculating the time it boots | |
| 09:15:15 | bauzas | this is just getting the uptime | |
| 09:19:50 | SvenKieske | systemd-analyze time gives you boot times :) | |
| 09:20:04 | SvenKieske | without any context | |
| 09:23:38 | bauzas | that's weird then | |
| 09:24:04 | bauzas | https://github.com/openstack/tempest/blob/28ba6ee268bdf5d18a13a0dc7c7fea2c3a3dfcce/tempest/common/utils/linux/remote_client.py#L84-L88 is just giving me when the server was booted | |
| 09:24:56 | bauzas | and https://github.com/openstack/tempest/blob/master/tempest/api/compute/servers/test_server_actions.py#L127-L128 is just comparing the timestamps to check that the VM was actually rebooted | |
| 09:25:11 | bauzas | this isn't at all a performance check | |
| 09:30:55 | SvenKieske | I'm not understanding your last sentence: do you mean this isn't supposed to be a performance check, or are you indicating that this should be a performance check but fails to do so? | |
| 09:31:52 | SvenKieske | depending on what stage the virtual machine is in, the time comparison could be flaky, during early boot especially, if time is not correctly synced. | |
| 09:33:18 | SvenKieske | I'd also strongly advise not to directly get stuff from proc-fs. why not use a native python function instead at least? | |
| 09:33:21 | bauzas | SvenKieske: Tempest gets the uptime, subs it from the current time so it gets the local time when the server was booted, then reboots the server and does the same math with uptime | |
| 09:34:07 | bauzas | and then there is an assertion check that verifies that the reboot timestamp is greater than the original timestamp (meaning the OS was actually rebooted) | |
| 09:34:40 | bauzas | https://github.com/openstack/tempest/blob/28ba6ee268bdf5d18a13a0dc7c7fea2c3a3dfcce/tempest/api/compute/servers/test_server_actions.py#L127-L128 and https://github.com/openstack/tempest/blob/28ba6ee268bdf5d18a13a0dc7c7fea2c3a3dfcce/tempest/api/compute/servers/test_server_actions.py#L109 | |
| 09:34:53 | SvenKieske | I hope that test never runs during dailight saving time adjustments *just kidding* | |
| 09:35:16 | bauzas | UTC exists for a good reason :) | |
| 09:35:52 | bauzas | basically, this failing check means that the server *wasn't* rebooted | |
| 09:35:54 | SvenKieske | I'm strongly reminded about this now, don't know why: https://gist.github.com/timvisee/fcda9bbdff88d45cc9061606b4b923ca | |
| 09:36:11 | SvenKieske | this is really a bad check, to be blunt | |
| 09:36:48 | SvenKieske | just use systemd for this stuff already guys, it's there, it works, I know it has it's own bugs, but it's better then the stuff that was before it. | |
| 09:37:09 | auniyal | https://paste.opendev.org/show/bk8aK3ZbjpeDYs6swcoQ/ | |
| 09:37:09 | auniyal | in test reboot_time should be more then (first) boot_time --- but its failing because it same. | |
| 09:37:14 | SvenKieske | e.g. you can easily check the boot counter with systemd | |
| 09:37:30 | bauzas | auniyal: which means that the server *wasn't* rebooted | |
| 09:37:41 | auniyal | yeah | |
| 09:37:56 | bauzas | I'm just checking the logs | |
| 09:38:07 | bauzas | we may have had an exception in the reboot | |
| 09:39:06 | bauzas | oh wait | |
| 09:39:15 | SvenKieske | well that could also be true :) but just to check the wall clock seems not really reliable in the long way, I bet you get false positives someday (when clocks will be out of sync in VMs, and they will be, if the ntp setup is not bulletproof) | |
| 09:39:23 | bauzas | if we ssh the guest too soon, the reboot may not be done yet | |
| 09:39:45 | bauzas | SvenKieske: I don't think it's an instrumentation issue | |
| 09:39:52 | SvenKieske | so you need to detect the reboot, before ssh'ing in order to detect the reboot ;) | |
| 09:40:05 | auniyal | but still will it saw same time ? time must have updated right ? | |
| 09:40:18 | bauzas | auniyal: not if the guest wasn't rebooted *yet* | |
| 09:40:36 | bauzas | yay, another waiter ! | |
| 09:42:43 | auniyal | yeah, so it took boot_time, reboot (and waited until get ACTIVE), then took new_boot_time, must be greater, | |
| 09:43:04 | auniyal | or we should set our own clock counter in test ?? | |
| 09:44:15 | auniyal | and verify if we are able to run any cmd at all in system after reboot ? | |
| 09:45:33 | SvenKieske | well you're already doing that if you read the system time, you "just" need to verify that this reading was successfull and not NULL :) | |
| 09:46:10 | auniyal | yeah, that should be enough to say server rebooted | |
| 09:54:59 | bauzas | okay, I think I spotted | |
| 09:55:13 | bauzas | we reverted the reboot due to us unable to reach cinder | |
| 10:00:47 | auniyal | bauzas https://zuul.opendev.org/t/openstack/build/0bf10393fa25419c8f3a072ab091f0da/log/controller/logs/screen-n-cpu.txt#12033 this right ? | |
| 10:01:43 | bauzas | yes | |
| 10:02:36 | bauzas | the keystone service catalog isn't containing the volumev3 endpoint url, that's what's saying the exception | |
| 10:04:54 | bauzas | but heh, https://1e11be38b60141dbb290-777f110ca49a5cd01022e1e8aeff1ed5.ssl.cf1.rackcdn.com/893401/5/check/neutron-ovn-tempest-ovs-release/f379752/controller/logs/local_conf.txt | |
| 10:04:59 | bauzas | "disable_service cinder" | |
| 10:05:10 | auniyal | okay, its reverted but still shouldn't /proc/uptime get updated ? | |
| 10:07:39 | auniyal | still great test though it caught it or navigated us to the reason | |
| 10:09:24 | bauzas | no, I mean, we have a problem in Nova : we can't longer hard reboot if you don't deploy Cinder | |
| 10:09:47 | bauzas | we should handle this case | |
| 10:10:09 | auniyal | yes | |
| 10:10:34 | bauzas | not sure sean-k-mooney is around but I'd appreciate his thoughts on the best way to alleviate https://github.com/openstack/nova/blob/88c73e2931dcb276982612dbbdc3b849d4977c16/nova/compute/manager.py#L4177 | |
| 10:12:03 | auniyal | so if cinder.attachment_get_all failed, with could not create client or endpoint not found skip the cinder check | |
| 10:12:45 | auniyal | I'll create a patch for that, | |
| 10:12:56 | bauzas | yeah that sounds the obvious fix, but I wonder whether there is a better way to flag that this environment isn't having cinder" | |
| 10:13:49 | bauzas | I'm just looking at how we manage such case elsewhere in the code | |
| 10:15:25 | bauzas | yeah found | |
| 10:16:04 | bauzas | auniyal: https://github.com/openstack/nova/blob/88c73e2931dcb276982612dbbdc3b849d4977c16/nova/compute/manager.py#L3173-L3178 | |
| 10:16:28 | bauzas | this is gross but meh | |
| 10:16:45 | auniyal | ack bauzas | |
| 10:18:50 | sean-k-mooney | bauzas what the issue | |
| 10:18:57 | bauzas | auniyal: would you be able to propose a patch soon ? | |
| 10:19:05 | auniyal | bauzas yes | |
| 10:19:09 | bauzas | sean-k-mooney: tl;dr: some jobs that don't pull cinder fail | |
| 10:19:21 | sean-k-mooney | ah | |
| 10:19:25 | bauzas | sean-k-mooney: because now we call cinder in reboot | |
| 10:19:41 | bauzas | so we need to wrap the right exceptions | |
| 10:19:44 | bauzas | https://bugs.launchpad.net/nova/+bug/2033752 | |
| 10:19:58 | sean-k-mooney | well we just need a decorator that check if keystone is installed and skipt the function | |
| 10:20:17 | bauzas | sean-k-mooney: keystone is installed, cinder isn't | |
| 10:20:20 | sean-k-mooney | treat this like how we test for api extentions in neutron | |
| 10:20:27 | sean-k-mooney | bauzas: yes | |
| 10:20:39 | sean-k-mooney | oh i said keystoen | |
| 10:20:50 | sean-k-mooney | i ment check in keystone if cinder is installed | |
| 10:20:51 | bauzas | yeah but I got your point | |
| 10:21:04 | bauzas | yeah, that's why I wanted to discuss this with you and dansmith | |
| 10:21:11 | bauzas | we have 100 ways to make it working | |
| 10:21:44 | bauzas | ideally I would think that our internal cinder client package would wrap this, but this sounds a huge work | |
| 10:21:53 | sean-k-mooney | so when discusssing this in the past we noted that if you cant connect ot cinder for any reason we shoudl jsut boot with the bdm we have | |
| 10:22:27 | bauzas | yeah and with shutdown we just have a long list of wrapped exceptions in order to let shutdown succeed anyway | |
| 10:22:33 | bauzas | https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L3164-L3188 | |