opendevreview | Julia Kreger proposed openstack/ironic master: DNM: Break tempest on purpose to figure out what tests might need to be addressed https://review.opendev.org/c/openstack/ironic/+/916128 | 00:14 |
---|---|---|
iurygregory | wow | 00:16 |
opendevreview | Julia Kreger proposed openstack/ironic-tempest-plugin master: Fix vif tests https://review.opendev.org/c/openstack/ironic-tempest-plugin/+/916299 | 00:19 |
TheJulia | iurygregory: huh? | 00:20 |
iurygregory | TheJulia, I liked the the title in your change :D I was like wow | 00:20 |
TheJulia | That DNM is likely going to be a permenant enablement of tempest and rabbit on that job | 00:20 |
TheJulia | just need to see since we set some slightly different config to match flat networking as well | 00:21 |
iurygregory | oh nice \o/ | 00:21 |
TheJulia | also https://review.opendev.org/c/openstack/ironic-specs/+/916126 | 00:21 |
* TheJulia whistles innocently | 00:22 | |
iurygregory | I saw that =) | 00:22 |
iurygregory | I gave a heads-up to lucas (since he might be interested) | 00:22 |
stevebaker[m] | TheJulia: I'm still getting the cinder detach error, you mentioned it requires a service token, which I think is the case. So I'm not sure what to try next https://zuul.opendev.org/t/openstack/build/2902d34980b841859ac8229bcb2e99ac/log/controller/logs/screen-ir-cond.txt#1645 | 02:54 |
stevebaker[m] | https://review.opendev.org/c/openstack/ironic/+/900265 | 02:54 |
TheJulia | stevebaker[m]: ill look in the morning | 02:55 |
stevebaker[m] | thanks, I'll revisit on Monday :) | 02:55 |
TheJulia | Iām unfortunately in line for post dinner desert at the moment | 02:55 |
TheJulia | Have a great weekend! | 02:55 |
stevebaker[m] | fortunately, surely | 02:55 |
opendevreview | OpenStack Proposal Bot proposed openstack/ironic-ui master: Imported Translations from Zanata https://review.opendev.org/c/openstack/ironic-ui/+/916360 | 03:24 |
opendevreview | cid proposed openstack/ironic master: wip: Remove special treatment of .json for API objects https://review.opendev.org/c/openstack/ironic/+/913467 | 03:34 |
opendevreview | Julia Kreger proposed openstack/ironic master: Run neutron for the functional test job https://review.opendev.org/c/openstack/ironic/+/916128 | 04:20 |
TheJulia | stevebaker[m]: took a look, I think I see the confusion there, the huge block of logic actually sort of holds the secret, but I linked to the more openstacksdk-ey example on how to do it. Because of the user request coming in, you want to start with sourcing the user authentication, and as you likely noticed if we didn't have it, then we just used our own credentials and did the endpoint massaging. There was reason for my | 04:34 |
TheJulia | madness | 04:34 |
stevebaker[m] | There is a version of that change which did the exact same as the neutron get_client, same result | 04:36 |
stevebaker[m] | TheJulia: It was Patchset 9 which just copied what you did for neutron get_client, which had the same result https://review.opendev.org/c/openstack/ironic/+/900265/9/ironic/common/cinder.py | 04:44 |
TheJulia | but neutron doesn't have the service auth requirement | 04:46 |
TheJulia | and I think we basically unwound the neutron stuff, but I'd had to take a look when I'm actually awake and not on the verge of heading to bed | 04:47 |
stevebaker[m] | yeah I'll absorb those connection.py docs and try again. Maybe not today though, I'm outta spoons | 04:49 |
TheJulia | understand, it is a bit weird of a thing to wrap your head around since it is a whole *other* dual token bearer thing | 04:49 |
opendevreview | Steve Baker proposed openstack/ironic master: Replace cinderclient usage with openstacksdk https://review.opendev.org/c/openstack/ironic/+/900265 | 05:13 |
rpittau | good morning ironic! happy friday o/ | 07:21 |
opendevreview | Merged openstack/ironic stable/2024.1: Add states.SERVICING and SERVICEWAIT to _FASTTRACK_HEARTBEAT_ALLOWED https://review.opendev.org/c/openstack/ironic/+/916085 | 08:42 |
opendevreview | Merged openstack/ironic stable/2023.2: Add states.SERVICING and SERVICEWAIT to _FASTTRACK_HEARTBEAT_ALLOWED https://review.opendev.org/c/openstack/ironic/+/916086 | 08:55 |
iurygregory | good morning Ironic | 11:45 |
opendevreview | Riccardo Pittau proposed openstack/ironic stable/2024.1: Fix redfish detach generic vmedia device method https://review.opendev.org/c/openstack/ironic/+/915341 | 12:57 |
rpittau | iurygregory, TheJulia, when you have a moment can you please check ^ ? thanks! | 12:58 |
iurygregory | rpittau, sure | 12:58 |
iurygregory | tbh I would love to see a unit test with the case of multiple devices (do we already have one?) | 13:00 |
* iurygregory checks | 13:00 | |
rpittau | iurygregory: we don't, I'm working on a patch to add them | 13:04 |
rpittau | well, specifically for the redfish attach/detach | 13:04 |
iurygregory | rpittau, oh ok ^^ | 13:04 |
iurygregory | ack | 13:04 |
rpittau | thanks for the review | 13:05 |
iurygregory | yw | 13:10 |
opendevreview | Julia Kreger proposed openstack/ironic-tempest-plugin master: Fix vif tests https://review.opendev.org/c/openstack/ironic-tempest-plugin/+/916299 | 13:40 |
TheJulia | hey folks, I'd appreciate reviews on ^^^ | 13:41 |
TheJulia | I've tagged it as ironic-week-prio. It is required to land a patch on ironic to enable neutron on the functional test jobs | 13:42 |
dtantsur | TheJulia: I see an issue in the fact that we use microversion 1.31 in a test that is literally testing microversion 1.2 | 13:55 |
TheJulia | then our only other option is to delete the test | 13:56 |
TheJulia | because otherwise the test is purely reliant upon those interfaces being default to not fail | 13:56 |
dtantsur | TheJulia: restore the microversion after changing the interfaces? | 13:56 |
dtantsur | it's still hacky, but something I can live with | 13:56 |
TheJulia | which is a crap behavior, but it is what it is | 13:57 |
TheJulia | dtantsur: I think I did that | 13:57 |
TheJulia | oh, wait | 13:57 |
dtantsur | TheJulia: not for inherited tests | 13:57 |
TheJulia | yeah, there are two other classes | 13:57 |
TheJulia | yeah | 13:57 |
TheJulia | give me a minute and I'll do that | 13:57 |
TheJulia | I completely forgot about the inhereted tests | 13:57 |
* TheJulia makes more money | 13:57 | |
TheJulia | err | 13:57 |
dtantsur | TheJulia: let's just set it again right in setUp | 13:57 |
TheJulia | coffee | 13:57 |
TheJulia | yeah | 13:57 |
dtantsur | both money and coffee are not too bad | 13:58 |
TheJulia | I did that originally but through refacctors hit the other issue | 13:58 |
dtantsur | I assume, update_node does not accept a different microversion? | 13:58 |
opendevreview | Merged openstack/ironic master: Fix device_type in attach/detach vmedia for Redfish https://review.opendev.org/c/openstack/ironic/+/916306 | 13:59 |
TheJulia | dtantsur: afaik, it does not, I sifted through the code yesterday and it didn't look like it off hand with the pattern taken | 14:03 |
opendevreview | Julia Kreger proposed openstack/ironic-tempest-plugin master: Fix vif tests https://review.opendev.org/c/openstack/ironic-tempest-plugin/+/916299 | 14:05 |
TheJulia | That should do it. I did it originally, but i later realized because what we're really testing is start node state behavior based on the enrolled version at the end of the day | 14:06 |
TheJulia | not re-adding the depends on patch to turn neutron on | 14:06 |
TheJulia | since... those tests are disjointed from neutron | 14:07 |
dtantsur | Okay, leaving the review open, will need to check some other things quickly | 14:08 |
TheJulia | sure, sure | 14:08 |
opendevreview | Riccardo Pittau proposed openstack/ironic-specs master: [WIP] Add work items for 2024.2 Dalmatian development cycle https://review.opendev.org/c/openstack/ironic-specs/+/916295 | 14:47 |
opendevreview | Riccardo Pittau proposed openstack/ironic-specs master: [WIP] Add work items for 2024.2 Dalmatian development cycle https://review.opendev.org/c/openstack/ironic-specs/+/916295 | 15:02 |
TheJulia | dtantsur: I'll need to rev that patch one more time to remove the dependency on it, but I'll let it finish up first before doing so | 15:10 |
opendevreview | Riccardo Pittau proposed openstack/ironic-specs master: Add work items for 2024.2 Dalmatian development cycle https://review.opendev.org/c/openstack/ironic-specs/+/916295 | 15:20 |
dtantsur | Tricky question, TheJulia, janders. If a node in active state and power *off* goes through servicing, must it return to power off without ever trying to boot into the instance? | 15:32 |
dtantsur | Same if power_off special step is used as the last step. | 15:33 |
TheJulia | oh jeeze | 15:33 |
TheJulia | I had always assumed you would only service things which are powered on | 15:34 |
dtantsur | yeah, I'm sitting with a confused face in front of the BMO code | 15:34 |
TheJulia | active and off is... hmm | 15:34 |
dtantsur | Can there be reasons to not power on a node until it undergoes servicing? | 15:34 |
TheJulia | I guess the key is to have ironic record the state, and return it to that state and not assume "active == on" | 15:34 |
dtantsur | yeaaaaaah | 15:35 |
TheJulia | I don't think there are reasons to service before powering on | 15:35 |
TheJulia | since service is all about being in the active state | 15:35 |
dtantsur | I'll make a mental note about it, but it's not my largest priority now to fix it. | 15:35 |
TheJulia | ok | 15:37 |
* dtantsur needs to demo a proof of concept of servicing with metal3 by end of month | 15:39 | |
opendevreview | Merged openstack/ironic stable/2024.1: Fix redfish detach generic vmedia device method https://review.opendev.org/c/openstack/ironic/+/915341 | 15:52 |
opendevreview | Merged openstack/ironic master: Follow-up: Use ``microversion-parse`` to parse version headers in API requests https://review.opendev.org/c/openstack/ironic/+/916149 | 15:52 |
rpittau | bye everyone, have a great weekend! o/ | 16:22 |
TheJulia | have a great weekend | 16:23 |
opendevreview | Julia Kreger proposed openstack/ironic-tempest-plugin master: Fix vif tests https://review.opendev.org/c/openstack/ironic-tempest-plugin/+/916299 | 16:32 |
opendevreview | cid proposed openstack/ironic master: wip: Remove special treatment of .json for API objects https://review.opendev.org/c/openstack/ironic/+/913467 | 16:45 |
opendevreview | cid proposed openstack/ironic-tempest-plugin master: Patch to enforce json extension works in existing API behaviour https://review.opendev.org/c/openstack/ironic-tempest-plugin/+/913926 | 16:48 |
JayF | I'm trying really hard to think of a valid use case for service+power off | 17:13 |
JayF | most of what I can come up with are bad ideas; e.g. baremetal node service --steps {'name': 'omghax_recovery'} | 17:14 |
TheJulia | storing and restoring a state is not really unheard of in our code base, we've got a state or two which does it, but then what if you explicitly want to power on or off as a step | 17:16 |
TheJulia | maybe that is best done afterward, dunno | 17:16 |
TheJulia | ... it is just... weird since we don't really think of nodes as deployed && off | 17:17 |
JayF | It's 100% supported, and even exposed via the nova server start/stop api | 17:17 |
JayF | so we at least have to have sane, documented behavior | 17:17 |
JayF | which imo means we either need to stick to: "if you desire a different power state than you started in, use a power_{on,off} step explicitly at the end" or "we always return you to the same power state you started with" | 17:18 |
JayF | I think #2 is the sanest way to go | 17:18 |
JayF | taking active+power off -> servicing -> active+power on ... this would be unexpected behavior imo | 17:18 |
clarkb | would it be valid for having a reserved node in that is fully deployed and ready to be started should power costs go down or demand go up? | 17:19 |
clarkb | then you don't have to worry about there not being any available nodes when you're in a position of "I need this now" | 17:19 |
TheJulia | clarkb: sort of an operator's use case and call | 17:20 |
TheJulia | I mean, you could , but you'd like want patches to be applied and whatnot | 17:21 |
TheJulia | so maybe it also is not the best idea... dunno | 17:21 |
JayF | The question is not if it's a good idea | 17:21 |
TheJulia | true | 17:21 |
JayF | the question is if there's a reasonable use case our API can be used for that we have to honor | 17:21 |
JayF | and I think we're clearly in "yes" territory | 17:21 |
JayF | (w/r/t ACTIVE+power_off) | 17:22 |
TheJulia | It is a case we didn't anticipate because most folks don't operate with something in that state | 17:22 |
TheJulia | since deploying is relatively quick | 17:22 |
JayF | I'll note that nova stop + nova lock is how we handled abuse at rackspace | 17:22 |
JayF | so powered off + active was something we used there for some case at all | 17:22 |
TheJulia | yeah, and I guess starting from off state *could* make sense if you had steps which... snapshooted it | 17:27 |
JayF | that's exactly what I was thinking re: omghax_recovery | 17:27 |
JayF | as a fakey step | 17:27 |
JayF | or lets say, major vuln in $firmware | 17:28 |
JayF | you can update via redfish but is only exploitable from OS | 17:28 |
JayF | power whole fleet off -> service to update them -> power on as they update | 17:28 |
JayF | (for the 'I want servicing to change power state' use case) ... but in that case i'd be OK with just saying "make an extra power status call" | 17:28 |
JayF | s/status/control/ | 17:29 |
TheJulia | power_on and power_off *are* available steps, we just don't have any sort of guard to prevent an operator from using it as the very last step and then powering the machine back on, but we anticipate you'd always want to return to the prior state of on | 17:31 |
TheJulia | recording and restoring is what we have around some of the power code for other cases, makes sense to do it around servicing to me | 17:32 |
JayF | agreed, would probably be valuable to indicate in docs that though -- so people don't think "ending service steps with power off/on" has the expected effect | 17:40 |
TheJulia | yeah | 17:46 |
JayF | TheJulia: you have a few minutes, perhaps? | 18:43 |
JayF | TheJulia: warning: IPA+WSGI discussion | 18:43 |
JayF | (and it's not great) | 18:43 |
TheJulia | a couple of moments please | 18:43 |
JayF | TheJulia: when you're ready: https://us06web.zoom.us/j/81363153778?pwd=iQBvg6gAJPE5sXghTY2bmfpbBOWibA.1 | 18:44 |
TheJulia | ok | 18:44 |
JayF | ty Julia for knocking us over the hump there | 19:21 |
* JayF holds a funeral for a stateless IPA | 19:21 | |
TheJulia | Awwwww | 19:37 |
TheJulia | crazy idea | 19:38 |
TheJulia | use a socket | 19:38 |
TheJulia | between the runner and wsgi app | 19:38 |
JayF | heh | 19:39 |
JayF | pick your poison | 19:39 |
JayF | json-rpc or sqlite | 19:39 |
JayF | adamcarthur5: ^ | 19:39 |
TheJulia | "hey, here is a thing", and "hey, here is your thing back" | 19:40 |
JayF | JSON-RPC is at least ... the right tool for the job | 19:40 |
TheJulia | kind of yeah | 19:40 |
JayF | any issues we'd have there should mimic issues we'd have IRL | 19:40 |
TheJulia | allows that call to get funneled into the actual source, if there is a clean way to serve it up | 19:40 |
JayF | today, IPA gets a DB or RPC | 19:41 |
JayF | tomorrow, IPA is just an ironic-api we spin up on a node /s | 19:41 |
* TheJulia twitches and falls over | 19:41 | |
JayF | the JSON-RPC route is nice because we could keep the wsgi/api application side stateless, too | 19:42 |
TheJulia | all it would need to start is the results from the lookup | 19:42 |
JayF | honestly this is going to be more work than I anticipated | 19:42 |
TheJulia | so it could filter/guard/validate requests at a high level and have that so it could heartbeat | 19:42 |
JayF | however it's going to end up being cleaner | 19:43 |
JayF | heh | 19:43 |
JayF | JSON-RPC heartbeat AND heartbeats to ironic-api | 19:43 |
* JayF adds two more decoder rings to his order form | 19:43 | |
adamcarthur5 | I really don't like the socket-to-socket idea? That seems like an anti-pattern. | 19:43 |
* TheJulia looks at the ephemeral order form and feels confused | 19:43 | |
adamcarthur5 | But that is almost certainly me not thinking in reference to Openstack enough | 19:44 |
JayF | adamcarthur5: well, we'd likely take the model from ironic-api <> ironic-conductor comms | 19:44 |
JayF | which can use amqp OR json-rpc | 19:44 |
JayF | obviously amqp is a terrible idea | 19:44 |
TheJulia | please no | 19:44 |
JayF | so we'd end up with (1-N) API app instances doing JSON-RPC to a single agent backend that did all the work | 19:44 |
JayF | basically the app at that point would just be doing similar to what ironic-api does for 90% of requests: validate the args, then pass them on to the backend | 19:45 |
JayF | s/backend/conductor/ | 19:45 |
TheJulia | over a socket would at least disjoint it so we're not worrying about wsgi and the such | 19:46 |
JayF | adamcarthur5: let me ask the question this way: What is a more appropriate form of IPC for this use case | 19:46 |
JayF | and socket can be unix socket or IP socket | 19:46 |
TheJulia | for the record, my please no was in regards to ampq | 19:47 |
JayF | rabbitmq is the most stable, reliable part of openstack and has never been an operational pain point ever! what are you talking about! | 19:48 |
JayF | ;) | 19:48 |
TheJulia | https://www.pinterest.com/pin/side-eye--332140541273152294/ | 19:49 |
* TheJulia go obtains corgi cuddles for a few minutes | 19:51 | |
TheJulia | My corgi gave me the same look | 19:52 |
adamcarthur5 | Probably sharing info through a socket ;((( | 19:54 |
JayF | I suspect you'd be surprised how often this happens :) | 19:54 |
adamcarthur5 | You are right. But I will write it up, we can make a final decision later | 19:54 |
JayF | I already added the alternateive to the etherpad | 19:54 |
adamcarthur5 | I think the bit that is more surprising is I'm having to deal with it. š | 19:55 |
adamcarthur5 | (can I send unicode characters in this without it breaking?) | 19:55 |
JayF | yes | 19:55 |
JayF | Another perspective would be: wow, I've levelled up to the problems that are shaped weird and require low level solutions | 19:55 |
adamcarthur5 | Yes... Always think positive right :)) | 19:56 |
JayF | Implementing something that's awesome and a little gross is kinda a rite of passage when dealing with hardware | 19:56 |
JayF | and tbh I don't even think a local unix socket with some RPC is really that gross | 19:56 |
JayF | better than COM on windows :D | 19:56 |
TheJulia | Dmitry noted they switched ironic to use a socket for inbound requests and that it worked perfectly | 19:59 |
TheJulia | with apache in front | 19:59 |
JayF | ...is this chicken<>egg? Would the JSON-RPC backend service just take a command and stop listening while it's going? | 20:02 |
JayF | that can't be true | 20:02 |
JayF | we still need asyncio or some kinda async comms, yeah? | 20:02 |
TheJulia | likely, but it would disjoint it form wsgi at least | 20:40 |
opendevreview | Julia Kreger proposed openstack/ironic master: docs: update redfish docs to detail swift url issues https://review.opendev.org/c/openstack/ironic/+/915931 | 21:42 |
opendevreview | Julia Kreger proposed openstack/ironic master: redfish: change default virtual media storage to local storage https://review.opendev.org/c/openstack/ironic/+/915932 | 21:43 |
opendevreview | Julia Kreger proposed openstack/ironic master: docs: Cleanup/revise Secure Boot docs https://review.opendev.org/c/openstack/ironic/+/915386 | 21:50 |
opendevreview | Julia Kreger proposed openstack/ironic-tempest-plugin master: CI: Increment stable jobs for 2024.1/drop Zed https://review.opendev.org/c/openstack/ironic-tempest-plugin/+/914931 | 21:54 |
janders | dtantsur TheJulia JayF in my ops life I've seen disasters caused by unintended power-on of a mission critical node that's been powered off due to issue (think old filesystem metadata server, a member of an HA group of sorts, cold-spare, whatever). Yeah there has to be pre-existing mess for that to happen and have bad impact but that doesn't mean it | 21:55 |
janders | can't happen. TL;DR: I reckon it would be good to make sure we don't boot the actual instance as a part of servicing and certainly power state pre and post servicing should match. If sorting out "ensuring not booting of the instance" is on the hard side we can handle that through documentation for starters ("DO NOT service a node that for some | 21:55 |
janders | reason must not be booted into the image deployed to it"). That's my perspective - sharing it in case it's useful. | 21:55 |
JayF | yep, that maps to my understanding ; although that being said | 21:55 |
JayF | if you're in that case, and you run servicing on your server | 21:56 |
JayF | you are a missed dhcp request away from breaking your env (so don't do that!) lol | 21:56 |
JayF | (at least in noop network interface) | 21:56 |
janders | yeah being in that scenario means someone is already in a situation sufficient to cause a disaster | 21:56 |
janders | it would just be good if Ironic doesn't happen to be the trigger that tips it over from a potential disaster to an actual one | 21:57 |
opendevreview | Julia Kreger proposed openstack/ironic master: docs: update ilo docs regarding status -> use redfish https://review.opendev.org/c/openstack/ironic/+/915387 | 21:58 |
opendevreview | Julia Kreger proposed openstack/ironic master: docs: document stance on partition image use https://review.opendev.org/c/openstack/ironic/+/915388 | 22:02 |
janders | OK Saturday here so time to wrap things up for the weekend. Have a great weekend everyone, see you Monday! | 22:04 |
JayF | o/ | 22:10 |
opendevreview | Merged openstack/ironic master: Docs: Remove outdated RBAC content https://review.opendev.org/c/openstack/ironic/+/915385 | 22:10 |
opendevreview | Julia Kreger proposed openstack/python-ironicclient master: Add Service Steps to client https://review.opendev.org/c/openstack/python-ironicclient/+/891140 | 22:41 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!