| opendevreview | Iury Gregory Melo Ferreira proposed openstack/sushy master: Don't fail to eject vmedia in case there is no vmedia https://review.opendev.org/c/openstack/sushy/+/952730 | 01:21 |
|---|---|---|
| iurygregory | TheJulia, dtantsur ^ initial idea | 01:23 |
| iurygregory | didn't add tests or releasenotes, just to have an idea if it's ok | 01:23 |
| rpittau | good morning ironic! o/ | 07:22 |
| queensly[m] | Good morning | 08:34 |
| opendevreview | Abhishek Bongale proposed openstack/ironic-tempest-plugin master: WIP Add Tempest tests for inspection rules in Ironic https://review.opendev.org/c/openstack/ironic-tempest-plugin/+/951761 | 10:56 |
| iurygregory | good morning ironic | 11:20 |
| dtantsur | iurygregory: I need to ponder it.. my other thought was to catch any exceptions initially, then re-load the resource and check if the image is still connected | 11:56 |
| iurygregory | dtantsur, so you were thinking in doing in the ironic side? https://github.com/openshift/openstack-ironic/blob/main/ironic/drivers/modules/redfish/boot.py#L414 | 11:58 |
| dtantsur | iurygregory: it could be done in sushy as well | 11:59 |
| dtantsur | you can call self.refresh(force=True) | 11:59 |
| iurygregory | oh right | 11:59 |
| * iurygregory need more coffee | 12:00 | |
| iurygregory | dtantsur, something along https://paste.opendev.org/show/bpD4ggSgzrqu7Ji1zyaB/ ? | 12:10 |
| dtantsur | iurygregory: I think we need to catch HTTPError, not literally any exceptions (sorry for confusion). Plus logging. Otherwise, yes, that's what I had in mind. | 12:19 |
| dtantsur | (assuming it works lol) | 12:20 |
| iurygregory | we would have two except exceptions.HTTPError, but it could be ok, and we should only re-load and check if the image is still connected in the second one | 12:23 |
| opendevreview | Julia Kreger proposed openstack/ironic-python-agent-builder master: Add Mellanox Spectrum to firmware to remove list https://review.opendev.org/c/openstack/ironic-python-agent-builder/+/952756 | 12:24 |
| TheJulia | good morning | 12:43 |
| opendevreview | Takashi Kajinami proposed openstack/ironic master: Revert "Replace license classifier" https://review.opendev.org/c/openstack/ironic/+/952763 | 13:23 |
| opendevreview | Riccardo Pittau proposed openstack/ironic bugfix/30.0: Disable metal3 job in bugfix/30.0 branch https://review.opendev.org/c/openstack/ironic/+/952764 | 13:24 |
| opendevreview | Abhishek Bongale proposed openstack/ironic-tempest-plugin master: WIP Add Tempest tests for inspection rules in Ironic https://review.opendev.org/c/openstack/ironic-tempest-plugin/+/951761 | 13:24 |
| tkajinam | there were some pushbacks about the classifier changes I made some time ago and it seems it's better to wait until we see a consensus within python/openstack community. I've prepared revert for the related ironic repos, so it'd be nice if we can merge these now https://review.opendev.org/q/topic:%22license-classifier%22+is:pure-revert | 13:25 |
| tkajinam | the commit message includes the link to the nova change where you can find part of our discussions | 13:26 |
| tkajinam | sorry for the confusion ... | 13:26 |
| rpittau | tkajinam: no worries, I've added +2 to all the reverts, just missing approval | 13:28 |
| tkajinam | rpittau, thx ! | 13:28 |
| opendevreview | Iury Gregory Melo Ferreira proposed openstack/sushy master: Don't fail to eject vmedia in case there is no vmedia https://review.opendev.org/c/openstack/sushy/+/952730 | 13:35 |
| TheJulia | I'm not sure I understand, because the classifier was also generating deprecation notices that your original patch apeared to remedy, so I guess I'm confused | 13:36 |
| iurygregory | dtantsur, let me know if you think I should add more logging, I've pushed a new version (working on the unit tests right now) | 13:38 |
| dtantsur | commented | 13:40 |
| tkajinam | TheJulia, the core problems is that setuptools introduced the warning to encourage people to adopt to PEP 639, while there are still active feedback about PEP 639 . | 13:41 |
| iurygregory | dtantsur, for the vmd_exc you want warning level? | 13:42 |
| dtantsur | iurygregory: yes, I think the missing message should be a warning, while the existing should come as a debug | 13:42 |
| iurygregory | ack o/ | 13:42 |
| iurygregory | will work on the unit test + release note and update the logging | 13:42 |
| opendevreview | Merged openstack/sushy-tools master: Revert "Replace license classifier" https://review.opendev.org/c/openstack/sushy-tools/+/952080 | 13:43 |
| TheJulia | iurygregory: I like the overall idea, I just think we likely need to also consider whatever the error value is.... since your already in fallback logic there | 13:43 |
| opendevreview | Merged openstack/python-ironicclient master: Revert "Replace license classifier" https://review.opendev.org/c/openstack/python-ironicclient/+/952083 | 13:45 |
| iurygregory | TheJulia, by error value you mean the status_code we would get? | 13:46 |
| TheJulia | tkajinam: wow, I'm not reading the messages on that one but surprising given the dates, classified as final, but then there is the huge thread still from last year | 13:46 |
| TheJulia | iurygregory: a status code or text or something, anything other than any HTTPError | 13:46 |
| TheJulia | Since, again, your already in fallback retry logic there too | 13:47 |
| dtantsur | I just asked not to rely on text and rely on HTTPError :) | 13:48 |
| dtantsur | how can we know it's not going to change in the next firmware patch update? | 13:48 |
| iurygregory | vendors... | 13:48 |
| dtantsur | especially since changing HTTP 500 to 4xx is usually not considered a breaking change? | 13:48 |
| TheJulia | I'd say error code, perhaps? I guess my concern is we're sort of assuming any error here is this exact issue, and it sort of makes sense. I dunno, maybe I'm being too persnickety | 13:49 |
| opendevreview | Merged openstack/ironic-inspector master: Revert "Remove license classifier" https://review.opendev.org/c/openstack/ironic-inspector/+/952084 | 13:49 |
| TheJulia | I guess my concern is what if we're in an auth failure or something else is going on and any HTTPError causes us to begin to think that | 13:49 |
| TheJulia | The refresh *does* sort of guard it, dunno | 13:50 |
| abongale | Hi all, I’m working on writing a tempest test for the inspection rules phase (early and preprocess) that simply performs a log action. From my understanding, this should be permitted but received 400 error. Only seeing the main phase available for this test? | 13:50 |
| abongale | I’m running the test against the ironic-tempest-functional-python3-2025.1 and wanted to check if anyone has encountered this issue or has insights on why the early and preprocess phases might not be available. | 13:50 |
| abongale | For reference, here are the logs I’m looking at: | 13:50 |
| abongale | https://storage.bhs.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_94b/openstack/94b5abf668bb4f05a5cf34807c1eef23/testr_results.html | 13:50 |
| TheJulia | iurygregory: what hardware was this observed on? | 13:50 |
| abongale | https://zuul.opendev.org/t/openstack/buildset/ab2ef6eae04147968aa1cf5a3bd2c03a | 13:50 |
| abongale | Any help would be appreciated! | 13:50 |
| abongale | cc: cid | 13:50 |
| dtantsur | TheJulia, iurygregory, we can do status code == 400 or >= 500 | 13:50 |
| dtantsur | (maybe include whatever code CONFLICT has, 406?) | 13:50 |
| TheJulia | dtantsur: that seems super reasonable to me | 13:50 |
| dtantsur | 409 (I never can remember it) | 13:51 |
| TheJulia | heh | 13:51 |
| opendevreview | Merged openstack/ironic-ui master: Revert "Replace license classifier" https://review.opendev.org/c/openstack/ironic-ui/+/952081 | 13:51 |
| TheJulia | I think if discussion recall is correct this was an idrac, I know at least one of the vendors only holds an attachment for like 15-20 minutes | 13:51 |
| TheJulia | but they may also eat any error then | 13:52 |
| iurygregory | TheJulia, Dell PowerEdge XR8620t | 13:52 |
| TheJulia | since your requesting the same state it is already in | 13:52 |
| TheJulia | Yeah, 409 makes tons of sense because if any vendor were to explicitly raise an error, it would be Dell | 13:53 |
| TheJulia | so going back to abongale's question, I think there is a mismatch between the introspection rules spec which he is working from and the actual end state result. | 13:53 |
| dtantsur | yeah, I don't think all features from the spec got implemented, some definitely got tabled | 13:54 |
| dtantsur | it's better to check the API reference | 13:54 |
| iurygregory | https://paste.opendev.org/show/bZsMKDnjzBl8e1ejERtB/ currently from the test it gives 500 | 13:54 |
| iurygregory | so adding status_code > 400 in the if not self.inserted ? | 13:59 |
| cid | abongale, I think only the 'main' phase is supported. Support for the other phases is yet to happen. | 13:59 |
| iurygregory | or you want to check specific codes (400, 409, >= 500) | 13:59 |
| cid | abongale, So your testing should likely only be for the 'main' phase. | 14:00 |
| TheJulia | iurygregory: i'd check specific codes | 14:00 |
| abongale | noted, thanks @cid | 14:00 |
| opendevreview | Merged openstack/ironic bugfix/30.0: Disable metal3 job in bugfix/30.0 branch https://review.opendev.org/c/openstack/ironic/+/952764 | 14:00 |
| iurygregory | TheJulia, ok | 14:04 |
| opendevreview | Merged openstack/sushy master: Revert "Replace license classifier" https://review.opendev.org/c/openstack/sushy/+/952082 | 14:14 |
| opendevreview | Merged openstack/ironic master: Revert "Replace license classifier" https://review.opendev.org/c/openstack/ironic/+/952763 | 14:17 |
| abongale | Hey cid, are there any plans to implement the preprocess and early phases in the future? | 14:35 |
| opendevreview | Abhishek Bongale proposed openstack/ironic stable/2025.1: fix: 'built_in' KeyError on invalid priority. https://review.opendev.org/c/openstack/ironic/+/952771 | 14:37 |
| cid | abongale, Not plans necessarily. But I think that's something that will has to happen eventually. | 14:40 |
| JayF | I don't think that work is someone GR-OSS would prioritize. | 14:50 |
| JayF | **something | 14:50 |
| JayF | happy to provide reviews and implementation help if someone does though | 14:50 |
| TheJulia | I read that as "should" happen, but not "will" happen. | 14:54 |
| TheJulia | and even more to the point, the minimum viable need has been met, the main phase. A note on the spec might be the most appropriate thing at this point. | 14:54 |
| cid | +++ | 14:57 |
| cid | I just replied to the API change, the commit message mentions a few of the functionalities that got tabled as otherwise specified. | 14:59 |
| cid | https://review.opendev.org/c/openstack/ironic/+/939217 | 14:59 |
| cid | Abongale, ^^. but yeah, I think the spec might need updating to make sure we capture exactly what's done and not done (yet). | 15:00 |
| JayF | can someone put a review+vote on https://review.opendev.org/c/openstack/ironic/+/952709 | 15:21 |
| JayF | I'd like to land it so we can test the wsgi change on top of it | 15:22 |
| iurygregory | JayF, done | 15:43 |
| JayF | ack ty | 15:43 |
| TheJulia | If we can get another review on https://review.opendev.org/c/openstack/ironic/+/952709 it would be great, I've got 10% ramdisk reduction change queued up which failed due to the overall pressure as well :( | 15:43 |
| TheJulia | alegacy: Commented on your spec, overall pretty good, some concerns but those concerns shouldn't be taken as direct action, they are more so items we're likely going to need to think about and keep in mind. Like logical progressions or step 2 after step 0 and step 1 are completed. | 15:55 |
| TheJulia | adamcarthur5: o/ So, I'm going to apologize now for the flood of comments ;) | 15:57 |
| adamcarthur5 | 👍 | 15:57 |
| JayF | We put that spec up to give a place for those comments to go :D | 16:09 |
| TheJulia | Yeah | 16:09 |
| TheJulia | I'm up to a comment on every 3-4 lines right now ;) | 16:10 |
| TheJulia | (a lot of it is because Julia has built DCs) | 16:10 |
| TheJulia | okay, maybe ~6 lines | 16:12 |
| TheJulia | Taking a break before moving beyond the base context ;) | 16:17 |
| TheJulia | Totally agree with JayF, lets pick 2 | 16:17 |
| opendevreview | Merged openstack/ironic master: Increase memory allocation for ironic-base https://review.opendev.org/c/openstack/ironic/+/952709 | 16:47 |
| JayF | I just got contacted by the ubuntu stable team | 16:57 |
| JayF | about patching dnsmasq | 16:57 |
| TheJulia | oh? | 17:00 |
| JayF | https://bugs.launchpad.net/ironic/+bug/2026757/comments/62 | 17:00 |
| JayF | if they can get me a direct deb link, I'll push an update to see if it does the trick | 17:01 |
| TheJulia | cool cool | 17:03 |
| opendevreview | Julia Kreger proposed openstack/ironic-python-agent-builder master: Add Mellanox Spectrum to firmware to remove list https://review.opendev.org/c/openstack/ironic-python-agent-builder/+/952756 | 17:19 |
| dtantsur | iurygregory: I'll forget to say tomorrow, and today I'm already tired so ignore me if I'm completely off. It seems like you have an issue with a Dell, but you're adding your code to the 'except' block written for HPE. Could you double check? | 18:14 |
| iurygregory | dtantsur, well, the code path showed in the logs show it attempts to execute the self._conn.post(target_uri, data={}) in the if condition that mentions HPE | 18:18 |
| dtantsur | hmmmm | 18:19 |
| dtantsur | and unfortunately we don't have any logs that could tell us why the first attempt failed | 18:19 |
| iurygregory | let me share here | 18:20 |
| dtantsur | (I'll check tomorrow, dinner time for real) | 18:21 |
| iurygregory | dtantsur, I will let the link here https://paste.opendev.org/show/b0pmScmBwiel2OqVGHW7/ seems like the first attempt says it fail because of media type so we do reach the code block https://github.com/openstack/sushy/blob/master/sushy/resources/manager/virtual_media.py#L261 | 18:27 |
| TheJulia | Its looking like https://review.opendev.org/c/openstack/ironic-python-agent-builder/+/952756 should be good | 18:38 |
| JayF | I just approved eventlet wsgi removal. ironic-standalone-redfish showing passed in zuul status page \o/ | 19:40 |
| TheJulia | \o/ | 20:24 |
| JayF | fwiw gate immediately failed due to a blip downloading etcd | 20:25 |
| JayF | I'm gonna recheck once it fails through | 20:26 |
| TheJulia | We ought to turn it off | 20:38 |
| opendevreview | Jay Faulkner proposed openstack/ironic master: WIP: Add node.instance_name as API microversion 1.100 https://review.opendev.org/c/openstack/ironic/+/952790 | 20:51 |
| opendevreview | Verification of a change to openstack/ironic master failed: Eventlet: Migrate API & JSON-RPC to cheroot https://review.opendev.org/c/openstack/ironic/+/951054 | 21:07 |
| * JayF kicks ^ | 21:09 | |
| opendevreview | Verification of a change to openstack/ironic master failed: Eventlet: Migrate API & JSON-RPC to cheroot https://review.opendev.org/c/openstack/ironic/+/951054 | 21:30 |
| opendevreview | Jay Faulkner proposed openstack/ironic master: WIP: Add node.instance_name as API microversion 1.100 https://review.opendev.org/c/openstack/ironic/+/952790 | 22:04 |
| -opendevstatus- NOTICE: Zuul jobs reporting POST_FAILURE were due to an incident with one of our cloud providers; this provider has been temporarily disabled and changes can be rechecked | 22:37 | |
Generated by irclog2html.py 4.0.0 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!