opendevreview | Jacob Anders proposed openstack/ironic master: Make redfish firmware update a service step https://review.opendev.org/c/openstack/ironic/+/922815 | 02:08 |
---|---|---|
janders | I need to stack this on top if the RPC/condutor patch, but I would be curious what iurygregory thinks about this approach to tests | 02:09 |
* iurygregory looks | 02:10 | |
iurygregory | janders, in general lgtm, I don't think we would need to cover all the cases since the procedure is similar to what we have in cleaning, and you added important scenarios on it | 02:13 |
janders | iurygregory TY! | 02:14 |
janders | so - should I remove some of these? | 02:14 |
iurygregory | janders, also needs a release note | 02:14 |
iurygregory | I would say is ok to keep as it's | 02:14 |
janders | releasenote++ | 02:14 |
iurygregory | since this is a feature =) | 02:15 |
iurygregory | hopefully we can include servicing in the demo for the OIS Asia presentation :D | 02:16 |
janders | ++ | 02:16 |
iurygregory | or we can at least mention :D | 02:16 |
janders | it's super easy to demo, each of my recent test runs would qualify as one | 02:16 |
opendevreview | Jacob Anders proposed openstack/ironic master: Make redfish firmware update a service step https://review.opendev.org/c/openstack/ironic/+/922815 | 02:17 |
iurygregory | yeah | 02:17 |
janders | (that was just the rebase on top of https://review.opendev.org/c/openstack/ironic/+/922811) | 02:17 |
janders | should help tests pass, there is just one more failing test I need to figure out | 02:17 |
janders | most seem to work fine | 02:18 |
iurygregory | ack o/ | 02:19 |
JayF | thanks janders; that was on my list to check \o/ | 02:21 |
iurygregory | CI should probably be happier since https://review.opendev.org/c/openstack/devstack/+/881580 is merged | 02:21 |
JayF | firmware update is basically why I wanted servicing/runbooks | 02:21 |
janders | JayF++ | 02:23 |
janders | we're getting pretty close to having it working quite nice in a minimum feature set | 02:25 |
janders | I think we need a servicing version of this: https://opendev.org/openstack/ironic/src/branch/master/ironic/conductor/utils.py#L449-L518 and the tests from 922815 should pass | 02:27 |
janders | (and likely cleanup_cleanwait_timeout() equivalent too) | 02:28 |
JayF | I would consider many of those backportable bugfixes | 02:31 |
JayF | since we advertised working servicing last cycle | 02:31 |
JayF | IDK how others would feel about that | 02:32 |
janders | JayF I think RPCAPI version bump due to missing methods will make it a bit harder | 02:32 |
janders | to sell | 02:32 |
JayF | I would say it's a complete nogo, but I would wonder: the behavior on a mixed-upgrade would be ... broken, just like it is right now? | 02:34 |
JayF | so I would have to read policy to see exactly if it's technically allowed | 02:34 |
JayF | but I suspect it's a case where the actual impact might be zero | 02:34 |
JayF | I wonder how that intersects with the release mappings... hmm | 02:35 |
TheJulia | so there are fixes, and then breaking changes | 02:37 |
TheJulia | versioning being breaking, but why would we need to bump the version to backport utility helpers and I guess a periodic ? | 02:37 |
TheJulia | if they are not rpc methods, it is fine | 02:37 |
TheJulia | and generally sorry I haven't been super interactive, I've been pulled in way too many directions | 02:38 |
janders | meanwhile I've gotten quiet because fixing that one-last unit test sent me down a rabbit hole of finding a whole more of servic(ing) functions that missing that aren't strictly speaking used by what I am doing | 02:39 |
TheJulia | so | 02:40 |
TheJulia | the issue is there is a bit of code in the conductor which calls back the rpc bus | 02:40 |
TheJulia | and that itself is a bug | 02:40 |
TheJulia | which is the pattern we later discovered created the continue_node_$thing | 02:41 |
TheJulia | I remember cid was looking at that as cleanup and discovered that bit of code and we kind of went "maybe we don't rip that out" | 02:42 |
TheJulia | anyway, there is always bugs in brand new massive things. Fix/Backport what we can. | 02:44 |
TheJulia | encourage folks to move forward to newer versions as they drop | 02:44 |
opendevreview | Jacob Anders proposed openstack/ironic master: Make redfish firmware update a service step https://review.opendev.org/c/openstack/ironic/+/922815 | 03:39 |
opendevreview | Jacob Anders proposed openstack/ironic master: Make redfish firmware update a service step https://review.opendev.org/c/openstack/ironic/+/922815 | 03:40 |
opendevreview | Jacob Anders proposed openstack/ironic master: Add RPC calls and handlers needed to support async service steps https://review.opendev.org/c/openstack/ironic/+/922811 | 04:14 |
janders | ^^ first lot of tests in, still WIP, need some chats/guidance when EMEA folks come online | 04:14 |
opendevreview | Verification of a change to openstack/ironic master failed: Fix execution of node servicing steps exposed by IPA's HardwareManager https://review.opendev.org/c/openstack/ironic/+/922024 | 06:38 |
rpittau | good morning ironic! o/ | 06:39 |
shajizad | Good morning o/ | 08:29 |
shajizad | Both my patches passed zuul btw | 08:29 |
masghar | nice! | 08:30 |
rpittau | shajizad: great :) | 08:37 |
opendevreview | Merged openstack/ironic master: Fix execution of node servicing steps exposed by IPA's HardwareManager https://review.opendev.org/c/openstack/ironic/+/922024 | 08:59 |
opendevreview | Jacob Anders proposed openstack/ironic master: Add RPC calls and handlers needed to support async service steps https://review.opendev.org/c/openstack/ironic/+/922811 | 11:14 |
janders | ^^ tests added and passing, iurygregory dtantsur when you have time I'd appreciate if you can have another look. | 11:16 |
iurygregory | good morning Ironic | 11:16 |
iurygregory | janders, sure I will take a second look o/ | 11:16 |
janders | iurygregory TY o/ | 11:17 |
janders | (and good morning) | 11:17 |
opendevreview | Jacob Anders proposed openstack/ironic master: Make redfish firmware update a service step https://review.opendev.org/c/openstack/ironic/+/922815 | 11:18 |
janders | ^^ rebased on the latest version of the RPC change | 11:19 |
shajizad | iurygregory: The reason i used 'port-uuid' in my testcases is because that's how the tests were written for the actual ironic api. | 11:21 |
shajizad | Here: https://github.com/openstack/ironic/commit/e8f6fdd56d7eccbdeba3ff3d1101a8dc6a8be72f | 11:21 |
shajizad | def test_vif_attach_port_uuid_and_portgroup_uuid(self, mock_attach,... (full message at <https://matrix.org/_matrix/media/v3/download/matrix.org/DhskmKqfrbfKvQhQblOybiyw>) | 11:22 |
shajizad | * def test_vif_attach_port_uuid_and_portgroup_uuid(self, mock_attach, | 11:22 |
shajizad | mock_get): | 11:22 |
shajizad | vif_id = uuidutils.generate_uuid() | 11:22 |
shajizad | request_body = { | 11:22 |
shajizad | 'id': vif_id, | 11:22 |
shajizad | 'port_uuid': 'port-uuid', | 11:22 |
shajizad | 'portgroup_uuid': 'portgroup-uuid' | 11:22 |
shajizad | } | 11:22 |
iurygregory | I see | 11:22 |
iurygregory | I added +1 is just a minor thing I noticed, let's see what others think | 11:23 |
iurygregory | I will try to check the other patches today to provide feedback o/ | 11:23 |
shajizad | Thank you | 11:23 |
iurygregory | janders, I've added some comments on it, just related to unit tests for the conductor utils and conductor rpcapi | 11:37 |
janders | iurygregory TY! | 11:38 |
opendevreview | Dmitry Tantsur proposed openstack/ironic master: Update the redfish interoperability profile https://review.opendev.org/c/openstack/ironic/+/920574 | 12:41 |
dtantsur | rpittau: typo fixed ^^ | 12:41 |
rpittau | +2 ed :) | 12:42 |
dtantsur | thx | 12:46 |
Sandzwerg[m] | hmm the sushy documentation still links to storyboard as bugtracker is that correct? | 14:01 |
rpittau | Sandzwerg[m]: no, that should be launchpad | 14:03 |
opendevreview | Riccardo Pittau proposed openstack/ironic master: Test empty media type in redfish virtual media boot https://review.opendev.org/c/openstack/ironic/+/922947 | 14:03 |
opendevreview | Mahnoor Asghar proposed openstack/ironic master: Fix log statement about starting inspection https://review.opendev.org/c/openstack/ironic/+/922948 | 14:16 |
opendevreview | Dmitry Tantsur proposed openstack/ironic master: Update the redfish interoperability profile https://review.opendev.org/c/openstack/ironic/+/920574 | 14:24 |
opendevreview | Dmitry Tantsur proposed openstack/ironic master: Render the redfish interop profile in the docs https://review.opendev.org/c/openstack/ironic/+/921298 | 14:34 |
opendevreview | cid proposed openstack/ironic master: Deprecated field in Redfish Driver https://review.opendev.org/c/openstack/ironic/+/922951 | 14:39 |
opendevreview | cid proposed openstack/ironic master: Deprecated field in Redfish Driver https://review.opendev.org/c/openstack/ironic/+/922951 | 14:40 |
rpittau | good night! o/ | 15:25 |
*** atmark_ is now known as atmark | 17:11 | |
opendevreview | cid proposed openstack/ironic master: Deprecated field in Redfish Driver https://review.opendev.org/c/openstack/ironic/+/922951 | 19:17 |
opendevreview | cid proposed openstack/ironic master: Deprecated field in Redfish Driver https://review.opendev.org/c/openstack/ironic/+/922951 | 19:20 |
opendevreview | cid proposed openstack/ironic master: Deprecated field in Redfish Driver https://review.opendev.org/c/openstack/ironic/+/922951 | 19:24 |
cid | o/ | 19:25 |
opendevreview | Jacob Anders proposed openstack/ironic master: Add RPC calls and handlers needed to support async service steps https://review.opendev.org/c/openstack/ironic/+/922811 | 23:10 |
janders | ^^ should be getting close with this one | 23:11 |
opendevreview | Jacob Anders proposed openstack/ironic master: Make redfish firmware update a service step https://review.opendev.org/c/openstack/ironic/+/922815 | 23:46 |
opendevreview | Jacob Anders proposed openstack/ironic master: Make redfish firmware update a service step https://review.opendev.org/c/openstack/ironic/+/922815 | 23:48 |
janders | ^ also | 23:53 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!