opendevreview | Arun S A G proposed openstack/ironic master: Add support for configdrive in anaconda interface https://review.opendev.org/c/openstack/ironic/+/780398 | 03:12 |
---|---|---|
iurygregory | good morning Ironic! TGIF | 06:31 |
arne_wiebalck | Good morning iurygregory and Ironic! | 06:36 |
iurygregory | morning arne_wiebalck o/ | 06:36 |
rpittau|afk | good morning ironic! Happy Friday! o/ | 07:13 |
*** rpittau|afk is now known as rpittau | 07:15 | |
arne_wiebalck | Good morning rpittau o/ | 07:18 |
rpittau | hey arne_wiebalck :) | 07:18 |
iurygregory | morning rpittau o/ | 07:36 |
rpittau | hey iurygregory :) | 07:36 |
janders | hey iurygregory arne_wiebalck rpittau and Ironic o/ | 07:59 |
janders | Happy Friday | 07:59 |
iurygregory | hey janders o/ | 07:59 |
rpittau | hey janders :) | 08:00 |
arne_wiebalck | good morning janders o/ | 08:12 |
opendevreview | Verification of a change to openstack/ironic failed: Fix ramdisk boot option handling https://review.opendev.org/c/openstack/ironic/+/797517 | 08:21 |
dtantsur | morning ironic | 08:43 |
dtantsur | TheJulia: do you suggest we block https://review.opendev.org/c/openstack/ironic/+/796879 until September? :-/ | 08:46 |
dtantsur | I'm not vehemently against that, but it may put the FJ drivers in disadvantage because of the remaining inconsistency | 08:47 |
dtantsur | JayF: hey, could we have some resolution on https://review.opendev.org/c/openstack/ironic/+/797512 please? Do you suggest we never improve or otherwise change error messages any more? | 08:48 |
opendevreview | Arne Wiebalck proposed openstack/ironic master: Cache AgentClient on Task, not globally https://review.opendev.org/c/openstack/ironic/+/797674 | 08:58 |
arne_wiebalck | dtantsur: I updated the task number to link it with the story ^^ (I guess this is how it is linked) | 08:59 |
dtantsur | yep, looks about right | 09:00 |
opendevreview | Dmitry Tantsur proposed openstack/ironic master: [WIP] Refactor validate_image_properties https://review.opendev.org/c/openstack/ironic/+/797875 | 09:02 |
cenne | morning everyone . | 09:02 |
dtantsur | hi cenne | 09:03 |
cenne | o/ | 09:03 |
iurygregory | morning dtantsur and cenne o/ | 09:04 |
arne_wiebalck | hi cenne o/ | 09:04 |
cenne | hey arne_wiebalck o/ | 09:05 |
arne_wiebalck | dtantsur: here is the issue I mentioned yesterday, it looks very similar to the one with stacked hearbeats (I initially thought is the same, but the error still appears): https://storyboard.openstack.org/#!/story/2009008 | 09:05 |
rpittau | good morning dtantsur and cenne :) | 09:06 |
cenne | hey rpittau o/ | 09:07 |
cenne | lol. : testtools.matchers._impl.MismatchError: 'bios' is not 'bios' | 09:08 |
cenne | should have seen that coming | 09:08 |
opendevreview | Verification of a change to openstack/ironic failed: Fix ramdisk boot option handling https://review.opendev.org/c/openstack/ironic/+/797517 | 09:10 |
dtantsur | cenne: heh, are you trying to compare string with "is"? | 09:30 |
cenne | Had by mistake. Fixed it to assertEqual | 09:30 |
dtantsur | good :) | 09:31 |
dtantsur | yeah, it's a common mistake, I must admit | 09:31 |
dtantsur | Python 3.9 finally started issuing a syntax warning for it | 09:31 |
janders | see you on Monday Ironic o/ | 09:41 |
arne_wiebalck | bye janders o/ | 09:41 |
rpittau | bye janders :) | 09:42 |
janders | having really bad connectivity interruptions on both main link and the tethered 5G | 09:42 |
janders | I think telcos are trying to tell me I should finish work for this week | 09:42 |
arne_wiebalck | dtantsur: the issue seems indeed very close to the heartbeat issue: whenever two RPCs to continue deployment are sent in quick succession, the ERROR is thrown (I updated the story with some more logging to show this) | 09:42 |
janders | have a great weekend everyone | 09:42 |
rpittau | thanks janders, you too | 09:43 |
iurygregory | bye janders enjoy the weekend | 09:44 |
dtantsur | oh, so no SPUC today? | 09:49 |
rpittau | dtantsur: I can't today :/ | 09:50 |
arne_wiebalck | dtantsur: I cannot today | 09:51 |
dtantsur | okay :) | 09:51 |
arne_wiebalck | dtantsur: hold on a sec before you accept my challenge | 09:51 |
arne_wiebalck | dtantsur: it may be due to the fact that the image does not have the heartbeat patch yet, and that this is indeed the same issue | 09:51 |
dtantsur | it may also be related to https://review.opendev.org/c/openstack/ironic/+/756354 | 09:52 |
arne_wiebalck | thanks! looking at the IPA logs, it seems the heartbeat is close to finishing the async command ... let me confirm there is an issue | 09:54 |
dtantsur | we still have kernel panics in the CI :( https://zuul.opendev.org/t/openstack/build/bd544ddfe344493bb44dbaaa08c52f17/log/controller/logs/ironic-bm-logs/node-1_no_ansi_2021-06-25-08:53:36_log.txt | 10:08 |
arne_wiebalck | "funny" you mention this: I am struggling with kernel panics as well in our deployment as well since some days ... seems to be due half-downloaded initrd file (and does not seem to be related to Ironic from what I see atm) | 10:13 |
opendevreview | cenne proposed openstack/ironic master: [WIP] Add `boot_mode` and `secure_boot` to node object and expose in api https://review.opendev.org/c/openstack/ironic/+/797055 | 10:15 |
dtantsur | arne_wiebalck: have you tried updating ipxe? | 10:18 |
arne_wiebalck | dtantsur: you mean updating the bios f/w | 10:24 |
arne_wiebalck | dtantsur: ? | 10:24 |
dtantsur | arne_wiebalck: do you have iPXE pre-flashed on your hardware? | 10:25 |
arne_wiebalck | dtantsur: I don't think we use iPXE | 10:26 |
arne_wiebalck | dtantsur: we do not re-flash the hardware, if that is your question | 10:26 |
dtantsur | what do you use then, plain PXE with TFTP? | 10:27 |
arne_wiebalck | dtantsur: TFTP, then HTTP, yes | 10:27 |
arne_wiebalck | dtantsur: the image download is via http | 10:27 |
dtantsur | "then HTTP" == either UEFI boot or iPXE, which one? | 10:27 |
arne_wiebalck | UEFI boot | 10:27 |
dtantsur | ahh | 10:28 |
dtantsur | I haven't encountered it yet. maybe upgrading firmware may help then. | 10:28 |
arne_wiebalck | dtantsur: we had cases in the past, where re-installing the bios helped, hardware colleagues are not convinced yet | 10:28 |
arne_wiebalck | dtantsur: but we are running out of ideas, so ... :) | 10:31 |
arne_wiebalck | dtantsur: anyway, not related to your issue | 10:32 |
cenne | dtantsur: when you have time, can you help me out here? : https://review.opendev.org/c/openstack/ironic/+/797055 | 10:37 |
opendevreview | Dmitry Tantsur proposed openstack/ironic master: Refactor deploy_utils.validate_image_properties https://review.opendev.org/c/openstack/ironic/+/797875 | 10:38 |
cenne | I don't understand why two of the units tests (i wrote) are failing. | 10:38 |
dtantsur | lemme see | 10:38 |
cenne | conductor.test_utils.CacheBootModeTestCase.test_failed_boot_mode and test_failed_secure | 10:40 |
dtantsur | yeah, I'm looking at them now | 10:41 |
iurygregory | . | 10:41 |
cenne | thank you | 10:42 |
cenne | hey iurygregory | 10:43 |
iurygregory | hey cenne | 10:44 |
dtantsur | cenne: I've spotted the issue, now checking the rest of the patch | 10:53 |
cenne | Thanks. Looking at it now | 10:54 |
* dtantsur has posted the comments | 10:54 | |
cenne | dtantsur: do you want me to split node_states_boot_mode function too? | 11:02 |
dtantsur | cenne: not necessarily | 11:02 |
cenne | okay | 11:03 |
* cenne embarrassed about missing out s/vendor/boot_mode even after going over that code many times | 11:08 | |
dtantsur | it took me quite a big of figuring out too, don't worry | 11:14 |
*** rpittau is now known as rpittau|bbl | 11:17 | |
opendevreview | Ching Kuo proposed openstack/bifrost master: Fix Redeploy Playbook https://review.opendev.org/c/openstack/bifrost/+/798079 | 11:52 |
opendevreview | cenne proposed openstack/ironic master: [WIP] Add `boot_mode` and `secure_boot` to node object and expose in api https://review.opendev.org/c/openstack/ironic/+/797055 | 12:36 |
opendevreview | kamlesh chauvhan proposed openstack/ironic master: Upgrade oslo.db version https://review.opendev.org/c/openstack/ironic/+/796811 | 12:39 |
TheJulia | dtantsur: I don't think we should block it if they can confirm that the machine is at least getting started (hence the reference to check the system console). It sounds like they are having a couple different issues, and understanding them *is* kind of key in my mind. | 12:53 |
*** rpittau|bbl is now known as rpittau | 12:56 | |
dtantsur | okay (and good morning) | 13:01 |
dtantsur | and I see someone is working on it | 13:01 |
TheJulia | dtantsur: awesome | 13:02 |
dtantsur | TheJulia: ronic.common.exception.IRMCOperationError: iRMC iRMC set_power_state failed. Reason: HTTPConnectionPool(host='192.168.6.250', port=80): Read timed out. (read timeout=60) | 13:02 |
TheJulia | dtantsur: I guess what I'm really afraid of is if we break it. | 13:03 |
TheJulia | wheeeeeeeee | 13:03 |
TheJulia | that sounds already broken quite nicely | 13:03 |
dtantsur | so the last build failed because of something internal to them apparently? I'll mention on the bug | 13:03 |
TheJulia | at least their logs are available again | 13:03 |
TheJulia | I mean, I could even be good at that, as long as they are good with it | 13:03 |
opendevreview | Verification of a change to openstack/ironic failed: Fix ramdisk boot option handling https://review.opendev.org/c/openstack/ironic/+/797517 | 13:05 |
dtantsur | this is insanity, why are my patches always fail in the gate so many times? | 13:06 |
dtantsur | TheJulia: btw "it looks like the CI job uses virtual media", no, it uses irmc-ipxe | 13:07 |
TheJulia | oh, I could have sworn I saw it getting loaded in the log | 13:07 |
TheJulia | well, devstack | 13:08 |
dtantsur | which is honestly upsetting, because covering virtual media is highly desired.. | 13:08 |
TheJulia | it is all a blur | 13:08 |
dtantsur | DEBUG oslo_service.service [-] enabled_boot_interfaces = ['irmc-pxe'] | 13:08 |
* TheJulia sighs | 13:08 | |
dtantsur | not even ipxe, just plain pxe | 13:08 |
TheJulia | le sigh | 13:08 |
TheJulia | maybe comment that? | 13:08 |
dtantsur | I will | 13:08 |
TheJulia | dtantsur: w/r/t failures all of the redundant jobs maybe?! | 13:09 |
TheJulia | or super slim lines of coverage | 13:09 |
TheJulia | gmann: by chance did you post a patch for tempest's client initialization? | 13:09 |
opendevreview | Julia Kreger proposed openstack/ironic-inspector master: Ignored error state cache for new requests https://review.opendev.org/c/openstack/ironic-inspector/+/785245 | 13:13 |
opendevreview | Julia Kreger proposed openstack/ironic master: Only return the requested fields from the DB https://review.opendev.org/c/openstack/ironic/+/792274 | 13:14 |
TheJulia | ajya|afk: ^ typo fixes | 13:16 |
opendevreview | Julia Kreger proposed openstack/ironic master: Add note regarding configuration drives to tuning docs https://review.opendev.org/c/openstack/ironic/+/789623 | 13:17 |
opendevreview | Verification of a change to openstack/ironic failed: Fix ramdisk boot option handling https://review.opendev.org/c/openstack/ironic/+/797517 | 13:28 |
* TheJulia blinks | 13:28 | |
arne_wiebalck | TheJulia: dtantsur: I guess https://review.opendev.org/c/openstack/ironic-python-agent/+/796882 is a backport candidate? | 13:39 |
TheJulia | marked as such :) | 13:40 |
arne_wiebalck | TheJulia: ty | 13:41 |
TheJulia | iurygregory: I'm not totally in the middle of https://review.opendev.org/c/openstack/ironic-specs/+/785742, but I posted a question which I wonder might be another way to approach it. Do we *really* need the field | 13:41 |
iurygregory | TheJulia, you mean event_types? | 13:43 |
iurygregory | it's required to create a subscription https://redfish.dmtf.org/schemas/v1/EventDestination.v1_0_6.json | 13:44 |
iurygregory | does this answer your question? | 13:46 |
TheJulia | but *does* it need to be defined by a user, i guess is where I'm going with the question | 13:48 |
TheJulia | it may be needed to create it, but is it *really* needed | 13:48 |
iurygregory | if we don't let the user specify the event types how we should handle this? | 13:49 |
iurygregory | we will use a default? "Alert" for example? | 13:50 |
iurygregory | for the OCP use case it would be ok since we are interested in Alerts from Redfish... | 13:51 |
iurygregory | but this would make the API very limited, I'm wondering if people will be ok with it | 13:52 |
iurygregory | if we as a community agree this is the way to go I'm completely fine with this path =) | 13:52 |
trandles | kkillsfirst, TheJulia where are we meeting at 10 MDT? Is it just on IRC or are we going to get on bluejeans/webex/zoom? | 14:13 |
opendevreview | Julia Kreger proposed openstack/ironic master: Implements node history: database https://review.opendev.org/c/openstack/ironic/+/768009 | 14:13 |
TheJulia | iurygregory: something is better than nothing :) | 14:16 |
TheJulia | and if the field is going away, then how do we navigate that I guess is the question | 14:17 |
TheJulia | not having it sooner makes things easier to navigate when it was a thing of the long ago past in like 2 years from now | 14:17 |
TheJulia | trandles: kkillsfirst: meetpad.opendev.org/ironic? | 14:17 |
iurygregory | TheJulia, humm so for now we would consider all subscriptions will be "Alert" and in the future when vendors have the support for RegistryPrefix and ResourceType we just add that to the api ? | 14:18 |
TheJulia | arne_wiebalck: i fixed the down revision on the node history patch. I think it is okay to merge in all honesty, although I raised some questions regarding indexes, like if we ever want to offer api query by conductor then we would want it indexed now and not later. | 14:18 |
TheJulia | iurygregory: I *feel* like that is the logical path since we can't see anything but the vague outline of the future | 14:19 |
iurygregory | so inside ironic when doing the request to redfish we would just "hardcode" to send "Alert" in the request? | 14:19 |
TheJulia | for now, yeah I think so | 14:19 |
iurygregory | TheJulia, ack I will update the spec and mention this =) | 14:20 |
iurygregory | and re-work my code a bit XD | 14:20 |
TheJulia | it is just an idea, it seems like that is a way to unblock it and not have us have a deprecation on a field soon() | 14:20 |
iurygregory | yeah makes sense to me =) | 14:20 |
trandles | TheJulia: that works for me! | 14:21 |
TheJulia | If anyone is willing, I'd <3 for reviews on my query performance improvement series of patches | 14:21 |
TheJulia | I also suspect every large operator would love it | 14:22 |
TheJulia | rloo: I've pondered adding chassis_uuid by join, but maybe it shouldn't be a default field populated? | 14:22 |
TheJulia | of course, doing detail view for all nodes *is* going to be slow | 14:23 |
* rloo wonders why we never removed chassis_uuid but anyway... | 14:23 | |
TheJulia | I guess, removal would be breaking | 14:24 |
TheJulia | but it could be by microversion. | 14:24 |
rloo | it seems to me that if someone explicitly wants chassis_uuid, we ought to populate it. so in a general 'detail', we should show it. | 14:24 |
TheJulia | so as is, without a join, it will be the secondary query per node | 14:25 |
TheJulia | yeah, needs to be joined then | 14:25 |
TheJulia | At least it is a 1-1 mapping | 14:26 |
TheJulia | err, 1 to many, but yeah | 14:26 |
TheJulia | 1 row ever for nodes | 14:26 |
rloo | have others complained about chassis-uuid aside from us? give me a few min, i want to see what we do downstream. it has been years ago... | 14:27 |
arne_wiebalck | TheJulia: I guess there is no harm to have it indexed now, is there? | 14:27 |
TheJulia | rloo: I think you guys just removed the lookup since you don't use it | 14:27 |
TheJulia | arne_wiebalck: not really, uuid would be slightly bloated because it woudl all be unique but its a solid hint to query planning/optimizers | 14:27 |
TheJulia | conductor indexes would be super efficient if queried by conductor. since you would like in your environment have <30 mappings to it would basically just have to give you the result set over the wire | 14:28 |
TheJulia | no actual hunting/matching field level values required | 14:28 |
rloo | ok, we added a config option downstream, which of course is always turned on, to set chassis_uuid to None for GET /v1/nodes & /v1/nodes/detail. | 14:29 |
arne_wiebalck | TheJulia: right | 14:29 |
rloo | according to notes (from juno...!) "Performance improvement was 15s for 10k nodes as compared to 50s with chassis_id" | 14:30 |
TheJulia | rloo: wow | 14:30 |
TheJulia | yeah, every single less additional query after the initial result sets helps a lot | 14:31 |
arne_wiebalck | TheJulia: could you update the patch accordingly? I guess you know already exactly what to add. | 14:31 |
TheJulia | arne_wiebalck: sure, and yeah | 14:32 |
rloo | i suspect that was done, before we changed the API to specify fields, so maybe it isn't that important now. | 14:32 |
arne_wiebalck | TheJulia: awesome, thanks a lot! | 14:33 |
* arne_wiebalck goes back to upgrade preparations | 14:33 | |
rloo | is chassis_uuid the only thing where we did a 2nd db lookup? I thought we did others but don't recall (and maybe you already addressed those). | 14:34 |
TheJulia | rloo: so... now we do have a couple other possible ones on if you ask for specific fields or detail view of the node. Like... allocations is another. That too could be a join instead with a little work. Node traits currently does an under the hood left outer join and row by row de-duplication (yes, crazy!), but my performance series changes that and actually starts moving us to using selectinload | 14:35 |
TheJulia | for all list operations, so it is much more targetted and efficent | 14:35 |
rloo | if possible, we should try to use the same 'mechanism' to improve performance. i'd be worried about trying to maintain/grok why retrieval of diff fields from the db are done differently. | 14:37 |
TheJulia | fwiw, with the current state of master branch on the nova query for nodes, I'm at ~18.49 seconds per 10k nodes if I scale the result set size down | 14:38 |
TheJulia | for single value fields, we can actually join. The selectinload are more for relationship joins where we get lists of fields attached to the base node object | 14:39 |
arne_wiebalck | TheJulia: this is the nova query in the nova ironic driver to get the list of all nodes? | 14:39 |
TheJulia | arne_wiebalck: yes.... my tests locally have 113542 nodes (creating nodes takes a long time and I stopped it short of 150k) | 14:40 |
opendevreview | cenne proposed openstack/ironic master: [WIP] Add `boot_mode` and `secure_boot` to node object and expose in api https://review.opendev.org/c/openstack/ironic/+/797055 | 14:40 |
TheJulia | arne_wiebalck: latest test is ~210 seconds to work through everything, granted no transport latency or final spitting out through the webserver, but all of that is fairly low overhead compared to object conversions and data lookups. | 14:41 |
arne_wiebalck | TheJulia: is this the one which is used to compare the number of nodes? | 14:42 |
arne_wiebalck | TheJulia: let me try to find a link for you, checking with Belmiro ... | 14:43 |
JayF | dtantsur: anytime I put a comment with a +1, like on https://review.opendev.org/c/openstack/ironic/+/797512, consider it exceedingly optional. I'll -1 if I feel strongly about something. | 14:43 |
TheJulia | arne_wiebalck: the query nova usees? | 14:43 |
dtantsur | JayF: I get it, but I'm afraid people may take it as a soft objection and not review.. | 14:44 |
arne_wiebalck | TheJulia: background is that we were looking into why the resource tracker time still grows and it seems nova is getting all nodes (but just to compare the numbers and to print a warning) | 14:44 |
arne_wiebalck | TheJulia: nova-conductor per group still gets all nodes | 14:44 |
TheJulia | arne_wiebalck: seriously?!? | 14:44 |
arne_wiebalck | TheJulia: it seems so | 14:45 |
TheJulia | just to get a count | 14:45 |
arne_wiebalck | TheJulia: so, we removed that last week | 14:45 |
arne_wiebalck | TheJulia: this cuts the RT time in half and suppresses a warning | 14:45 |
* arne_wiebalck goes and finds a link to the code ... | 14:45 | |
TheJulia | *so* field level query improvements + just asking for the list of UUIDs would be enough to get a result set that is filtered *all* the way down and as minimal as possible to respond to the api query | 14:46 |
TheJulia | oh... just fix nova completely | 14:46 |
TheJulia | NobodyCam: hey! | 14:46 |
rloo | TheJulia: Seems like the two choices for handling these fields are 1. join; 2. second db call. We're doing 2 now. How much performance gain to do 1 and how much work to do 1? I mean, I'd say do 1 unless it is a lot of work or hard to understand/maintain code. | 14:47 |
rpittau | bye everyone, see you in 10 days! o/ | 14:47 |
*** rpittau is now known as rpittau|afk | 14:47 | |
arne_wiebalck | bye rpittau|afk o/ | 14:48 |
arne_wiebalck | TheJulia: https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L9654 | 14:48 |
TheJulia | rloo: performance on a join would be good, it would be another object in the result set up front, The joins are relatively quick all things considered and saves the resulting 1000 db calls that would occur from the second db call | 14:49 |
TheJulia | 1 joined query is much nicer to the db than 1001 db queries | 14:49 |
TheJulia | and all the round trip time on the back end of that | 14:49 |
rloo | then lets go with joins. | 14:49 |
rloo | wrt what arne_wiebalck mentioned about nova -- we should be able to improve that. seems like someone actually asked to have an API that returned only numbers. but even if we don't go that route, the actual ironic query could filter out more stuff. | 14:52 |
arne_wiebalck | TheJulia: this line takes about 800secs for our ~8000 nodes ... not sure we are talking about the same query | 14:52 |
arne_wiebalck | TheJulia: but we removed this one (mind you, Nova is still on Stein, but the code does look the same on master) | 14:53 |
TheJulia | arne_wiebalck: I'll look in a moment | 14:59 |
TheJulia | arne_wiebalck: yeah, there is a HUGE amount of overhead under the hood in ironic that would cause that to be stupidly slow | 15:01 |
arne_wiebalck | TheJulia: :-D | 15:01 |
TheJulia | because it is asking the db for everything about the node, and then passing that through all the object transitions in ironic, and then going through policy and sanitization | 15:02 |
TheJulia | and then finally getting spit out | 15:02 |
arne_wiebalck | TheJulia: I guess that code was copied over from the VM driver, it makes sense there (and maybe in general), but has to be adapted for Ironic and 1000s of nodes | 15:02 |
TheJulia | yeah | 15:02 |
arne_wiebalck | TheJulia: from what we saw, it only compares the number and prints a warning :) | 15:02 |
TheJulia | rofl | 15:03 |
TheJulia | That seems pointless then | 15:03 |
arne_wiebalck | Yes, we should fix it there. I think Belmiro has not suggested this yet, since we are some versions behind on nova and he did not yet check if that works the same way on master. | 15:04 |
opendevreview | Julia Kreger proposed openstack/ironic master: Implements node history: database https://review.opendev.org/c/openstack/ironic/+/768009 | 15:04 |
arne_wiebalck | I doubt it changed, though. | 15:04 |
opendevreview | cenne proposed openstack/ironic master: [WIP] Add `boot_mode` and `secure_boot` to node object and expose in api https://review.opendev.org/c/openstack/ironic/+/797055 | 15:04 |
TheJulia | yeah, unlikely | 15:04 |
arne_wiebalck | Yeah, seems like this is just a "quick" consistency check ... :-D | 15:05 |
TheJulia | arne_wiebalck: I also renamed the stock index kaifeng included, since indexes are not divided by table they are against, they are namespaced across the entire db | 15:05 |
arne_wiebalck | On master, I do not see the result used anywhere besides LOG.warning() | 15:06 |
arne_wiebalck | TheJulia: node history: oh, I see, makes sense | 15:08 |
TheJulia | wow, that is literally the only place that method is used in nova... | 15:08 |
TheJulia | could we just return None and have that be a signal to skip the check?! | 15:08 |
TheJulia | (i realize, that *sounds* awful, so maybe an attribute on the driver or *something* | 15:09 |
arne_wiebalck | we need to decide if we need this check | 15:09 |
arne_wiebalck | if yes, it should be conductor_group aware I guess | 15:10 |
TheJulia | yeah, that wouldn't be too hard to do although the compare may still fail | 15:10 |
opendevreview | Merged openstack/ironic master: CI: change ilo_deploy_iso to deploy_iso https://review.opendev.org/c/openstack/ironic/+/796886 | 15:11 |
arne_wiebalck | which raises the question how useful it is | 15:11 |
TheJulia | yeah | 15:14 |
TheJulia | so it does do one thing | 15:18 |
TheJulia | that actually helps performance | 15:18 |
dtantsur | going a bit earlier today, have a great weekend | 15:19 |
arne_wiebalck | bye dtantsur o/ | 15:19 |
opendevreview | Merged openstack/ironic-python-agent master: Coalesce heartbeats https://review.opendev.org/c/openstack/ironic-python-agent/+/796882 | 15:21 |
TheJulia | actually, it doesn't | 15:23 |
TheJulia | it doesn't touch the cache at all, doesn't update it, it just queries needlessly | 15:23 |
arne_wiebalck | nice | 15:23 |
TheJulia | we could literally use the cache for that query | 15:23 |
TheJulia | I get why nova does it, they want to flag discrepencies in their power sync loop, but we're already keeping a cache | 15:25 |
gmann | TheJulia: not yet, I am going to do that after my breakfast | 15:25 |
TheJulia | gmann: ack | 15:25 |
TheJulia | gmann: thanks | 15:25 |
opendevreview | Merged openstack/ironic-python-agent master: Only mount the ESP if not yet mounted https://review.opendev.org/c/openstack/ironic-python-agent/+/796045 | 15:25 |
TheJulia | arne_wiebalck: so the get available nodes tracking *does* refresh the cache it looks like *and* is partition key aware | 15:29 |
TheJulia | so using the local in-driver cache seems to be the way to go | 15:29 |
TheJulia | and would remove the transactions over the wire unless the cache is just empty | 15:30 |
arne_wiebalck | TheJulia: oh, that is good | 15:43 |
arne_wiebalck | TheJulia: ofc, the comparison will still fail, there will be only a warning with no consequences, and all nova-compute process will ask for all nodes | 15:46 |
TheJulia | arne_wiebalck: so I'm not sure it would, because I think the cache is group aware and the count is by node anyway | 15:48 |
TheJulia | err, nova-compute instance | 15:48 |
TheJulia | so I think the warning will go away as well | 15:48 |
arne_wiebalck | the cache on the ironic side, you mean? but is nova-compute asking for a group as well? (maybe I misunderstand) | 15:50 |
TheJulia | arne_wiebalck: so, nova-compute processes keep a local cache of the ironic nodes. | 15:54 |
TheJulia | some of the hits go to the cache instead of back to ironic. Specific power sync status checks *do* go back to ironic in the end, but the query is just kind of redundant. | 15:54 |
opendevreview | Arne Wiebalck proposed openstack/ironic-python-agent stable/wallaby: Only mount the ESP if not yet mounted https://review.opendev.org/c/openstack/ironic-python-agent/+/798124 | 15:58 |
arne_wiebalck | TheJulia: oh, I thought power sync would go back to Ironic and the db | 16:00 |
TheJulia | arne_wiebalck: it *does* eventually | 16:00 |
arne_wiebalck | ok | 16:02 |
opendevreview | Arne Wiebalck proposed openstack/ironic-python-agent stable/wallaby: Coalesce heartbeats https://review.opendev.org/c/openstack/ironic-python-agent/+/798129 | 16:37 |
arne_wiebalck | Have a good week-end everyone, see you next week o/ | 16:43 |
TheJulia | I guess no spuc today? | 17:05 |
JayF | I can join if folks are there | 17:09 |
JayF | not fully braining well this morning lol | 17:09 |
gmann | TheJulia: try with this https://review.opendev.org/c/openstack/tempest/+/798130 | 17:11 |
gmann | TheJulia: how it create network for project scope only as there we have project_id which is needed for network creation | 17:12 |
TheJulia | JayF: https://pbs.twimg.com/profile_images/1234391573/brainslug_Fry_400x400.jpg | 17:12 |
TheJulia | JayF: nobody is there, and I feel like I have a brain slug | 17:12 |
TheJulia | gmann: ack | 17:12 |
JayF | Yeah. Kinda the same. Exhausted for no reason, can't think well today | 17:13 |
TheJulia | I'm going to go have a nice lunch with the wife, it has been a really long week | 17:14 |
opendevreview | Julia Kreger proposed openstack/ironic-tempest-plugin master: Use get_service_clients framework with basic Secure RBAC https://review.opendev.org/c/openstack/ironic-tempest-plugin/+/797521 | 17:15 |
gmann | TheJulia: this is basically create network resources only for the project scope. and this is something we need to make a general solution on "how all the openstack services project mapped resources can be operated by system scope token" | 17:15 |
TheJulia | gmann: ack, which makes me think our tempest tests may fail/break on it, but better now than later | 17:15 |
gmann | especially cross service dependent resources | 17:15 |
TheJulia | ++ | 17:15 |
TheJulia | I'll check the results of the job after lunch | 17:15 |
gmann | k | 17:16 |
TheJulia | I also suspect I'm working some this weekend :( | 17:17 |
cenne | It's Jupiter in retrograde, first friday. Thats why. : p Minds are clouded. Follow your hearts. | 17:18 |
cenne | jk. | 17:18 |
TheJulia | cenne: perfect answer :) | 17:21 |
cenne | Happy weekend. Sorry for silliness. Spilled over in lieu of spuc. | 17:23 |
TheJulia | :) | 17:23 |
TheJulia | silliness++ | 17:23 |
cenne | I'll | 17:23 |
TheJulia | We encourage silliness in this project! | 17:23 |
* TheJulia says with a serious face trying to hold back laughing | 17:23 | |
cenne | ^u^ | 17:25 |
cenne | I'll take leave now. Have a goof day. | 17:25 |
JayF | o/ | 17:26 |
TheJulia | have a wonderful weekend! | 17:26 |
cenne | umm.. *good day ofc. :p | 17:26 |
cenne | o/ | 17:26 |
opendevreview | Julia Kreger proposed openstack/ironic-tempest-plugin master: Add Wallaby jobs https://review.opendev.org/c/openstack/ironic-tempest-plugin/+/798143 | 17:47 |
* TheJulia goes and finds lunch | 17:56 | |
TheJulia | gmann: thanks for revising the change, hopefully the job will now be good | 19:37 |
gmann | TheJulia: yeah hope so, I am monitoring that in case. | 20:15 |
JayF | TheJulia: https://review.opendev.org/c/openstack/ironic/+/797984 WDYT about my suggestion there? If you're meh on it, I'll land the patch as-is | 20:31 |
TheJulia | JayF: If I edit it real quick and update it, I think you could +2+A it then | 21:27 |
opendevreview | Julia Kreger proposed openstack/ironic master: Deprecate [pxe]ip_version parameter https://review.opendev.org/c/openstack/ironic/+/797984 | 21:29 |
TheJulia | gmann: that tempest error is bizzar | 21:34 |
* TheJulia goes and makes coffee | 21:36 | |
gmann | I did the recheck for victoria jobs, should be green | 21:39 |
TheJulia | well, one of the jobs failed with a key error which made no sense | 21:44 |
TheJulia | I need ot go check the code of ironic's tempest plugin, it might need another change | 21:44 |
TheJulia | coffee() first | 21:44 |
opendevreview | Merged openstack/ironic master: Change UEFI ipxe bootloader default https://review.opendev.org/c/openstack/ironic/+/797973 | 21:50 |
TheJulia | hmm, looks unrelated | 21:53 |
TheJulia | so something is broken with regards to the memory checking, which seems totally unrelated. it is failing to apply the rule in inspector because the criteria is not being met | 22:04 |
TheJulia | and technically are ending up booting with just shy of the memory required for the test to pass... | 22:07 |
TheJulia | gmann: I see what it is | 22:14 |
TheJulia | https://review.opendev.org/c/openstack/ironic-python-agent/+/788921 needs to be cherry picked to victoria | 22:14 |
gmann | k, sorry I wasted the recheck there. but should be ok as its friday and less load in gate :) | 22:15 |
TheJulia | no worries, yeah... it is bizzar but apparently due to a bad patch on lshw in centos | 22:19 |
opendevreview | Julia Kreger proposed openstack/ironic-python-agent stable/victoria: Fix getting memory size in some lshw output https://review.opendev.org/c/openstack/ironic-python-agent/+/798168 | 22:22 |
TheJulia | And... I need to actually backport that one further | 22:22 |
opendevreview | Julia Kreger proposed openstack/ironic-python-agent stable/ussuri: Add function to calculate memory https://review.opendev.org/c/openstack/ironic-python-agent/+/798170 | 22:27 |
opendevreview | Julia Kreger proposed openstack/ironic-python-agent stable/ussuri: Fix getting memory size in some lshw output https://review.opendev.org/c/openstack/ironic-python-agent/+/798171 | 22:27 |
opendevreview | Julia Kreger proposed openstack/ironic-python-agent stable/train: Add function to calculate memory https://review.opendev.org/c/openstack/ironic-python-agent/+/798172 | 22:30 |
opendevreview | Julia Kreger proposed openstack/ironic-python-agent stable/train: Fix getting memory size in some lshw output https://review.opendev.org/c/openstack/ironic-python-agent/+/798173 | 22:30 |
* TheJulia files a downstream bug | 22:44 | |
TheJulia | gmann: I just rechecked a job to hopefully tie devstack+tempest+ironic-python-agent changes together to see if we can actually run our tempest tests with scope enforcement set. Wish it luck :) | 22:57 |
TheJulia | err, ironic-tempest-plugin changes | 22:57 |
opendevreview | Julia Kreger proposed openstack/ironic master: Set stage for objects to handle selected field lists. https://review.opendev.org/c/openstack/ironic/+/792275 | 23:04 |
opendevreview | Julia Kreger proposed openstack/ironic master: API to pass fields to node object list https://review.opendev.org/c/openstack/ironic/+/792296 | 23:04 |
opendevreview | Julia Kreger proposed openstack/ironic master: Allow node_sanitize function to be provided overrides https://review.opendev.org/c/openstack/ironic/+/794880 | 23:04 |
opendevreview | Julia Kreger proposed openstack/ironic master: Use selectinload for all list queries https://review.opendev.org/c/openstack/ironic/+/797337 | 23:05 |
gmann | TheJulia: +1 | 23:06 |
* TheJulia gets out the jeopardy music | 23:12 |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!