opendevreview | cid proposed openstack/ironic master: Remove unsupported rpc methods https://review.opendev.org/c/openstack/ironic/+/909263 | 02:09 |
---|---|---|
opendevreview | Merged openstack/ironic master: Ensure all errors are passed during cleaning https://review.opendev.org/c/openstack/ironic/+/908703 | 07:04 |
rpittau | good morning ironic! o/ | 08:35 |
rpittau | frickler: change looks good for jsonschema bump compatibility on our side | 08:40 |
frickler | rpittau: thanks, sadly not so good/no responses from other projects so far, so likely the bump will have to be deferred to the next cycle | 10:53 |
rpittau | that's perfect for us :) | 10:54 |
opendevreview | Alex Welsh proposed openstack/bifrost master: Improve downloaded deployment image support https://review.opendev.org/c/openstack/bifrost/+/884888 | 11:49 |
opendevreview | Kamil Gustab proposed openstack/sushy master: Adds Ethernet Interfaces for manager. https://review.opendev.org/c/openstack/sushy/+/909451 | 12:26 |
dking | TheJulia: So, after I started looking into it, I realize that I was just being silly. There is already a `by-path` option for root device hints. So, the work is already done. | 13:46 |
dking | It turns out that the problem is that it just isn't implemented yet in Metal3 Bare Metal Operator, as far as I can tell. | 13:47 |
dtantsur | dking: I believe it is, just in a fancy way | 13:48 |
dtantsur | dking: https://github.com/metal3-io/baremetal-operator/blob/main/pkg/provisioner/ironic/devicehints/devicehints.go#L19-L24 | 13:49 |
SvenKieske | hey there, I don't want to push anyone, but it would be nice to get reviews for this bugfix: https://review.opendev.org/c/openstack/networking-baremetal/+/903995 as there are users out there which are already impacted. thank you! :) | 13:57 |
dking | dtantsur: Thank you. I'm seeing that as I'm trying to sift through the code. Now I'm wondering why it didn't work for us. I'm going to try to replicate, and then I'll need to check version number. For some reason, it was searching with a `name` hint type. | 14:00 |
TheJulia | good morning | 14:11 |
SvenKieske | o/ (already 15:00 here :D ) | 14:11 |
dtantsur | Why do Europeans start their day in the afternoon? :D | 14:14 |
TheJulia | Magic?! | 14:22 |
TheJulia | Excellent Coffee?! | 14:23 |
TheJulia | The best cafes?!? | 14:23 |
SvenKieske | I for one try at least starting before 11:00 (succeeding most of the time, due to meetings) :D | 14:36 |
TheJulia | 0600 most days | 14:49 |
dtantsur | Since my wife is doing on-site job nowadays, 0700 is not uncommon for me any more | 15:03 |
JayF | I work 0700 to 1600 local time most days for similar reason - - my wife is a teacher and I start work when she leaves and then mostly done when she gets home | 15:42 |
clarkb | I'm a fan of having sunlight after work | 16:09 |
JayF | You know as well as I do that being off at 4pm isn't always enough to get sunlight where we live :D | 16:13 |
JayF | I guess to that effect, there are days in the depth of winter where "grey" is basically the color all day regardless of time of day lol | 16:13 |
clarkb | ya it doesn't always work out | 16:14 |
clarkb | I would take year round DST for this reason though. My kids already walk to school in the dark for non zero days of the year | 16:14 |
JayF | How shortsighted! You aren't considering the needs of 19th century farmers, clarkb. | 16:16 |
JayF | ;) | 16:16 |
rpittau | good night! o/ | 16:33 |
SvenKieske | I honestly don't understand ironics policies with regards to requirements bumps, but I guess we should discuss this in the review.. | 17:01 |
TheJulia | we have to be careful on stable branches specifically | 17:08 |
dtantsur | SvenKieske: not really Ironic policies, they're openstack-wide | 17:08 |
TheJulia | ++ | 17:08 |
TheJulia | And even then, not always possible to ensure as not all packagers respect requirements.txt | 17:08 |
dtantsur | if it's about https://review.opendev.org/c/openstack/networking-baremetal/+/903995/3/requirements.txt, you need to expect the current operators to use ANYTHING above 5.29.0. While I'd not recommend anyone to go with an ancient version, they might. | 17:08 |
dtantsur | I do agree that oslo libraries are kinda special since most consumers will upgrade them with the services. But they're not current exempt from the policies. | 17:09 |
TheJulia | *and* the only place that can *really* ever be changed is on the master branch | 17:09 |
TheJulia | we definitely cannot backport major changes like that | 17:10 |
dtantsur | SvenKieske: in situations like this, we tend to ask patch authors if the patches can be rewritten in a way that is compatible with older versions while also conditionally using newer ones | 17:10 |
* TheJulia drifts back to the island of writing documentation | 17:10 | |
dtantsur | e.g. if you want to use CONF.oslo_messaging_rabbit.rabbit_quorum_queue which, I presume, is not available earlier, you can use getattr(..., None) | 17:10 |
TheJulia | It is a dessert island where creme brulee is in the fridge awaiting dinner | 17:10 |
dtantsur | mmmmmmmm | 17:11 |
TheJulia | dtantsur: ++++ on the last two messages | 17:11 |
dtantsur | SvenKieske: another important angle to look at this problem. Some operators mix and match versions. I.e. they may be using a new Ironic on an ancient OpenStack. | 17:12 |
SvenKieske | okay, so this means, that ironic/networking-baremetal does not support quorum-queues before caracal, could that then at least be documented somewhere? | 17:12 |
TheJulia | or an ironic which requires archeology anytime they ask for insight | 17:13 |
* TheJulia wonders if a good new career could be digging up dinosaur bones | 17:13 | |
dtantsur | SvenKieske: which exactly part requires the version bump? We may figure out a conditional way to support them. | 17:13 |
dtantsur | if it's the configuration option, just use getattr | 17:14 |
SvenKieske | yeah we need someway to check if quorum queues are used, if we use olso native stuff for that this requires a bump. I think we could use some hacks to avoid the version bump | 17:14 |
dtantsur | and create a release note saying something along the lines of "If you use oslo.messaging before X.Y, you're in trouble" | 17:15 |
SvenKieske | still, this technically means networking-baremetal does not, in fact support quorum queues, because it can't be tested with it, if the minimum requirements do not require the needed oslo.messaging version | 17:15 |
SvenKieske | *besides the current master branch, once this is merged | 17:16 |
TheJulia | Hmm, I guess the model actually differs than what we expect most operators to actually do | 17:16 |
dtantsur | The minimum requirement changes nothing if it's already satisfied. I don't quite get your argument. | 17:17 |
TheJulia | single rabbit which comes back alive, but that doesn't work for those things which store long running state in the queue | 17:17 |
dtantsur | for instance, https://github.com/openstack/requirements/blob/stable/zed/upper-constraints.txt#L159 means that we *recommend* Zed users to have 14.0.3. | 17:17 |
* TheJulia had to go look up the history and origin behind quorum quues | 17:17 | |
dtantsur | So, while, we cannot *require* that on stable/zed, chances are 90% that the operators have it. And our CI definitely does. | 17:18 |
SvenKieske | well, I was asked to bump the minimum required version, because of course it needs to be guaranteed that the oslo.messaging mechanism of looking up if quorum queues are used works. | 17:18 |
SvenKieske | so the only way networking-baremetal can guarantee that quorum queues work in a given deployment is by having a minmium version of oslo.messaging which supports quorum queues, no? | 17:19 |
dtantsur | No. Or at least I cannot make this conclusion from your patch. | 17:19 |
dtantsur | I mean, for operators - yes. If they want to use this feature, they'll need the version. But *we* don't require it. | 17:20 |
SvenKieske | so, for what are minimum requirements than useful, please explain it to me :) | 17:20 |
dtantsur | SvenKieske: minimum requirement is: this project cannot work without that version of the library. | 17:20 |
SvenKieske | "But we don't require it" <- I don't understand this. If this patch is merged as is, we do in fact require it? | 17:20 |
dtantsur | Yes, but that's the reason we won't merge it on stable branches. | 17:21 |
SvenKieske | yeah and if I want to support feature foo in library bar which is available in version zed, I need to require version zed, no? | 17:21 |
dtantsur | Possibly. Sometimes we do conditional dependencies. In any case, supporting a new feature generally qualifies as a feature itself and is thus not backportable. | 17:21 |
dtantsur | Raising the version on the master branch is not a huge deal. | 17:22 |
TheJulia | I suspect a "fix" to handle the case of the usage is reasonable, given it is the default as of rabbitmq 3.8.0 released back in october 2019 | 17:22 |
dtantsur | There is a fine border between "I want to support a new feature" and "your project misbehaves in the presence of the new feature". | 17:22 |
dtantsur | I was reading your patch as fixing something like the latter. If it's more the former, let's merge it (and NOT backport). | 17:23 |
SvenKieske | that was not my point. my point was to ask to document - on older branches - that networking-baremetal does not support quorum queues, because that's what users actually deploy and want to use and run into bugs because nobody bumps the requirements of networking-baremetal. | 17:23 |
TheJulia | they renamed what was previously used as "classic" queues | 17:23 |
TheJulia | so the opportunity for confusion there is *huge* | 17:23 |
dtantsur | SvenKieske: then why don't document that on literally every openstack project that did not bump oslo.messaging? | 17:23 |
dtantsur | none of them will have quorum queues if oslo.message is old. but that belongs in the oslo.messages docs, no? | 17:24 |
SvenKieske | dtantsur: it would be good to do! I'm only aware of networking-baremetal until now :) | 17:24 |
dtantsur | Everything uses or can use oslo.messaging | 17:24 |
dtantsur | Neutron has old oslo.messaging requirement https://opendev.org/openstack/neutron/src/branch/master/requirements.txt#L34 | 17:25 |
SvenKieske | well all I'm trying to say is it would be nice if requirements would be bumped more often so users can actually use 2 year old "new" features. I get why requirements don't get bumped and I appreciate being able to also use older lib versions, but I don't think the current balance is a good one to be honest :) | 17:26 |
dtantsur | SvenKieske: I think you're mixing two things. Let's bump the requirement on master - well, you more or less convinced me. But there seems to be an actual bug fix in networking-baremetal (respecting the newer option), and I'd rather not tie it with the requirements bump. | 17:26 |
SvenKieske | dtantsur: okay, fine with me. I only did bundle it because another reviewer requested it in there and I'm only a drive-by contributor. So I really rely on the expertise of all the people who know more than me :) | 17:28 |
dtantsur | SvenKieske: sure, and we're very grateful for your contributions! Just trying to balance a lot of needs. | 17:28 |
SvenKieske | I really want to backport the actual fix, because people are running into this bug even on the zed release. | 17:28 |
dtantsur | I'd say: go with getattr() and no bump, backport that, then follow up with a version bump on master only. | 17:29 |
dtantsur | I'm saying that fully understanding that it's slightly more work for you.. Let us know if you want us to take over the 2nd part. | 17:29 |
SvenKieske | so, should I rewrite the detection logic of quorum queues to not use a newer oslo messaging version? I would need to think about how to do that in a not very ugly way :D | 17:29 |
SvenKieske | mhm, do you have maybe an example for this getattr() stuff somewhere? I'm fine with adapting but I don't know how to do it without an example I guess | 17:30 |
SvenKieske | I guess I can figure it out myself also, but a link would be handy :) | 17:30 |
dtantsur | SvenKieske: by detection, you mean the configuration option? | 17:31 |
SvenKieske | dtantsur: yes; I guess something like this? https://opendev.org/openstack/oslo.config/src/commit/305ecd817836a90a8f5e495cbf6a770dcea1f7e8/tests/test_cfg.py#L2159 | 17:32 |
dtantsur | SvenKieske: yes. A complete example https://review.opendev.org/c/openstack/networking-baremetal/+/903995/3/networking_baremetal/agent/ironic_neutron_agent.py | 17:33 |
dtantsur | actually, a small correction, hold on | 17:33 |
dtantsur | yep, now look | 17:34 |
SvenKieske | thanks for your time already, much appreciated. | 17:34 |
SvenKieske | will adjust the patch accordingly | 17:34 |
dtantsur | thank you! | 17:34 |
SvenKieske | mhm | 17:38 |
SvenKieske | what about the test though? https://review.opendev.org/c/openstack/networking-baremetal/+/903995/3/networking_baremetal/tests/unit/ironic_agent/test_ironic_agent.py | 17:38 |
SvenKieske | can I settattr that (is that even a thing) and older oslo releases just print a warning? | 17:39 |
dtantsur | SvenKieske: what's the default value of this option? | 17:40 |
opendevreview | Sven Kieske proposed openstack/networking-baremetal master: don't force amqp_auto_delete for quorum queues https://review.opendev.org/c/openstack/networking-baremetal/+/903995 | 17:40 |
SvenKieske | dtantsur: good question! it is actually "False" according to oslo.messaging docs, so no need to set it! | 17:43 |
dtantsur | problem solved :) | 17:44 |
opendevreview | Sven Kieske proposed openstack/networking-baremetal master: don't force amqp_auto_delete for quorum queues https://review.opendev.org/c/openstack/networking-baremetal/+/903995 | 17:47 |
SvenKieske | thanks again all, and sorry for being stubborn :D | 17:48 |
dtantsur | all good, I'm glad we all got better understanding of what is going on | 17:49 |
JayF | Does anyone have knowledge about ironic-standalone-redfish tests? | 17:49 |
dtantsur | now, nearly 7pm on our side of the Atlantic, Feierabend time | 17:49 |
JayF | I've found something whihc appears so broken I am stunned, which implies to me I'm missing something entirely | 17:49 |
JayF | TheJulia: ^ I know you've been looking at some of this | 17:49 |
SvenKieske | thinking about it, I guess I can propose a second patch bumping the requirements and getting rid of the getattr stuff then in master at least, would that be something nice to have? | 17:49 |
dtantsur | SvenKieske: it would. Just explain what is going on in the commit message. | 17:50 |
TheJulia | JayF: whats up, please bear with me I'm deep in writing how to adopt an ironic deployment documentation, so responses will be delayed | 17:50 |
SvenKieske | dtantsur: sure, probably will take until tomorrow, as you rightly said, it's getting late here :) | 17:50 |
dtantsur | JayF: feel free to pick my brain tomorrow your morning | 17:50 |
JayF | TheJulia: 2024-02-21 03:05:38.832137 | controller | +++ /opt/stack/ironic/devstack/lib/ironic:enroll_nodes:2619 : openstack --os-cloud devstack-system-admin baremetal node create --chassis e77c9ef2-ed44-4c22-a6fa-48059bf561cb --driver redfish --name node-3 --resource-class baremetal --property cpu_arch=x86_64 --property capabilities=boot_mode:uefi --driver-info redfish_address=http://10.208.225.208:9132 --driver-info | 18:01 |
JayF | redfish_username=admin --driver-info redfish_password=password --driver-info redfish_system_id=/redfish/v1/Systems/node-0 --driver-info redfish_system_id=/redfish/v1/Systems/node-1 --driver-info redfish_system_id=/redfish/v1/Systems/node-2 --driver-info redfish_system_id=/redfish/v1/Systems/node-3 -f value -c uuid | 18:01 |
JayF | https://storage.bhs.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_e3d/909263/3/check/ironic-standalone-redfish/e3d3d8a/job-output.txt | 18:01 |
JayF | we are setting redfish_sysem_id 3 separate times (?) | 18:01 |
JayF | my hypothesis is that we're relying on the behavior to be the last value wins | 18:08 |
JayF | which means this works | 18:08 |
JayF | hypothesis confirmed in logs | 18:10 |
TheJulia | eww, that is likely a bug in the creation logic where the value is just getting appended | 18:10 |
JayF | we've been relying on this for 8 years. | 18:10 |
TheJulia | \o/ | 18:10 |
JayF | we should bug this, right? | 18:11 |
JayF | because even if it works, it's terrible? | 18:11 |
TheJulia | that is awful | 18:13 |
TheJulia | And yeah, we should fix the logic in the plugin | 18:13 |
JayF | https://bugs.launchpad.net/ironic/+bug/2054597 | 18:26 |
JayF | cid said he'll pick this up soon | 18:26 |
cid | JayF: informative session :D | 18:31 |
cid | I'm looking forward to enjoying combing through logs like you | 18:31 |
JayF | you only think you are LOL | 18:31 |
JayF | reading copious amounts of debug logging is a big part of development work, especially systems development, but I'm not sure I've ever looked forward to it LOL | 18:32 |
JayF | I like INFO level logs when everything is going fine, just the services telling you about all the wonderful things they are doing LOL | 18:32 |
clarkb | eventually you get really good at using grep | 18:33 |
clarkb | its probably the one tool I don't have to look at the manpage periodically to remember flags for because I use it too often | 18:34 |
JayF | I've started to get a HUGE affinity for `rg` | 18:34 |
JayF | more useful in dev scenarios | 18:34 |
clarkb | isn't ripgrep largely just faster? Looks like it also respects .gitignore | 18:36 |
cid | this reassignment is an error LOL https://bugs.launchpad.net/ironic/+bug/1628422 | 18:36 |
cid | My bad, I was just asking what happens to hanging issues whose whose fix have been provide and even release. | 18:36 |
cid | JayF: I did not work on that, it was adamcarthur who did | 18:37 |
JayF | whoooop | 18:39 |
JayF | lol | 18:39 |
JayF | fixed :D | 18:39 |
JayF | clarkb: it's faster, respects .gitignore and ignores some other common stuff | 18:39 |
JayF | clarkb: but the "it's faster" is almost an understatement | 18:39 |
JayF | it's absurdly fast | 18:39 |
clarkb | JayF: I guess i never really notice that grep is slow and I'm often grepping in many gigabytes of data | 18:40 |
JayF | `git grep` vs `rg` in an openstack repo has a noticible speedup | 18:41 |
* JayF does a test to see if it's just placebo | 18:41 | |
clarkb | kinda like how people pick text editors because they are faster. Never really noticed that vim was slow and need to neovim | 18:41 |
JayF | less than half the time | 18:41 |
clarkb | ya but is that 200ms vs 100ms? | 18:42 |
clarkb | because that doesn't matter :P | 18:42 |
JayF | pretty much yeah :D | 18:42 |
JayF | I bet gentoo repo is where I felt the difference when I switched | 18:42 |
JayF | because that repo will make any computer weep | 18:42 |
JayF | yeah, that was .08 -> .42 seconds (rg vs grep) | 18:44 |
JayF | maybe doesn't matter bit it does feel better | 18:44 |
TheJulia | does anyone have a running ironic deployment handy? | 19:00 |
JayF | What do you need? | 19:00 |
* JayF can see how his devstack is doin' | 19:00 | |
JayF | actually probably not good since I had bad ram in that system | 19:00 |
TheJulia | doh | 19:00 |
TheJulia | does the following work: openstack baremetal node list -c uuid -f value --owner None | 19:01 |
JayF | cid: oh, I forgot to mention in our office hours: we have VMs coming for you. Two. One you can use to devstack and one you can use to do tests/etc without beating up on your local machine | 19:02 |
JayF | TheJulia: wow that is not what Iexpected | 19:03 |
TheJulia | i suspect that does not work | 19:04 |
JayF | https://usercontent.irccloud-cdn.com/file/vdug9OxP/image.png | 19:04 |
TheJulia | UUID, gah | 19:04 |
JayF | that returns a null set | 19:04 |
TheJulia | and I take it you have no owner set? | 19:04 |
TheJulia | well, your devstack-admin at the moment | 19:04 |
TheJulia | eh, should still match only what it can see | 19:05 |
JayF | that all have an owner set(!) | 19:05 |
JayF | I wasn't expecting that | 19:05 |
TheJulia | muahahahahahahaha | 19:06 |
JayF | let me unset on a couple ,this box is throwaway | 19:06 |
TheJulia | ack, thanks | 19:07 |
TheJulia | looks like it does a literal match, so we're expecting string input int he code | 19:07 |
JayF | empty set | 19:07 |
TheJulia | question sort of falls to sqlalchemy at the end of the day | 19:07 |
TheJulia | ugh, kind of what I suspected | 19:07 |
JayF | this is the root cause | 19:07 |
JayF | of why I added ?sharded= ?unsharded= | 19:07 |
JayF | because hooking up shard=None to filtering was a giant can of worms | 19:07 |
TheJulia | yeah | 19:08 |
TheJulia | out of curisoity, if you ask for json output and ask for all nodes by UUID,Owner, what do you get? | 19:08 |
JayF | BWAHAHAHA | 19:09 |
TheJulia | with field UUID,Owner | 19:09 |
JayF | TheJulia: how about I give you a working command | 19:09 |
JayF | https://usercontent.irccloud-cdn.com/file/6GfDqTgl/image.png | 19:09 |
TheJulia | that would be epic, but I think I'm going to need to filter it in jq | 19:09 |
TheJulia | oh, awesome! | 19:09 |
JayF | s/None/\'\'// | 19:09 |
TheJulia | perfect, thanks! | 19:09 |
JayF | you'll pay me back helping figure out why cid's job failed ironic-standalone-redfish if we aren't saved by the recheck ;) | 19:09 |
TheJulia | sure | 19:10 |
JayF | I think then I'll be down to only owing you a few dozen favors :P | 19:10 |
JayF | actually, wait, lemme set an owner on one of these | 19:10 |
JayF | and ensure it's filtered | 19:10 |
JayF | before I update the ledger :P | 19:10 |
opendevreview | Adam McArthur proposed openstack/ironic-python-agent master: WIP Adding support for viewing individual cpu process info https://review.opendev.org/c/openstack/ironic-python-agent/+/909346 | 19:10 |
JayF | TheJulia: uh | 19:11 |
JayF | TheJulia: that just doesn't filter at all, it seems :( | 19:11 |
TheJulia | oh, le sigh | 19:11 |
JayF | https://usercontent.irccloud-cdn.com/file/yjx7O6kl/image.png | 19:11 |
JayF | TheJulia: time to implement ?unowned= | 19:12 |
JayF | although I suspect if you're trying to solve a thing, that won't save you today | 19:12 |
TheJulia | i need it for the docs, if you can just dump the two columns via json, I'll do it in jq | 19:12 |
TheJulia | yeah, it won't | 19:12 |
TheJulia | openstack baremetal node list -c UUID,Owner -f json | 19:13 |
JayF | https://usercontent.irccloud-cdn.com/file/7NDGAAyt/image.png | 19:13 |
TheJulia | muchas gracias! | 19:13 |
JayF | seriously though | 19:13 |
JayF | we need unowned queries to work | 19:13 |
JayF | and this is a straightforward API improvement | 19:14 |
TheJulia | oh, how annoying, it uses the translated column name | 19:14 |
JayF | it should take both | 19:14 |
JayF | *should* | 19:14 |
JayF | not that it does | 19:14 |
JayF | but that it should | 19:14 |
JayF | from a value judgement not specification perspective I mean :D | 19:14 |
cid | JayF: I will really love that. in the wait mode now | 19:15 |
cid | recheck is failing | 19:16 |
TheJulia | "cat blah.json |jq .[].UUID -r" == win | 19:19 |
JayF | that doesn't filter for the owner you want? | 19:21 |
JayF | or you just need owner:uuid mappings? | 19:21 |
TheJulia | nah, not yet | 19:21 |
TheJulia | trying to get it to filter now | 19:21 |
TheJulia | having to look at jq docs for that | 19:21 |
TheJulia | cat blah.json |jq -r '.[] | select(.Owner == null) | .UUID' | 19:28 |
opendevreview | Adam McArthur proposed openstack/ironic-python-agent master: WIP Adding support for viewing individual cpu process info https://review.opendev.org/c/openstack/ironic-python-agent/+/909346 | 20:03 |
JayF | urgh https://review.opendev.org/c/openstack/ironic/+/909263 failed the same job in the sameish way | 20:29 |
TheJulia | ansible driver eh | 21:14 |
TheJulia | that is weird | 21:24 |
JayF | Ooh, zuul ansible bump was recentish iirc? | 22:04 |
clarkb | saturday ish we dropped ansible 6 and added ansible 8. Everything has been asnsible 8 by default for some months now and I sent notice of this change to the service-announce list last week | 22:09 |
JayF | Ack so unlikely cause there i suspect | 22:15 |
opendevreview | Steve Baker proposed openstack/sushy-tools master: Add virtual-media-boot to openstack driver https://review.opendev.org/c/openstack/sushy-tools/+/906768 | 22:18 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!