| opendevreview | Jacob Anders proposed openstack/ironic master: [WIP] Add support for verify steps https://review.opendev.org/c/openstack/ironic/+/800001 | 00:01 |
|---|---|---|
| opendevreview | Bob Fournier proposed openstack/ironic master: Add a section to redfish doc for BIOS registry fields https://review.opendev.org/c/openstack/ironic/+/793982 | 01:57 |
| iurygregory | good morning Ironic | 06:42 |
| arne_wiebalck | Good morning iurygregory and Ironic! | 06:43 |
| iurygregory | morning arne_wiebalck o/ | 06:43 |
| janders | good morning iurygregory arne_wiebalck and Ironic o/ | 06:56 |
| arne_wiebalck | hey janders o/ | 06:56 |
| iurygregory | good afternoon janders o/ | 06:56 |
| *** pmannidi is now known as pmannidi|brb | 07:09 | |
| * MahnoorAsghar Hey! | 07:41 | |
| * MahnoorAsghar Can someone tell me how lazy consensus on an RFE works? | 07:42 | |
| iurygregory | MahnoorAsghar, this will probably explain to you https://docs.openstack.org/ironic/latest/contributor/bugs.html#triaging-guide =) | 07:45 |
| * MahnoorAsghar thank you! | 07:46 | |
| iurygregory | np | 07:46 |
| dtantsur | good morning | 07:54 |
| *** pmannidi|brb is now known as pmannidi | 07:59 | |
| iurygregory | morning dtantsur | 08:04 |
| * MahnoorAsghar Do people normally add the needs-spec tag on the RFE themselves? | 08:29 | |
| dtantsur | MahnoorAsghar: normally it happens after a review on the weekly meeting | 08:30 |
| MahnoorAsghar | I see, I see | 08:31 |
| MahnoorAsghar | I will wait till the next meeting, then. | 08:32 |
| dtantsur | MahnoorAsghar: before that I suggest you clean up your proposal to have only one option. I think we more or less ruled out options 1 and 2 by this time, the option #3 looks interesting though. | 08:34 |
| dtantsur | "We would like to understand the values different vendors give in" is something that also needs to happen before the RFE is approved | 08:35 |
| dtantsur | also you're talking about different drivers. your proposal only applies to Redfish, so let's keep it Redfish specific | 08:35 |
| MahnoorAsghar | the mailing list is more appropriate for that discussion, I will remove that | 08:35 |
| dtantsur | on the other hand, I'm not sure it needs a spec | 08:35 |
| MahnoorAsghar | okay | 08:36 |
| dtantsur | I suggest you reframe the RFE so that it's talking about the Redfish RAID interface accepting an additional name schema | 08:36 |
| MahnoorAsghar | that sounds better | 08:36 |
| dtantsur | the other drivers may choose to implement something similar, but it may not be based on PhysLocation>PartLocation>ServiceLabel if they don't have them | 08:36 |
| MahnoorAsghar | yes, Dell and HP dont have those values | 08:37 |
| dtantsur | mmm, on what hardware you have you tested your approach then? | 08:37 |
| MahnoorAsghar | I have not developed/tested it yet. Just extracted the Redfish values on Dell hardware, and found data for HP hardware. | 08:38 |
| dtantsur | could you clarify your comment "Dell and HP don't have those values" | 08:39 |
| dtantsur | do they or do they not? | 08:39 |
| * MahnoorAsghar They have empty strings in place of PhysicalLocation.PartLocation.ServiceLabel | 08:39 | |
| dtantsur | have you found a hardware model that does have any information there? | 08:40 |
| * MahnoorAsghar No, just have Dell R740 servers to test on | 08:40 | |
| dtantsur | and it does not have the information you need? | 08:41 |
| dtantsur | I'm a bit worried about implementing a feature while we're not sure if any hardware supports it | 08:41 |
| * MahnoorAsghar It does not have this property. In that case, the proposed fallback concatenates "Drive"and "Id" | 08:42 | |
| MahnoorAsghar | Drive is the (fixed) resource name and Id is given by the vendor. In that case, I was thinking the disk name translation approach for each vendor driver is better. But should atleast 1 working demo to make a convincing case? | 08:43 |
| MahnoorAsghar | I remember the concern was that it is not possible to implement such a translation | 08:44 |
| dtantsur | MahnoorAsghar: Id is what the driver uses now, no? | 08:44 |
| MahnoorAsghar | yes | 08:44 |
| dtantsur | yeah, I'd prefer your approach to be proved working before we commit to it | 08:44 |
| MahnoorAsghar | there is no other glaring concern for this approach, right? | 08:45 |
| dtantsur | no, it's reasonable. please update the RFE and conduct some research on whether it's possible | 08:45 |
| MahnoorAsghar | okay. Sounds good! | 08:46 |
| dtantsur | btw we have some hardware in internal labs. if you tells us what to check (e.g. provide a python script), we can check on some hardware. | 08:46 |
| dtantsur | ping iurygregory or myself for that | 08:46 |
| * MahnoorAsghar That would be very helpful, thank you! I may send a python script | 08:47 | |
| iurygregory | yup =) | 08:47 |
| jssfr | I’m back at looking into adding IPMI MAC address detection to IPA. Looking at HardwareManager now. I want to add a `get_bmc_mac` method there. I think it might be sensible to let the default implementation return "00:00:00:00:00:00", the same value the real implementation in GenericHardwareManager would return if no LAN channel existed. | 10:38 |
| jssfr | That would help existing subclass implementations of HardwareManager to continue to function and not fail with an exception instead. | 10:38 |
| jssfr | consumers of the data need to be able to handle "00:00:00:00:00:00" if no LAN channel is present anyway. | 10:39 |
| jssfr | Alternatively, I could omit the field (by catching the IncompatibleHardwareMethodError exception in list_hardware_info) if the HardwareManager impl does not support getting the BMC MAC. | 10:39 |
| dtantsur | I'd rather omit this field | 10:54 |
| dtantsur | returning 0.0.0.0 for IPMI address is a legacy artefact IMO | 10:54 |
| jssfr | wfm! | 10:57 |
| MahnoorAsghar | dtantsur: Could you check the disk names on your hardware please? | 11:24 |
| MahnoorAsghar | or iurygregory: | 11:24 |
| dtantsur | how exactly? | 11:24 |
| dtantsur | I have one Dell and one HPE machine handy, janders may have SuperMicro around | 11:24 |
| * MahnoorAsghar 'pip install redfish-utilities' | 11:24 | |
| * MahnoorAsghar Also, 'git clone https://github.com/DMTF/Redfish-Tacklebox' | 11:25 | |
| * MahnoorAsghar Now, in ~/.local/lib/python3.6/site-packages/redfish_utilities/inventory.py, after line 212, add 'print(f'{catalog["Label"]}')' | 11:27 | |
| iurygregory | any curl that we can use to not have to install things in the machine? :D | 11:28 |
| * MahnoorAsghar And, in the directory in which you cloned the Redfish-Tacklebox repository, find the script: scripts/rf_sys_inventory.py; and comment the line 38 | 11:29 | |
| * MahnoorAsghar iurygregory: not sure...it uses modules installed in that package | 11:29 | |
| dtantsur | MahnoorAsghar: what Redfish resources are we looking at? | 11:30 |
| MahnoorAsghar | iurygregory: Can a python virtualenv work? | 11:31 |
| MahnoorAsghar | dtantsur: The Drive resource | 11:31 |
| iurygregory | ok so we can curl the Drive resource =) | 11:31 |
| * MahnoorAsghar Perfect! | 11:31 | |
| dtantsur | yep, on a Dell machine I'm looking at it's v1/Systems/System.Embedded.1/Storage/Drives/Disk.Direct.1-1:AHCI.Embedded.1-1 | 11:35 |
| dtantsur | you can use a Sushy connection to make your life a bit easier | 11:36 |
| dtantsur | MahnoorAsghar: Dell: https://paste.opendev.org/show/807464/ | 11:36 |
| * MahnoorAsghar oops,sorry | 11:36 | |
| * MahnoorAsghar oh I thought I pasted that somehow xD | 11:37 | |
| * MahnoorAsghar It should have had 'Physical Location' > 'Part Location' > 'Service Label' | 11:38 | |
| * MahnoorAsghar But the entire Physical Location property is not available | 11:39 | |
| dtantsur | on the HPE machine I don't see Storage resource at all | 11:39 |
| * MahnoorAsghar theres got to be some disks in there? | 11:39 | |
| dtantsur | sure, but they're not necessarily exposed in Redfish | 11:40 |
| dtantsur | it's HPE gen 9, its redfish support is incomplete | 11:40 |
| dtantsur | lemme see if I have gen 10 anywhere | 11:40 |
| * MahnoorAsghar I see | 11:40 | |
| dtantsur | MahnoorAsghar: I've found a gen10, and it doesn't have a Storage resource at all | 11:47 |
| dtantsur | I wonder how the drivers work... | 11:47 |
| dtantsur | stendulker_: if you're around ^^^ | 11:47 |
| iurygregory | magic | 11:47 |
| dtantsur | or rather: gen9 doesn't have a Storage resource, gen10 has it, but it has no members | 11:48 |
| * MahnoorAsghar dtantsur: thats strange | 11:48 | |
| * MahnoorAsghar Can wew try something like: redfish/v1/Chassis/{ChassisId}/Drives/{DriveId} | 11:49 | |
| dtantsur | drives are part of the storage, not chassis, no? | 11:49 |
| * MahnoorAsghar I think there are multiple ways of querying Drives | 11:51 | |
| * MahnoorAsghar (Pg 239 of https://www.dmtf.org/sites/default/files/standards/documents/DSP0268_2021.1.pdf) | 11:52 | |
| dtantsur | chassis doesn't have drives | 11:52 |
| dtantsur | (on the machine I'm looking at) | 11:52 |
| janders | mraineri I have a Redfish question related to virtual media. On some servers (particularly SuperMicro X11SPW-TF and SYS-210P-FRDN6T) we are observing issues where sushy library is treating Inserted property as read-write while the servers treat it as read-only. This is causing virtual media configuration failures. Which approach is correct as far as the standard is concerned? | 11:53 |
| * MahnoorAsghar dtantsur: okay... | 11:53 | |
| iurygregory | DriveCollection points to many different ways to get the info =( | 11:54 |
| iurygregory | page 76 | 11:54 |
| dtantsur | I'm looking at proliantutils code, and it seems that RAID is implemented via an OEM extension | 11:54 |
| janders | mraineri: do we need to get vendors to fix their firmware, or do we need to update sushy? Reference code: https://github.com/openstack/sushy/blob/master/sushy/resources/manager/virtual_media.py#L105. Thanks! | 11:55 |
| * MahnoorAsghar dtantsur: so RAID is not implemented via the iLO driver? | 11:56 | |
| dtantsur | MahnoorAsghar: it's implemented, but via an Oem extension rather than the standard Storage API | 11:58 |
| stendulker_ | MahnoorAsghar: Disk drives would be available under smartstorage in Gen10 | 11:58 |
| MahnoorAsghar | dtantsur: oh okay | 11:58 |
| * MahnoorAsghar stendulker: thank you! | 11:59 | |
| stendulker_ | MahnoorAsghar: You can see smaple data here https://opendev.org/x/proliantutils/src/branch/master/proliantutils/tests/redfish/json_samples/disk_drive.json#L64-L146 | 11:59 |
| * MahnoorAsghar stendulker: thanks! you shared this link with me earlier too ^-^ | 11:59 | |
| stendulker_ | MahnoorAsghar: yes, had pasted in storyboard | 12:00 |
| dtantsur | stendulker_: thank you! so I guess iLO gen10 doesn't support the Redfish standard RAID, only via SmartStorage? | 12:00 |
| stendulker_ | dtantsur: yes, it does not | 12:00 |
| dtantsur | ack | 12:01 |
| MahnoorAsghar | stendulker: We're trying to verify whether we can get a value for 'Physical Location'.' Part | 12:01 |
| MahnoorAsghar | Location'.' Service Label' on hp hardware | 12:01 |
| dtantsur | janders: what would a sushy fix look like? | 12:02 |
| dtantsur | (re Inserted) | 12:02 |
| stendulker_ | This is another place you can see the REST data for ilo5 https://hewlettpackard.github.io/ilo-rest-api-docs/ilo5/ | 12:03 |
| janders | dtantsur: I'm not 100% sure, considering two approaches | 12:03 |
| janders | dtantsur: naive: just patch sushy not to specify "Inserted" at all | 12:03 |
| janders | dtantsur: looking at the RF schema it seems optional. I never specified Inserted in my curl/API level testing and haven't seen any problems with that | 12:03 |
| stendulker_ | MahnoorAsghar: It's in this format "PhysicalDrives": [ | 12:04 |
| stendulker_ | { | 12:04 |
| stendulker_ | "LegacyBootPriority": "None", | 12:04 |
| stendulker_ | "Location": "P1I:3:1", | 12:04 |
| stendulker_ | "LocationFormat": "ControllerPort:Box:Bay" | 12:04 |
| stendulker_ | } | 12:04 |
| stendulker_ | ], | 12:04 |
| janders | dtantsur: a minimum-change approach: keep doing what we're doing for non-SuperMicro and when we see a SuperMicro server, then change behavior (and don't provide Inserted in the vmedia mount request) | 12:04 |
| MahnoorAsghar | stendulker_: Yes. In that case, the primary logic wouldnt work on HP hardware | 12:05 |
| dtantsur | janders: yeah, something like "if 'Inserted' in error: <repeat without Inserted>" | 12:05 |
| dtantsur | probably the safest bet | 12:05 |
| dtantsur | even if hardware is fixed, it's going to take months or years, so we need to work around | 12:05 |
| janders | dtantsur: I will also check if WriteProtected attribute would become the next problem if we removed Inserted | 12:06 |
| janders | dtantsur: clever! less conditionals in the code this way | 12:07 |
| janders | dtantsur: do we currently use read-write vMedia for anything (and if not are we planning to)? | 12:07 |
| dtantsur | do we change WriteProtected? | 12:09 |
| * dtantsur needs to check the code | 12:09 | |
| janders | yeah it is in the payload | 12:09 |
| janders | https://github.com/openstack/sushy/blob/master/sushy/resources/manager/virtual_media.py#L106 | 12:09 |
| janders | I have a sneaking suspicion that if I rip out Inserted, SM BMC will fail on WriteProtected | 12:09 |
| stendulker_ | MahnoorAsghar: In case of storage is configured in HBA mode, disk drive data woudl be like: | 12:10 |
| stendulker_ | { | 12:10 |
| stendulker_ | "@odata.context": "/redfish/v1/$metadata#HpeSmartStorageDiskDrive.HpeSmartStorageDiskDrive", | 12:10 |
| stendulker_ | "@odata.etag": "W/"7AAC6B40"", | 12:10 |
| stendulker_ | "@odata.id": "/redfish/v1/Systems/1/SmartStorage/HostBusAdapters/0/DiskDrives/0/", | 12:10 |
| stendulker_ | "@odata.type": "#HpeSmartStorageDiskDrive.v2_1_0.HpeSmartStorageDiskDrive", | 12:10 |
| stendulker_ | "CapacityMiB": 228352, | 12:10 |
| stendulker_ | "Description": "HPE Smart Storage Disk Drive View", | 12:10 |
| stendulker_ | "DiskDriveUse": "Raw", "FirmwareVersion": { | 12:10 |
| stendulker_ | "Current": { | 12:10 |
| stendulker_ | "VersionString": "HPGE " | 12:10 |
| stendulker_ | } | 12:10 |
| stendulker_ | }, | 12:10 |
| stendulker_ | "Id": "0", | 12:10 |
| stendulker_ | "Location": "Bay 3", | 12:10 |
| stendulker_ | "MediaType": "SSD", | 12:10 |
| stendulker_ | "Model": "MR000240GWFLU ", | 12:10 |
| stendulker_ | "Name": "HpeSmartStorageDiskDrive", | 12:10 |
| stendulker_ | "SerialNumber": "18211CA10641 ", "Status": { | 12:10 |
| stendulker_ | "Health": "OK", | 12:10 |
| stendulker_ | "State": "Enabled" | 12:10 |
| stendulker_ | } | 12:10 |
| stendulker_ | } | 12:10 |
| dtantsur | stendulker_: please use a paste service :) | 12:10 |
| * MahnoorAsghar just paste the Physical Location property | 12:10 | |
| stendulker_ | MahnoorAsghar: You can also explore the demo here https://ilorestfulapiexplorer.ext.hpe.com/ | 12:10 |
| janders | dtantsur++ | 12:11 |
| dtantsur | janders: oh, this is a different story IMO. I'm not sure why we even send it by default. | 12:11 |
| stendulker_ | MahnoorAsghar: Its just "Location": "Bay 3" | 12:11 |
| * MahnoorAsghar stendulker_: ah, okay. | 12:11 | |
| dtantsur | janders: unfortunately, there is no git history, so I don't remember | 12:11 |
| dtantsur | my gut feeling is that we should change write_protected to default to None and not send it at all | 12:12 |
| dtantsur | but maybe there is hardware that needs it? requires one more experiment (sigh) | 12:12 |
| * MahnoorAsghar stendulker_: other than hba mode, what other mode is present for configuring storage? | 12:12 | |
| janders | dtantsur: I will try to quickly test this, just fighting Slack to show me history of group chats :/ | 12:13 |
| janders | (I remember I need to poke in the same people that were in the group, just can't recall where to click :/) | 12:14 |
| janders | (janders+GUI=boom!) | 12:14 |
| janders | ok got it | 12:17 |
| janders | clear as mud but whatever | 12:18 |
| janders | :) | 12:18 |
| stendulker_ | MahnoorAsghar: Through Arraycontroller | 12:19 |
| * MahnoorAsghar stendulker_: okay. anything different in terms of redfish compliance? | 12:20 | |
| stendulker_ | Storage is not redfish compliant as such | 12:22 |
| * MahnoorAsghar stendulker_: okay | 12:26 | |
| janders | see you tomorrow Ironic o/ | 12:43 |
| mraineri | janders: The history behind some of the virtual media behavior is the original release of Redfish used those properties as read-write to insert/remove media; the side-effects observed with just PATCHing individual properties were odd | 13:15 |
| mraineri | Since then, we introduced actions to perform those operations; essentially if you see the "Actions" property in the VirtualMedia resource, then you should be using that; if not present, then fall back to performing PATCH operations to insert/remove media | 13:16 |
| dtantsur | mraineri: I think the question is more about whether Inserted is supposed to be changed during the old approach | 13:17 |
| mraineri | I would expect it to be writable with the old method | 13:21 |
| dtantsur | mraineri: janders discovered it's not always.. | 13:21 |
| mraineri | It's entirely possible implementations will automatically set it to true when you modify the Image property, but that's not an assumption we call out clients should make | 13:21 |
| mraineri | At least with the "Action" method, we call out default behavior for Inserted if not specified | 13:22 |
| dtantsur | I see | 13:22 |
| mraineri | I take it SuperMicro does not support the Action method and does not allow Inserted to be PATCHed? | 13:29 |
| mraineri | Are there other vendors that fall into that issue? | 13:29 |
| dtantsur | mraineri: I assume so. Unfortunately janders and you don't seem to have a timezone overlap | 13:29 |
| dtantsur | we haven't heard from anyone else | 13:29 |
| mraineri | I know Dell and HPE both support the Actions method for this | 13:38 |
| mraineri | And the Lenovo system I have access to uses the PATCH method | 13:39 |
| mraineri | But I don't have things set up at the moment to verify if they allow PATCH on Inserted (I *think* it does from when I last tested this) | 13:39 |
| viks__ | hi, how to resolve the below erro which i get when creating baremetal instance: | 13:40 |
| viks__ | ```InternalServerError: Unable to attach VIF b6e72b71-a7d2-43a5-a437-6cf869aaaed6 because Ironic can not update Neutron port b6e72b71-a7d2-43a5-a437-6cf869aaaed6 MAC address to match physical MAC address``` | 13:40 |
| ajya | Hi, what's happening with this patch - https://review.opendev.org/c/openstack/ironic/+/795505 ? It looks stuck somewhere with W+1. Anything else missing? | 13:42 |
| viks__ | ok... i think i got it... deleted the ports which was previously created due to some operation... now the above error is gone | 13:56 |
| dtantsur | ajya: adding another W+1 in a hope to fix it | 13:57 |
| dtantsur | if it doesn't help, you can rebase, and I'll re-approve | 13:57 |
| ajya | dtantsur: thanks, looks like Zuul reacted | 13:57 |
| dtantsur | yep | 13:58 |
| dtantsur | probably it was offline at the moment when the patch was approved | 13:58 |
| TheJulia | viks__: yeah, that can happen if you've got an orphaned port, typically people see that if they just start trying to rip out nodes from ironic and re-add them without properly tearing the node down | 15:07 |
| TheJulia | dtantsur: w/r/t addresses, I wonder if any of the vendors are setting the bmc address into smbios data yet | 15:08 |
| * dtantsur no idea | 15:09 | |
| TheJulia | There are two fields I think defined by one of the latest smbios data specs for it | 15:09 |
| TheJulia | but I don't think Dell or HPE supported that quite yet | 15:09 |
| opendevreview | Merged openstack/ironic master: Fix Redfish RAID interface_type physical disk hint https://review.opendev.org/c/openstack/ironic/+/795505 | 15:31 |
| opendevreview | Arne Wiebalck proposed openstack/ironic master: Burn-in: Add documentation https://review.opendev.org/c/openstack/ironic/+/800806 | 15:57 |
| opendevreview | Aija Jauntēva proposed x/sushy-oem-idrac stable/wallaby: Update .gitreview for stable/wallaby https://review.opendev.org/c/x/sushy-oem-idrac/+/800809 | 16:10 |
| opendevreview | Aija Jauntēva proposed x/sushy-oem-idrac stable/wallaby: Update TOX_CONSTRAINTS_FILE for stable/wallaby https://review.opendev.org/c/x/sushy-oem-idrac/+/800810 | 16:10 |
| NobodyCam | Top of the morning Ironic'ers | 16:42 |
| arne_wiebalck | Good morning NobodyCam o/ | 17:09 |
| arne_wiebalck | I observe that the logic we have in the IPA to sync the local time with chronyd does not (always?) work. And when the time stays unchanged, this breaks TLS. | 17:12 |
| arne_wiebalck | I see this on a node which had a Windows instance which apparently sets the local time to the h/w clock. | 17:13 |
| arne_wiebalck | When the IPA comes along, it takes this as UTC ... and we move into the future ... but not the Delorian type ... more the cert is not yet valid type. | 17:14 |
| arne_wiebalck | I am not sure if there is some delay with chronyd, but what I have found to be working is to stop all chronyd and run sth like "chronyd -q 'server TIMESERVER iburst'" | 17:15 |
| arne_wiebalck | Any thoughts? | 17:15 |
| NobodyCam | Good Morning arne_wiebalck | 17:20 |
| NobodyCam | I leave chrony running and run: | 17:22 |
| NobodyCam | chronyc -a 'burst 4/4' | 17:22 |
| NobodyCam | chronyc -a makestep | 17:22 |
| NobodyCam | Giving about 10 seconds in-between the two commands | 17:22 |
| arne_wiebalck | NobodyCam: Right, these are some of the commands I found. | 17:44 |
| arne_wiebalck | NobodyCam: I tried these while watching date/hwclock in another terminal and it seems the time is not (always) updated when these return ... which is bad for the cert we create. | 17:45 |
| iurygregory | morning NobodyCam | 18:03 |
| arne_wiebalck | I will do some more tests tomorrow. At least I understand the SSL errors ... and this time it *is* clock skew dtantsur :) And a Windows instance as mentioned in our docs which were helpful for this issue! | 18:12 |
| arne_wiebalck | Good night, everyone ... see you tomorrow o/ | 18:12 |
| iurygregory | bye arne_wiebalck | 18:18 |
| TheJulia | arne_wiebalck: sounds good to me, at least the earlier idea | 19:39 |
| TheJulia | I *think* when I patched ipa originaly it was all intended to be one time do it right now sort of stuff, so if it is not I think we shouldn't be trying to wait... but... maybe we need to?! | 19:40 |
| NobodyCam | Good Afternoon iurygregory | 19:57 |
| iurygregory | night for me already :D | 19:58 |
| NobodyCam | heheheh | 19:58 |
| TheJulia | good afternoon NobodyCam | 19:59 |
| TheJulia | iurygregory: go to bed :) | 19:59 |
| iurygregory | TheJulia, too early D: | 19:59 |
| TheJulia | or at *least* spend time away from the computer | 19:59 |
| NobodyCam | hey hey TheJulia Good Afternoon how is your day progressing | 20:00 |
| iurygregory | I can't because the unit tests are causing problems and I'm trying to fix :D | 20:00 |
| TheJulia | NobodyCam: it goes, feel like I'm writing a book today | 20:00 |
| NobodyCam | :) | 20:02 |
| iurygregory | it's official that the tests for new controller hates me =( https://paste.opendev.org/show/807482/ | 20:16 |
| TheJulia | iurygregory: is this a posted change yet? | 20:17 |
| iurygregory | no but I can push the one I have | 20:17 |
| TheJulia | sure, I'm happy to take a look | 20:17 |
| opendevreview | Iury Gregory Melo Ferreira proposed openstack/ironic master: Event Subscription support https://review.opendev.org/c/openstack/ironic/+/800831 | 20:24 |
| iurygregory | done =) | 20:24 |
| iurygregory | a few tests missing but I think I have the whole idea of the implementation | 20:24 |
| TheJulia | subscription.py, line 171 | 20:26 |
| TheJulia | why would self.node_ident be a thing? | 20:26 |
| TheJulia | also you have a node filter I guess? | 20:27 |
| * iurygregory checks | 20:27 | |
| iurygregory | in the Controller right? | 20:28 |
| TheJulia | yeah | 20:29 |
| TheJulia | your class doens't have a node_ident there | 20:29 |
| iurygregory | oh right I think while looking at some code it make sense | 20:30 |
| TheJulia | Yeah, but it doesn't really and you have no base object for it to find on the class which causes your test failure | 20:32 |
| TheJulia | iurygregory: am I making any sense? | 20:41 |
| iurygregory | yeah =) | 20:42 |
| iurygregory | I'm trying to fix | 20:42 |
| iurygregory | I was trying to check the policy via the node (so it would go wrong also) | 20:43 |
| TheJulia | oh | 20:43 |
| TheJulia | hmmm | 20:43 |
| * TheJulia looks again | 20:43 | |
| TheJulia | iurygregory: so there you can't | 20:44 |
| TheJulia | because your at an point in the API where your going "hi, give me all subscriptions" | 20:44 |
| iurygregory | yeah | 20:44 |
| TheJulia | so the only thing that can be done is on the DB query, is join with nodes *and* match where only access matches up if not a system scoped token | 20:45 |
| TheJulia | and that where acecss matches up is part of the DB query. We do similar on ports | 20:45 |
| iurygregory | wow the policy check for get_all in ports is crazy :D | 20:47 |
| TheJulia | yes! | 20:47 |
| iurygregory | jesus =O | 20:47 |
| TheJulia | heh | 20:47 |
| * TheJulia takes this as a compliment | 20:47 | |
| * TheJulia notes tzumainn did the heavy lifting really | 20:47 | |
| * iurygregory wondering if it could be done like the one in allocations :D | 20:47 | |
| TheJulia | subscriptions are many to many | 20:48 |
| TheJulia | allocations are one to one | 20:48 |
| TheJulia | err, i mean subscriptions are many to one | 20:48 |
| iurygregory | humm gotcha | 20:49 |
| iurygregory | TheJulia, the check to list all subscriptions would be something like https://paste.opendev.org/show/807485/ ? | 20:59 |
| TheJulia | line 18 is a little weird | 21:03 |
| TheJulia | rpc_node will always be None | 21:04 |
| TheJulia | fwiw | 21:04 |
| TheJulia | That... looks right otherwise | 21:04 |
| TheJulia | oh, conceal_linked_node doesn't seem like it would be a thing in that context | 21:05 |
| TheJulia | oh wait, it uses rpc_node | 21:05 |
| TheJulia | so part of that is because the ports controller can be executed 2 different ways | 21:05 |
| iurygregory | hummmm | 21:06 |
| iurygregory | well in the future I think we can have /v1/nodes/<node_id>/subscriptions also | 21:07 |
| iurygregory | I will remove the conceal_linked | 21:07 |
| iurygregory | well, now the error is because of owner :D hehe | 21:14 |
| iurygregory | WOOT \o/ | 21:22 |
| iurygregory | green \o/ | 21:22 |
| * iurygregory runs all unit tests to be sure that nothing is broken... | 21:22 | |
| TheJulia | \o. | 21:23 |
| TheJulia | err \o/ | 21:23 |
| iurygregory | now time to write the rest of the tests :D | 21:23 |
| TheJulia | the cross linked query is fine, as long as it is not by default | 21:23 |
| janders | good morning Ironic o/ | 21:29 |
| janders | mraineri: are you still around? | 21:29 |
| janders | mraineri: with SuperMicro, newer machines (e.g. X11) use Actions ( https://docs.google.com/document/d/1LzuzhPFjiEYEpvrOCJL4HmmQ-j23I0iBp1a3UwyrMWY/edit#heading=h.xp26ahxmbyo3 ) | 21:32 |
| janders | mraineri: older machines (e.g 5039MS) use Actions ( https://docs.google.com/document/d/1LzuzhPFjiEYEpvrOCJL4HmmQ-j23I0iBp1a3UwyrMWY/edit#heading=h.xp26ahxmbyo3 ) | 21:33 |
| TheJulia | janders: I think he works US-Central so he is more than done or about to leave for the day | 21:33 |
| janders | TheJulia: thanks. Very bad timezone alignment for the two of us (posted question just before 2300hrs, now posting follow up at 0730hrs :) ) | 21:34 |
| janders | that's just life :) | 21:34 |
| TheJulia | true | 21:37 |
| janders | mraineri: however the Actions are different between the old and the new. Old use *some* PATCHing but do not direcly write Inserted value (only Host and Image), Actions take care of that | 21:37 |
| janders | mraineri: the main area of interest for us right now is fixing those newer SuperMicros which use Actions (no PATCHing at all) yet fail when we specify Inserted=true and WriteProtected=true (they expect only Image to be set). Do we need to convince the vendor to stop failing when those are specified, or would it be resonable to stop providing Inserted and WriteProtected properties when Actions are used? | 21:41 |
| janders | dtantsur: WriteProtected will also cause the same issue (if we remove setting Inserted in sushy, it will fail on WriteProtected) | 21:45 |
| iurygregory | morning janders o/ | 21:46 |
| janders | dtantsur: requests only succeed when only Image property is set in the POST request to /redfish/v1/Managers/1/VirtualMedia/CD1/Actions/VirtualMedia.InsertMedia | 21:46 |
| janders | hey iurygregory o/ | 21:46 |
| janders | coffee/bfast time, bbiab | 21:49 |
| iurygregory | enjoy | 21:49 |
| janders | thanks iurygregory :) | 21:51 |
| iurygregory | tests failing because of a typo in the parameter name -> time to sleep :D | 22:28 |
| iurygregory | good night o/ | 22:28 |
| NobodyCam | night iurygregory | 23:40 |
| NobodyCam | hum any happen to know off the top of there head what tables would need to updated to manually change a nodes conductor.. I have a node that believes its mapped to a dead conductor | 23:42 |
| NobodyCam | hum anyONE happen to know off the top of there head what tables would need to updated to manually change a nodes conductor.. I have a node that believes its mapped to a dead conductor | 23:47 |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!