*** achanda has joined #openstack-ironic | 00:07 | |
*** naohirot has joined #openstack-ironic | 00:12 | |
*** achanda has quit IRC | 00:12 | |
*** subscope has quit IRC | 00:18 | |
*** kkoski has joined #openstack-ironic | 00:29 | |
*** Haomeng|2 has joined #openstack-ironic | 00:36 | |
*** mjturek1 has quit IRC | 00:37 | |
*** Haomeng has quit IRC | 00:39 | |
openstackgerrit | Michael Davies proposed openstack/ironic: New field 'name' not supported in port REST API https://review.openstack.org/163730 | 01:06 |
---|---|---|
openstackgerrit | Shivanand Tendulker proposed openstack/ironic: Enable agent_ilo for uefi-bios switching https://review.openstack.org/162043 | 01:09 |
*** alexpilotti has quit IRC | 01:19 | |
*** kkoski has quit IRC | 01:19 | |
*** mtanino has joined #openstack-ironic | 01:25 | |
*** chenglch has joined #openstack-ironic | 01:34 | |
*** ijw_ has quit IRC | 01:48 | |
*** Marga_ has joined #openstack-ironic | 01:57 | |
openstackgerrit | Shivanand Tendulker proposed openstack/ironic: ilo_iscsi driver do not validate boot_option https://review.openstack.org/164414 | 02:29 |
*** ramineni has joined #openstack-ironic | 02:47 | |
*** coolsvap|afk is now known as coolsvap | 02:58 | |
*** jrist has quit IRC | 03:01 | |
openstackgerrit | Anusha Ramineni proposed openstack/ironic: Add Cleaning Operations for iLO drivers https://review.openstack.org/157715 | 03:02 |
*** jrist has joined #openstack-ironic | 03:12 | |
*** ijw has joined #openstack-ironic | 03:23 | |
*** ChuckC has joined #openstack-ironic | 03:28 | |
*** achanda has joined #openstack-ironic | 04:10 | |
openstackgerrit | Anusha Ramineni proposed openstack/ironic: Add Cleaning Operations for iLO drivers https://review.openstack.org/157715 | 04:35 |
*** rameshg87 has joined #openstack-ironic | 04:37 | |
rameshg87 | good morning ironic | 04:38 |
*** mtanino is now known as mtanino_away | 04:40 | |
Haomeng|2 | rameshg87: morning, ramesh:) | 04:55 |
*** aswadr has joined #openstack-ironic | 05:15 | |
mrda | Morning rameshg87 and Haomeng|2 | 05:27 |
Haomeng|2 | mrda: morning:) | 05:28 |
*** kalpase has joined #openstack-ironic | 05:28 | |
*** ijw has quit IRC | 05:31 | |
rameshg87 | good morning mrda, Haomeng|2 :) | 05:38 |
Haomeng|2 | rameshg87: :) | 05:38 |
mrda | o/ | 05:39 |
*** ijw has joined #openstack-ironic | 05:41 | |
*** Nisha has joined #openstack-ironic | 05:48 | |
openstackgerrit | Nisha Agarwal proposed openstack/ironic: Automate boot iso creation with in ironic for iscsi-ilo https://review.openstack.org/155900 | 05:49 |
*** ijw has quit IRC | 05:51 | |
*** ijw has joined #openstack-ironic | 05:52 | |
openstackgerrit | Aparna proposed stackforge/proliantutils: ILO: Support for configuring httpboot through RIS https://review.openstack.org/163325 | 06:08 |
openstackgerrit | Sirushti Murugesan proposed openstack/ironic: Raise exception for Agent Deploy driver when using partition images https://review.openstack.org/164440 | 06:15 |
*** achanda has quit IRC | 06:15 | |
*** pradipta has joined #openstack-ironic | 06:25 | |
*** achanda has joined #openstack-ironic | 06:42 | |
openstackgerrit | chenglch proposed openstack/ironic-specs: Add console log support https://review.openstack.org/164586 | 06:52 |
*** Nisha_away has joined #openstack-ironic | 06:58 | |
*** kalpase1 has joined #openstack-ironic | 06:58 | |
*** Nisha has quit IRC | 06:59 | |
*** kalpase has quit IRC | 07:01 | |
*** stendulker has joined #openstack-ironic | 07:02 | |
*** achanda has quit IRC | 07:12 | |
*** pcaruana has quit IRC | 07:14 | |
*** Nisha_away is now known as Nisha | 07:18 | |
*** Marga_ has quit IRC | 07:23 | |
*** ijw has quit IRC | 07:28 | |
*** mtanino_away has quit IRC | 07:28 | |
openstackgerrit | chenglch proposed openstack/ironic-specs: Add console log support https://review.openstack.org/164586 | 07:30 |
*** andreykurilin_ has joined #openstack-ironic | 07:37 | |
*** rwsu has joined #openstack-ironic | 07:43 | |
*** Marga_ has joined #openstack-ironic | 07:45 | |
*** ukalifon1 has joined #openstack-ironic | 07:48 | |
*** yog__ has joined #openstack-ironic | 07:49 | |
*** sirushti has quit IRC | 07:55 | |
*** sirushti has joined #openstack-ironic | 07:55 | |
*** andreykurilin_ has quit IRC | 07:56 | |
*** Marga_ has quit IRC | 08:01 | |
*** ifarkas has joined #openstack-ironic | 08:07 | |
*** Nisha has quit IRC | 08:08 | |
*** Nisha has joined #openstack-ironic | 08:08 | |
*** mgoddard has joined #openstack-ironic | 08:19 | |
openstackgerrit | Ramakrishnan G proposed openstack/ironic-python-agent: Cache agent parameters for later invocations https://review.openstack.org/164596 | 08:20 |
*** yog__ has quit IRC | 08:21 | |
*** pradipta has quit IRC | 08:22 | |
openstackgerrit | Ramakrishnan G proposed openstack/ironic-python-agent: Cache agent parameters for later invocations https://review.openstack.org/164596 | 08:22 |
openstackgerrit | Ramakrishnan G proposed openstack/ironic-python-agent: Cache agent parameters for later invocations https://review.openstack.org/164596 | 08:24 |
*** jcoufal has joined #openstack-ironic | 08:33 | |
*** ndipanov has joined #openstack-ironic | 08:33 | |
*** yog__ has joined #openstack-ironic | 08:37 | |
*** dtantsur has joined #openstack-ironic | 08:40 | |
openstackgerrit | Ramakrishnan G proposed openstack/ironic: Add uefi support for agent iscsi deploy https://review.openstack.org/164386 | 08:41 |
dtantsur | Hey-hey, morning folks! I'm back :) | 08:42 |
Nisha | dtantsur, | 08:48 |
Nisha | hi | 08:49 |
Nisha | gm | 08:49 |
rameshg87 | dtantsur: o/ | 08:49 |
rameshg87 | welcome back | 08:49 |
dtantsur | o/ | 08:49 |
Haomeng|2 | dtantsur: o/ | 08:50 |
*** jcoufal has quit IRC | 08:57 | |
*** kalpase1 has quit IRC | 08:57 | |
*** kalpase has joined #openstack-ironic | 08:58 | |
*** jistr has joined #openstack-ironic | 09:11 | |
*** derekh has joined #openstack-ironic | 09:14 | |
*** MattMan has joined #openstack-ironic | 09:16 | |
*** lucasagomes has joined #openstack-ironic | 09:17 | |
openstackgerrit | Merged openstack/ironic-specs: Fix typo in ironic-specs/specs/kilo/partition-image-support-for-agent-driver.rst https://review.openstack.org/162593 | 09:17 |
openstackgerrit | Merged openstack/ironic-specs: Fix typo in ironic-specs/specs/kilo/cisco-ucs-pxe-driver.rst https://review.openstack.org/162613 | 09:18 |
openstackgerrit | Anusha Ramineni proposed openstack/ironic: Add Cleaning Operations for iLO drivers https://review.openstack.org/157715 | 09:22 |
GheRivero | morning all | 09:23 |
rameshg87 | GheRivero: o/ | 09:24 |
dtantsur | GheRivero, morning | 09:25 |
openstackgerrit | Merged openstack/ironic-specs: Fix typo in ironic-specs/specs/kilo/uefi-secure-boot.rst https://review.openstack.org/162590 | 09:26 |
openstackgerrit | Merged openstack/ironic-specs: Fix typos in ironic-specs/specs/kilo/drac-bios-mgmt.rst https://review.openstack.org/162578 | 09:26 |
openstackgerrit | Merged openstack/ironic-specs: Fix typo in ironic-specs/specs/kilo/local-boot-support-with-partition-images.rst https://review.openstack.org/162608 | 09:28 |
openstackgerrit | Tan Lin proposed openstack/ironic: Use Mock.patch decorator to handle patching amt management module https://review.openstack.org/164615 | 09:28 |
*** mgoddard has quit IRC | 09:30 | |
openstackgerrit | Merged openstack/ironic-specs: Fix typo in ironic-specs/specs/kilo/automate-uefi-bios-iso-creation.rst https://review.openstack.org/162605 | 09:31 |
* dtantsur fast-tracked some typo fixes ^^^ | 09:32 | |
pshige | Thanks! | 09:33 |
openstackgerrit | Merged openstack/ironic-specs: Fix typo in ironic-specs/specs/kilo/new-ironic-state-machine.rst https://review.openstack.org/162607 | 09:34 |
stendulker | Haomeng|2, dtantsur: Can you please have a look at this secure boot code review when you get time. https://review.openstack.org/#/c/153974/ | 09:37 |
*** mgoddard has joined #openstack-ironic | 09:46 | |
openstackgerrit | Ramakrishnan G proposed openstack/ironic-python-agent: Add uefi support in image extension https://review.openstack.org/163739 | 09:49 |
*** naohirot has quit IRC | 09:51 | |
rameshg87 | dtantsur: i too feel it's true https://review.openstack.org/#/c/155561/27/ironic/conductor/manager.py | 09:54 |
rameshg87 | dtantsur: how about i just fix it up in a patch and throw a new one ? or should we wait for JoshNang ? | 09:55 |
rameshg87 | dtantsur: the unit test should have caught it :( | 09:55 |
openstackgerrit | Lucas Alvares Gomes proposed openstack/ironic: IPA: Add support for root device hints https://review.openstack.org/163857 | 09:55 |
dtantsur | rameshg87, well, we generally do it for nits, not for logic errors.... but if you propose a follow-up fixing it (and tests), I'll approve this one ;) | 09:56 |
openstackgerrit | Ghe Rivero proposed openstack/ironic-python-agent: Use oslo_log lib https://review.openstack.org/162790 | 09:56 |
openstackgerrit | Ghe Rivero proposed openstack/ironic-python-agent: Sync from oslo.incubator https://review.openstack.org/162791 | 09:56 |
rameshg87 | dtantsur: follow-up patch or a new patch-set on the same patch ? | 09:56 |
*** rsalevsky has joined #openstack-ironic | 09:56 | |
*** lazy_prince has quit IRC | 09:57 | |
dtantsur | rameshg87, does not matter to me :) | 09:58 |
rameshg87 | okay | 09:58 |
rameshg87 | throwing a follow-up patch | 09:58 |
*** lazy_prince has joined #openstack-ironic | 09:59 | |
*** lazy_prince has quit IRC | 10:00 | |
*** lazy_prince has joined #openstack-ironic | 10:00 | |
openstackgerrit | Nisha Agarwal proposed openstack/ironic: Automate boot iso creation with in ironic for iscsi-ilo https://review.openstack.org/155900 | 10:03 |
Nisha | dtantsur, lucasagomes ^^^^ | 10:04 |
dtantsur | I'm still recovering from PTO, but will try somewhat later :) | 10:04 |
Nisha | dtantsur, thanks | 10:04 |
openstackgerrit | Ramakrishnan G proposed openstack/ironic: Add uefi support for agent iscsi deploy https://review.openstack.org/164386 | 10:07 |
*** coolsvap is now known as coolsvap|afk | 10:19 | |
openstackgerrit | Gopi Krishna S proposed openstack/ironic: Add pxe_ucs driver to manage Cisco UCS servers https://review.openstack.org/159734 | 10:25 |
*** kalpase1 has joined #openstack-ironic | 10:27 | |
*** kalpase has quit IRC | 10:29 | |
*** saripurigopi has joined #openstack-ironic | 10:35 | |
*** dtantsur is now known as dtantsur|bbl | 10:49 | |
openstackgerrit | Ramakrishnan G proposed openstack/ironic: Address comments on cleaning commit https://review.openstack.org/164645 | 10:50 |
rameshg87 | dtantsur: ^^^ | 10:50 |
rameshg87 | lucasagomes: one quick question: https://review.openstack.org/#/c/164386/5/ironic/drivers/modules/deploy_utils.py | 10:51 |
rameshg87 | lucasagomes: the comment on L635 | 10:51 |
rameshg87 | lucasagomes: i use the same dictionary for knowing the devices for the partitions first and updating them with uuids in the loop | 10:52 |
rameshg87 | lucasagomes: not sure if that's obvious, let me know if you want me to use up another dictionary | 10:52 |
openstackgerrit | Anusha Ramineni proposed openstack/ironic: Support agent_ilo driver to perform cleaning https://review.openstack.org/164646 | 10:53 |
*** ramineni has quit IRC | 11:05 | |
lucasagomes | rameshg87, hi yes that's fine | 11:05 |
lucasagomes | rameshg87, but I thought that when you do 'efi system partition uuid': part_dict.get('efi system partition') | 11:05 |
lucasagomes | you get None already if it's not present | 11:05 |
lucasagomes | so the "if part_dev: ... else..." | 11:06 |
lucasagomes | the else is not needed because the value of the dict is already none | 11:06 |
lucasagomes | if it contains something like "/dev/part1" it will be replaced by the block_uuid() return value | 11:06 |
*** stendulker has quit IRC | 11:13 | |
Nisha | lucasagomes, i responded to comments on https://review.openstack.org/#/c/155900/ | 11:16 |
Nisha | please check | 11:16 |
rameshg87 | lucasagomes: oh okay | 11:17 |
rameshg87 | lucasagomes: got it part_dict.get('efi system partition') will return None, so it will be None anyway | 11:17 |
rameshg87 | lucasagomes: yeah makes sense | 11:17 |
*** yuanying has quit IRC | 11:18 | |
rameshg87 | lucasagomes: will remove it, thanks | 11:18 |
lucasagomes | np :) | 11:18 |
lucasagomes | Nisha, will do | 11:18 |
Nisha | lucasagomes, actually umount is called as part of "with utils.tempdir as <dir> name:" block | 11:19 |
Nisha | so when the block ends it unmounts | 11:19 |
openstackgerrit | Nisha Agarwal proposed openstack/ironic: iLO implementation for hardware inspection https://review.openstack.org/151596 | 11:21 |
lucasagomes | Nisha, https://github.com/openstack/ironic/blob/master/ironic/common/utils.py#L379-L389 ? | 11:21 |
lucasagomes | it deletes it | 11:22 |
lucasagomes | runs "losetup -a" after you run that code | 11:22 |
lucasagomes | you might see the loop device associated with the iso you just mounted no? | 11:22 |
lucasagomes | or simply check the logs to see if you can find "'Could not remove tmpdir:" | 11:23 |
Nisha | i have seen it umounts but may be i need to figure out the path. anyway i will pots the patch if it doesnt unmounts | 11:23 |
lucasagomes | cause it may not be able to delete the contained due the resource will be busy | 11:23 |
lucasagomes | right, I don't see where it does it :/ | 11:24 |
lucasagomes | that's why we should use the unittests to make sure | 11:24 |
lucasagomes | umount is being called | 11:24 |
lucasagomes | even if it's part of tempdir, we are mocking umount so we can test assert_called_once_with() | 11:24 |
*** kalpase1 has quit IRC | 11:27 | |
openstackgerrit | Nisha Agarwal proposed openstack/ironic: iLO driver updates node capabilities during inspection https://review.openstack.org/163572 | 11:28 |
openstackgerrit | Ramakrishnan G proposed openstack/ironic: Add uefi support for agent iscsi deploy https://review.openstack.org/164386 | 11:28 |
Nisha | lucasagomes, i will test that and update the patch if required. meanwhile it will help if you and dtantsur|bbl can review inspection patches | 11:29 |
Nisha | #link https://review.openstack.org/151596, https://review.openstack.org/163572, https://review.openstack.org/148804 | 11:29 |
openstackgerrit | Ramakrishnan G proposed openstack/ironic-python-agent: Add uefi support in image extension https://review.openstack.org/163739 | 11:30 |
lucasagomes | Nisha, ack thanks | 11:31 |
* rameshg87 goes home | 11:32 | |
*** rameshg87 has left #openstack-ironic | 11:32 | |
*** Nisha has quit IRC | 11:34 | |
openstackgerrit | Merged openstack/ironic: Use Mock.patch decorator to handle patching amt management module https://review.openstack.org/164615 | 11:38 |
*** takadayuiko has joined #openstack-ironic | 11:51 | |
*** saripurigopi has quit IRC | 11:57 | |
*** dtantsur|bbl is now known as dtantsur | 11:57 | |
*** dprince has joined #openstack-ironic | 12:07 | |
*** kalpase has joined #openstack-ironic | 12:26 | |
*** alexpilotti has joined #openstack-ironic | 12:27 | |
openstackgerrit | Dmitry Tantsur proposed openstack/ironic: Start using in-band inspection https://review.openstack.org/161132 | 12:33 |
*** lucasagomes is now known as lucas-hungry | 12:34 | |
*** subscope has joined #openstack-ironic | 12:37 | |
*** dlpartain has joined #openstack-ironic | 12:40 | |
*** openstackgerrit has quit IRC | 12:50 | |
*** openstackgerrit has joined #openstack-ironic | 12:50 | |
*** dlpartain has left #openstack-ironic | 12:56 | |
*** wuhg has joined #openstack-ironic | 12:57 | |
*** ijw has joined #openstack-ironic | 13:06 | |
*** rameshg87 has joined #openstack-ironic | 13:08 | |
jroll | morning all :) | 13:08 |
Shrews | mordred: left you some comments on the shade stuff | 13:09 |
Shrews | dang it. wrong channel | 13:12 |
Shrews | jroll: morning! | 13:12 |
jroll | morning Shrews :) | 13:13 |
dtantsur | jroll, Shrews, morning! | 13:14 |
Shrews | hi dtantsur | 13:15 |
dtantsur | jroll, Shrews, could you guys review https://review.openstack.org/148804 ? Landing it would greatly simplify testing some new states including inspection | 13:15 |
jroll | morning dtantsur :) | 13:15 |
jroll | welcome back! | 13:15 |
dtantsur | thanks :) | 13:15 |
Shrews | dtantsur: sure, will take a look | 13:16 |
rameshg87 | dtantsur: oh, i just upgraded my vote :) | 13:17 |
rameshg87 | dtantsur: wanted others to have a look ? | 13:17 |
dtantsur | :) | 13:17 |
rameshg87 | dtantsur: i gave +1 back when i didn't have +2 voting rights | 13:18 |
Shrews | :) | 13:18 |
dtantsur | at least it works in my testing... but I'm the wrong person to ask, I want this to land too much :D | 13:18 |
jroll | oh, good patch :) | 13:18 |
* jroll got ninja'd | 13:19 | |
dtantsur | I mean, I just wanted someone to +A it | 13:19 |
*** stendulker has joined #openstack-ironic | 13:19 | |
Shrews | so, that patch is contributing to the issues devananda pointed out with our client (exposing stuff that might not be relevant for an older version of the server). But I don't think we've discussed a proper solution for that. | 13:21 |
jroll | right | 13:21 |
rameshg87 | oh, never looked from that part :( | 13:21 |
jroll | I think it can be fixed later | 13:21 |
dtantsur | I think it _should_ be fixed later | 13:23 |
dtantsur | once we understand how to fix it :) | 13:23 |
Shrews | I think it _will_ be fixed later :-P | 13:23 |
rameshg87 | _may_ be :D | 13:23 |
rameshg87 | dtantsur: why are we not enabled discoverd for agent* drivers ? - https://review.openstack.org/#/c/161132/6 | 13:25 |
rameshg87 | *enabling | 13:25 |
dtantsur | ehmmmm... because I didn't think about it :) | 13:25 |
dtantsur | also we need to know how J* and devananda feel about it | 13:25 |
dtantsur | jroll, JayF ^^^ | 13:25 |
trown | lol at nick-globbing | 13:26 |
* jroll reads | 13:26 | |
jroll | dtantsur: because nobody did the work? idk | 13:26 |
jroll | I don't see why we wouldn't want to do it | 13:27 |
*** chenglch has quit IRC | 13:28 | |
rameshg87 | dtantsur: also, any idea why we did we change the behaviour of not instantiating DiscoverdInspect if CONF option is disabled ? | 13:29 |
rameshg87 | dtantsur: i mean any real reason ? (other than not wanting to instantiate if it's not going to be used) | 13:29 |
jroll | rameshg87: iirc it was related to node-validate | 13:29 |
dtantsur | rameshg87, you'd better ask devananda when he's here... | 13:29 |
dtantsur | I don't understand either | 13:30 |
jroll | node-validate was showing | inspect | False | | 13:30 |
dtantsur | right | 13:30 |
jroll | but really it should be None, not False | 13:30 |
dtantsur | no | 13:30 |
openstackgerrit | Imre Farkas proposed stackforge/ironic-discoverd: Save BIOS and RAID configuration to node.extra https://review.openstack.org/164695 | 13:30 |
jroll | because it isn't a validation error | 13:30 |
jroll | it's just disabled | 13:30 |
dtantsur | jroll, then we need a new kind of error | 13:30 |
dtantsur | disabled != not implemented | 13:30 |
rameshg87 | *disabled* | 13:30 |
jroll | hrm | 13:30 |
jroll | idk, I think the goal was: | 13:31 |
dtantsur | And the difference is: not implemented = go ping Ironic devs; disabled = go ping your admin | 13:31 |
jroll | True == should work | 13:31 |
jroll | False == something is broken | 13:31 |
openstackgerrit | Shivanand Tendulker proposed openstack/ironic: ilo_iscsi driver do not validate boot_option https://review.openstack.org/164414 | 13:31 |
jroll | None == not available | 13:31 |
jroll | but idk. | 13:31 |
rameshg87 | jroll: but error messages it's going to provide are even less difficult to understand | 13:32 |
openstackgerrit | Shivanand Tendulker proposed openstack/ironic: ilo_iscsi driver do not validate boot_option https://review.openstack.org/164414 | 13:32 |
dtantsur | IIRC we don't call it "not available", we call it "not supported" | 13:32 |
rameshg87 | jroll: someone can't make out easily if it's disabled because of CONF option | 13:32 |
openstackgerrit | Shivanand Tendulker proposed openstack/ironic: Enable agent_ilo for uefi-bios switching https://review.openstack.org/162043 | 13:32 |
rameshg87 | jroll: if we don't show inspect in node-validate OR if we show "pxe_ipmitool doesn't support inspect" | 13:33 |
rameshg87 | jroll: doesn't come out why is it so ? (when doc says it is supported) | 13:33 |
*** ChuckC has quit IRC | 13:33 | |
jroll | sure | 13:33 |
jroll | idk what the right thing to do is | 13:33 |
jroll | both options are confusing to the user | 13:33 |
jroll | in different ways | 13:34 |
jroll | s/confusing to the user/inconsistent/ maybe | 13:34 |
openstackgerrit | Shivanand Tendulker proposed openstack/ironic: Ilo drivers sets capabilities:boot_mode in node https://review.openstack.org/155731 | 13:34 |
*** ramineni has joined #openstack-ironic | 13:34 | |
rameshg87 | yeah | 13:34 |
*** lucas-hungry is now known as lucasagomes | 13:35 | |
openstackgerrit | Shivanand Tendulker proposed openstack/ironic: Common changes for secure boot support https://review.openstack.org/153974 | 13:35 |
openstackgerrit | Shivanand Tendulker proposed openstack/ironic: Secure boot support for pxe_ilo driver https://review.openstack.org/154808 | 13:35 |
openstackgerrit | Shivanand Tendulker proposed openstack/ironic: Secure boot support for iscsi_ilo driver https://review.openstack.org/154814 | 13:36 |
openstackgerrit | Shivanand Tendulker proposed openstack/ironic: Secure boot support for agent_ilo driver https://review.openstack.org/154816 | 13:36 |
*** rloo has joined #openstack-ironic | 13:41 | |
rameshg87 | lucasagomes: https://review.openstack.org/#/c/164596/3 is somewhat related to root device hints | 13:43 |
rameshg87 | lucasagomes: please let me know your thoughts when you have a look at that | 13:43 |
rameshg87 | lucasagomes: i needed to do this for when agent is booted with virtual media - to make iscsi_ilo work with agent ramdisk | 13:44 |
*** sambetts_ is now known as sambetts | 13:45 | |
*** kalpase has quit IRC | 13:46 | |
*** kalpase1 has joined #openstack-ironic | 13:46 | |
*** kalpase has joined #openstack-ironic | 13:48 | |
*** stendulker has quit IRC | 13:48 | |
lucasagomes | rameshg87, ack | 13:51 |
*** kalpase1 has quit IRC | 13:51 | |
*** mjturek1 has joined #openstack-ironic | 13:51 | |
jroll | JoshNang: I'm hacking on devstack for this change btw https://review.openstack.org/#/c/161453/ | 13:52 |
openstackgerrit | Dmitry Tantsur proposed stackforge/ironic-discoverd: eDeploy plugin to save BIOS and RAID configuration to node.extra https://review.openstack.org/164695 | 13:52 |
ramineni | jroll: hi | 13:56 |
jroll | hi ramineni | 13:57 |
ramineni | jroll: could you please have a look at https://review.openstack.org/#/c/157715/16 , waiting for reviews :) | 13:57 |
jroll | ramineni: I'll do my best to get to it today | 13:58 |
ramineni | jroll: great.thanks | 13:58 |
*** kkoski has joined #openstack-ironic | 13:59 | |
*** ukalifon1 has quit IRC | 14:00 | |
jroll | np :) | 14:00 |
*** PaulCzar has joined #openstack-ironic | 14:01 | |
lucasagomes | rameshg87, commented | 14:01 |
*** ukalifon has joined #openstack-ironic | 14:01 | |
lucasagomes | jroll, ramineni morning :) | 14:02 |
dtantsur | rameshg87, are you going to the meeting tonight? | 14:02 |
lucasagomes | rameshg87, lemme know if what I commented makes sense, i didn't get why we cached that self.parameters in the test class | 14:02 |
lucasagomes | that's not needed | 14:02 |
ramineni | lucasagomes: morning :) | 14:02 |
jroll | morning lucasagomes :) | 14:02 |
*** ukalifon2 has joined #openstack-ironic | 14:03 | |
rameshg87 | lucasagomes: thanks | 14:04 |
rameshg87 | dtantsur: i hope you mean ironic meeting, i will be there .. :) | 14:04 |
rameshg87 | lucasagomes: that was just a precaution to make sure that i don't play around with any other test | 14:04 |
devananda | g'morning, all | 14:05 |
lucasagomes | rameshg87, right, hmm I don't know how is that helping really | 14:05 |
lucasagomes | I mean, it's doing nothing to be honest | 14:05 |
rameshg87 | lucasagomes: do you feel it is not required ? i wanted to make sure that i don't touch the state of the global in any case, that's why i thought of saving the current value and restoring it back | 14:05 |
lucasagomes | self.parameters is not being used in any tests | 14:05 |
jroll | heya devananda :) | 14:05 |
*** ukalifon has quit IRC | 14:05 | |
dtantsur | devananda, morning :) feeling like some holy way about validation in the discoverd driver? :D | 14:05 |
rameshg87 | lucasagomes: it's not being used, but i restore the value of the global after the test is done: L146 | 14:06 |
rameshg87 | devananda: o/ | 14:06 |
lucasagomes | rameshg87, right... hmm not sure I mean right after we cache we do utils.AGENT_PARAMS_CACHED =dict () | 14:06 |
lucasagomes | which overwrites global | 14:06 |
lucasagomes | so | 14:06 |
rameshg87 | yeah | 14:06 |
dtantsur | rameshg87, I was going to ask you to raise question about validation in discoverd, but as devananda is here, we may start now :) | 14:06 |
lucasagomes | devananda, morning | 14:07 |
rameshg87 | dtantsur: yeah sure :) | 14:07 |
rameshg87 | lucasagomes: but after test is done, i restore the value back in L146 - tearDown() | 14:07 |
rameshg87 | lucasagomes: so modifying the value of global before the test case, restoring it back after the test case - this is what i meant | 14:07 |
rameshg87 | lucasagomes: i am still not sure if it's required though :) | 14:08 |
lucasagomes | rameshg87, I see but what's the benefit? on the next test running the value of the global will be {} because we jsut set it to it on setUp | 14:08 |
lucasagomes | rameshg87, gotcha, yeah I'm a bit confused whether it's needed or not to be honest | 14:08 |
lucasagomes | looks like not | 14:08 |
rameshg87 | lucasagomes: yeah but the value will again be restored back in next test case's tearDown() | 14:08 |
lucasagomes | rameshg87, perhaps we could have a _get_cached_agent_params function | 14:09 |
lucasagomes | that will return the global | 14:09 |
lucasagomes | to help with tests u can mock that | 14:09 |
rameshg87 | lucasagomes: yeah :) | 14:09 |
lucasagomes | you can create a side_effect that will return [{}, {info}] | 14:09 |
openstackgerrit | Merged stackforge/ironic-discoverd: eDeploy plugin to save BIOS and RAID configuration to node.extra https://review.openstack.org/164695 | 14:09 |
rameshg87 | that makes more sense infact | 14:09 |
lucasagomes | so you see if info is present we don't call get_params from vmedia or cmdline again | 14:09 |
rameshg87 | unless people question why you need a one-line method :D | 14:09 |
lucasagomes | rameshg87, yeah I think that's a better approach, and we don't have to play with globals | 14:09 |
jroll | rameshg87: I've done this before to mock time.time() | 14:10 |
lucasagomes | rameshg87, well we can add a comment saying it's a helper for tests, it's not unsual | 14:10 |
lucasagomes | jroll, yeah | 14:10 |
rameshg87 | yeah okay | 14:10 |
rameshg87 | makes sense, will change it ... | 14:10 |
lucasagomes | rameshg87, cool, thanks other than that it lgtm :) | 14:10 |
rameshg87 | (since jroll pitched in) jroll, please have a look at and see if it's okay https://review.openstack.org/#/c/164596/3 | 14:11 |
jroll | gah | 14:11 |
rameshg87 | jroll: it's touching the heart of agent (getting agent params) :) | 14:11 |
jroll | ok | 14:11 |
rameshg87 | folks, i wanted to raise one question regarding a bug i reported: https://bugs.launchpad.net/ironic/+bug/1432093 | 14:12 |
openstack | Launchpad bug 1432093 in Ironic "Instance tear down fail with KeyError: 'is_whole_disk_image'" [Undecided,New] | 14:13 |
NobodyCam | Good morning Ironicers :) | 14:13 |
rameshg87 | i had an instance created before ironic had whole-disk-image-support, i took latest code (which supports whole-disk-images), and then i tried to tear down and it failed | 14:14 |
rameshg87 | now i am wondering whether we should consider such tests for CI | 14:14 |
rameshg87 | i mean deploy an instance without the patch, try rebuild/tear down with the patch | 14:14 |
rameshg87 | does that make sense ? | 14:14 |
*** ChuckC has joined #openstack-ironic | 14:15 | |
lucasagomes | rameshg87, I think it does | 14:15 |
*** kkoski has quit IRC | 14:15 | |
lucasagomes | tearing down a node is part of the normal flow, we should stress that | 14:15 |
jroll | rameshg87: I agree with lucasagomes on that agent params, I'll look after you push a new one :) | 14:16 |
rameshg87 | jroll: okay | 14:16 |
*** rameshg87 is now known as rameshg87-brb | 14:16 | |
devananda | 14:07:42 < rameshg87> lucasagomes: so modifying the value of global before the test case, restoring it back after the test case - this is what i meant | 14:18 |
devananda | you can't do tht | 14:18 |
devananda | tests run in parallel | 14:18 |
NobodyCam | mornig devananda lucasagomes jroll and rameshg87-brb | 14:18 |
lucasagomes | devananda, yea, we want to have a helper method that mock the global | 14:18 |
*** kkoski has joined #openstack-ironic | 14:18 | |
lucasagomes | I mean that return the global so it can be mocked | 14:18 |
lucasagomes | for tests | 14:18 |
devananda | k | 14:19 |
devananda | also, why are you using a global? | 14:19 |
lucasagomes | devananda, rameshg87-brb wants to cache the parameters passed to the IPA ramdisk | 14:19 |
lucasagomes | the parametes via cmdline or vmedia | 14:19 |
lucasagomes | because they never change anyway, and vmedia may not be available apparently if the temp url expires | 14:19 |
lucasagomes | devananda, https://review.openstack.org/#/c/164596/3 | 14:20 |
devananda | a global-global? or a module-level variable? | 14:20 |
* devananda reads | 14:20 | |
lucasagomes | a module level variable | 14:20 |
devananda | oh | 14:20 |
devananda | right - that's not a global | 14:20 |
lucasagomes | I think it's fine to cache it after parsing. But for tests we better have the helper method so we can have control the values by mocking it | 14:21 |
*** stendulker has joined #openstack-ironic | 14:22 | |
*** rameshg87-brb is now known as rameshg87 | 14:22 | |
rameshg87 | morning NobodyCam | 14:23 |
devananda | sorry, early morning brain. yea, module-level variables accessed by "global" is usually unnecessary, and makes testing quite challenging | 14:23 |
lucasagomes | devananda, yup :) it's all good | 14:24 |
devananda | so I really dont understand this change | 14:25 |
devananda | why is it necessary to deepcopy a global?? | 14:25 |
dtantsur | NobodyCam, morning | 14:25 |
NobodyCam | morning dtantsur :) | 14:25 |
jroll | morning NobodyCam :) | 14:26 |
NobodyCam | :) | 14:26 |
* NobodyCam hopes everyone had a great weekend! | 14:27 | |
lucasagomes | devananda, I wonder if other functions doing get_agent_params() might be injecting stuff into the dict | 14:28 |
lucasagomes | and therefor passing a reference would also change the global (fresh parsed) | 14:28 |
* lucasagomes gotta looks more into the IPA codebase to be sure of that | 14:29 | |
devananda | rameshg87: ok, i think i understand the goal of this patch now. you should use a guard and a singleton, like this; https://github.com/openstack/ironic/blob/master/ironic/db/sqlalchemy/api.py#L58 | 14:29 |
devananda | lucasagomes: is ^ basically what you were already suggesting? | 14:29 |
devananda | global is guarded by a private accessor method that can easily be mocked for testing | 14:30 |
lucasagomes | devananda, yes | 14:30 |
devananda | cool | 14:30 |
lucasagomes | like the _create_facade_lazily() will populate the global | 14:30 |
devananda | exactly | 14:31 |
lucasagomes | and then we have a get_function that returns it if not set | 14:31 |
lucasagomes | if set* | 14:31 |
rameshg87 | devananda: yeah okay | 14:32 |
rameshg87 | devananda: was writing up something like this :) | 14:32 |
*** BadCub_Away is now known as BadCub | 14:33 | |
* lucasagomes english level is pretty low today, doing 2 things in parallel and trying to keep concentrated :) | 14:33 | |
*** yog__ has quit IRC | 14:33 | |
rameshg87 | devananda: lucasagomes: and assume we dont write unit tests for _create_facade_lazily :) | 14:33 |
* rameshg87 checks | 14:33 | |
BadCub | morning everyone | 14:33 |
rameshg87 | BadCub: o/ | 14:34 |
NobodyCam | morning BadCub :_ | 14:34 |
dtantsur | BadCub, o/ | 14:34 |
lucasagomes | rameshg87, you could write tests, doesn't matter really. Because for the other tests where that variable matter the get_() will be mocked and you have control over what is returned | 14:35 |
lucasagomes | BadCub, good morning! | 14:35 |
*** zz_jgrimm- is now known as jgrimm | 14:35 | |
rameshg87 | lucasagomes: yeah | 14:35 |
devananda | ok, with that settled and backscroll in other channels similarly read, I'm going to spend today focusing on reviews | 14:36 |
devananda | and getting ready for K3 milestone this week | 14:36 |
dtantsur | devananda, rameshg87, can we discuss https://review.openstack.org/#/c/161132/ ? | 14:36 |
lucasagomes | NobodyCam, morning :) | 14:37 |
devananda | BadCub: in the reviewjam last week, I think we all (myself included) forgot about our old sync page -- https://etherpad.openstack.org/p/IronicReviewDay | 14:37 |
lucasagomes | NobodyCam, btw, I added a new patch set due a merge conflict for the root device hints on https://review.openstack.org/#/c/163857/ | 14:37 |
devananda | so I updated it on the weekend | 14:37 |
BadCub | devananda: Hmmm. I believe we did | 14:38 |
jroll | nice | 14:38 |
devananda | BadCub: it had no reviews on it on saturday. it's quite full now :) | 14:38 |
NobodyCam | lucasagomes: looking | 14:38 |
devananda | it's possible I missed things, but that page should now have a link to every review tagged by a targeted blueprint | 14:38 |
rameshg87 | dtantsur: i am in ... | 14:39 |
*** Marga_ has joined #openstack-ironic | 14:40 | |
*** Marga_ has quit IRC | 14:41 | |
*** Marga_ has joined #openstack-ironic | 14:41 | |
*** ukalifon2 has quit IRC | 14:41 | |
dtantsur | devananda, rameshg87 and I have some doubts that "Driver pxe_ipmitool does not support inspect" is less confusing to a user than reporting validation failure | 14:43 |
*** ChuckC has quit IRC | 14:43 | |
openstackgerrit | Imre Farkas proposed openstack/ironic: DRAC BIOS vendor_passthru: enable rebooting the node https://review.openstack.org/163071 | 14:43 |
stendulker | devananda: Not much of of core-reviewer review has happened for secure boot related patches. With just few days left for feature freeze, just wondering if it is still in the priority list. | 14:45 |
*** ukalifon1 has joined #openstack-ironic | 14:47 | |
stendulker | lucagomes: Please have a look at this review when you get time "Ilo drivers sets capabilities:boot_mode in node" https://review.openstack.org/#/c/155731/ | 14:49 |
stendulker | lucasgomes: Please have a look at this review when you get time "Ilo drivers sets capabilities:boot_mode in node" https://review.openstack.org/#/c/155731/ | 14:50 |
openstackgerrit | Ramakrishnan G proposed openstack/ironic-python-agent: Cache agent parameters for later invocations https://review.openstack.org/164596 | 14:53 |
lucasagomes | stendulker, lucas<tab> | 14:55 |
lucasagomes | stendulker, will do :) I will have a call in 5 min but I will do it right after | 14:55 |
stendulker | lucagomes: thank you :) | 14:55 |
lucasagomes | I see the patch was just rebased and few nits pointed by ruby were fixed | 14:55 |
lucasagomes | since my last review (which I +2'ed | 14:55 |
lucasagomes | )* | 14:55 |
stendulker | lucasgomes: Yes. Some optimization suggested by Ruby, they are also incorporated. | 14:56 |
lucasagomes | cool :) | 14:57 |
devananda | dtantsur: but parameter validation is not the correct way to advertise that | 14:58 |
dtantsur | devananda, that's right... but what we have now is IMO worse, we just hide the fact we have any inspection at all | 14:59 |
devananda | dtantsur: it's disabled. therefor we should skip validation | 14:59 |
dtantsur | devananda, so e.g. instead of contacting admins, a user will assume we don't have a feature :) | 14:59 |
devananda | the feature isn't available in that environment | 14:59 |
devananda | if, as you propose, the node fails validation, the user is going to assume they've done something wrong | 15:00 |
devananda | because that's what Result = false means for every other interface | 15:00 |
dtantsur | devananda, we don't report it as disabled. we report that driver does not support the feature which is just not true | 15:00 |
dtantsur | do we need one more case for that? | 15:00 |
*** ramineni has quit IRC | 15:02 | |
devananda | I'd be fine with result = 'disabled' | 15:02 |
devananda | dtantsur: your scenario is a user noticing 'disabled" message and contacting their cloud operator to ask to enable the feature | 15:04 |
devananda | dtantsur: that doesn't make any sense -- discovery is only useful to operators | 15:04 |
*** mgoddard1 has joined #openstack-ironic | 15:04 | |
dtantsur | right | 15:05 |
devananda | even when we add policy settings to allow delegation of different operational responsibility, it's still not a "user facing feature" | 15:05 |
rameshg87 | devananda: but even for the operators, isn't it confusing "pxe_ipmitool doesn't support inspect" ? | 15:05 |
rameshg87 | devananda: i agree with validation failure | 15:05 |
NobodyCam | lucasagomes: LGTM! | 15:05 |
dtantsur | devananda, should we then at least issue a log warning/info that discoverd is disabled? | 15:06 |
lucasagomes | NobodyCam, :) thanks | 15:06 |
dtantsur | devananda, so that to give at least some clues? | 15:06 |
*** mgoddard has quit IRC | 15:06 | |
devananda | dtantsur: oh - logging an info or notice about the feature being turned off -- absolutely | 15:06 |
devananda | that's the right way to signal to operators "hey, there's this thing, it's turned off, but you want to know about it" | 15:07 |
dtantsur | devananda, log on importing ironic.driver.modules.discoverd then? because not code from the class is ever called... | 15:07 |
devananda | my cocnern with validate result = false is that automated tools which enroll nodes and parse the output of "ironic node-validate" are going to see False as a failure | 15:07 |
devananda | which it, in this case, is not | 15:07 |
devananda | and then those tools will need special handling to, eg, grep for 'disabled' in the message string | 15:07 |
devananda | which sounds like a terrible user interface | 15:07 |
dtantsur | devananda, note that we already have deploy validation = False most of the time | 15:08 |
openstackgerrit | Ghe Rivero proposed openstack/ironic: Sync from oslo.incubator https://review.openstack.org/162505 | 15:08 |
devananda | dtantsur: log in the if: blocks here https://review.openstack.org/#/c/161132/6/ironic/drivers/pxe.py | 15:08 |
dtantsur | devananda, makes sense too (will create a helper function then). rameshg87 does it work for you? | 15:09 |
devananda | dtantsur: deploy validation = false when ironic doesn't have enough info to deploy. after a client (eg, nova, or ansible, or what ever) passes that in, then it should change to True | 15:10 |
devananda | that's how clients know they've satisfied all the required info | 15:10 |
dtantsur | yeah, my comment was about "automated tools which enroll nodes and parse the output of "ironic node-validate" | 15:10 |
jroll | devananda: this reminds me, I want to see your ansible thing: ) | 15:10 |
rameshg87 | dtantsur: but still the error msg "Driver pxe_ssh does not support inspect." | 15:11 |
devananda | dtantsur: oh, you're right :) s/enroll/do things with/ | 15:11 |
rameshg87 | dtantsur: is that fine still ? | 15:11 |
dtantsur | not fine, but the alternative sucks too :) | 15:12 |
*** stendulker has quit IRC | 15:12 | |
rameshg87 | yeah, :( | 15:13 |
dtantsur | devananda, what if we change the error to: "Driver pxe_ipmitool does not support inspect (not implemented or disabled)" | 15:13 |
dtantsur | wdyt? | 15:13 |
devananda | dtantsur: just to be clear, which error? | 15:14 |
*** aswadr has quit IRC | 15:14 | |
* dtantsur is looking | 15:14 | |
rameshg87 | devananda: when some one tries to do an inspect when it is disabled in CONF file | 15:15 |
rameshg87 | devananda: ironic node-set-provision-state <> inspect | 15:15 |
dtantsur | devananda, https://github.com/openstack/ironic/blob/master/ironic/common/exception.py#L344 | 15:15 |
*** david-lyle_afk is now known as david-lyle | 15:15 | |
rameshg87 | dtantsur: and https://github.com/openstack/ironic/blob/master/ironic/conductor/manager.py#L1480-L1482 | 15:16 |
openstackgerrit | Ramakrishnan G proposed openstack/ironic-python-agent: Add uefi support in image extension https://review.openstack.org/163739 | 15:17 |
*** achanda has joined #openstack-ironic | 15:17 | |
devananda | dtantsur: that seems reasonable | 15:19 |
dtantsur | ack, will change too | 15:19 |
* BadCub needs mroe coffee | 15:19 | |
BadCub | more even | 15:19 |
devananda | dtantsur: his is the first time we're adding an interface with a CONF option. i'm not surprised we're running into a few things that makes us change exception or log messages | 15:20 |
devananda | *this | 15:20 |
dtantsur | yeah | 15:20 |
JoshNang | jroll: so i was working on that on friday, and i'm thinking the better solution is to not power off the nodes in tear down, and do it at the end of cleaning. fixes a lot of cruft. | 15:22 |
rameshg87 | lucasagomes: are you aruond ? | 15:22 |
JoshNang | jroll: (at least for the agent driver) | 15:22 |
JoshNang | err drivers | 15:22 |
rameshg87 | lucasagomes: i am kind of getting occassional failures here: https://github.com/openstack/ironic-python-agent/blob/master/ironic_python_agent/extensions/image.py#L51 | 15:22 |
rameshg87 | lucasagomes: on actual hardware | 15:23 |
rameshg87 | lucasagomes: just wondering if partition table is not getting read properly fast enough | 15:23 |
lucasagomes | rameshg87, I will take a look in a sec | 15:23 |
rameshg87 | lucasagomes: did you test on real hardware ? image extension ? | 15:23 |
rameshg87 | lucasagomes: i am just testing with retry and delay_on_retry | 15:24 |
*** rameshg87 is now known as rameshg87-brb | 15:24 | |
*** achanda has quit IRC | 15:25 | |
jroll | JoshNang: don't you need to power them off to pxe boot them? | 15:26 |
JoshNang | jroll: derp, ignore me | 15:27 |
jroll | :) | 15:27 |
JoshNang | so getting it to work in devstack is all fine and well, but that'd be a new mandatory config option for deployers :/ | 15:27 |
openstackgerrit | Ruby Loo proposed openstack/ironic: Log message is missing a blank space https://review.openstack.org/164735 | 15:27 |
jroll | JoshNang: oh well? | 15:28 |
dtantsur | devananda, last thing about fake discoverd driver: I think it should fail to load, if discoverd is disabled. after all it's the driver designed to test discoverd support... I would leave it as it was | 15:28 |
*** mjturek1 has quit IRC | 15:28 | |
jroll | JoshNang: we should probably check for that at startup | 15:28 |
*** mjturek1 has joined #openstack-ironic | 15:29 | |
JoshNang | jroll: probably a good idea | 15:30 |
openstackgerrit | John L. Villalovos proposed openstack/ironic: Update unittests and use NamedTemporaryFile https://review.openstack.org/162672 | 15:32 |
jlvillal | rloo: You were talking about checking exception and log messages before in testing. I am proposing this patch in regards to that: https://review.openstack.org/164226 | 15:33 |
lucasagomes | rameshg87-brb, back, so hmmmm it should be good because I use partx -u to make the kernel re-read the part table | 15:36 |
lucasagomes | rameshg87-brb, when you do a delay_on_try does it solve the problem? | 15:36 |
*** rameshg87-brb is now known as rameshg87 | 15:37 | |
rameshg87 | lucasagomes: i guess it doesn't | 15:37 |
rameshg87 | lucasagomes: because stdout of lsblk is empty | 15:37 |
lucasagomes | hmm | 15:37 |
rloo | jlvillal: you were the one talking about it. I think I just said, yeah, it might be nice ;) | 15:37 |
rameshg87 | lucasagomes: will try checking if stdout is empty and adding a retry block waiting after 1 sec | 15:37 |
lucasagomes | rameshg87, I wondering whether partx is doing the right thing there, or we should use something like partprobe | 15:37 |
rameshg87 | partx man says reread partition table | 15:38 |
jlvillal | rloo: I thought you were discussing an issue with possible problems with checking the content of log messages because of I18N issues. | 15:38 |
rameshg87 | so looks like right thing :) | 15:38 |
lucasagomes | rameshg87, right, if you have ur hands on it, can u test wiht partprobe if that doesn't work with partx ? | 15:38 |
rameshg87 | to me | 15:38 |
jroll | lucasagomes: partprobe wasn't working for us in the agent in the gate :/ | 15:38 |
rloo | jlvillal: yeah, I did mention that. | 15:38 |
lucasagomes | rameshg87, yeah, but urgh I already had so many problems with both partprobe and partx man | 15:38 |
lucasagomes | jroll, oh | 15:38 |
NobodyCam | brb | 15:38 |
jroll | lucasagomes: (though it worked fine on real metal) | 15:38 |
rameshg87 | lucasagomes: on real hardware ? | 15:38 |
lucasagomes | jroll, not working how? | 15:38 |
lucasagomes | rameshg87, yeah | 15:38 |
lucasagomes | jroll, ouch, with vms? | 15:38 |
jroll | lucasagomes: can't remember, let me look | 15:39 |
rameshg87 | lucasagomes: okay let me check ... | 15:39 |
lucasagomes | rameshg87, out of context, but I had a cloning disk tool | 15:39 |
lucasagomes | which I used to play a lot of MBR and stuff | 15:39 |
jroll | lucasagomes: https://bugs.launchpad.net/ironic/+bug/1418833 | 15:39 |
openstack | Launchpad bug 1418833 in Ironic "check-tempest-dsvm-ironic-agent_ssh fails due to ConfigDriveWriteError" [Critical,Fix committed] - Assigned to Jay Faulkner (jason-oldos) | 15:39 |
lucasagomes | and relied on those tools to apply the images via network | 15:39 |
lucasagomes | rameshg87, https://github.com/umago/carbono | 15:39 |
lucasagomes | jroll, T.T | 15:40 |
lucasagomes | jroll, so did partx worked for the VMs in gate seems | 15:40 |
rameshg87 | lucasagomes: okay | 15:41 |
jroll | lucasagomes: yeah, and iirc it works fine on metal too, I think we have that change in our environment | 15:41 |
lucasagomes | gotcha | 15:41 |
jroll | JayF could confirm that when he's around | 15:41 |
lucasagomes | jroll, that's all good, thanks for the input | 15:41 |
devananda | dtantsur: ah - fair point about fake-discover | 15:41 |
rameshg87 | lucasagomes: okay, may be i need to check, but i am sure lsblk is returning empty for some reason. will confirm ... | 15:42 |
jroll | lucasagomes: yeah, np | 15:42 |
lucasagomes | rameshg87, so hmm... perhaps the smartest thing to do, is to weather call partx -u/partprobe a couple of times with a sleep between them | 15:42 |
lucasagomes | like a retry function | 15:42 |
devananda | dtantsur: if you could just put a comment in there saying why it's following a different pattern than the other drivers, that would help us 6mo from now when we're wondering why :) | 15:42 |
dtantsur | sure :) | 15:42 |
lucasagomes | it may take some time to the kernel notice the changes on the device | 15:42 |
devananda | lucasagomes: is there something you can poll, instead of sleep? | 15:42 |
rameshg87 | lucasagomes: yeah was thinking of that .. | 15:43 |
rameshg87 | lucasagomes: will give a try and come back ... | 15:43 |
lucasagomes | devananda, hmm we could look at devfs see if the partition we created just appeared there | 15:46 |
rameshg87 | devananda: i guess you just commented on an older patchset. can you please check if latest patchset is what you meant: https://review.openstack.org/#/c/164596/4/ironic_python_agent/utils.py | 15:46 |
lucasagomes | 1. calls partx -u. 2. looks at the fs by-uuid to see if the partition already appeared there | 15:47 |
rameshg87 | devananda: i have an _get and _set methods | 15:47 |
lucasagomes | we got the root uuid partition | 15:47 |
* NobodyCam is back | 15:47 | |
*** achanda has joined #openstack-ironic | 15:48 | |
jlvillal | Good morning Ironic.... | 15:48 |
NobodyCam | morning jlvillal :) | 15:48 |
*** wuhg has quit IRC | 15:49 | |
jlvillal | NobodyCam: Thanks. Did you get faster internet? If so, I hope it is working well :) | 15:49 |
BadCub | devananda: looked over the Review Day list. Looks like we have a few that can get closed out pretty easy, I think. | 15:50 |
devananda | BadCub: nice. please mark them on the page to get attention | 15:50 |
BadCub | Will do! | 15:50 |
NobodyCam | jlvillal: oh ya!!! real internetz is great! | 15:51 |
jlvillal | NobodyCam: Woo hoo! :D | 15:51 |
* rameshg87 will be back for the meeting ... | 15:53 | |
*** rameshg87 is now known as rameshg87-away | 15:53 | |
NobodyCam | night rameshg87-away | 15:53 |
devananda | rameshg87: that will work. but I still dont see why you need to use deepcopy | 15:53 |
*** rameshg87-away is now known as rameshg87 | 15:53 | |
rameshg87 | devananda: just to make sure if there is nested dictionary in params | 15:54 |
jlvillal | rameshg87: The meeting is in about 15 hours | 15:54 |
jlvillal | I think | 15:54 |
rameshg87 | oh | 15:54 |
rameshg87 | i thought it was in 1 hour :) | 15:54 |
jlvillal | rameshg87: Unless it is a different meeting | 15:54 |
rameshg87 | my mistake :) | 15:54 |
rameshg87 | then it's my tomorrow actually | 15:54 |
rameshg87 | thanks jlvillal | 15:54 |
jlvillal | rameshg87: You're welcome | 15:54 |
devananda | 13 hours | 15:55 |
rameshg87 | devananda: isn't deepcopy required to make sure that we have complete copy if it was a nested dictionary ? | 15:55 |
devananda | rameshg87: why is copy needed at all, i mean? | 15:56 |
devananda | rameshg87: why not simply return the dict (which is really a reference to it) | 15:56 |
devananda | is there a concern that a calling function is going to modify the global? | 15:56 |
rameshg87 | devananda: but what if some code modified it ? | 15:56 |
rameshg87 | devananda: yeah | 15:56 |
devananda | and so what? is that a bad thing? | 15:56 |
devananda | (I dont know this code that well -- that's why I'm asking) | 15:57 |
rameshg87 | devananda: if *someone* modified the values of keys or deleted some keys | 15:57 |
rameshg87 | devananda: the code trying to access later on might never have it | 15:57 |
BadCub | brb | 15:57 |
BadCub | devananda: I put notes on two of them to start | 15:57 |
rameshg87 | devananda: i am not saying it will happen, but can happen. just wanted to be on safer side as it will be cache will be used for the lifetime of the agent | 15:57 |
devananda | rameshg87: k. so, if that's a concern, there's a better way than copying it twice | 15:58 |
devananda | i'll comment on the review | 15:58 |
rameshg87 | devananda: okay .. thanks .. | 15:58 |
rameshg87 | will check .. | 15:58 |
*** rameshg87 is now known as rameshg87-away | 15:59 | |
*** absubram has quit IRC | 16:01 | |
openstackgerrit | Dmitry Tantsur proposed openstack/ironic: Start using in-band inspection https://review.openstack.org/161132 | 16:02 |
dtantsur | devananda, rameshg87-away ^^^ | 16:02 |
openstackgerrit | Dmitry Tantsur proposed openstack/ironic: Write documentation for hardware inspection https://review.openstack.org/161775 | 16:03 |
*** achanda has quit IRC | 16:04 | |
*** stendulker has joined #openstack-ironic | 16:07 | |
*** ijw has quit IRC | 16:07 | |
*** stendulker_ has joined #openstack-ironic | 16:09 | |
devananda | this https://review.openstack.org/#/c/162449/4/ironic/drivers/modules/ilo/deploy.py,cm could use eyes from an IPA core -- jroll? | 16:11 |
*** openstackgerrit has quit IRC | 16:11 | |
* jroll will look | 16:11 | |
devananda | I'm a bit confused by having a continue_deploy and a _continue_deploy method, which are not related | 16:12 |
*** openstackgerrit has joined #openstack-ironic | 16:12 | |
NobodyCam | lucasagomes: question looking at 162449: the commit message refferences blueprint ipa-as-default-ramdisk I not seeing that BP. could be /me's lack of coffee! | 16:12 |
*** stendulker has quit IRC | 16:12 | |
lucasagomes | NobodyCam, https://blueprints.launchpad.net/ironic/+spec/ipa-as-default-ramdisk | 16:12 |
* lucasagomes checks if he misspelledit | 16:13 | |
NobodyCam | Implemented Edit | 16:13 |
NobodyCam | doh | 16:13 |
lucasagomes | looks correct, not sure why it can't find it tho hmmm | 16:14 |
NobodyCam | it listed as Implemented | 16:14 |
lucasagomes | oh | 16:14 |
NobodyCam | BadCub: ^^ | 16:14 |
BadCub | huh | 16:14 |
NobodyCam | 162449 should be need review! | 16:14 |
BadCub | set to needs review | 16:15 |
NobodyCam | awesome THank you | 16:15 |
NobodyCam | and now it shows up :) | 16:16 |
openstackgerrit | Merged openstack/ironic: Implement execute clean steps https://review.openstack.org/155561 | 16:21 |
openstackgerrit | Merged openstack/ironic: Use oslo_context package https://review.openstack.org/162499 | 16:22 |
JoshNang | \o/ thanks for all the help everyone ^ | 16:22 |
dtantsur | :) | 16:22 |
NobodyCam | w00 h00 :) | 16:22 |
devananda | JoshNang: o/^5 | 16:22 |
openstackgerrit | Merged openstack/ironic: Use oslo_policy package https://review.openstack.org/162501 | 16:22 |
jlvillal | JoshNang: Congrats on all the work to get that done! :) | 16:23 |
jroll | devananda: I think you're misunderstanding 162449 | 16:23 |
devananda | jroll: that's quite possible | 16:24 |
jroll | this is the same change lucas just did to enable agent/iscsi | 16:24 |
jroll | this enables agent/iscsi on ilo | 16:24 |
jroll | continue_deploy() comes from the base class and is for use with the agent ramdisk, _continue_deploy() is for use with bash ramdisk | 16:24 |
NobodyCam | lucasagomes: question (again) looking at https://review.openstack.org/#/c/162449/4/ironic/drivers/modules/ilo/deploy.py lines 622-623 should we be checking auth_strategy conf setting there? we could be set to no-auth | 16:24 |
*** ijw has joined #openstack-ironic | 16:24 | |
NobodyCam | https://github.com/openstack/ironic/blob/master/etc/ironic/ironic.conf.sample#L203 | 16:25 |
devananda | jroll: ah. so *AgentDriver is compatible with both ipa and dib ramdisks? | 16:25 |
*** rloo has quit IRC | 16:25 | |
devananda | sorry if that is a thing I should already know - my brain isn't fully awake yet | 16:25 |
*** rloo has joined #openstack-ironic | 16:26 | |
jroll | devananda: no, Pxe*Driver is compatible with ipa and bash ramdisks | 16:26 |
devananda | jroll: ok. that's what I thought | 16:26 |
lucasagomes | NobodyCam, oh that's a good point | 16:26 |
NobodyCam | want me to comment on review? | 16:26 |
*** rloo_ has joined #openstack-ironic | 16:27 | |
*** rloo has quit IRC | 16:27 | |
openstackgerrit | Merged openstack/ironic: Address comments on cleaning commit https://review.openstack.org/164645 | 16:27 |
openstackgerrit | Merged openstack/python-ironicclient: Add support for inspection to node-set-provision-state https://review.openstack.org/148804 | 16:27 |
lucasagomes | NobodyCam, please do... also, I see we are doing that in other parts of the code | 16:28 |
lucasagomes | I don't know how that would play with noauth | 16:28 |
*** Nisha has joined #openstack-ironic | 16:28 | |
NobodyCam | :( | 16:28 |
lucasagomes | NobodyCam, re https://github.com/openstack/ironic/blob/master/ironic/conductor/manager.py#L1142-L1143 | 16:28 |
jroll | NobodyCam: I think that's fine... | 16:28 |
jroll | noauth is saying "ironic doesn't validate tokens" | 16:29 |
jroll | that grabs a token to talk to glance | 16:29 |
jroll | do we have a glance auth_strategy setting? | 16:29 |
lucasagomes | maybe the keystone module will just ignore that if noauth | 16:29 |
rloo_ | hi JoshNang: wrt https://review.openstack.org/164313, I was hoping we could code it so that we didn't have to update it when we added new states like zap*, validat*, but maybe we can't avoid it. | 16:29 |
devananda | lucasagomes: so there are deploymets without keystone or glance | 16:29 |
devananda | *of ironic without | 16:30 |
NobodyCam | my fear is there could not be a keystone service | 16:30 |
JoshNang | rloo_: hmmm | 16:30 |
lucasagomes | devananda, yeah, after nobodycam comments it just came to mind | 16:30 |
jroll | hrm, maybe | 16:30 |
devananda | jroll: ok, I still dont quite grok the change's effect on IloVirtualMediaIscsiDriver -- but that aside, having unrelated methods whose names are only different by a leading "_" is just weird | 16:30 |
lucasagomes | I'm checking cause seems in other parts of the code we are also calling keystone.get_admin_token() without checking noauth | 16:30 |
jroll | devananda: the change is, that driver can now use the IPA ramdisk. | 16:31 |
jroll | devananda: and that is weird, but I believe IscsiVendorWhatever does the same. | 16:32 |
devananda | lucasagomes: so I believe we work around that currently by passing a fake token in the HTTP request, and setting keystone auth_mode = noauth in the config | 16:32 |
JoshNang | rloo_: i think i'd rather keep it explicit rather than implicit. | 16:32 |
devananda | jroll: gotcha. | 16:32 |
lucasagomes | devananda, right, yeha so that bits on 622-623 will have no problem right? | 16:32 |
*** kalpase has quit IRC | 16:34 | |
devananda | jroll: reversed my -1 | 16:34 |
jroll | devananda: I +2'd that change, leaving it to you to +A | 16:34 |
jroll | oh | 16:34 |
jroll | ok, I can +A | 16:34 |
devananda | oh | 16:34 |
devananda | k, thx | 16:34 |
NobodyCam | lol | 16:34 |
NobodyCam | gah you folk are quick | 16:35 |
NobodyCam | reverted my vote | 16:36 |
NobodyCam | lol | 16:36 |
*** rameshg87-away is now known as rameshg87 | 16:36 | |
rameshg87 | oh just in time :) | 16:36 |
rameshg87 | devananda: will clean up the function name - it has to be done for pxe driver too | 16:36 |
rameshg87 | devananda: will do that in a patch tomorrow (my time) | 16:37 |
rloo_ | JoshNang: that's fine. for now anyway. just means we need to update it. maybe we should just add in all the states even if they aren't used yet. | 16:37 |
devananda | rameshg87: thanks much | 16:37 |
NobodyCam | mornign rloo_ :) | 16:37 |
* NobodyCam gets more coffee | 16:38 | |
lucasagomes | devananda, ur comment remids me that I've to rename _continue_deploy to pass_deploy_info | 16:38 |
lucasagomes | on pxe | 16:38 |
devananda | ++ | 16:38 |
rloo_ | morning NobodyCam. Already my afternoon and I'm still trying to catch up on what you all did in the past few days ;) | 16:38 |
rameshg87 | lucasagomes: i will take care if you are not getting to it before tomorrow | 16:38 |
rameshg87 | lucasagomes: i will do for both ilo and pxe driver | 16:38 |
lucasagomes | rameshg87, I will put one up for the pxe today | 16:38 |
lucasagomes | rameshg87, cool | 16:38 |
* BadCub needs to grab food | 16:39 | |
NobodyCam | rloo_: heheheh :) we've been busy! | 16:40 |
JoshNang | rloo_: i just added everything already in states.py. getting a small patch in to nova when we add states shouldn't be hard | 16:40 |
rloo_ | JoshNang: "shouldn't be hard" ;) | 16:41 |
dtantsur | rloo_, morning! | 16:41 |
JoshNang | rloo_: oh trust me...i know :) | 16:41 |
openstackgerrit | Ramakrishnan G proposed openstack/ironic-python-agent: Cache agent parameters for later invocations https://review.openstack.org/164596 | 16:41 |
JayF | jroll: we don't have that change in our env yet | 16:41 |
rloo_ | JoshNang: :D | 16:41 |
jroll | JayF: really? not preprod either? | 16:42 |
devananda | getting food and more coffee... bbiab | 16:42 |
JayF | jroll: we aren't past my HW Manager refactor b/c the onmetal-ironic-hardware-manager hasn't been updated to that interface yet | 16:42 |
rloo_ | hi dtantsur and everyone else :) | 16:42 |
rameshg87 | dtantsur: ack | 16:42 |
jroll | JayF: oh right | 16:42 |
jroll | thanks | 16:42 |
jroll | I think we did test it on some metal, at least | 16:42 |
JayF | If you did, sure. I haven't. | 16:44 |
*** anderbubble has joined #openstack-ironic | 16:45 | |
jroll | hm | 16:45 |
*** anderbubble has left #openstack-ironic | 16:45 | |
rameshg87 | dtantsur: https://review.openstack.org/#/c/161132/7/ironic/drivers/drac.py | 16:45 |
rameshg87 | dtantsur: better if it was pxe_drac, right ? | 16:45 |
rameshg87 | dtantsur: instead of PXEDracDriver | 16:46 |
dtantsur | rameshg87, I don't if code should assume knowledge about setup.cfg | 16:46 |
rameshg87 | dtantsur: at that point driver_factor is not completely initialized, right ? | 16:46 |
dtantsur | I think so | 16:46 |
*** ukalifon1 has quit IRC | 16:48 | |
rameshg87 | okay | 16:48 |
* rameshg87 goes to sleep | 16:54 | |
rameshg87 | good night all ... | 16:54 |
*** openstackgerrit has quit IRC | 16:54 | |
*** rameshg87 has left #openstack-ironic | 16:54 | |
*** openstackgerrit has joined #openstack-ironic | 16:54 | |
openstackgerrit | Ruby Loo proposed openstack/python-ironicclient: Metavar name should be hyphenated https://review.openstack.org/164781 | 16:57 |
NobodyCam | devananda: did you want to take another look at 164596? I'm about to add second +2 | 16:58 |
openstackgerrit | Dmitry Tantsur proposed stackforge/ironic-discoverd: [WIP] Add DevStack plugin for ironic-discoverd https://review.openstack.org/164782 | 16:58 |
*** mtanino has joined #openstack-ironic | 16:59 | |
*** Marga_ has quit IRC | 17:01 | |
dtantsur | see you tomorrow | 17:02 |
*** dtantsur is now known as dtantsur|afk | 17:02 | |
openstackgerrit | Lucas Alvares Gomes proposed openstack/ironic: Rename _continue_deploy() to pass_deploy_info() https://review.openstack.org/164783 | 17:04 |
* devananda returns | 17:09 | |
devananda | dtantsur|afk: I believe discoverd disabled should be an INFO or NOTICE, not WARN | 17:10 |
devananda | dtantsur|afk: also there's a typo in the log. /me fixes | 17:11 |
NobodyCam | night dtantsur|afk | 17:11 |
*** stendulker_ has quit IRC | 17:12 | |
devananda | NobodyCam: rut roh. new output from validate this morning in stand-alone mode is now this: | 17:14 |
devananda | | deploy | False | Couldn't get the URL of the Ironic API service from the configuration file or keystone catalog. Keystone error: Keystone API endpoint is missing | | 17:14 |
NobodyCam | is that 161132? I was just opening that one? | 17:15 |
devananda | that's master | 17:15 |
devananda | well,actually the discoverd change, but it shouldn't be related | 17:16 |
* devananda looks further | 17:16 | |
*** ijw has quit IRC | 17:16 | |
*** mgoddard1 has quit IRC | 17:18 | |
openstackgerrit | Devananda van der Veen proposed openstack/ironic: Fixup log message for discoverd https://review.openstack.org/164792 | 17:19 |
openstackgerrit | Nisha Agarwal proposed openstack/ironic: iLO implementation for hardware inspection https://review.openstack.org/151596 | 17:20 |
devananda | who wants to land discoverd support? | 17:22 |
devananda | I think it's good to go -- just need eyes on https://review.openstack.org/#/c/161132/ | 17:22 |
NobodyCam | devananda: reviewing now | 17:22 |
devananda | great | 17:22 |
*** rsalevsky has quit IRC | 17:23 | |
NobodyCam | oh I like the create_if_enabled stuff :) | 17:26 |
*** mgoddard has joined #openstack-ironic | 17:26 | |
*** harlowja has joined #openstack-ironic | 17:28 | |
rloo_ | devananda: did you test 161132? I'm ok with it but was thinking of testing it but if you already did... | 17:28 |
openstackgerrit | Nisha Agarwal proposed openstack/ironic: iLO driver updates node capabilities during inspection https://review.openstack.org/163572 | 17:29 |
*** ijw has joined #openstack-ironic | 17:29 | |
*** Marga_ has joined #openstack-ironic | 17:30 | |
*** lazy_prince is now known as killer_prince | 17:31 | |
*** ijw has quit IRC | 17:31 | |
*** ijw has joined #openstack-ironic | 17:31 | |
*** ijw has quit IRC | 17:33 | |
*** ijw has joined #openstack-ironic | 17:34 | |
*** dprince has quit IRC | 17:36 | |
devananda | rloo_: I have tested that all the things load and report correctly -- i have not actually tested ironic-discoverd with it, though | 17:40 |
rloo_ | devananda: ok, that's good enough, since that's what the patch is about. thx. | 17:40 |
devananda | yup. cheers | 17:40 |
NobodyCam | devananda: did you see my question on: 164596? I added a second +2 but didn't +a as you had a -1, just checking for you wanted to have a quick look? | 17:43 |
*** ukalifon1 has joined #openstack-ironic | 17:43 | |
devananda | NobodyCam: thanks, looking | 17:44 |
NobodyCam | Nisha: are you around? | 17:48 |
Nisha | NobodyCam, yes | 17:48 |
NobodyCam | hey there | 17:48 |
NobodyCam | :) | 17:48 |
Nisha | NobodyCam, :) | 17:48 |
NobodyCam | I looking at the comments on 151596 | 17:48 |
Nisha | ok | 17:48 |
*** MattMan has quit IRC | 17:48 | |
NobodyCam | is there documantation we can point to for the port number stuff | 17:49 |
NobodyCam | i didn't see anything on http://docs.openstack.org/developer/ironic/drivers/ilo.html | 17:49 |
Nisha | No its only the RIBCL output | 17:49 |
NobodyCam | that may help over come some of the questions | 17:49 |
Nisha | yes | 17:49 |
Nisha | NobodyCam, just a min | 17:50 |
NobodyCam | ie. some examples in the doc's of getting and setting that param would help I believe | 17:50 |
*** achanda has joined #openstack-ironic | 17:50 | |
NobodyCam | sure | 17:50 |
*** dprince has joined #openstack-ironic | 17:50 | |
*** jistr has quit IRC | 17:51 | |
BadCub | Anyone interested in taking a gander at : https://review.openstack.org/#/c/163857/ | 17:51 |
Nisha | #link we can quote "http://h20565.www2.hp.com/hpsc/doc/public/display?docId=emr_na-c03334058-13&docLocale=" | 17:51 |
Nisha | the sample return from GET_EMBEDDED_HEALTH | 17:52 |
*** ukalifon1 has quit IRC | 17:52 | |
Nisha | NobodyCam, http://paste.openstack.org/show/192686/ | 17:53 |
*** achanda_ has joined #openstack-ironic | 17:55 | |
NobodyCam | Nisha: yes! I understand what needs to be there based on the review and the sample in _get_desired_macs_for_port_creation | 17:57 |
Nisha | NobodyCam, any action item for me? | 17:57 |
NobodyCam | but would it be worth as part of this patch to add to the iLo docs a section on getting the correct port numbers to use and an example of setting them on hte node? | 17:58 |
*** achanda has quit IRC | 17:58 | |
NobodyCam | ie as a op how will i know which port I have pluged my cable into? | 17:58 |
Nisha | NobodyCam, as a operator if you have plugged the cable you will know the port number from the hardware itself | 17:59 |
lucasagomes | folks I will call it a day | 17:59 |
NobodyCam | have a good night lucasagomes | 17:59 |
Nisha | its written clearly on the hardware (for embedded NICs atleast | 17:59 |
BadCub | g'night lucasagomes | 18:00 |
JayF | Nisha: I think there's still value in being able to pull that info out of band | 18:00 |
lucasagomes | I will be off tomorrow due the St. Patricks day | 18:00 |
lucasagomes | enjoy it folks :) | 18:00 |
Nisha | JayF, i didnt get | 18:00 |
*** lucasagomes is now known as lucas-dinner | 18:00 | |
JayF | Nisha: I've had issues where the documentation given to me from $physical_hands_in_datacenter was incorrect, and only because we were double-verifying we cought it | 18:00 |
openstackgerrit | Devananda van der Veen proposed openstack/ironic-python-agent: Fix nit in test_get_agent_params_from_cache https://review.openstack.org/164805 | 18:00 |
Nisha | how will we know OOB which port is plugged | 18:00 |
krotscheck | jroll: Hey, sorry about disappearing on you on friday. You mentioned the #rackspace channel? | 18:00 |
NobodyCam | Nisha: so all hp server have somehting like: http://h20564.www2.hp.com/hpsc/doc/public/imageServlet?DOCID=emr_na-c00385409-9/c00386425.gif | 18:01 |
JayF | krotscheck: if you /join #rackspace on this network, talk to kenperkins | 18:01 |
Nisha | only when OS is running underlying protocol like RIS/RIBCL can give the NIC status | 18:01 |
JayF | krotscheck: pkgcloud not supported in browser today but apparently is on a todo, so perhaps the chat will help | 18:01 |
krotscheck | JayF: Thansk | 18:01 |
Nisha | NobodyCam, yes i checked on gen8 and gen9 | 18:02 |
devananda | JoshNang: you rebasing https://review.openstack.org/#/c/159322/ ? | 18:05 |
openstackgerrit | Josh Gachnang proposed openstack/ironic: Add /nodes/<uuid>/cleaning/steps API https://review.openstack.org/159322 | 18:07 |
JoshNang | devananda: lol literally what i was working on | 18:07 |
devananda | JoshNang: lol | 18:07 |
devananda | NobodyCam: fwiw, 164792 is the sort of change that I think we can start safely approving with only one +2 | 18:08 |
NobodyCam | that your log update? | 18:08 |
devananda | it is | 18:08 |
devananda | i saw you just +2'd it | 18:08 |
NobodyCam | i didn't just because the depent patchwas not yet approved | 18:08 |
NobodyCam | :-p | 18:08 |
devananda | oh | 18:08 |
NobodyCam | it may be now! | 18:09 |
NobodyCam | :-p | 18:09 |
devananda | so you can +A a patch further down a chain like that, and jenkins will wait until the chain is approved and then merge all together | 18:09 |
NobodyCam | ok... | 18:09 |
NobodyCam | :) | 18:09 |
devananda | but yes, it looks like rloo approved the discoverd patch | 18:09 |
NobodyCam | lol +a'd | 18:10 |
NobodyCam | Nisha: maybe something like: Comma seperated values of ethernet ports to be identified when creating the node ports. Valid values may be: for lines 79,80 of https://review.openstack.org/#/c/151596/37/ironic/drivers/modules/ilo/common.py | 18:12 |
*** derekh has quit IRC | 18:12 | |
NobodyCam | maybe other can weigh in with thoughts too | 18:12 |
NobodyCam | others even | 18:12 |
BadCub | devananda: MAKED https://blueprints.launchpad.net/ironic/+spec/inband-properties-discovery as implemented | 18:13 |
BadCub | marked even | 18:13 |
devananda | woot | 18:13 |
devananda | BadCub: well, wait ... it's not all merged yet | 18:14 |
BadCub | oop | 18:14 |
devananda | BadCub: that's a tiny bit pre-emptive, even though it's all approved something could fail to merge | 18:14 |
Nisha | NobodyCam, Looks fine, we may add saying "Comma seperated values of ethernet ports(which are cable connected) to be identified when creating node ports.... | 18:14 |
BadCub | *grumbles at Jenkins* | 18:14 |
NobodyCam | Nisha: nit: s/which are/ which have/ | 18:15 |
NobodyCam | :-p | 18:15 |
Nisha | NobodyCam, ok. I will update the patch with this. any other comments | 18:16 |
Nisha | please leave comments on the patch if any more | 18:16 |
Nisha | NobodyCam, one more query | 18:16 |
NobodyCam | I think that will help clear it for folks | 18:16 |
NobodyCam | sure | 18:16 |
Nisha | for delete_ports what shall be the action from ironic side: | 18:17 |
Nisha | 1. delete ports only for the non-associated macs | 18:17 |
Nisha | or | 18:17 |
Nisha | 2. delete all the ports which are not requested in 'inspect_ports' | 18:18 |
Nisha | cons for 1. -> we may have additional ports which may not have cable connected | 18:19 |
devananda | JoshNang: so in 159322, you add a new controller to NodesController -- this is going to change the base (v1.1) API | 18:19 |
Nisha | cons for 2 -> it may delete the already created macs which may not be in 'inspect_ports' | 18:20 |
devananda | JoshNang: take a look at my proposed changes in 164369 -- it actually hides the controller from requests predating that version | 18:20 |
Nisha | NobodyCam, i guess risk of 1 is less | 18:20 |
*** rloo_ has quit IRC | 18:20 | |
Nisha | than risk of 2 | 18:20 |
devananda | JoshNang: if we really want to preserve API semantics depending on the version, then you'll need to do that | 18:20 |
NobodyCam | oh thats not that easy, what would happen if as a op I inspected a node, then manually added a port for a nic which I installed, like a dolphin ethernet card | 18:21 |
Nisha | that will be there | 18:21 |
Nisha | i think i will go with option 1 | 18:22 |
Nisha | risk of 2 is big | 18:22 |
NobodyCam | ++ | 18:22 |
JoshNang | devananda: that's reasonable. jroll had initially proposed returning a 404, but it seems that's different than actually not existing :/ | 18:22 |
devananda | yes | 18:22 |
devananda | JoshNang: thta is apparently an issue with rest.RestController | 18:23 |
devananda | with some help from ryanpetrello, i worked around it | 18:23 |
JoshNang | alright, i'll take queues from 164369 | 18:23 |
JoshNang | err cues | 18:23 |
*** mgoddard has quit IRC | 18:23 | |
krotscheck | If I were to create an isomorphic ironic API javascript library, what would I name it? | 18:28 |
krotscheck | That's a lot of words. | 18:28 |
krotscheck | If I were to create a javascript thing that talks to ironic, what would I name it? | 18:28 |
krotscheck | javascript-ironic-lib? | 18:29 |
JayF | js-ironicclient? | 18:29 |
devananda | JoshNang: none of the drivers actually implement clean_step yet, correct? (or am i just blind) | 18:29 |
devananda | ironic-javascript-client? | 18:30 |
krotscheck | JayF: It's not truly a client though - no commandline | 18:30 |
devananda | krotscheck: ajil | 18:30 |
krotscheck | ajil? | 18:30 |
devananda | krotscheck: a-javascript-ironic-library | 18:30 |
NobodyCam | lol | 18:30 |
NobodyCam | ieek | 18:30 |
devananda | also, sounds like AGILE | 18:30 |
krotscheck | ...... | 18:30 |
JayF | krotscheck: I disagree that a client has to include the CLI, but I get your thought process | 18:30 |
devananda | ^_^ | 18:30 |
* krotscheck sadtrombones all over the place. | 18:30 | |
JoshNang | devananda: correct. i have a patch up to add them to the agent for inband, ramieni has one for ilo out of band | 18:31 |
devananda | krotscheck: no sad trombone. we love bad puns here :) | 18:31 |
JoshNang | (which needs devstack fixes...booting an agent outside of the deploy cycle is...challenging) | 18:31 |
krotscheck | Well, who knows. We might get to the point where javascript files get a shebang line. | 18:31 |
openstackgerrit | Merged openstack/ironic: iscsi_ilo driver to support agent ramdisk https://review.openstack.org/162449 | 18:31 |
*** wanyen has joined #openstack-ironic | 18:31 | |
krotscheck | aaactually | 18:31 |
openstackgerrit | Merged openstack/ironic: Log message is missing a blank space https://review.openstack.org/164735 | 18:32 |
JayF | I've def. seen #!/usr/bin/node as the first line of a file, he | 18:32 |
JayF | h | 18:32 |
devananda | JoshNang: great. also, i didn't see https://review.openstack.org/#/c/164646/1 on the 'pad, so just added it | 18:32 |
JoshNang | devananda: ++ | 18:33 |
*** saripurigopi has joined #openstack-ironic | 18:34 | |
krotscheck | Oh wow. That actually just worked | 18:35 |
krotscheck | Today, the commandline.... | 18:35 |
krotscheck | ...tomorrow, THE WORLD! | 18:35 |
*** dprince has quit IRC | 18:35 | |
NobodyCam | ok pinky | 18:37 |
NobodyCam | :-p | 18:37 |
NobodyCam | oh wait that was brain who said that | 18:38 |
openstackgerrit | Merged openstack/ironic-python-agent: Cache agent parameters for later invocations https://review.openstack.org/164596 | 18:41 |
devananda | JoshNang: should clean priorities be config options ? re: https://review.openstack.org/#/c/157715/16/etc/ironic/ironic.conf.sample,cm | 18:41 |
JoshNang | devananda: definitely. it lets the operator enable, disable, and reorder (if necesssary) | 18:42 |
devananda | k k | 18:42 |
devananda | JoshNang: also, i posted a nit following up on ^^ here https://review.openstack.org/#/c/164805/ | 18:42 |
JoshNang | devananda: ahh saw your comment on the other patch. reviewing now | 18:43 |
*** dprince has joined #openstack-ironic | 18:44 | |
openstackgerrit | Nisha Agarwal proposed openstack/ironic: Automate boot iso creation with in ironic for iscsi-ilo https://review.openstack.org/155900 | 18:48 |
devananda | JoshNang: why does get-clean_steps for the agent require the agent to be booted? | 18:50 |
devananda | JoshNang: a 409 CONFLICT becaues the node is in the wrong power state doesn't make sense for a GET request ... | 18:50 |
JayF | devananda: the agent can dynamically include/exclude hardware managers and decom steps using hardware managers | 18:51 |
NobodyCam | JayF: | 18:51 |
JayF | devananda: without a running agent, there's no way to know what hardware managers would be in/enabled for that agent/hw pairing, and therefore no way of knowing the decom steps | 18:51 |
JayF | s/decom/cleaning/ | 18:51 |
* JayF brb -> lunch | 18:51 | |
devananda | JayF: but a 409 seems a bit extreme | 18:51 |
JayF | We talked a little about the behavior, I was on the side of failing the query if it was missing info (because agent wasn't booted) | 18:52 |
JoshNang | ^ | 18:52 |
JayF | I don't have a personal preference as to the http code in return | 18:52 |
devananda | JayF: what about a response from the agent driver to the effect of "in-band cleaning will be done by the agent, details unknown at this time" | 18:52 |
JayF | I really gotta go to lunch though, catered today so I don't wanna be late. be back shortly. plus JoshNang can stay, he's not here to enjoy the delicious free food | 18:53 |
devananda | ack | 18:53 |
JoshNang | :( | 18:53 |
JayF | devananda: I think communicating that would be difficult/impossible without an error status | 18:53 |
JayF | as an operator, if I ask for a thing, if you throw a 2xx, I should be able to assume it's complete | 18:53 |
* JayF actually gone | 18:53 | |
NobodyCam | open question: should our tests actually create / delete files? REF: https://review.openstack.org/#/c/162672/12/ironic/tests/drivers/test_ipmitool.py | 18:53 |
devananda | JoshNang, JayF: my concern with ^ is that the API semantics // workflow for using cleaning is going to be different for different drivers | 18:54 |
devananda | NobodyCam: if they are properly using tempdirs, I think that's fine | 18:54 |
NobodyCam | :) ack :) | 18:54 |
NobodyCam | TY | 18:54 |
JoshNang | devananda: i was considering caching the cleaning steps on the node object as a potential way around it | 18:54 |
devananda | JoshNang: ++ | 18:55 |
JoshNang | but there's still the hangup that you could boot a different agent with a different set of cleaning steps | 18:55 |
devananda | well. initial thought aside, that's potentially incorrect too | 18:55 |
devananda | right | 18:55 |
devananda | what the agent did last time doesn't necessarily mean it will be able to do the same thing this time | 18:55 |
JoshNang | right | 18:55 |
BadCub | devananda: I moved https://blueprints.launchpad.net/ironic/+spec/inband-properties-discovery up to the Approved/Waiting to Land category on Whiteboard | 18:56 |
*** ijw_ has joined #openstack-ironic | 18:56 | |
*** Marga_ has quit IRC | 18:57 | |
devananda | BadCub: ++ | 18:57 |
*** Marga_ has joined #openstack-ironic | 18:57 | |
JoshNang | devananda: can we pass a query param to the effect of "no cache, give me the current or fail?" | 18:57 |
JoshNang | and default it to no? i think for most deployments, the agent cleaning steps aren't going to be changing a ton | 18:58 |
devananda | JoshNang: my objection is that I dont want a different API for in vs out of band | 18:58 |
NobodyCam | jlvillal: question on 162672 | 18:58 |
JoshNang | gotcha. hmm. | 18:59 |
*** ijw has quit IRC | 18:59 | |
devananda | JoshNang: half-formed thought: maybe make the whole endpoint only accessible while in manageable state, and have the agent running by default in that state? | 18:59 |
NobodyCam | should we be taking https://github.com/openstack/ironic/blob/master/etc/ironic/ironic.conf.sample#L316 into account in _make_password_file | 19:00 |
JoshNang | devananda: that...fixes a lot of things | 19:00 |
devananda | JoshNang: heh | 19:00 |
JoshNang | so we still have the issue that booting an agent + neutron dhcp means we need a network for out-of-deploy-cycle agent booting | 19:01 |
JoshNang | i wrote support for it already, requiring a conf option of the network uuid. not sure this is the best way, but fwiw it's similar to what we do downstream | 19:02 |
*** Marga_ has quit IRC | 19:03 | |
*** Marga_ has joined #openstack-ironic | 19:03 | |
JoshNang | devananda: (the above applies to booting any ramdisk outside the deploy cycle where nova creates the neutron ports for us) | 19:05 |
*** mgoddard has joined #openstack-ironic | 19:06 | |
*** Nisha has quit IRC | 19:06 | |
*** Nisha has joined #openstack-ironic | 19:07 | |
devananda | ooh, right | 19:11 |
devananda | so that applies to any time the agent needs to get on the network | 19:11 |
JoshNang | yup | 19:12 |
JoshNang | so i don't think its terribly unreasonable to require the operator to provide a neutron network, but we'd really need to document it | 19:12 |
devananda | yea, so that's not part of these patches | 19:12 |
JoshNang | it's in the agent cleaning patch | 19:13 |
devananda | oh, it is? | 19:13 |
JoshNang | https://review.openstack.org/#/c/161453/8/ironic/dhcp/neutron.py | 19:13 |
devananda | ahh | 19:13 |
JoshNang | which seems like it should be more general | 19:14 |
devananda | yup | 19:14 |
devananda | what if I want to use hte agent w/o neutron? | 19:14 |
devananda | which, in fact, I do | 19:14 |
JoshNang | hey me too! | 19:15 |
JoshNang | assuming you're using a flat network with tftp, you'd just reboot with pxe | 19:15 |
devananda | yup | 19:16 |
devananda | JoshNang: the client can't actually disable any clean steps via the REST API | 19:17 |
JoshNang | correct | 19:17 |
devananda | JoshNang: so we're merely exposing the ability to list them ... why is that actually important to the client? | 19:17 |
JoshNang | i definitely think operators are going to want a list of what steps a node would take, especially since they'll be different depending on drivers | 19:18 |
JoshNang | but i don't think it's essential, just good for operator visibility | 19:18 |
JoshNang | the operator could get the list from the agent and knows what he/she set in the config options, but if we can provide an api, i think we should | 19:20 |
openstackgerrit | Merged openstack/ironic: Start using in-band inspection https://review.openstack.org/161132 | 19:21 |
openstackgerrit | Merged openstack/ironic: Write documentation for hardware inspection https://review.openstack.org/161775 | 19:21 |
*** Marga_ has quit IRC | 19:21 | |
*** ParsectiX_ has joined #openstack-ironic | 19:22 | |
*** Marga_ has joined #openstack-ironic | 19:22 | |
devananda | JoshNang: are the agent's choices being driven by the conductor config option settings? | 19:22 |
*** ParsectiX_ has quit IRC | 19:22 | |
devananda | JoshNang: or is the agent freely discovering new things and doing stuff the conductor doesn't know about? | 19:22 |
JoshNang | devananda: the agent cleaning steps are determined by the hardware managers | 19:23 |
JoshNang | each manager returns a list | 19:23 |
*** Marga_ has quit IRC | 19:25 | |
*** Marga_ has joined #openstack-ironic | 19:26 | |
JoshNang | the agent reports the list of steps it has, and the conductor combines all the steps then starts telling drivers to execute each step | 19:26 |
*** rloo has joined #openstack-ironic | 19:27 | |
*** saripurigopi has quit IRC | 19:31 | |
jroll | 18:54:22 devananda | NobodyCam: if they are properly using tempdirs, I think that's fine <- this may fail under selinux with certain configs, we've had a bug opened and fixed about this before, I believe it was greghaynes | 19:37 |
greghaynes | jroll: ohai | 19:39 |
greghaynes | yes | 19:39 |
jroll | hi :) | 19:39 |
greghaynes | I was running with noexec /tmp | 19:39 |
jroll | oh, right, only exec'ing temp files | 19:40 |
jroll | so this case is fine ( https://review.openstack.org/#/c/162672/12/ironic/tests/drivers/test_ipmitool.py ) | 19:40 |
greghaynes | Yea, really I dont think its that bad to say "test require an executable /tmp" or similar, as long as you fail in a way where thats reasonable to identify ;) | 19:41 |
jroll | heh | 19:42 |
NobodyCam | okay question: is the unmount except block in this example ( http://paste.openstack.org/show/KLDOud9mRSmDqNQ3gYQF ) needed | 19:43 |
jroll | NobodyCam: not the unmount in the except block, no | 19:44 |
NobodyCam | * is the unmount in the except block | 19:44 |
NobodyCam | jroll: :) TY thats what I thought | 19:44 |
jroll | finally always gets run, no matter what | 19:45 |
jroll | even if the try block has a return | 19:45 |
NobodyCam | :) | 19:45 |
NobodyCam | Ty jroll :) | 19:46 |
*** alexpilotti has quit IRC | 19:50 | |
*** r-daneel has joined #openstack-ironic | 19:58 | |
openstackgerrit | Josh Gachnang proposed openstack/ironic: Add /nodes/<uuid>/cleaning/steps API https://review.openstack.org/159322 | 19:59 |
*** mgoddard has quit IRC | 20:02 | |
openstackgerrit | Josh Gachnang proposed openstack/ironic: Add /nodes/<uuid>/cleaning/steps API https://review.openstack.org/159322 | 20:02 |
JoshNang | heh thought of another test as soon as i submitted the first one | 20:03 |
* devananda lunches | 20:04 | |
NobodyCam | oh food | 20:06 |
BadCub | mmmm... food | 20:11 |
mrda | Morning Ironic | 20:12 |
jlvillal | NobodyCam: I just saw your message. | 20:13 |
jlvillal | NobodyCam: I think on: https://review.openstack.org/162672 | 20:13 |
NobodyCam | morning mrda :) | 20:13 |
NobodyCam | jlvillal: yep | 20:14 |
jlvillal | NobodyCam: You had a question? | 20:14 |
NobodyCam | left comment on the review | 20:15 |
NobodyCam | :) | 20:15 |
*** ndipanov has quit IRC | 20:16 | |
jlvillal | NobodyCam: Okay. Not sure what is expected on the list of exceptions that it returns. | 20:16 |
NobodyCam | jlvillal: ya that just a minnor nit | 20:17 |
NobodyCam | its the other one | 20:17 |
jlvillal | NobodyCam: I'm unsure how to address the exception list. I can update it to use the tempdir value though. | 20:17 |
jlvillal | NobodyCam: Thanks for the review. | 20:17 |
rloo | hey JoshNang, question about 159322. I don't see where 1.7 was added for cleaning. Shouldn't it be in api/controllers/v1/__init__.py? | 20:18 |
NobodyCam | the tempdir conf setting is the biggie | 20:18 |
jlvillal | NobodyCam: Would that be like: cfg.CONF.tempdir? | 20:18 |
JoshNang | rloo: oh! i didn't realize that was there. i thought it was a bit strange we didn't have a list of versions.. | 20:19 |
* JoshNang fixes | 20:19 | |
*** ifarkas has quit IRC | 20:19 | |
rloo | JoshNang: ok, that answers my question then ;) | 20:19 |
rloo | JoshNang: you have to up the max_version | 20:19 |
*** alexpilotti has joined #openstack-ironic | 20:19 | |
JoshNang | huh, suprised tests passed passing a version greater than max.. | 20:19 |
NobodyCam | jlvillal: here is what I did? I'm sure there are better ways: https://review.openstack.org/#/c/160383/13/ironic/drivers/modules/ipmitool.py | 20:20 |
jlvillal | NobodyCam: Great :) Thanks! | 20:20 |
mrda | morning rloo. Since you found the problem, do you want to take a look at review 163730 to see if it covers the missing functionality? | 20:22 |
rloo | morning mrda. what problem? /me looks | 20:22 |
mrda | logical name in port section of REST API | 20:22 |
mrda | rloo ^^ | 20:22 |
*** dprince has quit IRC | 20:22 | |
openstackgerrit | Josh Gachnang proposed openstack/ironic: Add /nodes/<uuid>/cleaning/steps API https://review.openstack.org/159322 | 20:22 |
JoshNang | rloo: ^ fixed. thanks for that! | 20:22 |
rloo | mrda: oh that. mrda. I was trying to focus on feature stuff that had to be in before k-3. i think the port stuff can be put in later but maybe not. will look. | 20:23 |
mrda | ok, if it can wait, then feel free to leave | 20:25 |
rloo | mrda: i think it can wait, and if it can't for wahtever reason, i think it can wait for liberty. the major part is already done. and i think there are some features that are higher priority than this. i did put a comment in the bug about create-port and whether we wanted to allow node name there too instead of uuid. | 20:28 |
mrda | thanks rloo, much appreciated! | 20:28 |
rloo | mrda: yw I guess. although now I feel bad cuz you're thanking me for not reviewing your stuff til later :-( | 20:30 |
mrda | rloo: pfft! :) I only raised ti because you identified the original problem. The key is getting a great K out the door, and if this can wait, then it can wait - it's just code (*our* code), it's not personal :) | 20:31 |
*** igordcard_ has joined #openstack-ironic | 20:32 | |
rloo | mrda: :D | 20:32 |
NobodyCam | brb | 20:35 |
TheJulia | win 7 | 20:37 |
TheJulia | doh | 20:37 |
JayF | only 7? You need to IRC harder julia :P | 20:37 |
BadCub | hi TheJulia | 20:38 |
TheJulia | JayF: oh, I have many windows, just I happened to be going to that one, because I'm lazy and like typing /win :) | 20:38 |
openstackgerrit | John L. Villalovos proposed openstack/ironic: Update unittests and use NamedTemporaryFile https://review.openstack.org/162672 | 20:39 |
jlvillal | NobodyCam: ^^^^ Hopefully I addressed your issue :) | 20:39 |
jlvillal | NobodyCam: I did it kind of like this: https://github.com/openstack/ironic/blob/master/ironic/common/utils.py#L380-L382 | 20:41 |
*** lucas-dinner has quit IRC | 20:50 | |
*** andreykurilin_ has joined #openstack-ironic | 20:50 | |
*** Nisha has quit IRC | 20:54 | |
*** Nisha_away has joined #openstack-ironic | 20:54 | |
*** harlowja is now known as harlowja_away | 20:58 | |
NobodyCam | jlvillal: how does NamedTemporaryFile act if conf.tempDir is none? | 21:01 |
jlvillal | NobodyCam: That is the default | 21:02 |
jlvillal | NobodyCam: https://docs.python.org/2/library/tempfile.html | 21:02 |
NobodyCam | :) ack | 21:03 |
jlvillal | :D | 21:03 |
*** ijw_ has quit IRC | 21:03 | |
*** Nisha_away has quit IRC | 21:04 | |
*** ijw has joined #openstack-ironic | 21:04 | |
*** Nisha has joined #openstack-ironic | 21:04 | |
*** ijw has quit IRC | 21:06 | |
*** ijw has joined #openstack-ironic | 21:06 | |
*** jgrimm is now known as zz_jgrimm | 21:07 | |
openstackgerrit | Nisha Agarwal proposed openstack/ironic: iLO implementation for hardware inspection https://review.openstack.org/151596 | 21:08 |
*** andreykurilin_ has quit IRC | 21:09 | |
* devananda returns from lunch | 21:09 | |
NobodyCam | welcome back | 21:11 |
* devananda takes a look at the review board | 21:14 | |
NobodyCam | :) | 21:14 |
mrda | jlvillal: I've just commented on 162672 too. HTH. | 21:18 |
openstackgerrit | Devananda van der Veen proposed openstack/ironic: Fixup log message for discoverd https://review.openstack.org/164792 | 21:18 |
jlvillal | mrda: Thanks :) | 21:19 |
devananda | NobodyCam: ^ needs re-review. I missed a test | 21:19 |
openstackgerrit | Nisha Agarwal proposed openstack/ironic: iLO implementation for hardware inspection https://review.openstack.org/151596 | 21:19 |
mrda | jlvillal: on the previous version | 21:19 |
NobodyCam | devananda: ack | 21:20 |
NobodyCam | mrda: looking at your comment on 162672 would your concern be removed with the "pre-flight" cjhecks I have here: https://review.openstack.org/#/c/160383 | 21:21 |
devananda | JoshNang: what's the impact if we don't land the GET /nodes/clean API ? | 21:21 |
JoshNang | devananda: less operator visibility, not huge IMO | 21:21 |
JoshNang | especially since the number of steps is pretty small (0 right now) | 21:22 |
*** sambetts has quit IRC | 21:22 | |
devananda | JoshNang: cleaning /progress/ is still made visible as a node goes through it | 21:22 |
JoshNang | correct | 21:23 |
JayF | you can't know progress (in terms of % completion) | 21:23 |
JayF | without knowing the full set of steps being run, right? | 21:23 |
NobodyCam | devananda: re +a'd :) | 21:24 |
devananda | NobodyCam: ty | 21:24 |
*** sambetts has joined #openstack-ironic | 21:25 | |
mrda | NobodyCam: looking now | 21:25 |
devananda | JoshNang: since https://review.openstack.org/#/c/161453/ is curently dependent on the GET API change, if we didn't land that API change in kilo, | 21:25 |
devananda | JoshNang: is it reasonable to split execute out and land that now? | 21:26 |
openstackgerrit | Nisha Agarwal proposed openstack/ironic: iLO driver updates node capabilities during inspection https://review.openstack.org/163572 | 21:28 |
mrda | NobodyCam: That's great stuff! I think that makes the use of the CONF.tempdir is fine with those checks preceeding it | 21:28 |
JoshNang | (sorry for delayed responses, in a meeting) i don't think that's dependent on the API change | 21:29 |
NobodyCam | question: I believe line 58 of https://review.openstack.org/#/c/151596/38/ironic/drivers/modules/ilo/inspect.py should ne a log not a raise. do others agree? | 21:29 |
JoshNang | devananda: ^ | 21:29 |
* mrda needs more coffee so he can speak in sentences | 21:30 | |
NobodyCam | mrda: I wouldn't say great but its a basic pre-flight check | 21:30 |
NobodyCam | :-p | 21:31 |
devananda | JoshNang: in gerrit it is listed that way | 21:31 |
JoshNang | devananda: it's dependent on the execute clean steps, which merged this morning. i need a devstack patch to land agent execute add the cleaning network config option, working on that right now, but then it should be good to go | 21:32 |
mrda | NobodyCam: people use configuartion-specified tempdir's without checking that sort of stuff all the time in software. It's a good first step :) | 21:34 |
devananda | JoshNang: ooh. yah. i'm looking at the wrong tab (maybe I shouldn't have a hundred open?) | 21:34 |
NobodyCam | mrda: :) | 21:34 |
JoshNang | devananda: heh i know that feeling | 21:34 |
devananda | JoshNang: in the interest of crunch time, i'm inclined to bump the REST API changes | 21:35 |
JoshNang | devananda: wfm | 21:36 |
devananda | k | 21:36 |
JoshNang | i'd rather focus on landing the agent and ilo bits | 21:36 |
devananda | yah | 21:36 |
JoshNang | the ilo management one looked good to go | 21:36 |
devananda | make cleaning work, be configurable, tested, etc | 21:36 |
JoshNang | agreed | 21:36 |
devananda | exposing the list of steps seems secondary at this point | 21:36 |
devananda | good - but not as critical | 21:36 |
JoshNang | (and work on docs! my plan for the end of the week) | 21:36 |
openstackgerrit | Nisha Agarwal proposed openstack/ironic: Automate boot iso creation with in ironic for iscsi-ilo https://review.openstack.org/155900 | 21:37 |
* JayF will write cleaning docs | 21:37 | |
devananda | JoshNang: we have a couple weeks to hammer out docs and bugs | 21:37 |
JayF | all the docs | 21:37 |
devananda | JayF: any objection to ^ ? | 21:37 |
devananda | Nisha: ^ | 21:37 |
JayF | I know all my clean steps, api or not ;) | 21:37 |
JayF | heh | 21:37 |
JayF | I'd say it's OK. | 21:37 |
devananda | JayF: lol, of course you do :) | 21:37 |
devananda | ok, bumping | 21:38 |
JoshNang | i'd guess most people are going to be using defaults in the agent (erase disks) | 21:38 |
JoshNang | and same with ilo, looked like sane defaults | 21:38 |
JayF | Worth noting that today in the agent | 21:38 |
JayF | if your disk doesn't support ATA security erase, CLEANFAIL | 21:39 |
devananda | ooh. yes | 21:39 |
devananda | must make that optional then | 21:39 |
JoshNang | ++ | 21:41 |
*** ijw_ has joined #openstack-ironic | 21:42 | |
*** ijw has quit IRC | 21:45 | |
BadCub | brb | 21:46 |
devananda | any IPA folks reviewed lucas' patch for root device hints? https://review.openstack.org/#/c/163857/ | 21:46 |
JayF | I put it in a tab but can't review for a littl ebit | 21:47 |
JayF | it's in my queue :) | 21:47 |
Nisha | devananda, for docs? | 21:51 |
Nisha | sorry i was away i saw it now only | 21:51 |
devananda | Nisha: the GET /v1/nodes/NNN/clean/steps API change -- we were discussing bumping that to Liberty, and I have just done so | 21:52 |
Nisha | devananda, does that affects complete cleaning? | 21:55 |
devananda | Nisha: it shouldn't affect the functionality of performing cleaning operations on nodes | 21:55 |
JoshNang | correct | 21:55 |
Nisha | as i think it impacts only ironic client? | 21:55 |
devananda | Nisha: it is removing the REST API that allows you to query the steps that the driver believes it will take. | 21:55 |
Nisha | ok | 21:56 |
rloo | devananda: wrt that patch that JoshNang had for the API. We want some of it (the microversion/hiding of clean_steps info) in kilo but it can be done after kilo-3. | 21:56 |
rloo | devananda: well, it should be done now cuz of microversions, it is a bug. | 21:56 |
devananda | rloo: i think we can defer all the clean_step API stuff | 21:56 |
devananda | erm | 21:56 |
rloo | devananda: yeah the API part. not the microversion part that should have been done in some earlier patch that added clean_step I think. | 21:57 |
devananda | ugh | 21:57 |
rloo | devananda: i don't know if that made sense but i hope you understood. | 21:57 |
devananda | rloo: i think so. verifying right now | 21:58 |
rloo | devananda: i added comments to that patch. | 21:58 |
Nisha | devananda, and other core reviewers ...i have few patches out for review the inspection patches. https://review.openstack.org/151596, https://review.openstack.org/163572 and https://review.openstack.org/155900 | 21:59 |
devananda | rloo: so these fields are not guarded by a microveresion: inspection_started_at inspection_finished_at | 21:59 |
rloo | devananda: i thought the inspect* stuff was guarded already. | 21:59 |
Nisha | devananda, they are for microversion 1.6 | 21:59 |
Nisha | rloo, correct | 22:00 |
devananda | they are not | 22:00 |
devananda | http://paste.openstack.org/show/192738/ | 22:00 |
rloo | devananda: ugh. you're right. i thought they were, but those were missed too. | 22:01 |
Nisha | devananda, guarded means? | 22:03 |
devananda | Nisha: checking the microversion in the request and changing behavior as necessary to present that version of the API | 22:03 |
JoshNang | devananda: (meeting done) i can pul the guard bits out into their own tiny patch | 22:04 |
JoshNang | pull, even | 22:04 |
Nisha | devananda, so in the pasted output what was expected | 22:04 |
Nisha | oh it should have shown for 1.1 | 22:05 |
devananda | Nisha: the inspect_*_at fields should not be present when X-OpenStack-Ironic-API-Version: 1.1 | 22:05 |
Nisha | devananda, got it | 22:05 |
NobodyCam | Nisha: looks like https://review.openstack.org/#/c/151596/38/ironic/drivers/modules/ilo/inspect.py needs doc string and message updates to support the new None option? | 22:05 |
devananda | while looking for the clean* fields, I noticed the inspect* fields were present (it's a separate issue) | 22:05 |
Nisha | NobodyCam, yes | 22:06 |
NobodyCam | :) | 22:06 |
Nisha | NobodyCam, let me see | 22:06 |
*** absubram has joined #openstack-ironic | 22:06 | |
*** kkoski has quit IRC | 22:07 | |
NobodyCam | _get_desired_macs_for_port_creation method's doc string, line 201 :) | 22:07 |
NobodyCam | s/string,/string and/ | 22:08 |
Nisha | NobodyCam, but the function _get_desired_macs_for_port_creation() is not called when argument is "none" | 22:08 |
*** rloo is now known as rloo_afk | 22:09 | |
Nisha | NobodyCam, please leave comments at exact locations | 22:09 |
Nisha | in the patch | 22:09 |
Nisha | if it is just comments and all i shud be able to throw new version now | 22:09 |
*** harlowja_away is now known as harlowja | 22:11 | |
*** ijw_ has quit IRC | 22:13 | |
devananda | JoshNang: see this? http://logs.openstack.org/46/164646/1/check/check-tempest-dsvm-ironic-agent_ssh/9b78745/logs/screen-ir-cond.txt.gz?level=ALL#_2015-03-16_11_28_34_320 | 22:23 |
JoshNang | yup, that's why we need a devstack patch | 22:23 |
JoshNang | i'm testing locally right now | 22:23 |
jroll | JoshNang: I had some ideas on how to test / figure that out, if you get stuck or whatever | 22:23 |
devananda | so that looks downright broken | 22:24 |
devananda | which leads me to wonder how it landed | 22:24 |
jroll | it didn't land? | 22:24 |
jroll | that's a gate job, how would it land | 22:24 |
devananda | oh blah | 22:24 |
devananda | too many tabs again | 22:24 |
jroll | :P | 22:24 |
devananda | that hasn' tlanded | 22:24 |
JoshNang | yeah definitely not landed :) | 22:24 |
NobodyCam | :-p | 22:25 |
jroll | devananda: do you know what's going on there? I can quickly explain | 22:25 |
jroll | short version: to pxe boot the ramdisk to do cleaning, we need a neutron network. we don't know what the neutron network is supposed to be, so the deployer needs to configure it. josh is working on doing this programatically in devstack | 22:26 |
openstackgerrit | Nisha Agarwal proposed openstack/ironic: iLO implementation for hardware inspection https://review.openstack.org/151596 | 22:26 |
devananda | jroll: right | 22:27 |
devananda | i was looking at the agent_ilo patch, saw that error, and saw that a patch in the chain had landed, without realizing that the error was in the second (unlanded) patch out of three | 22:27 |
jroll | got it | 22:28 |
openstackgerrit | Nisha Agarwal proposed openstack/ironic: iLO driver updates node capabilities during inspection https://review.openstack.org/163572 | 22:28 |
* devananda deletes the already-merged things from etherpad | 22:30 | |
*** ijw has joined #openstack-ironic | 22:34 | |
adam_g | anyone seen these unit tests failures locally? http://paste.openstack.org/show/192749/ my is up to date | 22:38 |
devananda | adam_g: not in a very long time | 22:38 |
adam_g | hmm | 22:38 |
* adam_g recreates | 22:39 | |
devananda | adam_g: see ironic/tests/__init__.py | 22:39 |
adam_g | oh, right | 22:40 |
adam_g | probably something to do with running via nose | 22:40 |
BadCub | thanks devananda , I was just striking out the merged ones | 22:51 |
devananda | BadCub: yup, that's good too. but there were so many of them :) | 22:51 |
*** wanyen has quit IRC | 22:52 | |
BadCub | Yeah, it was getting rather colorful | 22:52 |
BadCub | devananda: question on 159322.... Bumping that particular item does not kill the BP, correct? | 22:52 |
devananda | BadCub: we should indicate that something was completed on launchpad, yes | 22:53 |
devananda | BadCub: however the work isn't actually finished -- http://specs.openstack.org/openstack/ironic-specs/specs/kilo/implement-cleaning-states.html#rest-api-impact | 22:54 |
devananda | the spec clearly lists two things that we just decided to bump | 22:54 |
devananda | as a project, we haven't had to deal with this before, and rather than parrot how other projects (nova, neutron) have done it, I think we will all discuss it over the coming weeks | 22:55 |
devananda | we'll definitely need to track what parts of the spec weren't done, and probably go update it to indicate as much | 22:55 |
devananda | so that the historical record of what was done in Kilo is accurate | 22:56 |
BadCub | okay | 22:56 |
BadCub | I only noted one that was bumped. Probably missed the second | 22:56 |
devananda | eg, of the 5 items under REST API Impact, we've done 3 | 22:56 |
openstackgerrit | Nisha Agarwal proposed openstack/ironic: iLO driver updates node capabilities during inspection https://review.openstack.org/163572 | 23:03 |
*** ChuckC has joined #openstack-ironic | 23:09 | |
*** ramineni has joined #openstack-ironic | 23:11 | |
*** Nisha has quit IRC | 23:14 | |
*** stendulker has joined #openstack-ironic | 23:15 | |
*** r-daneel has quit IRC | 23:19 | |
*** andreykurilin_ has joined #openstack-ironic | 23:20 | |
NobodyCam | JoshNang: the topic on https://review.openstack.org/#/c/161453 seems strange. does it need a commit message *-bug tag? | 23:23 |
*** absubram has quit IRC | 23:23 | |
JoshNang | NobodyCam: i think that's left over from the execute clean steps patch. (which did have that bug) | 23:23 |
openstackgerrit | Shivanand Tendulker proposed openstack/ironic: Common changes for secure boot support https://review.openstack.org/153974 | 23:23 |
NobodyCam | JoshNang: ack | 23:24 |
*** yuanying has joined #openstack-ironic | 23:24 | |
NobodyCam | just checking | 23:24 |
JoshNang | updated! | 23:24 |
NobodyCam | :-p | 23:24 |
openstackgerrit | Shivanand Tendulker proposed openstack/ironic: Secure boot support for pxe_ilo driver https://review.openstack.org/154808 | 23:25 |
JoshNang | so for that patch, setting a network uuid to one of the neutron networks lets the node boot, but the node never seems to get to the agent. i see it tftp booting, get ".....ready." a blank screen and nothing | 23:25 |
openstackgerrit | Anusha Ramineni proposed openstack/ironic: Add Cleaning Operations for iLO drivers https://review.openstack.org/157715 | 23:26 |
JoshNang | anyone have any ideas? | 23:26 |
ramineni | JoshNang: have one question here, while creating neutron port , dont you need to provide the baremetal port(mac_address) also as argument in the body along with network_id? | 23:33 |
BadCub | jroll: would you mind taking a look at https://review.openstack.org/#/c/163857/ when you have a free minute has two +2s. Needs a +A from IPA | 23:34 |
ramineni | JoshNang: I mean in create_cleaning_ports | 23:34 |
JoshNang | ramineni: hmm lemme look | 23:35 |
JoshNang | might be the reason my nodes are hanging :) | 23:35 |
* BadCub needs a smoke | 23:37 | |
devananda | mrda: did you look into logical name support for /v1/ports API ? | 23:38 |
*** r-daneel has joined #openstack-ironic | 23:42 | |
ramineni | JoshNang: :) one more question, how do we resume back the cleaning once we clean is failed , I see a transition from CLEAN_FAIL to CLEAN , but not sure how to trigger/resume back | 23:42 |
JoshNang | ramineni: the provision state api | 23:43 |
JoshNang | with the verb 'provide' | 23:44 |
ramineni | ironic node-set-provision-state 'provide'? | 23:45 |
JoshNang | hrm no that won't work | 23:45 |
ramineni | JoshNang: i dont see any option in CLI | 23:46 |
JoshNang | the verb is 'clean' in states.py | 23:46 |
*** r-daneel has quit IRC | 23:47 | |
ramineni | JoshNang: ya, got that, but for the end user to initiate? | 23:47 |
*** andreykurilin_ has quit IRC | 23:48 | |
JoshNang | ramineni: yeah i think this got lost with all the patchsets :( i had thought the 'provide' verb would do it | 23:50 |
jroll | BadCub: +3'd | 23:50 |
jroll | JoshNang: doesn't it? | 23:50 |
BadCub | jroll: thank ya kindly sir! :-) | 23:51 |
JoshNang | jroll: apparently not. the verb from cleanfail->clean is 'clean', which isn't in the list of verbs or the api | 23:51 |
jroll | JoshNang: oh, to restart cleaning. I see | 23:51 |
jroll | fixfixfix | 23:51 |
jroll | :P | 23:51 |
jroll | well | 23:51 |
jroll | using provide would put it in managed, right? | 23:52 |
JoshNang | well i've got a handful of minutes while waiting for devstack to restack >:( | 23:52 |
*** Haomeng has joined #openstack-ironic | 23:52 | |
jroll | err | 23:52 |
jroll | using manage would put it in managed, then provide would clean and make it available, yes? | 23:52 |
JoshNang | correct | 23:52 |
jroll | ramineni: ^ there's a workaround for nowe | 23:52 |
jroll | now | 23:52 |
JoshNang | i think that was semi intended tbh | 23:53 |
openstackgerrit | Devananda van der Veen proposed openstack/python-ironicclient: Add support for logical names https://review.openstack.org/158520 | 23:53 |
jroll | yeah, go to managed to fix | 23:53 |
ramineni | jroll: ok | 23:54 |
NobodyCam | lol I was just about you about that one devananda | 23:54 |
NobodyCam | gah | 23:54 |
devananda | :) | 23:54 |
NobodyCam | I was just about to ask about that one | 23:54 |
NobodyCam | :-p | 23:54 |
BadCub | devananda: by looking at notes in spreadsheet, we don't need https://review.openstack.org/#/c/142178/ to land https://blueprints.launchpad.net/ironic/+spec/root-device-hints correct? | 23:55 |
*** Haomeng|2 has quit IRC | 23:55 | |
devananda | BadCub: cross project deps are a bit odd to track. but I think ^ is very likely to land | 23:56 |
ramineni | jroll: should this be triggered using API diretky? | 23:56 |
jroll | ramineni: you can use the client afaik | 23:56 |
BadCub | I was going to move https://blueprints.launchpad.net/ironic/+spec/root-device-hints up to Approved/Waiting to Land | 23:57 |
*** alexpilotti has quit IRC | 23:57 | |
jroll | though maybe our client doesn't support those yet, maybe run master version of client? | 23:57 |
devananda | BadCub: would it be good to have another section for "dependencies on other projects" ? | 23:58 |
jroll | ramineni: oh, this https://review.openstack.org/#/c/148804/ | 23:58 |
*** takadayuiko has quit IRC | 23:58 | |
BadCub | devananda: that may be a good idea. I will add a new section on white board for that under Approved/Waiting to Land and put that one there since the dep is the only thing left standing | 23:59 |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!