| opendevreview | Jay Faulkner proposed openstack/ironic master: DB & Object layer for node.shard https://review.opendev.org/c/openstack/ironic/+/864236 | 00:04 |
|---|---|---|
| opendevreview | Jay Faulkner proposed openstack/ironic master: API support for CRUD node.shard https://review.opendev.org/c/openstack/ironic/+/866235 | 00:04 |
| opendevreview | Jay Faulkner proposed openstack/ironic master: Allow port queries by shard list https://review.opendev.org/c/openstack/ironic/+/872235 | 00:04 |
| JayF | I either massively broke tests with my changes, or it's broken locally | 00:04 |
| JayF | both things are possible but I'll find out tomorrow | 00:04 |
| JayF | o/ | 00:04 |
| vanou | Good night o/ | 00:07 |
| vanou | Yes. Even I remove vendor interface part of code, this patch deals with firmware incompatibility. Existing operator notice difference only when operator deploys new node (this patch adds verify step to deals with firmware HTTP incompatibility) | 00:09 |
| opendevreview | Vanou Ishii proposed openstack/ironic master: [WIP] Deal with iRMC virtual media incompatibility https://review.opendev.org/c/openstack/ironic/+/823790 | 05:48 |
| rpittau | good morning ironic! o/ | 07:59 |
| opendevreview | Riccardo Pittau proposed openstack/ironic-python-agent bugfix/8.3: Fix CI https://review.opendev.org/c/openstack/ironic-python-agent/+/871980 | 10:07 |
| opendevreview | Riccardo Pittau proposed openstack/ironic-python-agent bugfix/8.3: Fix CI https://review.opendev.org/c/openstack/ironic-python-agent/+/871980 | 10:08 |
| iurygregory | morning Ironic | 11:40 |
| *** tkajinam is now known as Guest3003 | 12:02 | |
| opendevreview | Jakub Jelinek proposed openstack/ironic master: Erase swift inventory entry on node deletion https://review.opendev.org/c/openstack/ironic/+/871394 | 13:56 |
| TheJulia | good morning | 14:13 |
| dtantsur | morning TheJulia | 14:32 |
| opendevreview | OpenStack Release Bot proposed openstack/ironic-python-agent bugfix/9.3: Update .gitreview for bugfix/9.3 https://review.opendev.org/c/openstack/ironic-python-agent/+/872287 | 15:06 |
| opendevreview | OpenStack Release Bot proposed openstack/ironic-inspector bugfix/11.3: Update .gitreview for bugfix/11.3 https://review.opendev.org/c/openstack/ironic-inspector/+/872288 | 15:07 |
| opendevreview | OpenStack Release Bot proposed openstack/ironic bugfix/21.3: Update .gitreview for bugfix/21.3 https://review.opendev.org/c/openstack/ironic/+/872319 | 15:24 |
| kubajj | TheJulia, dtantsur: if you had a minute, I added some tests to the follow-up https://review.opendev.org/c/openstack/ironic/+/871394 | 15:38 |
| dtantsur | in a meeting, will check later | 15:56 |
| opendevreview | Jay Faulkner proposed openstack/ironic master: DB & Object layer for node.shard https://review.opendev.org/c/openstack/ironic/+/864236 | 16:40 |
| opendevreview | Jay Faulkner proposed openstack/ironic master: API support for CRUD node.shard https://review.opendev.org/c/openstack/ironic/+/866235 | 16:40 |
| opendevreview | Jay Faulkner proposed openstack/ironic master: Allow port queries by shard list https://review.opendev.org/c/openstack/ironic/+/872235 | 16:41 |
| rpittau | good night! o/ | 16:48 |
| JayF | /win 19 | 16:54 |
| JayF | whoops | 16:54 |
| TheJulia | heh | 17:04 |
| * TheJulia has an idea to get out of her brain... after the current call | 17:05 | |
| JayF | https://review.opendev.org/c/openstack/ironic/+/869614 is a high impact thing we can land, I already am +2, someone should land it (Julia's patch for service RBAC role) | 17:53 |
| iurygregory | will take a look in few | 18:12 |
| TheJulia | if there are any questions lmk | 18:40 |
| TheJulia | I'm trying to get an idea out of my head at the moment | 18:41 |
| opendevreview | Merged openstack/ironic master: Bump cirros to version 0.6.1 https://review.opendev.org/c/openstack/ironic/+/871812 | 19:35 |
| opendevreview | Merged openstack/ironic-inspector bugfix/11.0: Remove reno job and fix CI https://review.opendev.org/c/openstack/ironic-inspector/+/871977 | 19:35 |
| opendevreview | Merged openstack/ironic-inspector bugfix/10.9: Remove reno job and cap tox to version lower than 4 https://review.opendev.org/c/openstack/ironic-inspector/+/871978 | 19:35 |
| opendevreview | Merged openstack/ironic bugfix/19.0: Remove reno job https://review.opendev.org/c/openstack/ironic/+/871973 | 19:35 |
| opendevreview | Merged openstack/ironic bugfix/20.2: Remove reno job https://review.opendev.org/c/openstack/ironic/+/871971 | 19:35 |
| opendevreview | Merged openstack/ironic bugfix/21.0: Remove reno job https://review.opendev.org/c/openstack/ironic/+/871972 | 19:35 |
| opendevreview | Merged openstack/ironic-python-agent bugfix/8.3: Fix CI https://review.opendev.org/c/openstack/ironic-python-agent/+/871980 | 19:35 |
| opendevreview | Merged openstack/ironic-python-agent master: update NVIDIA NIC firmware images and settings by ironic-python-agent https://review.opendev.org/c/openstack/ironic-python-agent/+/566544 | 19:35 |
| opendevreview | Julia Kreger proposed openstack/ironic-specs master: Add modify steps framework https://review.opendev.org/c/openstack/ironic-specs/+/872349 | 19:36 |
| TheJulia | janders: NobodyCam: JayF: ^^ kind of the active steps idea, but I thought about it and thought "that is kind of misleading since a system being modified is not active". Please provide an initial review pass when you have some time | 19:39 |
| opendevreview | Merged openstack/ironic-python-agent bugfix/9.0: Remove reno job and cap tox to version lower than 4 https://review.opendev.org/c/openstack/ironic-python-agent/+/871979 | 19:53 |
| opendevreview | Merged openstack/ironic-python-agent bugfix/8.1: Remove reno job and cap tox to version lower than 4 https://review.opendev.org/c/openstack/ironic-python-agent/+/871981 | 19:53 |
| JayF | gonna be honest, I'm not prioritizing spec reviews given we've got a lot of things already for this cycle that aren't ready yet :/ | 20:08 |
| JayF | I'm sure I'll take a look because I'm curious and will get around to it eventually though | 20:08 |
| TheJulia | JayF: fwiw, the same idea was listed as an item for this cycle under active steps | 20:28 |
| JayF | oh, I didn't realize that was even in the stuff for this cycle | 20:31 |
| opendevreview | Merged openstack/ironic master: Add `service` role RBAC policy support https://review.opendev.org/c/openstack/ironic/+/869614 | 21:54 |
| JayF | TheJulia: I'm not sure it'll be possible to implement a /v1/nodes?shard=null | 22:16 |
| JayF | same for ports | 22:16 |
| JayF | I assume this is OK? | 22:16 |
| JayF | I could add a separate, boolean filter for shardless nodes | 22:16 |
| JayF | it's not an unreasonable filter to want | 22:16 |
| TheJulia | oh, eww | 22:16 |
| TheJulia | yeah | 22:16 |
| JayF | at the DBAPI level, I can't get a None thru to the DB | 22:17 |
| JayF | which kinda makes sense | 22:17 |
| JayF | at least with how it's structured (all shard queries are listified and passed in via an in query) | 22:17 |
| TheJulia | yeah, if none the logic has to change on the query | 22:17 |
| TheJulia | yeah, I mean if it is all discoverable, maybe it is okay | 22:18 |
| TheJulia | I'd need to think about it, but I think your right | 22:18 |
| JayF | I'm not sure it makes sense to say /v1/nodes?shard=shard1,shard2,None | 22:18 |
| JayF | but being able to say /v1/nodes?shardless=true does, I think | 22:18 |
| JayF | I think if I hooked up "None" queries at the DBAPI layer (not even thinking about how to communicate it via API); I'd probably have to do the "is null" query AND the in "list-of-shards" and combine the results | 22:19 |
| JayF | which does not seem idea | 22:19 |
| JayF | **ideal | 22:19 |
| JayF | TheJulia: RFC on what to call the boolean? unsharded? shardless? noshard? Or do you think I should take on the complexity of allowing None to be supplied in the list | 22:21 |
| JayF | I really, really, really, do not think it's a good idea to take on the complexity unless there's a trick I don't know | 22:21 |
| TheJulia | well, if null then is not in... I'm don't know of the query handler to do that though | 22:22 |
| TheJulia | so | 22:22 |
| JayF | not in is for a negated query | 22:22 |
| TheJulia | I was always thinking that we would get a string in, not a literal null | 22:23 |
| JayF | let me put it this way: I don't know how to ask the DB for "select * from nodes where shard in (null, 'blah', 'lol', 'cats'); | 22:23 |
| JayF | because IIRC you can't put null in that list | 22:23 |
| TheJulia | because the only way to do that would be GET /url?shared=?arg2=blah | 22:23 |
| * TheJulia wonders if that is even possible | 22:23 | |
| JayF | TheJulia: I assumed we'd just say 'null' or 'None' or similar | 22:23 |
| JayF | this is gross, we have to make noshard=True or similar | 22:23 |
| TheJulia | and if we had the string, we could then just go "oh, that is special value, invert the get! | 22:24 |
| JayF | or else we're essentially going to end up with a 'magic string' in our API | 22:24 |
| TheJulia | well, select | 22:24 |
| JayF | I don't think you fully understand; or I don't fully understand you | 22:24 |
| TheJulia | well, down in the dbapi query most likely, but maybe we just punt on that | 22:24 |
| JayF | right now, the way I have it hooked up for both nodes and ports is | 22:24 |
| JayF | /v1/nodes?shards=shard1,shard2,shard3,shardbanana | 22:24 |
| JayF | it always wants a list | 22:24 |
| TheJulia | okay | 22:25 |
| JayF | so I don't want to *negate* the match, I want to be able to specify "no shard" as one of the potential shards in my list | 22:25 |
| JayF | which would be e.g. select * from nodes where shard in (null, 'shard1', 'shard2', 'shard3'); (which you can't do) | 22:25 |
| TheJulia | okay | 22:25 |
| TheJulia | which patch is this in? | 22:26 |
| JayF | so I believe the only way to make this work in one query, regardless of what the API looked like, would be something gross like | 22:26 |
| TheJulia | the latter? | 22:26 |
| TheJulia | err, last one? | 22:26 |
| JayF | trying to push it but got rebased | 22:26 |
| opendevreview | Jay Faulkner proposed openstack/ironic master: DB & Object layer for node.shard https://review.opendev.org/c/openstack/ironic/+/864236 | 22:27 |
| opendevreview | Jay Faulkner proposed openstack/ironic master: API support for CRUD node.shard https://review.opendev.org/c/openstack/ironic/+/866235 | 22:27 |
| opendevreview | Jay Faulkner proposed openstack/ironic master: Allow port queries by shard list https://review.opendev.org/c/openstack/ironic/+/872235 | 22:27 |
| TheJulia | okay, so the challenge is we end up with inconsistency on ports which becomes weird | 22:27 |
| JayF | I don't think it makes sense to ask "give me ports without a shard" | 22:27 |
| JayF | the whole time we said we'd treat no shard as another shard, I think I'm learning that's impossilbe | 22:27 |
| JayF | unless we want to force not-null on node.shard and populate it in the initial migration | 22:28 |
| JayF | which is a bad idea for $scale_reasons | 22:28 |
| TheJulia | I think that is a reasonable take and I was kind of thinking the same | 22:28 |
| JayF | so we need to be able to say "show me nodes that need a shard assigned" | 22:28 |
| TheJulia | ++ | 22:28 |
| JayF | which could be like, sharded=false | 22:28 |
| JayF | I can't find a good name for it | 22:28 |
| TheJulia | I think that could be reasonable | 22:28 |
| TheJulia | instead of a magical value | 22:28 |
| TheJulia | and you could... hmm | 22:28 |
| JayF | you think I'm better off saying "sharded=false" or "no_shard=true" | 22:28 |
| JayF | I almost think I should word it so the bool would be true | 22:29 |
| TheJulia | oh where oh where is the provisioned filter hiding | 22:29 |
| JayF | I know where it is | 22:29 |
| JayF | top of the sqlalchemy dbapi | 22:30 |
| JayF | look for a constant | 22:30 |
| TheJulia | https://review.opendev.org/c/openstack/ironic/+/866235/20/ironic/db/sqlalchemy/api.py#425 is kind of where you might consider it | 22:30 |
| JayF | yep, I'd add one to _NODE_NON_NULL_FILTERS | 22:30 |
| JayF | looks like all of them are positive | 22:30 |
| TheJulia | ++ | 22:30 |
| JayF | so I'd need to make it sharded: shard | 22:30 |
| TheJulia | perfect | 22:31 |
| JayF | I think I'll need to revise the spec? | 22:31 |
| TheJulia | nah | 22:31 |
| JayF | :-| | 22:31 |
| * JayF hates as a dev or operator reading the spec and it not being useful to understanding what's going on | 22:31 | |
| JayF | maybe not a prereq but I will probably circle back and make it reflect the real api | 22:31 |
| TheJulia | okay, minor update then :) | 22:31 |
| TheJulia | I'll single core approve it based upon implementation reality ;) | 22:31 |
| JayF | perhaps after I'm done lol | 22:31 |
| TheJulia | okay | 22:31 |
| TheJulia | I'm going to try and get some more words down before I step away, getting a migraine | 22:32 |
| JayF | ack; I'm going to finish this patch as written | 22:32 |
| JayF | and make the boolean-shard-checker as a follow-on | 22:32 |
| TheJulia | ack | 22:32 |
| JayF | which technically might be nonideal, because I'll have a patch that bumps the API microversion then two after that still use it | 22:32 |
| JayF | but it's better for them to be split, and an atomic microversion | 22:32 |
| TheJulia | true | 22:33 |
| JayF | but it's better for them to be split patches, and I prefer clarity for review over being hardasses about policy | 22:33 |
| TheJulia | in rapid succession I'd worry less, tbh | 22:34 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!