opendevreview | Julia Kreger proposed openstack/ironic master: Fix service role support https://review.opendev.org/c/openstack/ironic/+/907148 | 00:55 |
---|---|---|
opendevreview | Merged openstack/ironic-inspector master: Remove dependency on pytz https://review.opendev.org/c/openstack/ironic-inspector/+/906947 | 00:58 |
opendevreview | Julia Kreger proposed openstack/ironic master: Fix service role support https://review.opendev.org/c/openstack/ironic/+/907148 | 01:00 |
TheJulia | I think that sort of does it | 01:03 |
TheJulia | stevebaker[m]: hjensas: ^ | 01:03 |
*** jph7 is now known as jph | 01:40 | |
stevebaker[m] | ack | 01:41 |
opendevreview | Merged openstack/ironic-inspector master: Bump hacking to 6.1.0 https://review.opendev.org/c/openstack/ironic-inspector/+/907046 | 02:13 |
opendevreview | OpenStack Proposal Bot proposed openstack/ironic-inspector master: Imported Translations from Zanata https://review.opendev.org/c/openstack/ironic-inspector/+/906935 | 03:48 |
rpittau | good morning ironic! o/ | 08:10 |
opendevreview | Dmitry Tantsur proposed openstack/ironic-python-agent master: Trivial: avoid deprecated utcnow https://review.opendev.org/c/openstack/ironic-python-agent/+/907298 | 09:09 |
masghar | Good morning! | 09:25 |
rpittau | hey masghar :) | 09:25 |
*** zigo_ is now known as zigo | 09:43 | |
iurygregory | good morning ironic | 11:12 |
TheJulia | good morning | 13:53 |
iurygregory | good morning TheJulia =) | 13:53 |
iurygregory | Metal3 Team Meetup happening now | 14:01 |
TheJulia | I feel only sort of here, yay for mild food poisoning | 14:03 |
iurygregory | ouch =( | 14:04 |
iurygregory | sorry to hear that, take care TheJulia | 14:04 |
TheJulia | Today will just be a slow day for me | 14:05 |
dtantsur | get better! | 14:06 |
arne_wiebalck | Good morning, Ironic! | 14:08 |
arne_wiebalck | For the Ironic gathering before the OpenInfra meetup, suggestions on the title? | 14:08 |
arne_wiebalck | (we are preparing the event site) | 14:09 |
arne_wiebalck | in discussions here we called it "Ironic Meetup / Bare Metal SIG / BM Operator Hour" ... not too catchy | 14:10 |
arne_wiebalck | JayF: ^^ | 14:10 |
TheJulia | "Bare Metal Sig/Ironic Meetup" ? | 14:10 |
arne_wiebalck | the title should convey "all welcome" somehow, I guess | 14:13 |
JayF | I suspect there's enough cultural shenanigans wrapped up in that assessment that you might be the best person to figure out how to say that where you're at | 14:14 |
arne_wiebalck | the bare metal event will be linked from the general meetup page, so everyone considering to come to CERN for the meetup will see this .. just want to make sure we do not give the impression the BM event is some "closed" event | 14:19 |
arne_wiebalck | can do this in the small print, I guess | 14:19 |
arne_wiebalck | in big letters :) | 14:19 |
JayF | I'm literally happy to call it anything. We can call it open source office hours we can call it a cloud party with the bare metal kids | 14:19 |
JayF | So if you want to go completely out of the box, I'm game for ideas | 14:20 |
arne_wiebalck | "Cloud party with the cool bare metal kids" :-D | 14:20 |
opendevreview | Merged openstack/virtualbmc master: [codespell] Fixing Spelling Mistakes https://review.opendev.org/c/openstack/virtualbmc/+/906803 | 14:20 |
tkajinam | o/ anyone mind merging this CI fix? https://review.opendev.org/c/openstack/python-ironic-inspector-client/+/907058 | 14:20 |
JayF | Add me as reviewer if nobody has gotten it by the time I get to an actual computer, I'll merge it. An hour at most. | 14:22 |
iurygregory | approved | 14:25 |
dtantsur | TheJulia: I wonder if there is another way to push (a bit of) data through Redfish to the OS... | 14:38 |
dtantsur | some EFI variables? dunno.. | 14:38 |
opendevreview | Merged openstack/python-ironic-inspector-client master: Bump ironic-inspector used in functionl tests https://review.opendev.org/c/openstack/python-ironic-inspector-client/+/907058 | 14:38 |
TheJulia | maybe.... | 14:38 |
TheJulia | "maybe" | 14:38 |
JayF | I'll note that any such mechanism would have to be able to be trivially disabled for truly multi-tenant installs | 14:39 |
dtantsur | yeah, unless we have a reliable way to clean it as well | 14:39 |
TheJulia | and a lot of hardware starts acting super weird when you touch uefi vars | 14:40 |
TheJulia | specifically items in nvram | 14:40 |
dtantsur | yeah, I can imagine | 14:40 |
TheJulia | we've managed to sort of identify the weirdness with injection of bootloaders, but those are also standardized records | 14:50 |
TheJulia | Remember that shim bug we helped raise visibility of? The one which crashed machines? | 14:54 |
TheJulia | JayF: commented on your comment on https://review.opendev.org/c/openstack/ironic/+/907148 | 15:20 |
JayF | With that context, I think that the change makes sense. I also think we need to have a very strongly worded upgrade note about ensuring you change that value to something valid for your environment | 15:23 |
JayF | Either that or default the service project name to none, but I can see value in aligning with many of those other projects | 15:24 |
JayF | Like I said in the comment, I want to avoid a situation where someone can easily upgrade themselves into granting someone an authorized access | 15:24 |
TheJulia | Yeah, we already grant access based upon role:service, just the filter gets applied by default | 15:31 |
TheJulia | so really, go back to a boolean knob *and* allow default, so it can be entirely disabled | 15:34 |
TheJulia | eh.... | 15:34 |
TheJulia | setting something to None is a little awkard in a configuration | 15:34 |
JayF | I like the idea of having a boolean knob to turn on and off service access, separate from the configuration of what project | 15:42 |
JayF | That way we can allow deployment methods that are going to set up the project's properly to have it working by default, but don't have to worry about upgrading someone into insecurity | 15:42 |
TheJulia | I'm just not going to do it for all role:service access, just this higher level "see everything" | 15:43 |
JayF | ++ | 16:19 |
TheJulia | https://review.opendev.org/c/openstack/ironic/+/906914 and https://review.opendev.org/c/openstack/ironic/+/906913 are two relatively clean/quick backports which would be nice | 16:21 |
JayF | lookin | 16:21 |
JayF | +2A to all | 16:22 |
TheJulia | much appreciated, thanks! | 16:23 |
TheJulia | I'll revise the rbac one a little later | 16:23 |
opendevreview | Merged openstack/ironic-python-agent master: Drop usage of run_as_root https://review.opendev.org/c/openstack/ironic-python-agent/+/906375 | 16:29 |
rpittau | good night! o/ | 17:18 |
opendevreview | Merged openstack/ironic stable/2023.1: Fixes Secureboot with Anaconda deploy https://review.opendev.org/c/openstack/ironic/+/906913 | 18:05 |
opendevreview | Merged openstack/ironic stable/2023.1: Kickstart: Don't error unit tests ksvalidate is present https://review.opendev.org/c/openstack/ironic/+/906914 | 18:06 |
JayF | For tests in ironic-tempest-plugin, are nodes I create cleared *per test* or *per class* | 18:25 |
JayF | I'm pretty cure it's per class but not 100% sure | 18:25 |
JayF | yeah, absolutely per class I think | 18:25 |
TheJulia | uhh... it is invoked per class per test I believe | 18:29 |
TheJulia | because it is in setup it get created, tests are executed on a one-on-one basis multiple runners may handle all tests in a class | 18:29 |
opendevreview | Jay Faulkner proposed openstack/ironic-tempest-plugin master: Basic API tests for sharding https://review.opendev.org/c/openstack/ironic-tempest-plugin/+/906749 | 18:34 |
JayF | yeah, that is where my logic fell as well | 18:35 |
JayF | I don't have my devstack to test that, so if you're around and want to poke it with any obvious issues it'd be awesome, otherwise I'll wait for zuul to tell me | 18:35 |
* JayF working on auto shop wifi getting a tire patched | 18:35 | |
TheJulia | err, I'm thinking tempest default, but I think it is similar | 18:35 |
TheJulia | Yeah,, I think you need to refactor your setup on the test level with unique shard names | 18:42 |
JayF | in the first class, yes | 18:43 |
JayF | the second class I did it right | 18:43 |
JayF | because second class I create a set of data, tests are only querying against it | 18:43 |
JayF | (yeah?) | 18:44 |
TheJulia | nah, I think it is wrong because setup gets run per test | 18:44 |
JayF | but cleanup doesn't happen per test | 18:44 |
TheJulia | at the end it does | 18:44 |
JayF | cleanup happens at class teardown | 18:44 |
JayF | jenkies :( | 18:44 |
TheJulia | teardown is called for each test execution | 18:44 |
JayF | So this is fine then? I'll set nodes setup and torn down each execution | 18:45 |
JayF | I don't care what the names are, right? | 18:45 |
TheJulia | yeah as long as you have unique names to avoid collission | 18:45 |
JayF | but teardown deletes the nodes(?) | 18:45 |
TheJulia | it should if they are created in the test | 18:45 |
JayF | I don't understand how that works with my current understanding of setup/teardown, but that at least gives me a pattern I can mimic | 18:47 |
JayF | if setup gets run N times, and teardown gets run N times, and after each teardown $stuff is leftover... I don't get it | 18:48 |
JayF | but I get my car back, bbs | 18:48 |
TheJulia | we go to execute a test, to setup that test the class setup gets called, test run and then teardown | 18:48 |
clarkb | HACKING.rst documents these behaviors though I'm not sure how up to date that doc is | 18:48 |
JayF | Then what names could conflict with what names? That's where I'm stuck on? | 19:08 |
JayF | if teardown has run, nodes created in setup are gone, setup re-runs for the next test | 19:08 |
JayF | unless there's parallelism and operating off the same ironic instance to do so | 19:08 |
JayF | hmmmm | 19:08 |
opendevreview | Jay Faulkner proposed openstack/ironic-tempest-plugin master: Basic API tests for sharding https://review.opendev.org/c/openstack/ironic-tempest-plugin/+/906749 | 19:32 |
TheJulia | and this is your regular reminder, that StrOpt by accident set to boolean False, still returns True :) | 20:06 |
opendevreview | Julia Kreger proposed openstack/ironic master: Fix service role support https://review.opendev.org/c/openstack/ironic/+/907148 | 20:08 |
TheJulia | JayF: Revised ^ with release note | 20:09 |
TheJulia | well, upgrade note in the reno | 20:09 |
JayF | TheJulia: Sorry, but I had to -1 again. I think we're still not on the same page. I was under the impression we were going to default that elevated access off | 20:30 |
JayF | I am going to have a hard time finding a +2 for a change which could be done in custom policy, but without custom policy will enable additional access for operators unless they do something about it :/ | 20:30 |
TheJulia | we can do that, ultimately everyone will just flip the setting by default which is not operator friendly | 20:30 |
JayF | I don't think that's generally true TBH | 20:31 |
JayF | Is there some other path? | 20:31 |
TheJulia | only to configure services as service scope, which folks don't really want to do | 20:32 |
TheJulia | or are not tooled to do | 20:32 |
TheJulia | This is really all fallout of the openstack wide rbac position change | 20:32 |
TheJulia | everyone who wants nova-compute not to care about the owner/lessee fields will either need to flip the setting or use system scope auth, or write whole custom policies which is not viable for distribution users really | 20:33 |
JayF | I think today most folks use system scoped auth | 20:39 |
JayF | I just forsee that at many large installs I would've worked, they would've wanted to set the service_project name to something non-default, and isolate Ironic service project/scope users to Ironic only | 20:39 |
JayF | the idea of "this token is good for service access to read from (for instance) neutron" all the sudden getting Ironic admin access would be incident-inducing in some of those environment | 20:40 |
JayF | The real solution I want is somehow to know on upgrade it's an upgrade, set "false" and for new installs to get "true" | 20:41 |
JayF | but I can want that all I want, we have no way to get there AFAIK | 20:41 |
TheJulia | I don't think that is actually true, but the patch enables that to be setup. What it defaults towards is the generalized defaults leveraged in deployment tools (tripleo, kolla, airship, devstack (see lib/keystone SERVICE_TENANT_NAME)), the only variation I've found is DIY puppet deployment where the default value is "services" instead of "service" for the project name which an admin would need to create. Yes, one could | 20:45 |
TheJulia | isolate ironic to it's own project, but that is not stock sample template configuration. | 20:45 |
TheJulia | To completely disable service scoped access would require it just not to be somehow enabled/present in keystone, and depending on level of paranoia, require carrying custom policy to strike out the default capability, but that is separate from the service project | 20:46 |
TheJulia | I could see it making sense to merge as true, and backport as false, fwiw | 20:46 |
JayF | So to be clear: current (master) RBAC model is -> Service role, on any given project, has service access to that project | 20:47 |
JayF | proposed change would take us to RBAC model is -> service role, on a SPECIFIC project, has a big chunk of what I'd consider system scoped manager/admin access | 20:47 |
JayF | the fact that specific project *is* the same as what is setup by the other deployment tools is exactly what my concern is | 20:48 |
TheJulia | yes, basically | 20:48 |
TheJulia | I'd note, we're the only folks in all of openstack to do that | 20:48 |
TheJulia | everyone else is just "role:service" is just shy of admin or is aliased to admin | 20:48 |
JayF | we are also the only folks in all of openstack to migrate from an admin-only API to an RBAC-aware/user exposed one | 20:48 |
JayF | so I think it's reasonable to be more aware of how things migrate into it | 20:48 |
JayF | TheJulia: https://us06web.zoom.us/j/81323207222?pwd=ltnb7rKfYitDKZWJgzzib2mIfboybb.1 sync? | 20:49 |
TheJulia | True, I do agree with that statement, I think the challenge is a willingness to or ability to leverage a system scope | 20:49 |
TheJulia | sure | 20:49 |
opendevreview | Merged openstack/python-ironic-inspector-client master: Bump hacking to 6.1.0 https://review.opendev.org/c/openstack/python-ironic-inspector-client/+/907045 | 22:07 |
opendevreview | Jay Faulkner proposed openstack/ironic-tempest-plugin master: WIP: Basic API tests for sharding https://review.opendev.org/c/openstack/ironic-tempest-plugin/+/906749 | 22:56 |
opendevreview | Merged openstack/python-ironic-inspector-client master: [codespell] Fixing Spelling Mistakes https://review.opendev.org/c/openstack/python-ironic-inspector-client/+/907115 | 23:07 |
JayF | adamcarthur5: I think all your changes are 1) landed, 2) have valid comments with a -1 or 3) are waiting on gate fixes ... good work! | 23:12 |
* JayF goes to check to see if adamcarthur5 has more commits in Ironic projects than him this cycle before workflowing anything else ;) | 23:12 | |
opendevreview | Julia Kreger proposed openstack/ironic master: Fix service role support https://review.opendev.org/c/openstack/ironic/+/907148 | 23:12 |
TheJulia | JayF: ^^ | 23:14 |
JayF | I clicked away from gerrit opened to that page to acknowledge this ;) | 23:14 |
TheJulia | heh | 23:16 |
JayF | TheJulia: https://review.opendev.org/c/openstack/ironic/+/907148/5#message-c4789466f934d57540ce4110f1adde68c6804c6d a suggestion for wording in the config description, but +1 to indicate overall general agreement | 23:18 |
TheJulia | JayF: maybe just drop the last sentence? | 23:20 |
opendevreview | Julia Kreger proposed openstack/ironic master: Fix service role support https://review.opendev.org/c/openstack/ironic/+/907148 | 23:28 |
JayF | +2 | 23:29 |
opendevreview | Jay Faulkner proposed openstack/ironic-tempest-plugin master: WIP: Basic API tests for sharding https://review.opendev.org/c/openstack/ironic-tempest-plugin/+/906749 | 23:33 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!