Friday, 2022-09-30

vanougood morning ironic00:09
arne_wiebalckGood morning, Ironic!06:17
opendevreviewJakob Meng proposed openstack/bifrost master: [DNM] Explicitly fetch baremetal ports and use id on openstacksdk>=0.99.0  https://review.opendev.org/c/openstack/bifrost/+/85943006:56
opendevreviewJakob Meng proposed openstack/bifrost master: [DNM] Explicitly fetch baremetal ports and use id on openstacksdk>=0.99.0  https://review.opendev.org/c/openstack/bifrost/+/85943007:06
rpittaugood morning ironic, happy Friday! o/07:12
opendevreviewRiccardo Pittau proposed openstack/ironic-python-agent bugfix/8.6: [DNM] TEST CI  https://review.opendev.org/c/openstack/ironic-python-agent/+/85978207:21
opendevreviewJakob Meng proposed openstack/bifrost master: [DNM] Explicitly fetch baremetal ports and use id on openstacksdk>=0.99.0  https://review.opendev.org/c/openstack/bifrost/+/85943008:47
rpittauJayF: hi! just reading your mail on openstack-discuss, what's the reasoning behind wanting to release from bugfix branches? I don't think that's really needed since we release already from stable branches09:26
JayFrpittau: honestly it's as simple as: 1) we have backports there, 2) it should be just as easy to push for n+3 releases as for n12:16
JayFrpittau: it's possible you know something I don't, so is there a reason that's not true?12:17
rpittauJayF: I wonder how that would work as versions in bugfix branches are the same as the corresponding stable branches12:17
iurygregorymorning Ironic12:18
JayFrpittau: I think it's just a patch release (e.g. 21.0.1)12:19
rpittauJayF: same as stable branches12:20
iurygregorytbh, I don't think we would need releases from bugfix branches12:22
opendevreviewJakob Meng proposed openstack/bifrost master: Use openstacksdk<0.99.0 and a-c-o<2.0.0 on bifrost-*collections-* jobs  https://review.opendev.org/c/openstack/bifrost/+/85998412:47
opendevreviewJakob Meng proposed openstack/bifrost master: Explicitly fetch baremetal ports and use id on openstacksdk>=0.99.0  https://review.opendev.org/c/openstack/bifrost/+/85943012:50
TheJuliagood morning everyone13:09
* TheJulia suspects dtantsur is out today13:09
TheJuliareleases are cheap. If there is another reason or concern driving a different perspective, we should get it on the table13:38
dtantsurTheJulia: I started late and forgot to turn on IRC :)13:43
dtantsurall yours now13:43
TheJuliathe allocations name field... varchar(255)13:43
TheJuliaAny objection if I change this to a text field?13:44
TheJuliaand add an index?13:44
dtantsurindex on a text field, is it even a thing?13:44
TheJuliaI ask because it is incompatible with mariadb w/innodb13:44
TheJuliayes, it is13:44
* dtantsur is surprised13:44
TheJuliawe would need it for querying since text fields are stored differently13:45
dtantsurI have no objections, but can you elaborate on the exact issue?13:45
dtantsur"255 varchars exceeds the field max size of 767 bytes for varchar" WUT13:45
TheJuliaMariadb innodb tables have an underlying field byte limit of 767 characters, however, when you say you want varchar(255), and the system is using utf8, your really asking for 255*4 bytes13:46
TheJuliayup13:46
dtantsurfuuuuuuuuuuu13:46
TheJuliayeah13:46
dtantsurmysql is absurd13:46
* dtantsur has always been a postgres person13:46
TheJuliaheh13:46
dtantsurshould I even try contemplating where 767 is coming from?13:46
TheJulia767/4 == ???13:47
dtantsuror is it too dangerous for the rest of my sanity13:47
TheJulia:)13:47
TheJuliatoo dangerous13:47
dtantsurokay, I assume they really like this Boeing model13:47
dtantsur767 was actually not too bad: a wide one, but not packed like 777 and on13:47
* dtantsur is in Friday mood already13:47
TheJuliaheh13:47
dtantsurTheJulia: do we have the same problem with node names?13:50
dtantsurbecause I think I modelled the allocation name after them13:50
TheJulia Ode names are 63 chars13:51
TheJuliaNode13:51
dtantsuroooookay13:52
dtantsurmaybe we should shrink the allocation name13:52
TheJulia Can’t without nuking uniqueness and query by name13:53
TheJuliaIndexing it should keep query performance sane if we just kick it over to text13:54
dtantsurbecause of how alter table will behave?13:54
TheJuliaBecause we break the contract for being able to use a node by name13:54
TheJuliaAnd risk non unique names as a result13:54
TheJuliaThen again, we could preflight warn…13:54
dtantsuryeah, I understand that it's problematic if someone has allocation names that are longer than, say, 128 chars13:55
dtantsurwhich doesn't sound super likely, so yeah, we can probably just ask people to change that pre-upgrade13:55
TheJuliaThat could work… though. Force them to reduce length before upgrading13:55
TheJuliaWe’ll need to backport the upgrade check13:55
TheJuliaBut that is easy enough13:55
TheJuliaand once we start merging sqlalchemy2 stuff, we're moving our minimum for sqlalchemy which means we cut a whole numbered release, so including it should be fine13:56
TheJuliathe alter for the schema, if everything is the right length, shouldn't be too horrible of a db update13:58
* TheJulia goes and finds breakfast and then off to a doctor's appointment13:59
dtantsurSo, the 767 limit applies to text fields (any fields) too14:00
dtantsuryeah, good luck. just thinking aloud.14:00
dtantsurI wonder how it actually looks for a large text. will it only index the first 767 bytes?14:00
TheJuliatext should be 64k14:03
dtantsurTheJulia: reading https://dev.mysql.com/doc/refman/5.6/en/column-indexes.html. with text fields we need to specify the limit14:03
dtantsurbut we can also specify the limit for varchar?14:03
dtantsurmaybe we can work around the problem by asking it to only index the frist 767 (or whatever) bytes?14:03
TheJuliaahh14:03
TheJuliayeah, for the index, that makes some sense14:03
TheJuliait can't be ensured unique then14:04
dtantsursame for text :(14:04
TheJuliayup14:04
dtantsurthen we probably need to shrink the field14:04
TheJuliayeah14:04
rpittaubye everyone, have a great weekend! o/14:04
dtantsurc u rpittau 14:04
* TheJulia notes rpittau is rightfully running away from db discussions :)14:04
* TheJulia seriously needs to go be on time for her appointment14:05
dtantsursorry for holding you :)14:05
TheJuliaodds on 2 or more vaccines today?!14:05
dtantsurOo14:05
* TheJulia guesses monkeypox and covid bivalent14:06
ajyamraineri: when you have a time, can you take a look at my comment here - https://review.opendev.org/c/openstack/sushy/+/856597/9#message-8fed241b28280cd93b65023f8d00383a609f94c6 Is there a generic way how to tell which property is modifiable on active resource, which on settings? cc janders 14:07
dtantsurTheJulia: you have both of these available already? wow14:09
* dtantsur is still unsure if he gets a covid one before x-mas holidays..14:10
TheJuliaWe have a monkey pox outbreak locally14:14
dtantsurouch14:15
JayFTheJulia: rpittau: 'releases are cheap' is basically 99.99% of my reasoning for releasing bugfix branches. We've all done hard work to backport stuff there, and it's worth it even if only one or two users pulls the update14:28
* TheJulia somehow obtained coffee hot enough to burn the sun14:32
JayFTheJulia: I'd take that over this defective jelly donut14:33
JayFTheJulia: literally only filled in like, 1/3rd of the donut14:33
TheJuliaSadness :(14:33
JayFit's okay tbh the donuts are tasty enough w/o the filling. benefit of local places14:33
TheJuliaThat is always a benefit14:37
dtantsurJayF: keep in mind that bugfix releases (the last time I checked) were done manually by the release team14:40
JayFdtantsur: if it's not free/cheap, that's absolutely a factor I'm unaware of and will be more judicious14:40
JayFBut like, if we're going to push bugfix releases to pypi, with some amount of patch support, we need to push patch releases there too14:40
dtantsurit's not a huge deal, but it does involve a living person doing `git tag/git push`14:41
JayFI'm sorta ok with "bugfix releases are always em-style branches" or "bugfix releases are released to pypi infrequently" but just tossing one out there and leaving it from then on seems crummy :( 14:41
JayFbut again, I don't think we have the insights to know if literally any humans at all are consuming those releases14:41
JayFwhich is unfortunate14:42
dtantsurI think they're em-style in my brain14:42
dtantsurbut I'm open to changing that14:42
dtantsuryea...14:42
JayFwe should document that and not ever release it if we go that route14:42
dtantsurwe in OCP consume branches, nto releases14:42
JayFthe in between is mainly what's uncomfy14:42
dtantsurI don't think we've done any bugfix branch releases in a while14:42
dtantsurI did a few in the very beginning, then stopped14:43
JayFthat's exactly my motivation for doing it :D 14:43
JayFand for stable branches too, plus it lets me know what the process is. I wouldn't hate getting on some kind of cadence for that ... tbh not neccessarily as a project, but like for me-personally pushing releases, because I'm not good at things on a "when needed" cadence vs some form of regularity14:43
dtantsurwe already decide on a feature release cadence, we can also decide how often we do releases from all branches14:44
TheJuliaThe fact a human has to do it really means that the tooling needs to be fixed14:44
dtantsur(with out-of-order releases possible too, esp. for libs)14:44
JayFdtantsur: Can you clarify what you mean by that?14:45
dtantsurFixing tooling ++, but since we're the only team doing bugfix branches.. yeah14:45
dtantsurJayF: we do feature releases every 2 months, right?14:45
JayFthat is not documented in the process, even if it was practice14:45
dtantsurthis is documented, actually14:45
* JayF finds and re-reads the release cadence docs14:46
dtantsurhttps://specs.openstack.org/openstack/ironic-specs/specs/approved/new-release-model.html#releasing14:46
dtantsursays exactly this14:46
dtantsurJayF: ^^14:46
JayF> They are also released on a regular cadence as opposed to on-demand, namely three times a release cycle (roughly a release every 2 months).14:46
JayFYep, you're 100% right14:46
JayFIt's just awkward b/c with that kind of cadence, it's not the kind of release I'd expect to get "stable" support14:46
JayFbut because our contributors care about them getting stable support, we do it14:46
dtantsuryeah, for me bugfix branches are more of an opportunity to collaborate rather than a firm support commitment14:47
dtantsurkinds of what em is14:47
dtantsur(I'm ready to change this position if we decide to as a team)14:47
JayFYeah, I just hate the idea that there's a 22.0 release out there, on pypi, that if installed is missing months of fixes14:48
JayFs/22/21/ 14:48
dtantsurit's not unheard of, when projects (outside of OpenStack) declare some releases as LTS/whatever, while some only receive limited support14:49
dtantsurI wish pypi had some metadata for that14:49
JayFthe thing that is weird for me is that these *are effectively* LTS releases14:49
dtantsurwdym?14:49
JayFwe're just ducking actually pushing the long term supported bits as patch releases14:49
dtantsurwell... we also have 2 types of "long" :)14:49
dtantsurTrain has been supported for how much now? I don't think any bugfix branch will survive that long.14:50
JayFI sure as hell hope not lol 14:50
JayFbut also like, an intermediate release getting 18-24 months of backported bugfixes is kinda awesome14:50
JayFwe're doing lots of cool work maintaining the stable branches and mostly they are hidden under a rock14:51
JayF(keeping it under our red-hat? lol)14:51
dtantsurthere is also https://specs.openstack.org/openstack/ironic-specs/specs/approved/new-release-model.html#support-phases, but I don't think we really comply with that14:51
dtantsurheh14:51
JayFdtantsur: METRICS! https://pepy.tech/project/ironic?versions=21.0.015:34
JayFI knew they had to exist somewhere if I could find out how 15:35
JayFlol15:35
dtantsurWow, interesting!15:35
dtantsurwon't account downstream packaging and tarballs.o.o, but this is a start15:35
JayFI mean, when I think about a release, I primarily think about pypi15:36
JayFbecause frankly: 1) downstream packagers pull from git 2) tarballs.o.o customers would have to take explicit steps to upgrade (it's not "free")15:36
dtantsur#1 is not 100% correct15:36
JayFInteresting, does metal3 pull from pypi or something?15:36
dtantsurRDO master packaging pulls from git, RDO releases use tarballs.o.o15:36
dtantsurmetal3 upstream uses RDO packages then15:37
JayFInteresting. I had no idea. 15:37
JayFI've literally *never* worked anywhere that deployed ironic out of anything but git15:37
JayFprobably just a matter of the scale of environment I've worked in15:37
JayFeither way I think that says "nonzero numbers of people are using this"15:37
JayFI'm going to ask $SmartPeopleITrust if pypi has like, a noise level in the stats15:37
dtantsure.g. https://github.com/rdo-packages/ironic-distgit/blob/rpm-master/openstack-ironic.spec#L1615:37
JayFe.g. if ~1000/day could be just bot noise15:38
dtantsura good question15:38
JayFAh, the 1000/day is *all versions*, that stat doesn't change with filtering15:41
JayFlooks like we've got hundreds/week still for bugfix/2* releases in openstack/ironic15:41
JayFI am going to like, aggregate this data15:42
JayFPlug our bugfix releases in the Zed announcement with numbers about downloads :) 15:42
opendevreviewDmitry Tantsur proposed openstack/ironic stable/yoga: Prevent clear_job_queue and reset_idrac failures on older iDRACs  https://review.opendev.org/c/openstack/ironic/+/85991516:02
opendevreviewDmitry Tantsur proposed openstack/ironic bugfix/20.2: Prevent clear_job_queue and reset_idrac failures on older iDRACs  https://review.opendev.org/c/openstack/ironic/+/85991616:03
opendevreviewDmitry Tantsur proposed openstack/ironic bugfix/20.0: Prevent clear_job_queue and reset_idrac failures on older iDRACs  https://review.opendev.org/c/openstack/ironic/+/85991716:03
opendevreviewDmitry Tantsur proposed openstack/ironic bugfix/19.0: Prevent clear_job_queue and reset_idrac failures on older iDRACs  https://review.opendev.org/c/openstack/ironic/+/85991816:04
dtantsurjanders: ^^^ for https://issues.redhat.com/browse/OCPBUGS-174016:04
JayFI'll put approvals on them, reviewed the first and it looked good, just gotta check the rest16:05
dtantsurthx!16:06
JayF(+2 not +A, wrong worD)16:06
JayFactually, backported by core16:06
JayFI'll +a the first one16:06
JayFdtantsur: I don't see bugfix/20.0 on our list of branches on whiteboard16:07
JayFdtantsur: I really, really hope that's not in use because I've not backported a single patch of the stuff I've backported there16:07
dtantsurpossibly, I don't remember it by heart. feel free to abandon with a comment16:07
JayFack okay you had me worried lol16:07
dtantsur20.0 is not something we use at least16:08
JayFyeah, not 20.016:08
JayFI made that part of the whiteboard more useful for backporting btw16:08
JayFby putting the stable/ branches in there16:08
dtantsur++16:08
JayFso we have a reference of literally "go down this list to backport"16:08
jm1dtantsur: hello :) if you have some time please review https://review.opendev.org/c/openstack/bifrost/+/859984 17:23
dtantsurlooking17:24
dtantsurjm1: this lgtm, but on a related note.. I'm not sure how I feel about no longer returning NICs from node_info17:27
dtantsurjm1: isn't it sorta weird that we accept nics when creating a node but don't return them?17:27
jm1dtantsur: you should feel good because the module only fetches what you ask for ;)17:27
dtantsurso, can I ask for nics?17:28
dtantsur(it's also a bit unfortunately that we called them "nics".. and now have port_info)17:29
dtantsurjm1: ^^17:29
jm1dtantsur: aoc modules are thin wrappers around openstacksdk funtionality. node calls register_machine which has a nics parameter but it does returns a Node object only.17:31
dtantsursigh. we're quite inconsistent indeed.17:31
jm1dtantsur: imho any "extra" functionality which does not involve calling openstacksdk functions should moved from aoc modules to openstacksdk or dropped if redundant. calling port_info is much more efficient than proactively enumerating all ports inside node module. the patch for bifrost is so straight forward, backward compatible and simple that imho its worth to do it: https://review.opendev.org/c/openstack/bifrost/+/859430/17:34
dtantsuryeah, I wish Ironic could return ports as part of the node (optionally)17:36
dtantsurjm1: do we have any prior art? like extra_specs or flavors on servers?17:36
jm1dtantsur: you mean like a flag which you could pass to a module to ask for extra info such as nics? nope17:37
jm1dtantsur: at least not that i am aware of. but who knows what strange behaviour those not-yet-ported modules have ;)17:37
dtantsuryeah, I seem to remember some logic around it17:38
jm1dtantsur: some modules had similar functionality where a module would enrich the output from the sdk. but as in this case it would happen always and it also was a *_info module, but i cannot remember which.. anyway, we dropped that part for the same reason. wont scale, user did not ask for it and if user really wanted it he would and should use the appropriate module17:40
jm1dtantsur: or do you want to return nics inside Node object when register_machine is called?17:51
dtantsurjm1: this is probably a good idea, but the SDK's Node object doesn't account for it17:52
jm1dtantsur: we have a couple of resource objects in openstacksdk which have special fields which are not always filled, only on special occasions like what you want to do with nics. one example is public_v4/public_v6/interface_ip fields in openstacksdk's Server object17:53
jm1dtantsur: aoc modules will happily return whatever openstacksdk returns, so when you pass gtema's review, it will be in aoc as well ;)17:54
dtantsur:)17:54
jm1dtantsur: but honestly i like ironic's (and thus openstacksdk's) clear distinction in Node and Port. baremetal_node's nics parameter looks like a relict from the past, much like nics parameter from server module. the latter can be really confusing because it not always does what one expects. 17:59
dtantsurcreating nics together with the node has an important role: cleaning cannot succeed without ports17:59
jm1dtantsur: sure, but then why isnt it implemented in ironic itself?18:00
dtantsurjm1: ironic has more enrollment steps than the collection exposes18:01
dtantsuryou see, ironic itself is not declarative18:01
jm1dtantsur: "cleaning cannot succeed without ports" << implementing such a sensitive operation on clientside seems odd. if client looses connection then what?18:04
dtantsurjm1: what register_machine does is: 1) enroll a node, 2) enroll ports, 3) ensure the node is available for deploy18:05
dtantsur#3 includes cleaning. if #2 fails, it does not happen18:05
jm1dtantsur: the reason i am wondering why ironic does not accept a nics/networks/whatever paramter is that Nova does exactly that: The server creation api accepts a networks parameter which will attach a server to networks and will create ports.18:10
dtantsurironic's workflow is much more complex18:10
dtantsurthe server creating IS also the server deployment18:10
dtantsurit's not the case for bare metal18:11
jm1dtantsur: yeah i might got the wrong impression when looking at the sdk logic for register_machine18:17
dtantsurokay folks, time to enjoy the Friday evening18:24
dtantsurhave a good weekend, see you on Tuesday18:24
jm1dtantsur: ah damn, i just had a good idea. have a nice weekend!18:27
dtantsursorry, half past eight here already :)18:27
jm1dtantsur: same here ;)18:27
dtantsurthen go open a beer, you deserved it!18:27
* dtantsur slowly blends with the background18:28
jm1dtantsur: just wanted to write it down, we can think about it next weekend: inside register_machine you already have a list of ports, so you could simply replace the "ports" attribute of the node returned by register_machine with the real Port resource objects https://opendev.org/openstack/openstacksdk/src/commit/13117610fe7a6e39f2b01d93c3ea556aa83a5d85/openstack/cloud/_baremetal.py#L25518:29
jm1dtantsur: no need for a new "nics" field. it would simply move the functionality from the module to register_machine but without the additional api calls18:29
jm1dtantsur: now have a good weekend ,)18:30
opendevreviewJulia Kreger proposed openstack/ironic master: Add upgrade check warning for Allocation.name field  https://review.opendev.org/c/openstack/ironic/+/86002618:49
opendevreviewJulia Kreger proposed openstack/ironic master: Change allocations name length check to an upgrade failure  https://review.opendev.org/c/openstack/ironic/+/86002718:49
TheJuliaI'll figure out how to do the shortening of the field next week18:51
TheJuliaThanks Dave Neary, I'm craving pie now.18:51
opendevreviewMerged openstack/ironic bugfix/20.2: Prevent clear_job_queue and reset_idrac failures on older iDRACs  https://review.opendev.org/c/openstack/ironic/+/85991618:56
JayFgoing to land that to yoga18:57
JayFSo I ran the numbers via the pypi stats->bigquery interface; we have had 6196 downloads of the zed-based bugfix releases 18:59
JayF(21.0.0 and 20.2.0)18:59
ashinclouds[m]Wow, not bad18:59
JayFI'm going to call it 6200 in my slides19:00
JayFif I need to, I can pip install it 4 times in a venv ;) 19:00
JayFDo we know what releases of metal3/rdo included Ironic bugfix releases? Would be neat to call them out that way as well19:02
TheJulianot off the top of my head19:05
iurygregoryafaik metal3 will push the latest rpm available 19:05
JayFack so not really like, a number I could point to in that slide19:05
iurygregoryhttps://github.com/metal3-io/ironic-image/19:05
JayFI have an "Ironic Zed - by the numbers" slide in the highlights thing19:06
JayFso trying to come up with interesting things to point out (the pypi numbers are going in that, to give me a chance to plug our intermeidate bugfix releases)19:06
JayFand I think our integration with other non-openstack projects is a cool, unique-to-ironic thing19:06
* TheJulia cries that instacart's website seems to be unhappy19:12
opendevreviewMerged openstack/ironic stable/yoga: Prevent clear_job_queue and reset_idrac failures on older iDRACs  https://review.opendev.org/c/openstack/ironic/+/85991520:56
opendevreviewMerged openstack/ironic bugfix/19.0: Prevent clear_job_queue and reset_idrac failures on older iDRACs  https://review.opendev.org/c/openstack/ironic/+/85991822:17
JayFdtantsur: janders: I landed those drac fixes while you all slept; hope the user is fixed and happy o/22:25

Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!