Monday, 2015-03-16

*** achanda has joined #openstack-ironic00:07
*** naohirot has joined #openstack-ironic00:12
*** achanda has quit IRC00:12
*** subscope has quit IRC00:18
*** kkoski has joined #openstack-ironic00:29
*** Haomeng|2 has joined #openstack-ironic00:36
*** mjturek1 has quit IRC00:37
*** Haomeng has quit IRC00:39
openstackgerritMichael Davies proposed openstack/ironic: New field 'name' not supported in port REST API  https://review.openstack.org/16373001:06
openstackgerritShivanand Tendulker proposed openstack/ironic: Enable agent_ilo for uefi-bios switching  https://review.openstack.org/16204301:09
*** alexpilotti has quit IRC01:19
*** kkoski has quit IRC01:19
*** mtanino has joined #openstack-ironic01:25
*** chenglch has joined #openstack-ironic01:34
*** ijw_ has quit IRC01:48
*** Marga_ has joined #openstack-ironic01:57
openstackgerritShivanand Tendulker proposed openstack/ironic: ilo_iscsi driver do not validate boot_option  https://review.openstack.org/16441402:29
*** ramineni has joined #openstack-ironic02:47
*** coolsvap|afk is now known as coolsvap02:58
*** jrist has quit IRC03:01
openstackgerritAnusha Ramineni proposed openstack/ironic: Add Cleaning Operations for iLO drivers  https://review.openstack.org/15771503:02
*** jrist has joined #openstack-ironic03:12
*** ijw has joined #openstack-ironic03:23
*** ChuckC has joined #openstack-ironic03:28
*** achanda has joined #openstack-ironic04:10
openstackgerritAnusha Ramineni proposed openstack/ironic: Add Cleaning Operations for iLO drivers  https://review.openstack.org/15771504:35
*** rameshg87 has joined #openstack-ironic04:37
rameshg87good morning ironic04:38
*** mtanino is now known as mtanino_away04:40
Haomeng|2rameshg87: morning, ramesh:)04:55
*** aswadr has joined #openstack-ironic05:15
mrdaMorning rameshg87 and Haomeng|205:27
Haomeng|2mrda: morning:)05:28
*** kalpase has joined #openstack-ironic05:28
*** ijw has quit IRC05:31
rameshg87good morning mrda, Haomeng|2 :)05:38
Haomeng|2rameshg87: :)05:38
mrdao/05:39
*** ijw has joined #openstack-ironic05:41
*** Nisha has joined #openstack-ironic05:48
openstackgerritNisha Agarwal proposed openstack/ironic: Automate boot iso creation with in ironic for iscsi-ilo  https://review.openstack.org/15590005:49
*** ijw has quit IRC05:51
*** ijw has joined #openstack-ironic05:52
openstackgerritAparna proposed stackforge/proliantutils: ILO: Support for configuring httpboot through RIS  https://review.openstack.org/16332506:08
openstackgerritSirushti Murugesan proposed openstack/ironic: Raise exception for Agent Deploy driver when using partition images  https://review.openstack.org/16444006:15
*** achanda has quit IRC06:15
*** pradipta has joined #openstack-ironic06:25
*** achanda has joined #openstack-ironic06:42
openstackgerritchenglch proposed openstack/ironic-specs: Add console log support  https://review.openstack.org/16458606:52
*** Nisha_away has joined #openstack-ironic06:58
*** kalpase1 has joined #openstack-ironic06:58
*** Nisha has quit IRC06:59
*** kalpase has quit IRC07:01
*** stendulker has joined #openstack-ironic07:02
*** achanda has quit IRC07:12
*** pcaruana has quit IRC07:14
*** Nisha_away is now known as Nisha07:18
*** Marga_ has quit IRC07:23
*** ijw has quit IRC07:28
*** mtanino_away has quit IRC07:28
openstackgerritchenglch proposed openstack/ironic-specs: Add console log support  https://review.openstack.org/16458607:30
*** andreykurilin_ has joined #openstack-ironic07:37
*** rwsu has joined #openstack-ironic07:43
*** Marga_ has joined #openstack-ironic07:45
*** ukalifon1 has joined #openstack-ironic07:48
*** yog__ has joined #openstack-ironic07:49
*** sirushti has quit IRC07:55
*** sirushti has joined #openstack-ironic07:55
*** andreykurilin_ has quit IRC07:56
*** Marga_ has quit IRC08:01
*** ifarkas has joined #openstack-ironic08:07
*** Nisha has quit IRC08:08
*** Nisha has joined #openstack-ironic08:08
*** mgoddard has joined #openstack-ironic08:19
openstackgerritRamakrishnan G proposed openstack/ironic-python-agent: Cache agent parameters for later invocations  https://review.openstack.org/16459608:20
*** yog__ has quit IRC08:21
*** pradipta has quit IRC08:22
openstackgerritRamakrishnan G proposed openstack/ironic-python-agent: Cache agent parameters for later invocations  https://review.openstack.org/16459608:22
openstackgerritRamakrishnan G proposed openstack/ironic-python-agent: Cache agent parameters for later invocations  https://review.openstack.org/16459608:24
*** jcoufal has joined #openstack-ironic08:33
*** ndipanov has joined #openstack-ironic08:33
*** yog__ has joined #openstack-ironic08:37
*** dtantsur has joined #openstack-ironic08:40
openstackgerritRamakrishnan G proposed openstack/ironic: Add uefi support for agent iscsi deploy  https://review.openstack.org/16438608:41
dtantsurHey-hey, morning folks! I'm back :)08:42
Nishadtantsur,08:48
Nishahi08:49
Nishagm08:49
rameshg87dtantsur: o/08:49
rameshg87welcome back08:49
dtantsuro/08:49
Haomeng|2dtantsur: o/08:50
*** jcoufal has quit IRC08:57
*** kalpase1 has quit IRC08:57
*** kalpase has joined #openstack-ironic08:58
*** jistr has joined #openstack-ironic09:11
*** derekh has joined #openstack-ironic09:14
*** MattMan has joined #openstack-ironic09:16
*** lucasagomes has joined #openstack-ironic09:17
openstackgerritMerged openstack/ironic-specs: Fix typo in ironic-specs/specs/kilo/partition-image-support-for-agent-driver.rst  https://review.openstack.org/16259309:17
openstackgerritMerged openstack/ironic-specs: Fix typo in ironic-specs/specs/kilo/cisco-ucs-pxe-driver.rst  https://review.openstack.org/16261309:18
openstackgerritAnusha Ramineni proposed openstack/ironic: Add Cleaning Operations for iLO drivers  https://review.openstack.org/15771509:22
GheRiveromorning all09:23
rameshg87GheRivero: o/09:24
dtantsurGheRivero, morning09:25
openstackgerritMerged openstack/ironic-specs: Fix typo in ironic-specs/specs/kilo/uefi-secure-boot.rst  https://review.openstack.org/16259009:26
openstackgerritMerged openstack/ironic-specs: Fix typos in ironic-specs/specs/kilo/drac-bios-mgmt.rst  https://review.openstack.org/16257809:26
openstackgerritMerged openstack/ironic-specs: Fix typo in ironic-specs/specs/kilo/local-boot-support-with-partition-images.rst  https://review.openstack.org/16260809:28
openstackgerritTan Lin proposed openstack/ironic: Use Mock.patch decorator to handle patching amt management module  https://review.openstack.org/16461509:28
*** mgoddard has quit IRC09:30
openstackgerritMerged openstack/ironic-specs: Fix typo in ironic-specs/specs/kilo/automate-uefi-bios-iso-creation.rst  https://review.openstack.org/16260509:31
* dtantsur fast-tracked some typo fixes ^^^09:32
pshigeThanks!09:33
openstackgerritMerged openstack/ironic-specs: Fix typo in ironic-specs/specs/kilo/new-ironic-state-machine.rst  https://review.openstack.org/16260709:34
stendulkerHaomeng|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-ironic09:46
openstackgerritRamakrishnan G proposed openstack/ironic-python-agent: Add uefi support in image extension  https://review.openstack.org/16373909:49
*** naohirot has quit IRC09:51
rameshg87dtantsur: i too feel it's true https://review.openstack.org/#/c/155561/27/ironic/conductor/manager.py09:54
rameshg87dtantsur: how about i just fix it up in a patch and throw a new one ? or should we wait for JoshNang ?09:55
rameshg87dtantsur: the unit test should have caught it :(09:55
openstackgerritLucas Alvares Gomes proposed openstack/ironic: IPA: Add support for root device hints  https://review.openstack.org/16385709:55
dtantsurrameshg87, 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
openstackgerritGhe Rivero proposed openstack/ironic-python-agent: Use oslo_log lib  https://review.openstack.org/16279009:56
openstackgerritGhe Rivero proposed openstack/ironic-python-agent: Sync from oslo.incubator  https://review.openstack.org/16279109:56
rameshg87dtantsur: follow-up patch or a new patch-set on the same patch ?09:56
*** rsalevsky has joined #openstack-ironic09:56
*** lazy_prince has quit IRC09:57
dtantsurrameshg87, does not matter to me :)09:58
rameshg87okay09:58
rameshg87throwing a follow-up patch09:58
*** lazy_prince has joined #openstack-ironic09:59
*** lazy_prince has quit IRC10:00
*** lazy_prince has joined #openstack-ironic10:00
openstackgerritNisha Agarwal proposed openstack/ironic: Automate boot iso creation with in ironic for iscsi-ilo  https://review.openstack.org/15590010:03
Nishadtantsur, lucasagomes ^^^^10:04
dtantsurI'm still recovering from PTO, but will try somewhat later :)10:04
Nishadtantsur, thanks10:04
openstackgerritRamakrishnan G proposed openstack/ironic: Add uefi support for agent iscsi deploy  https://review.openstack.org/16438610:07
*** coolsvap is now known as coolsvap|afk10:19
openstackgerritGopi Krishna S proposed openstack/ironic: Add pxe_ucs driver to manage Cisco UCS servers  https://review.openstack.org/15973410:25
*** kalpase1 has joined #openstack-ironic10:27
*** kalpase has quit IRC10:29
*** saripurigopi has joined #openstack-ironic10:35
*** dtantsur is now known as dtantsur|bbl10:49
openstackgerritRamakrishnan G proposed openstack/ironic: Address comments on cleaning commit  https://review.openstack.org/16464510:50
rameshg87dtantsur: ^^^10:50
rameshg87lucasagomes: one quick question: https://review.openstack.org/#/c/164386/5/ironic/drivers/modules/deploy_utils.py10:51
rameshg87lucasagomes: the comment on L63510:51
rameshg87lucasagomes: i use the same dictionary for knowing the devices for the partitions first and updating them with uuids in the loop10:52
rameshg87lucasagomes: not sure if that's obvious, let me know if you want me to use up another dictionary10:52
openstackgerritAnusha Ramineni proposed openstack/ironic: Support agent_ilo driver to perform cleaning  https://review.openstack.org/16464610:53
*** ramineni has quit IRC11:05
lucasagomesrameshg87, hi yes that's fine11:05
lucasagomesrameshg87, but I thought that when you do 'efi system partition uuid': part_dict.get('efi system partition')11:05
lucasagomesyou get None already if it's not present11:05
lucasagomesso the "if part_dev: ... else..."11:06
lucasagomesthe else is not needed because the value of the dict is already none11:06
lucasagomesif it contains something like "/dev/part1" it will be replaced by the block_uuid() return value11:06
*** stendulker has quit IRC11:13
Nishalucasagomes, i responded to comments on https://review.openstack.org/#/c/155900/11:16
Nishaplease check11:16
rameshg87lucasagomes: oh okay11:17
rameshg87lucasagomes: got it part_dict.get('efi system partition') will return None, so it will be None anyway11:17
rameshg87lucasagomes: yeah makes sense11:17
*** yuanying has quit IRC11:18
rameshg87lucasagomes: will remove it, thanks11:18
lucasagomesnp :)11:18
lucasagomesNisha, will do11:18
Nishalucasagomes, actually umount is called as part of "with utils.tempdir as <dir> name:" block11:19
Nishaso when the block ends it unmounts11:19
openstackgerritNisha Agarwal proposed openstack/ironic: iLO implementation for hardware inspection  https://review.openstack.org/15159611:21
lucasagomesNisha, https://github.com/openstack/ironic/blob/master/ironic/common/utils.py#L379-L389 ?11:21
lucasagomesit deletes it11:22
lucasagomesruns "losetup -a" after you run that code11:22
lucasagomesyou might see the loop device associated with the iso you just mounted no?11:22
lucasagomesor simply check the logs to see if you can find "'Could not remove tmpdir:"11:23
Nishai have seen it umounts but may be i need to figure out the path. anyway i will pots the patch if it doesnt unmounts11:23
lucasagomescause it may not be able to delete the contained due the resource will be busy11:23
lucasagomesright, I don't see where it does it :/11:24
lucasagomesthat's why we should use the unittests to make sure11:24
lucasagomesumount is being called11:24
lucasagomeseven if it's part of tempdir, we are mocking umount so we can test assert_called_once_with()11:24
*** kalpase1 has quit IRC11:27
openstackgerritNisha Agarwal proposed openstack/ironic: iLO driver updates node capabilities during inspection  https://review.openstack.org/16357211:28
openstackgerritRamakrishnan G proposed openstack/ironic: Add uefi support for agent iscsi deploy  https://review.openstack.org/16438611:28
Nishalucasagomes, i will test that and update the patch if required. meanwhile it will help if you and dtantsur|bbl can review inspection patches11:29
Nisha#link https://review.openstack.org/151596, https://review.openstack.org/163572, https://review.openstack.org/14880411:29
openstackgerritRamakrishnan G proposed openstack/ironic-python-agent: Add uefi support in image extension  https://review.openstack.org/16373911:30
lucasagomesNisha, ack thanks11:31
* rameshg87 goes home 11:32
*** rameshg87 has left #openstack-ironic11:32
*** Nisha has quit IRC11:34
openstackgerritMerged openstack/ironic: Use Mock.patch decorator to handle patching amt management module  https://review.openstack.org/16461511:38
*** takadayuiko has joined #openstack-ironic11:51
*** saripurigopi has quit IRC11:57
*** dtantsur|bbl is now known as dtantsur11:57
*** dprince has joined #openstack-ironic12:07
*** kalpase has joined #openstack-ironic12:26
*** alexpilotti has joined #openstack-ironic12:27
openstackgerritDmitry Tantsur proposed openstack/ironic: Start using in-band inspection  https://review.openstack.org/16113212:33
*** lucasagomes is now known as lucas-hungry12:34
*** subscope has joined #openstack-ironic12:37
*** dlpartain has joined #openstack-ironic12:40
*** openstackgerrit has quit IRC12:50
*** openstackgerrit has joined #openstack-ironic12:50
*** dlpartain has left #openstack-ironic12:56
*** wuhg has joined #openstack-ironic12:57
*** ijw has joined #openstack-ironic13:06
*** rameshg87 has joined #openstack-ironic13:08
jrollmorning all :)13:08
Shrewsmordred: left you some comments on the shade stuff13:09
Shrewsdang it. wrong channel13:12
Shrewsjroll: morning!13:12
jrollmorning Shrews :)13:13
dtantsurjroll, Shrews, morning!13:14
Shrewshi dtantsur13:15
dtantsurjroll, Shrews, could you guys review https://review.openstack.org/148804 ? Landing it would greatly simplify testing some new states including inspection13:15
jrollmorning dtantsur :)13:15
jrollwelcome back!13:15
dtantsurthanks :)13:15
Shrewsdtantsur: sure, will take a look13:16
rameshg87dtantsur: oh, i just upgraded my vote :)13:17
rameshg87dtantsur: wanted others to have a look ?13:17
dtantsur:)13:17
rameshg87dtantsur: i gave +1 back when i didn't have +2 voting rights13:18
Shrews:)13:18
dtantsurat least it works in my testing... but I'm the wrong person to ask, I want this to land too much :D13:18
jrolloh, good patch :)13:18
* jroll got ninja'd13:19
dtantsurI mean, I just wanted someone to +A it13:19
*** stendulker has joined #openstack-ironic13:19
Shrewsso, 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
jrollright13:21
rameshg87oh, never looked from that part :(13:21
jrollI think it can be fixed later13:21
dtantsurI think it _should_ be fixed later13:23
dtantsuronce we understand how to fix it :)13:23
ShrewsI think it _will_ be fixed later  :-P13:23
rameshg87_may_ be :D13:23
rameshg87dtantsur: why are we not enabled discoverd for agent* drivers ? - https://review.openstack.org/#/c/161132/613:25
rameshg87*enabling13:25
dtantsurehmmmm... because I didn't think about it :)13:25
dtantsuralso we need to know how J* and devananda feel about it13:25
dtantsurjroll, JayF ^^^13:25
trownlol at nick-globbing13:26
* jroll reads13:26
jrolldtantsur: because nobody did the work? idk13:26
jrollI don't see why we wouldn't want to do it13:27
*** chenglch has quit IRC13:28
rameshg87dtantsur: also, any idea why we did we change the behaviour of not instantiating DiscoverdInspect if CONF option is disabled ?13:29
rameshg87dtantsur: i mean any real reason ? (other than not wanting to instantiate if it's not going to be used)13:29
jrollrameshg87: iirc it was related to node-validate13:29
dtantsurrameshg87, you'd better ask devananda when he's here...13:29
dtantsurI don't understand either13:30
jrollnode-validate was showing | inspect | False |13:30
dtantsurright13:30
jrollbut really it should be None, not False13:30
dtantsurno13:30
openstackgerritImre Farkas proposed stackforge/ironic-discoverd: Save BIOS and RAID configuration to node.extra  https://review.openstack.org/16469513:30
jrollbecause it isn't a validation error13:30
jrollit's just disabled13:30
dtantsurjroll, then we need a new kind of error13:30
dtantsurdisabled != not implemented13:30
rameshg87*disabled*13:30
jrollhrm13:30
jrollidk, I think the goal was:13:31
dtantsurAnd the difference is: not implemented = go ping Ironic devs; disabled = go ping your admin13:31
jrollTrue == should work13:31
jrollFalse == something is broken13:31
openstackgerritShivanand Tendulker proposed openstack/ironic: ilo_iscsi driver do not validate boot_option  https://review.openstack.org/16441413:31
jrollNone == not available13:31
jrollbut idk.13:31
rameshg87jroll: but error messages it's going to provide are even less difficult to understand13:32
openstackgerritShivanand Tendulker proposed openstack/ironic: ilo_iscsi driver do not validate boot_option  https://review.openstack.org/16441413:32
dtantsurIIRC we don't call it "not available", we call it "not supported"13:32
rameshg87jroll: someone can't make out easily if it's disabled because of CONF option13:32
openstackgerritShivanand Tendulker proposed openstack/ironic: Enable agent_ilo for uefi-bios switching  https://review.openstack.org/16204313:32
rameshg87jroll: if we don't show inspect in node-validate OR if we show "pxe_ipmitool doesn't support inspect"13:33
rameshg87jroll: doesn't come out why is it so ? (when doc says it is supported)13:33
*** ChuckC has quit IRC13:33
jrollsure13:33
jrollidk what the right thing to do is13:33
jrollboth options are confusing to the user13:33
jrollin different ways13:34
jrolls/confusing to the user/inconsistent/ maybe13:34
openstackgerritShivanand Tendulker proposed openstack/ironic: Ilo drivers sets capabilities:boot_mode in node  https://review.openstack.org/15573113:34
*** ramineni has joined #openstack-ironic13:34
rameshg87yeah13:34
*** lucas-hungry is now known as lucasagomes13:35
openstackgerritShivanand Tendulker proposed openstack/ironic: Common changes for secure boot support  https://review.openstack.org/15397413:35
openstackgerritShivanand Tendulker proposed openstack/ironic: Secure boot support for pxe_ilo driver  https://review.openstack.org/15480813:35
openstackgerritShivanand Tendulker proposed openstack/ironic: Secure boot support for iscsi_ilo driver  https://review.openstack.org/15481413:36
openstackgerritShivanand Tendulker proposed openstack/ironic: Secure boot support for agent_ilo driver  https://review.openstack.org/15481613:36
*** rloo has joined #openstack-ironic13:41
rameshg87lucasagomes: https://review.openstack.org/#/c/164596/3 is somewhat related to root device hints13:43
rameshg87lucasagomes: please let me know your thoughts when you have a look at that13:43
rameshg87lucasagomes: i needed to do this for when agent is booted with virtual media - to make iscsi_ilo work with agent ramdisk13:44
*** sambetts_ is now known as sambetts13:45
*** kalpase has quit IRC13:46
*** kalpase1 has joined #openstack-ironic13:46
*** kalpase has joined #openstack-ironic13:48
*** stendulker has quit IRC13:48
lucasagomesrameshg87, ack13:51
*** kalpase1 has quit IRC13:51
*** mjturek1 has joined #openstack-ironic13:51
jrollJoshNang: I'm hacking on devstack for this change btw https://review.openstack.org/#/c/161453/13:52
openstackgerritDmitry Tantsur proposed stackforge/ironic-discoverd: eDeploy plugin to save BIOS and RAID configuration to node.extra  https://review.openstack.org/16469513:52
raminenijroll: hi13:56
jrollhi ramineni13:57
raminenijroll: could you please have a look at https://review.openstack.org/#/c/157715/16 , waiting for reviews :)13:57
jrollramineni: I'll do my best to get to it today13:58
raminenijroll: great.thanks13:58
*** kkoski has joined #openstack-ironic13:59
*** ukalifon1 has quit IRC14:00
jrollnp :)14:00
*** PaulCzar has joined #openstack-ironic14:01
lucasagomesrameshg87, commented14:01
*** ukalifon has joined #openstack-ironic14:01
lucasagomesjroll, ramineni morning :)14:02
dtantsurrameshg87, are you going to the meeting tonight?14:02
lucasagomesrameshg87, lemme know if what I commented makes sense, i didn't get why we cached that self.parameters in the test class14:02
lucasagomesthat's not needed14:02
raminenilucasagomes: morning :)14:02
jrollmorning lucasagomes :)14:02
*** ukalifon2 has joined #openstack-ironic14:03
rameshg87lucasagomes: thanks14:04
rameshg87dtantsur: i hope you mean ironic meeting, i will be there .. :)14:04
rameshg87lucasagomes: that was just a precaution to make sure that i don't play around with any other test14:04
devanandag'morning, all14:05
lucasagomesrameshg87, right, hmm I don't know how is that helping really14:05
lucasagomesI mean, it's doing nothing to be honest14:05
rameshg87lucasagomes: 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 back14:05
lucasagomesself.parameters is not being used in any tests14:05
jrollheya devananda :)14:05
*** ukalifon has quit IRC14:05
dtantsurdevananda, morning :) feeling like some holy way about validation in the discoverd driver? :D14:05
rameshg87lucasagomes: it's not being used, but i restore the value of the global after the test is done: L14614:06
rameshg87devananda: o/14:06
lucasagomesrameshg87, right... hmm not sure I mean right after we cache we do utils.AGENT_PARAMS_CACHED =dict ()14:06
lucasagomeswhich overwrites global14:06
lucasagomesso14:06
rameshg87yeah14:06
dtantsurrameshg87, I was going to ask you to raise question about validation in discoverd, but as devananda is here, we may start now :)14:06
lucasagomesdevananda, morning14:07
rameshg87dtantsur: yeah sure :)14:07
rameshg87lucasagomes: but after test is done, i restore the value back in L146 - tearDown()14:07
rameshg87lucasagomes: so modifying the value of global before the test case, restoring it back after the test case - this is what i meant14:07
rameshg87lucasagomes: i am still not sure if it's required though :)14:08
lucasagomesrameshg87, 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 setUp14:08
lucasagomesrameshg87, gotcha, yeah I'm a bit confused whether it's needed or not to be honest14:08
lucasagomeslooks like not14:08
rameshg87lucasagomes: yeah but the value will again be restored back in next test case's tearDown()14:08
lucasagomesrameshg87, perhaps we could have a _get_cached_agent_params function14:09
lucasagomesthat will return the global14:09
lucasagomesto help with tests u can mock that14:09
rameshg87lucasagomes: yeah :)14:09
lucasagomesyou can create a side_effect that will return [{}, {info}]14:09
openstackgerritMerged stackforge/ironic-discoverd: eDeploy plugin to save BIOS and RAID configuration to node.extra  https://review.openstack.org/16469514:09
rameshg87that makes more sense infact14:09
lucasagomesso you see if info is present we don't call get_params from vmedia or cmdline again14:09
rameshg87unless people question why you need a one-line method :D14:09
lucasagomesrameshg87, yeah I think that's a better approach, and we don't have to play with globals14:09
jrollrameshg87: I've done this before to mock time.time()14:10
lucasagomesrameshg87, well we can add a comment saying it's a helper for tests, it's not unsual14:10
lucasagomesjroll, yeah14:10
rameshg87yeah okay14:10
rameshg87makes sense, will change it ...14:10
lucasagomesrameshg87, 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/314:11
jrollgah14:11
rameshg87jroll: it's touching the heart of agent (getting agent params) :)14:11
jrollok14:11
rameshg87folks, i wanted to raise one question regarding a bug i reported: https://bugs.launchpad.net/ironic/+bug/143209314:12
openstackLaunchpad bug 1432093 in Ironic "Instance tear down fail with KeyError: 'is_whole_disk_image'" [Undecided,New]14:13
NobodyCamGood morning Ironicers :)14:13
rameshg87i 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 failed14:14
rameshg87now i am wondering whether we should consider such tests for CI14:14
rameshg87i mean deploy an instance without the patch, try rebuild/tear down with the patch14:14
rameshg87does that make sense ?14:14
*** ChuckC has joined #openstack-ironic14:15
lucasagomesrameshg87, I think it does14:15
*** kkoski has quit IRC14:15
lucasagomestearing down a node is part of the normal flow, we should stress that14:15
jrollrameshg87: I agree with lucasagomes on that agent params, I'll look after you push a new one :)14:16
rameshg87jroll: okay14:16
*** rameshg87 is now known as rameshg87-brb14:16
devananda14: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 meant14:18
devanandayou can't do tht14:18
devanandatests run in parallel14:18
NobodyCammornig devananda lucasagomes jroll and rameshg87-brb14:18
lucasagomesdevananda, yea, we want to have a helper method that mock the global14:18
*** kkoski has joined #openstack-ironic14:18
lucasagomesI mean that return the global so it can be mocked14:18
lucasagomesfor tests14:18
devanandak14:19
devanandaalso, why are you using a global?14:19
lucasagomesdevananda, rameshg87-brb wants to cache the parameters passed to the IPA ramdisk14:19
lucasagomesthe parametes via cmdline or vmedia14:19
lucasagomesbecause they never change anyway, and vmedia may not be available apparently if the temp url expires14:19
lucasagomesdevananda, https://review.openstack.org/#/c/164596/314:20
devanandaa global-global? or a module-level variable?14:20
* devananda reads14:20
lucasagomesa module level variable14:20
devanandaoh14:20
devanandaright - that's not a global14:20
lucasagomesI 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 it14:21
*** stendulker has joined #openstack-ironic14:22
*** rameshg87-brb is now known as rameshg8714:22
rameshg87morning NobodyCam14:23
devanandasorry, early morning brain. yea, module-level variables accessed by "global" is usually unnecessary, and makes testing quite challenging14:23
lucasagomesdevananda, yup :) it's all good14:24
devanandaso I really dont understand this change14:25
devanandawhy is it necessary to deepcopy a global??14:25
dtantsurNobodyCam, morning14:25
NobodyCammorning dtantsur :)14:25
jrollmorning NobodyCam :)14:26
NobodyCam:)14:26
* NobodyCam hopes everyone had a great weekend!14:27
lucasagomesdevananda, I wonder if other functions doing get_agent_params() might be injecting stuff into the dict14:28
lucasagomesand 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 that14:29
devanandarameshg87: 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#L5814:29
devanandalucasagomes: is ^ basically what you were already suggesting?14:29
devanandaglobal is guarded by a private accessor method that can easily be mocked for testing14:30
lucasagomesdevananda, yes14:30
devanandacool14:30
lucasagomeslike the _create_facade_lazily() will populate the global14:30
devanandaexactly14:31
lucasagomesand then we have a get_function that returns it if not set14:31
lucasagomesif set*14:31
rameshg87devananda: yeah okay14:32
rameshg87devananda: was writing up something like this :)14:32
*** BadCub_Away is now known as BadCub14:33
* lucasagomes english level is pretty low today, doing 2 things in parallel and trying to keep concentrated :) 14:33
*** yog__ has quit IRC14:33
rameshg87devananda: lucasagomes: and assume we dont write unit tests for _create_facade_lazily :)14:33
* rameshg87 checks14:33
BadCubmorning everyone14:33
rameshg87BadCub: o/14:34
NobodyCammorning BadCub :_14:34
dtantsurBadCub, o/14:34
lucasagomesrameshg87, 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 returned14:35
lucasagomesBadCub, good morning!14:35
*** zz_jgrimm- is now known as jgrimm14:35
rameshg87lucasagomes: yeah14:35
devanandaok, with that settled and backscroll in other channels similarly read, I'm going to spend today focusing on reviews14:36
devanandaand getting ready for K3 milestone this week14:36
dtantsurdevananda, rameshg87, can we discuss https://review.openstack.org/#/c/161132/ ?14:36
lucasagomesNobodyCam, morning :)14:37
devanandaBadCub: in the reviewjam last week, I think we all (myself included) forgot about our old sync page -- https://etherpad.openstack.org/p/IronicReviewDay14:37
lucasagomesNobodyCam, 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
devanandaso I updated it on the weekend14:37
BadCubdevananda: Hmmm. I believe we did14:38
jrollnice14:38
devanandaBadCub: it had no reviews on it on saturday. it's quite full now :)14:38
NobodyCamlucasagomes: looking14:38
devanandait's possible I missed things, but that page should now have a link to every review tagged by a targeted blueprint14:38
rameshg87dtantsur: i am in ...14:39
*** Marga_ has joined #openstack-ironic14:40
*** Marga_ has quit IRC14:41
*** Marga_ has joined #openstack-ironic14:41
*** ukalifon2 has quit IRC14:41
dtantsurdevananda, rameshg87 and I have some doubts that "Driver pxe_ipmitool does not support inspect" is less confusing to a user than reporting validation failure14:43
*** ChuckC has quit IRC14:43
openstackgerritImre Farkas proposed openstack/ironic: DRAC BIOS vendor_passthru: enable rebooting the node  https://review.openstack.org/16307114:43
stendulkerdevananda: 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-ironic14:47
stendulkerlucagomes: 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
stendulkerlucasgomes: 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
openstackgerritRamakrishnan G proposed openstack/ironic-python-agent: Cache agent parameters for later invocations  https://review.openstack.org/16459614:53
lucasagomesstendulker, lucas<tab>14:55
lucasagomesstendulker, will do :) I will have a call in 5 min but I will do it right after14:55
stendulkerlucagomes: thank you :)14:55
lucasagomesI see the patch was just rebased and few nits pointed by ruby were fixed14:55
lucasagomessince my last review (which I +2'ed14:55
lucasagomes)*14:55
stendulkerlucasgomes: Yes. Some optimization suggested by Ruby, they are also incorporated.14:56
lucasagomescool :)14:57
devanandadtantsur: but parameter validation is not the correct way to advertise that14:58
dtantsurdevananda, that's right... but what we have now is IMO worse, we just hide the fact we have any inspection at all14:59
devanandadtantsur: it's disabled. therefor we should skip validation14:59
dtantsurdevananda, so e.g. instead of contacting admins, a user will assume we don't have a feature :)14:59
devanandathe feature isn't available in that environment14:59
devanandaif, as you propose, the node fails validation, the user is going to assume they've done something wrong15:00
devanandabecause that's what Result = false means for every other interface15:00
dtantsurdevananda, we don't report it as disabled. we report that driver does not support the feature which is just not true15:00
dtantsurdo we need one more case for that?15:00
*** ramineni has quit IRC15:02
devanandaI'd be fine with result = 'disabled'15:02
devanandadtantsur: your scenario is a user noticing 'disabled" message and contacting their cloud operator to ask to enable the feature15:04
devanandadtantsur: that doesn't make any sense -- discovery is only useful to operators15:04
*** mgoddard1 has joined #openstack-ironic15:04
dtantsurright15:05
devanandaeven when we add policy settings to allow delegation of different operational responsibility, it's still not a "user facing feature"15:05
rameshg87devananda: but even for the operators, isn't it confusing "pxe_ipmitool doesn't support inspect" ?15:05
rameshg87devananda: i agree with validation failure15:05
NobodyCamlucasagomes: LGTM!15:05
dtantsurdevananda, should we then at least issue a log warning/info that discoverd is disabled?15:06
lucasagomesNobodyCam, :) thanks15:06
dtantsurdevananda, so that to give at least some clues?15:06
*** mgoddard has quit IRC15:06
devanandadtantsur: oh - logging an info or notice about the feature being turned off -- absolutely15:06
devanandathat'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
dtantsurdevananda, log on importing ironic.driver.modules.discoverd then? because not code from the class is ever called...15:07
devanandamy 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 failure15:07
devanandawhich it, in this case, is not15:07
devanandaand then those tools will need special handling to, eg, grep for 'disabled' in the message string15:07
devanandawhich sounds like a terrible user interface15:07
dtantsurdevananda, note that we already have deploy validation  = False most of the time15:08
openstackgerritGhe Rivero proposed openstack/ironic: Sync from oslo.incubator  https://review.openstack.org/16250515:08
devanandadtantsur: log in the if: blocks here https://review.openstack.org/#/c/161132/6/ironic/drivers/pxe.py15:08
dtantsurdevananda, makes sense too (will create a helper function then). rameshg87 does it work for you?15:09
devanandadtantsur: 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 True15:10
devanandathat's how clients know they've satisfied all the required info15:10
dtantsuryeah, my comment was about "automated tools which enroll nodes and parse the output of "ironic node-validate"15:10
jrolldevananda: this reminds me, I want to see your ansible thing: )15:10
rameshg87dtantsur: but still the error msg "Driver pxe_ssh does not support inspect."15:11
devanandadtantsur: oh, you're right :) s/enroll/do things with/15:11
rameshg87dtantsur: is that fine still ?15:11
dtantsurnot fine, but the alternative sucks too :)15:12
*** stendulker has quit IRC15:12
rameshg87yeah, :(15:13
dtantsurdevananda, what if we change the error to: "Driver pxe_ipmitool does not support inspect (not implemented or disabled)"15:13
dtantsurwdyt?15:13
devanandadtantsur: just to be clear, which error?15:14
*** aswadr has quit IRC15:14
* dtantsur is looking15:14
rameshg87devananda: when some one tries to do an inspect when it is disabled in CONF file15:15
rameshg87devananda: ironic node-set-provision-state <> inspect15:15
dtantsurdevananda, https://github.com/openstack/ironic/blob/master/ironic/common/exception.py#L34415:15
*** david-lyle_afk is now known as david-lyle15:15
rameshg87dtantsur: and https://github.com/openstack/ironic/blob/master/ironic/conductor/manager.py#L1480-L148215:16
openstackgerritRamakrishnan G proposed openstack/ironic-python-agent: Add uefi support in image extension  https://review.openstack.org/16373915:17
*** achanda has joined #openstack-ironic15:17
devanandadtantsur: that seems reasonable15:19
dtantsurack, will change too15:19
* BadCub needs mroe coffee15:19
BadCubmore even15:19
devanandadtantsur: 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 messages15:20
devananda*this15:20
dtantsuryeah15:20
JoshNangjroll: 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
rameshg87lucasagomes: are you aruond ?15:22
JoshNangjroll: (at least for the agent driver)15:22
JoshNangerr drivers15:22
rameshg87lucasagomes: i am kind of getting occassional failures here: https://github.com/openstack/ironic-python-agent/blob/master/ironic_python_agent/extensions/image.py#L5115:22
rameshg87lucasagomes: on actual hardware15:23
rameshg87lucasagomes: just wondering if partition table is not getting read properly fast enough15:23
lucasagomesrameshg87, I will take a look in a sec15:23
rameshg87lucasagomes: did you test on real hardware ? image extension ?15:23
rameshg87lucasagomes: i am just testing with retry and delay_on_retry15:24
*** rameshg87 is now known as rameshg87-brb15:24
*** achanda has quit IRC15:25
jrollJoshNang: don't you need to power them off to pxe boot them?15:26
JoshNangjroll: derp, ignore me15:27
jroll:)15:27
JoshNangso getting it to work in devstack is all fine and well, but that'd be a new mandatory config option for deployers :/15:27
openstackgerritRuby Loo proposed openstack/ironic: Log message is missing a blank space  https://review.openstack.org/16473515:27
jrollJoshNang: oh well?15:28
dtantsurdevananda, 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 was15:28
*** mjturek1 has quit IRC15:28
jrollJoshNang: we should probably check for that at startup15:28
*** mjturek1 has joined #openstack-ironic15:29
JoshNangjroll: probably a good idea15:30
openstackgerritJohn L. Villalovos proposed openstack/ironic: Update unittests and use NamedTemporaryFile  https://review.openstack.org/16267215:32
jlvillalrloo: 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/16422615:33
lucasagomesrameshg87-brb, back, so hmmmm it should be good because I use partx -u to make the kernel re-read the part table15:36
lucasagomesrameshg87-brb, when you do a delay_on_try does it solve the problem?15:36
*** rameshg87-brb is now known as rameshg8715:37
rameshg87lucasagomes: i guess it doesn't15:37
rameshg87lucasagomes: because stdout of lsblk is empty15:37
lucasagomeshmm15:37
rloojlvillal: you were the one talking about it. I think I just said, yeah, it might be nice ;)15:37
rameshg87lucasagomes: will try checking if stdout is empty and adding a retry block waiting after 1 sec15:37
lucasagomesrameshg87, I wondering whether partx is doing the right thing there, or we should use something like partprobe15:37
rameshg87partx man says reread partition table15:38
jlvillalrloo: I thought you were discussing an issue with possible problems with checking the content of log messages because of I18N issues.15:38
rameshg87so looks like right thing :)15:38
lucasagomesrameshg87, right, if you have ur hands on it, can u test wiht partprobe if that doesn't work with partx ?15:38
rameshg87to me15:38
jrolllucasagomes: partprobe wasn't working for us in the agent in the gate :/15:38
rloojlvillal: yeah, I did mention that.15:38
lucasagomesrameshg87, yeah, but urgh I already had so many problems with both partprobe and partx man15:38
lucasagomesjroll, oh15:38
NobodyCambrb15:38
jrolllucasagomes: (though it worked fine on real metal)15:38
rameshg87lucasagomes: on real hardware ?15:38
lucasagomesjroll, not working how?15:38
lucasagomesrameshg87, yeah15:38
lucasagomesjroll, ouch, with vms?15:38
jrolllucasagomes: can't remember, let me look15:39
rameshg87lucasagomes: okay let me check ...15:39
lucasagomesrameshg87, out of context, but I had a cloning disk tool15:39
lucasagomeswhich I used to play a lot of MBR and stuff15:39
jrolllucasagomes: https://bugs.launchpad.net/ironic/+bug/141883315:39
openstackLaunchpad 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
lucasagomesand relied on those tools to apply the images via network15:39
lucasagomesrameshg87, https://github.com/umago/carbono15:39
lucasagomesjroll, T.T15:40
lucasagomesjroll, so did partx worked for the VMs in gate seems15:40
rameshg87lucasagomes: okay15:41
jrolllucasagomes: yeah, and iirc it works fine on metal too, I think we have that change in our environment15:41
lucasagomesgotcha15:41
jrollJayF could confirm that when he's around15:41
lucasagomesjroll, that's all good, thanks for the input15:41
devanandadtantsur: ah - fair point about fake-discover15:41
rameshg87lucasagomes: okay, may be i need to check, but i am sure lsblk is returning empty for some reason. will confirm ...15:42
jrolllucasagomes: yeah, np15:42
lucasagomesrameshg87, so hmm... perhaps the smartest thing to do, is to weather call partx -u/partprobe a couple of times with a sleep between them15:42
lucasagomeslike a retry function15:42
devanandadtantsur: 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
dtantsursure :)15:42
lucasagomesit may take some time to the kernel notice the changes on the device15:42
devanandalucasagomes: is there something you can poll, instead of sleep?15:42
rameshg87lucasagomes: yeah was thinking of that ..15:43
rameshg87lucasagomes: will give a try and come back ...15:43
lucasagomesdevananda, hmm we could look at devfs see if the partition we created just appeared there15:46
rameshg87devananda: 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.py15:46
lucasagomes1. calls partx -u. 2. looks at the fs by-uuid to see if the partition already appeared there15:47
rameshg87devananda: i have an _get and _set methods15:47
lucasagomeswe got the root uuid partition15:47
* NobodyCam is back15:47
*** achanda has joined #openstack-ironic15:48
jlvillalGood morning Ironic....15:48
NobodyCammorning jlvillal :)15:48
*** wuhg has quit IRC15:49
jlvillalNobodyCam: Thanks.  Did you get faster internet?  If so, I hope it is working well :)15:49
BadCubdevananda: looked over the Review Day list. Looks like we have a few that can get closed out pretty easy, I think.15:50
devanandaBadCub: nice. please mark them on the page to get attention15:50
BadCubWill do!15:50
NobodyCamjlvillal: oh ya!!! real internetz is great!15:51
jlvillalNobodyCam: Woo hoo! :D15:51
* rameshg87 will be back for the meeting ...15:53
*** rameshg87 is now known as rameshg87-away15:53
NobodyCamnight rameshg87-away15:53
devanandarameshg87: that will work. but I still dont see why you need to use deepcopy15:53
*** rameshg87-away is now known as rameshg8715:53
rameshg87devananda: just to make sure if there is nested dictionary in params15:54
jlvillalrameshg87: The meeting is in about 15 hours15:54
jlvillalI think15:54
rameshg87oh15:54
rameshg87i thought it was in 1 hour :)15:54
jlvillalrameshg87: Unless it is a different meeting15:54
rameshg87my mistake :)15:54
rameshg87then it's my tomorrow actually15:54
rameshg87thanks jlvillal15:54
jlvillalrameshg87: You're welcome15:54
devananda13 hours15:55
rameshg87devananda: isn't deepcopy required to make sure that we have complete copy if it was a nested dictionary ?15:55
devanandarameshg87: why is copy needed at all, i mean?15:56
devanandarameshg87: why not simply return the dict (which is really a reference to it)15:56
devanandais there a concern that a calling function is going to modify the global?15:56
rameshg87devananda: but what if some code modified it ?15:56
rameshg87devananda: yeah15:56
devanandaand 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
rameshg87devananda: if *someone* modified the values of keys or deleted some keys15:57
rameshg87devananda: the code trying to access later on might never have it15:57
BadCubbrb15:57
BadCubdevananda: I put notes on two of them to start15:57
rameshg87devananda: 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 agent15:57
devanandarameshg87: k. so, if that's a concern, there's a better way than copying it twice15:58
devanandai'll comment on the review15:58
rameshg87devananda: okay .. thanks ..15:58
rameshg87will check ..15:58
*** rameshg87 is now known as rameshg87-away15:59
*** absubram has quit IRC16:01
openstackgerritDmitry Tantsur proposed openstack/ironic: Start using in-band inspection  https://review.openstack.org/16113216:02
dtantsurdevananda, rameshg87-away ^^^16:02
openstackgerritDmitry Tantsur proposed openstack/ironic: Write documentation for hardware inspection  https://review.openstack.org/16177516:03
*** achanda has quit IRC16:04
*** stendulker has joined #openstack-ironic16:07
*** ijw has quit IRC16:07
*** stendulker_ has joined #openstack-ironic16:09
devanandathis 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 IRC16:11
* jroll will look16:11
devanandaI'm a bit confused by having a continue_deploy and a _continue_deploy method, which are not related16:12
*** openstackgerrit has joined #openstack-ironic16:12
NobodyCamlucasagomes: 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 IRC16:12
lucasagomesNobodyCam, https://blueprints.launchpad.net/ironic/+spec/ipa-as-default-ramdisk16:12
* lucasagomes checks if he misspelledit16:13
NobodyCamImplemented Edit16:13
NobodyCamdoh16:13
lucasagomeslooks correct, not sure why it can't find it tho hmmm16:14
NobodyCamit listed as Implemented16:14
lucasagomesoh16:14
NobodyCamBadCub: ^^16:14
BadCubhuh16:14
NobodyCam162449 should be need review!16:14
BadCubset to needs review16:15
NobodyCamawesome THank you16:15
NobodyCamand now it shows up :)16:16
openstackgerritMerged openstack/ironic: Implement execute clean steps  https://review.openstack.org/15556116:21
openstackgerritMerged openstack/ironic: Use oslo_context package  https://review.openstack.org/16249916:22
JoshNang\o/ thanks for all the help everyone ^16:22
dtantsur:)16:22
NobodyCamw00 h00 :)16:22
devanandaJoshNang: o/^516:22
openstackgerritMerged openstack/ironic: Use oslo_policy package  https://review.openstack.org/16250116:22
jlvillalJoshNang: Congrats on all the work to get that done! :)16:23
jrolldevananda: I think you're misunderstanding 16244916:23
devanandajroll: that's quite possible16:24
jrollthis is the same change lucas just did to enable agent/iscsi16:24
jrollthis enables agent/iscsi on ilo16:24
jrollcontinue_deploy() comes from the base class and is for use with the agent ramdisk, _continue_deploy() is for use with bash ramdisk16:24
NobodyCamlucasagomes: 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-auth16:24
*** ijw has joined #openstack-ironic16:24
NobodyCamhttps://github.com/openstack/ironic/blob/master/etc/ironic/ironic.conf.sample#L20316:25
devanandajroll: ah. so *AgentDriver is compatible with both ipa and dib ramdisks?16:25
*** rloo has quit IRC16:25
devanandasorry if that is a thing I should already know - my brain isn't fully awake yet16:25
*** rloo has joined #openstack-ironic16:26
jrolldevananda: no, Pxe*Driver is compatible with ipa and bash ramdisks16:26
devanandajroll: ok. that's what I thought16:26
lucasagomesNobodyCam, oh that's a good point16:26
NobodyCamwant me to comment on review?16:26
*** rloo_ has joined #openstack-ironic16:27
*** rloo has quit IRC16:27
openstackgerritMerged openstack/ironic: Address comments on cleaning commit  https://review.openstack.org/16464516:27
openstackgerritMerged openstack/python-ironicclient: Add support for inspection to node-set-provision-state  https://review.openstack.org/14880416:27
lucasagomesNobodyCam, please do... also, I see we are doing that in other parts of the code16:28
lucasagomesI don't know how that would play with noauth16:28
*** Nisha has joined #openstack-ironic16:28
NobodyCam:(16:28
lucasagomesNobodyCam, re https://github.com/openstack/ironic/blob/master/ironic/conductor/manager.py#L1142-L114316:28
jrollNobodyCam: I think that's fine...16:28
jrollnoauth is saying "ironic doesn't validate tokens"16:29
jrollthat grabs a token to talk to glance16:29
jrolldo we have a glance auth_strategy setting?16:29
lucasagomesmaybe the keystone module will just ignore that if noauth16: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
devanandalucasagomes: so there are deploymets without keystone or glance16:29
devananda*of ironic without16:30
NobodyCammy fear is there could not be a keystone service16:30
JoshNangrloo_: hmmm16:30
lucasagomesdevananda, yeah, after nobodycam comments it just came to mind16:30
jrollhrm, maybe16:30
devanandajroll: 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 weird16:30
lucasagomesI'm checking cause seems in other parts of the code we are also calling keystone.get_admin_token() without checking noauth16:30
jrolldevananda: the change is, that driver can now use the IPA ramdisk.16:31
jrolldevananda: and that is weird, but I believe IscsiVendorWhatever does the same.16:32
devanandalucasagomes: 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 config16:32
JoshNangrloo_: i think i'd rather keep it explicit rather than implicit.16:32
devanandajroll: gotcha.16:32
lucasagomesdevananda, right, yeha so that bits on 622-623 will have no problem right?16:32
*** kalpase has quit IRC16:34
devanandajroll: reversed my -116:34
jrolldevananda: I +2'd that change, leaving it to you to +A16:34
jrolloh16:34
jrollok, I can +A16:34
devanandaoh16:34
devanandak, thx16:34
NobodyCamlol16:34
NobodyCamgah you folk are quick16:35
NobodyCamreverted my vote16:36
NobodyCamlol16:36
*** rameshg87-away is now known as rameshg8716:36
rameshg87oh just in time :)16:36
rameshg87devananda: will clean up the function name - it has to be done for pxe driver too16:36
rameshg87devananda: 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
devanandarameshg87: thanks much16:37
NobodyCammornign rloo_ :)16:37
* NobodyCam gets more coffee16:38
lucasagomesdevananda, ur comment remids me that I've to rename _continue_deploy to pass_deploy_info16:38
lucasagomeson pxe16: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
rameshg87lucasagomes: i will take care if you are not getting to it before tomorrow16:38
rameshg87lucasagomes: i will do for both ilo and pxe driver16:38
lucasagomesrameshg87, I will put one up for the pxe today16:38
lucasagomesrameshg87, cool16:38
* BadCub needs to grab food16:39
NobodyCamrloo_: heheheh :) we've been busy!16:40
JoshNangrloo_: i just added everything already in states.py. getting a small patch in to nova when we add states shouldn't be hard16:40
rloo_JoshNang: "shouldn't be hard" ;)16:41
dtantsurrloo_, morning!16:41
JoshNangrloo_: oh trust me...i know :)16:41
openstackgerritRamakrishnan G proposed openstack/ironic-python-agent: Cache agent parameters for later invocations  https://review.openstack.org/16459616:41
JayFjroll: we don't have that change in our env yet16:41
rloo_JoshNang: :D16:41
jrollJayF: really? not preprod either?16:42
devanandagetting food and more coffee... bbiab16:42
JayFjroll: we aren't past my HW Manager refactor b/c the onmetal-ironic-hardware-manager hasn't been updated to that interface yet16:42
rloo_hi dtantsur and everyone else :)16:42
rameshg87dtantsur: ack16:42
jrollJayF: oh right16:42
jrollthanks16:42
jrollI think we did test it on some metal, at least16:42
JayFIf you did, sure. I haven't.16:44
*** anderbubble has joined #openstack-ironic16:45
jrollhm16:45
*** anderbubble has left #openstack-ironic16:45
rameshg87dtantsur: https://review.openstack.org/#/c/161132/7/ironic/drivers/drac.py16:45
rameshg87dtantsur: better if it was pxe_drac, right ?16:45
rameshg87dtantsur: instead of PXEDracDriver16:46
dtantsurrameshg87, I don't if code should assume knowledge about setup.cfg16:46
rameshg87dtantsur: at that point driver_factor is not completely initialized, right ?16:46
dtantsurI think so16:46
*** ukalifon1 has quit IRC16:48
rameshg87okay16:48
* rameshg87 goes to sleep 16:54
rameshg87good night all ...16:54
*** openstackgerrit has quit IRC16:54
*** rameshg87 has left #openstack-ironic16:54
*** openstackgerrit has joined #openstack-ironic16:54
openstackgerritRuby Loo proposed openstack/python-ironicclient: Metavar name should be hyphenated  https://review.openstack.org/16478116:57
NobodyCamdevananda: did you want to take another look at 164596? I'm about to add second +216:58
openstackgerritDmitry Tantsur proposed stackforge/ironic-discoverd: [WIP] Add DevStack plugin for ironic-discoverd  https://review.openstack.org/16478216:58
*** mtanino has joined #openstack-ironic16:59
*** Marga_ has quit IRC17:01
dtantsursee you tomorrow17:02
*** dtantsur is now known as dtantsur|afk17:02
openstackgerritLucas Alvares Gomes proposed openstack/ironic: Rename _continue_deploy() to pass_deploy_info()  https://review.openstack.org/16478317:04
* devananda returns17:09
devanandadtantsur|afk: I believe discoverd disabled should be an INFO or NOTICE, not WARN17:10
devanandadtantsur|afk: also there's a typo in the log. /me fixes17:11
NobodyCamnight dtantsur|afk17:11
*** stendulker_ has quit IRC17:12
devanandaNobodyCam: 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
NobodyCamis that 161132? I was just opening that one?17:15
devanandathat's master17:15
devanandawell,actually the discoverd change, but it shouldn't be related17:16
* devananda looks further17:16
*** ijw has quit IRC17:16
*** mgoddard1 has quit IRC17:18
openstackgerritDevananda van der Veen proposed openstack/ironic: Fixup log message for discoverd  https://review.openstack.org/16479217:19
openstackgerritNisha Agarwal proposed openstack/ironic: iLO implementation for hardware inspection  https://review.openstack.org/15159617:20
devanandawho wants to land discoverd support?17:22
devanandaI think it's good to go -- just need eyes on https://review.openstack.org/#/c/161132/17:22
NobodyCamdevananda: reviewing now17:22
devanandagreat17:22
*** rsalevsky has quit IRC17:23
NobodyCamoh I like the create_if_enabled stuff :)17:26
*** mgoddard has joined #openstack-ironic17:26
*** harlowja has joined #openstack-ironic17: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
openstackgerritNisha Agarwal proposed openstack/ironic: iLO driver updates node capabilities during inspection  https://review.openstack.org/16357217:29
*** ijw has joined #openstack-ironic17:29
*** Marga_ has joined #openstack-ironic17:30
*** lazy_prince is now known as killer_prince17:31
*** ijw has quit IRC17:31
*** ijw has joined #openstack-ironic17:31
*** ijw has quit IRC17:33
*** ijw has joined #openstack-ironic17:34
*** dprince has quit IRC17:36
devanandarloo_: I have tested that all the things load and report correctly -- i have not actually tested ironic-discoverd with it, though17:40
rloo_devananda: ok, that's good enough, since that's what the patch is about. thx.17:40
devanandayup. cheers17:40
NobodyCamdevananda: 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-ironic17:43
devanandaNobodyCam: thanks, looking17:44
NobodyCamNisha: are you around?17:48
NishaNobodyCam, yes17:48
NobodyCamhey there17:48
NobodyCam:)17:48
NishaNobodyCam, :)17:48
NobodyCamI looking at the comments on 15159617:48
Nishaok17:48
*** MattMan has quit IRC17:48
NobodyCamis there documantation we can point to for the port number stuff17:49
NobodyCami didn't see anything on http://docs.openstack.org/developer/ironic/drivers/ilo.html17:49
NishaNo its only the RIBCL output17:49
NobodyCamthat may help over come some of the questions17:49
Nishayes17:49
NishaNobodyCam, just a min17:50
NobodyCamie. some examples in the doc's of getting and setting that param would help I believe17:50
*** achanda has joined #openstack-ironic17:50
NobodyCamsure17:50
*** dprince has joined #openstack-ironic17:50
*** jistr has quit IRC17:51
BadCubAnyone 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
Nishathe sample return from GET_EMBEDDED_HEALTH17:52
*** ukalifon1 has quit IRC17:52
NishaNobodyCam, http://paste.openstack.org/show/192686/17:53
*** achanda_ has joined #openstack-ironic17:55
NobodyCamNisha: yes! I understand what needs to be there based on the review and the sample in _get_desired_macs_for_port_creation17:57
NishaNobodyCam, any action item for me?17:57
NobodyCambut 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 IRC17:58
NobodyCamie as a op how will i know which port I have pluged my cable into?17:58
NishaNobodyCam, as a operator if you have plugged the cable you will know the port number from the hardware itself17:59
lucasagomesfolks I will call it a day17:59
NobodyCamhave a good night lucasagomes17:59
Nishaits written clearly on the hardware (for embedded NICs atleast17:59
BadCubg'night lucasagomes18:00
JayFNisha: I think there's still value in being able to pull that info out of band18:00
lucasagomesI will be off tomorrow due the St. Patricks day18:00
lucasagomesenjoy it folks :)18:00
NishaJayF, i didnt get18:00
*** lucasagomes is now known as lucas-dinner18:00
JayFNisha: 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 it18:00
openstackgerritDevananda van der Veen proposed openstack/ironic-python-agent: Fix nit in test_get_agent_params_from_cache  https://review.openstack.org/16480518:00
Nishahow will we know OOB which port is plugged18:00
krotscheckjroll: Hey, sorry about disappearing on you on friday. You mentioned the #rackspace channel?18:00
NobodyCamNisha: so all hp server have somehting like: http://h20564.www2.hp.com/hpsc/doc/public/imageServlet?DOCID=emr_na-c00385409-9/c00386425.gif18:01
JayFkrotscheck: if you /join #rackspace on this network, talk to kenperkins18:01
Nishaonly when OS is running underlying protocol like RIS/RIBCL can give the NIC status18:01
JayFkrotscheck: pkgcloud not supported in browser today but apparently is on a todo, so perhaps the chat will help18:01
krotscheckJayF: Thansk18:01
NishaNobodyCam, yes i checked on gen8 and gen918:02
devanandaJoshNang: you rebasing https://review.openstack.org/#/c/159322/ ?18:05
openstackgerritJosh Gachnang proposed openstack/ironic: Add /nodes/<uuid>/cleaning/steps API  https://review.openstack.org/15932218:07
JoshNangdevananda: lol literally what i was working on18:07
devanandaJoshNang: lol18:07
devanandaNobodyCam: fwiw, 164792 is the sort of change that I think we can start safely approving with only one +218:08
NobodyCamthat your log update?18:08
devanandait is18:08
devanandai saw you just +2'd it18:08
NobodyCami didn't just because the depent patchwas not yet approved18:08
NobodyCam:-p18:08
devanandaoh18:08
NobodyCamit may be now!18:09
NobodyCam:-p18:09
devanandaso you can +A a patch further down a chain like that, and jenkins will wait until the chain is approved and then merge all together18:09
NobodyCamok...18:09
NobodyCam:)18:09
devanandabut yes, it looks like rloo approved the discoverd patch18:09
NobodyCamlol +a'd18:10
NobodyCamNisha: 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.py18:12
*** derekh has quit IRC18:12
NobodyCammaybe other can weigh in with thoughts too18:12
NobodyCamothers even18:12
BadCubdevananda: MAKED https://blueprints.launchpad.net/ironic/+spec/inband-properties-discovery as implemented18:13
BadCubmarked even18:13
devanandawoot18:13
devanandaBadCub: well, wait ... it's not all merged yet18:14
BadCuboop18:14
devanandaBadCub: that's a tiny bit pre-emptive, even though it's all approved something could fail to merge18:14
NishaNobodyCam, 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
NobodyCamNisha: nit: s/which are/ which have/18:15
NobodyCam:-p18:15
NishaNobodyCam, ok. I will update the patch with this. any other comments18:16
Nishaplease leave comments on the patch if any more18:16
NishaNobodyCam, one more query18:16
NobodyCamI think that will help clear it for folks18:16
NobodyCamsure18:16
Nishafor delete_ports what shall be the action from ironic side:18:17
Nisha1. delete ports only for the non-associated macs18:17
Nishaor18:17
Nisha2. delete all the ports which are not requested in 'inspect_ports'18:18
Nishacons for 1. -> we may have additional ports which may not have cable connected18:19
devanandaJoshNang: so in 159322, you add a new controller to NodesController -- this is going to change the base (v1.1) API18:19
Nishacons for 2 -> it may delete the already created macs which may not be in 'inspect_ports'18:20
devanandaJoshNang: take a look at my proposed changes in 164369 -- it actually hides the controller from requests predating that version18:20
NishaNobodyCam, i guess risk of 1 is less18:20
*** rloo_ has quit IRC18:20
Nishathan risk of 218:20
devanandaJoshNang: if we really want to preserve API semantics depending on the version, then you'll need to do that18:20
NobodyCamoh 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 card18:21
Nishathat will be there18:21
Nishai think i will go with option 118:22
Nisharisk of 2 is big18:22
NobodyCam++18:22
JoshNangdevananda: that's reasonable. jroll had initially proposed returning a 404, but it seems that's different than actually not existing :/18:22
devanandayes18:22
devanandaJoshNang: thta is apparently an issue with rest.RestController18:23
devanandawith some help from ryanpetrello, i worked around it18:23
JoshNangalright, i'll take queues from 16436918:23
JoshNangerr cues18:23
*** mgoddard has quit IRC18:23
krotscheckIf I were to create an isomorphic ironic API javascript library, what would I name it?18:28
krotscheckThat's a lot of words.18:28
krotscheckIf I were to create a javascript thing that talks to ironic, what would I name it?18:28
krotscheckjavascript-ironic-lib?18:29
JayFjs-ironicclient?18:29
devanandaJoshNang: none of the drivers actually implement clean_step yet, correct? (or am i just blind)18:29
devanandaironic-javascript-client?18:30
krotscheckJayF: It's not truly a client though - no commandline18:30
devanandakrotscheck: ajil18:30
krotscheckajil?18:30
devanandakrotscheck: a-javascript-ironic-library18:30
NobodyCamlol18:30
NobodyCamieek18:30
devanandaalso, sounds like AGILE18:30
krotscheck......18:30
JayFkrotscheck: I disagree that a client has to include the CLI, but I get your thought process18:30
devananda^_^18:30
* krotscheck sadtrombones all over the place.18:30
JoshNangdevananda: correct. i have a patch up to add them to the agent for inband, ramieni has one for ilo out of band18:31
devanandakrotscheck: 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
krotscheckWell, who knows. We might get to the point where javascript files get a shebang line.18:31
openstackgerritMerged openstack/ironic: iscsi_ilo driver to support agent ramdisk  https://review.openstack.org/16244918:31
*** wanyen has joined #openstack-ironic18:31
krotscheckaaactually18:31
openstackgerritMerged openstack/ironic: Log message is missing a blank space  https://review.openstack.org/16473518:32
JayFI've def. seen #!/usr/bin/node as the first line of a file, he18:32
JayFh18:32
devanandaJoshNang: great. also, i didn't see https://review.openstack.org/#/c/164646/1 on the 'pad, so just added it18:32
JoshNangdevananda: ++18:33
*** saripurigopi has joined #openstack-ironic18:34
krotscheckOh wow. That actually just worked18:35
krotscheckToday, the commandline....18:35
krotscheck...tomorrow, THE WORLD!18:35
*** dprince has quit IRC18:35
NobodyCamok pinky18:37
NobodyCam:-p18:37
NobodyCamoh wait that was brain who said that18:38
openstackgerritMerged openstack/ironic-python-agent: Cache agent parameters for later invocations  https://review.openstack.org/16459618:41
devanandaJoshNang: should clean priorities be config options ? re: https://review.openstack.org/#/c/157715/16/etc/ironic/ironic.conf.sample,cm18:41
JoshNangdevananda: definitely. it lets the operator enable, disable, and reorder (if necesssary)18:42
devanandak k18:42
devanandaJoshNang: also, i posted a nit following up on ^^ here https://review.openstack.org/#/c/164805/18:42
JoshNangdevananda: ahh saw your comment on the other patch. reviewing now18:43
*** dprince has joined #openstack-ironic18:44
openstackgerritNisha Agarwal proposed openstack/ironic: Automate boot iso creation with in ironic for iscsi-ilo  https://review.openstack.org/15590018:48
devanandaJoshNang: why does get-clean_steps for the agent require the agent to be booted?18:50
devanandaJoshNang: a 409 CONFLICT becaues the node is in the wrong power state doesn't make sense for a GET request ...18:50
JayFdevananda: the agent can dynamically include/exclude hardware managers and decom steps using hardware managers18:51
NobodyCamJayF:18:51
JayFdevananda: 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 steps18:51
JayFs/decom/cleaning/18:51
* JayF brb -> lunch18:51
devanandaJayF: but a 409 seems a bit extreme18:51
JayFWe 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
JayFI don't have a personal preference as to the http code in return18:52
devanandaJayF: 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
JayFI 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 food18:53
devanandaack18:53
JoshNang:(18:53
JayFdevananda: I think communicating that would be difficult/impossible without an error status18:53
JayFas an operator, if I ask for a thing, if you throw a 2xx, I should be able to assume it's complete18:53
* JayF actually gone18:53
NobodyCamopen question: should our tests actually create / delete files? REF: https://review.openstack.org/#/c/162672/12/ironic/tests/drivers/test_ipmitool.py18:53
devanandaJoshNang, JayF: my concern with ^ is that the API semantics // workflow for using cleaning is going to be different for different drivers18:54
devanandaNobodyCam: if they are properly using tempdirs, I think that's fine18:54
NobodyCam:) ack :)18:54
NobodyCamTY18:54
JoshNangdevananda: i was considering caching the cleaning steps on the node object as a potential way around it18:54
devanandaJoshNang: ++18:55
JoshNangbut there's still the hangup that you could boot a different agent with a different set of cleaning steps18:55
devanandawell. initial thought aside, that's potentially incorrect too18:55
devanandaright18:55
devanandawhat the agent did last time doesn't necessarily mean it will be able to do the same thing this time18:55
JoshNangright18:55
BadCubdevananda: I moved https://blueprints.launchpad.net/ironic/+spec/inband-properties-discovery up to the Approved/Waiting to Land category on Whiteboard18:56
*** ijw_ has joined #openstack-ironic18:56
*** Marga_ has quit IRC18:57
devanandaBadCub: ++18:57
*** Marga_ has joined #openstack-ironic18:57
JoshNangdevananda: can we pass a query param to the effect of "no cache, give me the current or fail?"18:57
JoshNangand default it to no? i think for most deployments, the agent cleaning steps aren't going to be changing a ton18:58
devanandaJoshNang: my objection is that I dont want a different API for in vs out of band18:58
NobodyCamjlvillal: question on 16267218:58
JoshNanggotcha. hmm.18:59
*** ijw has quit IRC18:59
devanandaJoshNang: 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
NobodyCamshould we be taking https://github.com/openstack/ironic/blob/master/etc/ironic/ironic.conf.sample#L316 into account in _make_password_file19:00
JoshNangdevananda: that...fixes a lot of things19:00
devanandaJoshNang: heh19:00
JoshNangso we still have the issue that booting an agent + neutron dhcp means we need a network for out-of-deploy-cycle agent booting19:01
JoshNangi 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 downstream19:02
*** Marga_ has quit IRC19:03
*** Marga_ has joined #openstack-ironic19:03
JoshNangdevananda: (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-ironic19:06
*** Nisha has quit IRC19:06
*** Nisha has joined #openstack-ironic19:07
devanandaooh, right19:11
devanandaso that applies to any time the agent needs to get on the network19:11
JoshNangyup19:12
JoshNangso i don't think its terribly unreasonable to require the operator to provide a neutron network, but we'd really need to document it19:12
devanandayea, so that's not part of these patches19:12
JoshNangit's in the agent cleaning patch19:13
devanandaoh, it is?19:13
JoshNanghttps://review.openstack.org/#/c/161453/8/ironic/dhcp/neutron.py19:13
devanandaahh19:13
JoshNangwhich seems like it should be more general19:14
devanandayup19:14
devanandawhat if I want to use hte agent w/o neutron?19:14
devanandawhich, in fact, I do19:14
JoshNanghey me too!19:15
JoshNangassuming you're using a flat network with tftp, you'd just reboot with pxe19:15
devanandayup19:16
devanandaJoshNang: the client can't actually disable any clean steps via the REST API19:17
JoshNangcorrect19:17
devanandaJoshNang: so we're merely exposing the ability to list them ... why is that actually important to the client?19:17
JoshNangi definitely think operators are going to want a list of what steps a node would take, especially since they'll be different depending on drivers19:18
JoshNangbut i don't think it's essential, just good for operator visibility19:18
JoshNangthe 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 should19:20
openstackgerritMerged openstack/ironic: Start using in-band inspection  https://review.openstack.org/16113219:21
openstackgerritMerged openstack/ironic: Write documentation for hardware inspection  https://review.openstack.org/16177519:21
*** Marga_ has quit IRC19:21
*** ParsectiX_ has joined #openstack-ironic19:22
*** Marga_ has joined #openstack-ironic19:22
devanandaJoshNang: are the agent's choices being driven by the conductor config option settings?19:22
*** ParsectiX_ has quit IRC19:22
devanandaJoshNang: or is the agent freely discovering new things and doing stuff the conductor doesn't know about?19:22
JoshNangdevananda: the agent cleaning steps are determined by the hardware managers19:23
JoshNangeach manager returns a list19:23
*** Marga_ has quit IRC19:25
*** Marga_ has joined #openstack-ironic19:26
JoshNangthe agent reports the list of steps it has, and the conductor combines all the steps then starts telling drivers to execute each step19:26
*** rloo has joined #openstack-ironic19:27
*** saripurigopi has quit IRC19:31
jroll18: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 greghaynes19:37
greghaynesjroll: ohai19:39
greghaynesyes19:39
jrollhi :)19:39
greghaynesI was running with noexec /tmp19:39
jrolloh, right, only exec'ing temp files19:40
jrollso this case is fine ( https://review.openstack.org/#/c/162672/12/ironic/tests/drivers/test_ipmitool.py )19:40
greghaynesYea, 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
jrollheh19:42
NobodyCamokay question: is the unmount except block in this example ( http://paste.openstack.org/show/KLDOud9mRSmDqNQ3gYQF ) needed19:43
jrollNobodyCam: not the unmount in the except block, no19:44
NobodyCam* is the unmount in the except block19:44
NobodyCamjroll: :) TY thats what I thought19:44
jrollfinally always gets run, no matter what19:45
jrolleven if the try block has a return19:45
NobodyCam:)19:45
NobodyCamTy jroll :)19:46
*** alexpilotti has quit IRC19:50
*** r-daneel has joined #openstack-ironic19:58
openstackgerritJosh Gachnang proposed openstack/ironic: Add /nodes/<uuid>/cleaning/steps API  https://review.openstack.org/15932219:59
*** mgoddard has quit IRC20:02
openstackgerritJosh Gachnang proposed openstack/ironic: Add /nodes/<uuid>/cleaning/steps API  https://review.openstack.org/15932220:02
JoshNangheh thought of another test as soon as i submitted the first one20:03
* devananda lunches20:04
NobodyCamoh food20:06
BadCubmmmm... food20:11
mrdaMorning Ironic20:12
jlvillalNobodyCam: I just saw your message.20:13
jlvillalNobodyCam: I think on: https://review.openstack.org/16267220:13
NobodyCammorning mrda :)20:13
NobodyCamjlvillal: yep20:14
jlvillalNobodyCam: You had a question?20:14
NobodyCamleft comment on the review20:15
NobodyCam:)20:15
*** ndipanov has quit IRC20:16
jlvillalNobodyCam: Okay.  Not sure what is expected on the list of exceptions that it returns.20:16
NobodyCamjlvillal: ya that just a minnor nit20:17
NobodyCamits the other one20:17
jlvillalNobodyCam: I'm unsure how to address the exception list.  I can update it to use the tempdir value though.20:17
jlvillalNobodyCam: Thanks for the review.20:17
rloohey 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
NobodyCamthe tempdir conf setting is the biggie20:18
jlvillalNobodyCam: Would that be like: cfg.CONF.tempdir?20:18
JoshNangrloo: 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 fixes20:19
*** ifarkas has quit IRC20:19
rlooJoshNang: ok, that answers my question then ;)20:19
rlooJoshNang: you have to up the max_version20:19
*** alexpilotti has joined #openstack-ironic20:19
JoshNanghuh, suprised tests passed passing a version greater than max..20:19
NobodyCamjlvillal: here is what I did? I'm sure there are better ways: https://review.openstack.org/#/c/160383/13/ironic/drivers/modules/ipmitool.py20:20
jlvillalNobodyCam: Great :)  Thanks!20:20
mrdamorning 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
rloomorning mrda. what problem? /me looks20:22
mrdalogical name in port section of REST API20:22
mrdarloo ^^20:22
*** dprince has quit IRC20:22
openstackgerritJosh Gachnang proposed openstack/ironic: Add /nodes/<uuid>/cleaning/steps API  https://review.openstack.org/15932220:22
JoshNangrloo: ^ fixed. thanks for that!20:22
rloomrda: 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
mrdaok, if it can wait, then feel free to leave20:25
rloomrda: 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
mrdathanks rloo, much appreciated!20:28
rloomrda: yw I guess. although now I feel bad cuz you're thanking me for not reviewing your stuff til later :-(20:30
mrdarloo: 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-ironic20:32
rloomrda: :D20:32
NobodyCambrb20:35
TheJuliawin 720:37
TheJuliadoh20:37
JayFonly 7? You need to IRC harder julia :P20:37
BadCubhi TheJulia20:38
TheJuliaJayF: oh, I have many windows, just I happened to be going to that one, because I'm lazy and like typing /win :)20:38
openstackgerritJohn L. Villalovos proposed openstack/ironic: Update unittests and use NamedTemporaryFile  https://review.openstack.org/16267220:39
jlvillalNobodyCam: ^^^^ Hopefully I addressed your issue :)20:39
jlvillalNobodyCam: I did it kind of like this: https://github.com/openstack/ironic/blob/master/ironic/common/utils.py#L380-L38220:41
*** lucas-dinner has quit IRC20:50
*** andreykurilin_ has joined #openstack-ironic20:50
*** Nisha has quit IRC20:54
*** Nisha_away has joined #openstack-ironic20:54
*** harlowja is now known as harlowja_away20:58
NobodyCamjlvillal: how does NamedTemporaryFile act if conf.tempDir is none?21:01
jlvillalNobodyCam: That is the default21:02
jlvillalNobodyCam: https://docs.python.org/2/library/tempfile.html21:02
NobodyCam:) ack21:03
jlvillal:D21:03
*** ijw_ has quit IRC21:03
*** Nisha_away has quit IRC21:04
*** ijw has joined #openstack-ironic21:04
*** Nisha has joined #openstack-ironic21:04
*** ijw has quit IRC21:06
*** ijw has joined #openstack-ironic21:06
*** jgrimm is now known as zz_jgrimm21:07
openstackgerritNisha Agarwal proposed openstack/ironic: iLO implementation for hardware inspection  https://review.openstack.org/15159621:08
*** andreykurilin_ has quit IRC21:09
* devananda returns from lunch21:09
NobodyCamwelcome back21:11
* devananda takes a look at the review board21:14
NobodyCam:)21:14
mrdajlvillal: I've just commented on 162672 too.  HTH.21:18
openstackgerritDevananda van der Veen proposed openstack/ironic: Fixup log message for discoverd  https://review.openstack.org/16479221:18
jlvillalmrda: Thanks :)21:19
devanandaNobodyCam: ^ needs re-review. I missed a test21:19
openstackgerritNisha Agarwal proposed openstack/ironic: iLO implementation for hardware inspection  https://review.openstack.org/15159621:19
mrdajlvillal: on the previous version21:19
NobodyCamdevananda: ack21:20
NobodyCammrda: looking at your comment on 162672 would your concern be removed with the "pre-flight" cjhecks I have here: https://review.openstack.org/#/c/16038321:21
devanandaJoshNang: what's the impact if we don't land the GET /nodes/clean API ?21:21
JoshNangdevananda: less operator visibility, not huge IMO21:21
JoshNangespecially since the number of steps is pretty small (0 right now)21:22
*** sambetts has quit IRC21:22
devanandaJoshNang: cleaning /progress/ is still made visible as a node goes through it21:22
JoshNangcorrect21:23
JayFyou can't know progress (in terms of % completion)21:23
JayFwithout knowing the full set of steps being run, right?21:23
NobodyCamdevananda: re +a'd :)21:24
devanandaNobodyCam: ty21:24
*** sambetts has joined #openstack-ironic21:25
mrdaNobodyCam: looking now21:25
devanandaJoshNang: 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
devanandaJoshNang: is it reasonable to split execute out and land that now?21:26
openstackgerritNisha Agarwal proposed openstack/ironic: iLO driver updates node capabilities during inspection  https://review.openstack.org/16357221:28
mrdaNobodyCam: That's great stuff!  I think that makes the use of the CONF.tempdir is fine with those checks preceeding it21:28
JoshNang(sorry for delayed responses, in a meeting) i don't think that's dependent on the API change21:29
NobodyCamquestion: 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
JoshNangdevananda: ^21:29
* mrda needs more coffee so he can speak in sentences21:30
NobodyCammrda: I wouldn't say great but its a basic pre-flight check21:30
NobodyCam:-p21:31
devanandaJoshNang: in gerrit it is listed that way21:31
JoshNangdevananda: 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 go21:32
mrdaNobodyCam: 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
devanandaJoshNang: ooh. yah. i'm looking at the wrong tab (maybe I shouldn't have a hundred open?)21:34
NobodyCammrda: :)21:34
JoshNangdevananda: heh i know that feeling21:34
devanandaJoshNang: in the interest of crunch time, i'm inclined to bump the REST API changes21:35
JoshNangdevananda: wfm21:36
devanandak21:36
JoshNangi'd rather focus on landing the agent and ilo bits21:36
devanandayah21:36
JoshNangthe ilo management one looked good to go21:36
devanandamake cleaning work, be configurable, tested, etc21:36
JoshNangagreed21:36
devanandaexposing the list of steps seems secondary at this point21:36
devanandagood - but not as critical21:36
JoshNang(and work on docs! my plan for the end of the week)21:36
openstackgerritNisha Agarwal proposed openstack/ironic: Automate boot iso creation with in ironic for iscsi-ilo  https://review.openstack.org/15590021:37
* JayF will write cleaning docs21:37
devanandaJoshNang: we have a couple weeks to hammer out docs and bugs21:37
JayFall the docs21:37
devanandaJayF: any objection to ^ ?21:37
devanandaNisha: ^21:37
JayFI know all my clean steps, api or not ;)21:37
JayFheh21:37
JayFI'd say it's OK.21:37
devanandaJayF: lol, of course you do :)21:37
devanandaok, bumping21:38
JoshNangi'd guess most people are going to be using defaults in the agent (erase disks)21:38
JoshNangand same with ilo, looked like sane defaults21:38
JayFWorth noting that today in the agent21:38
JayFif your disk doesn't support ATA security erase, CLEANFAIL21:39
devanandaooh. yes21:39
devanandamust make that optional then21:39
JoshNang++21:41
*** ijw_ has joined #openstack-ironic21:42
*** ijw has quit IRC21:45
BadCubbrb21:46
devanandaany IPA folks reviewed lucas' patch for root device hints? https://review.openstack.org/#/c/163857/21:46
JayFI put it in a tab but can't review for a littl ebit21:47
JayFit's in my queue :)21:47
Nishadevananda, for docs?21:51
Nishasorry i was away i saw it now only21:51
devanandaNisha: the GET /v1/nodes/NNN/clean/steps API change -- we were discussing bumping that to Liberty, and I have just done so21:52
Nishadevananda, does that affects complete cleaning?21:55
devanandaNisha: it shouldn't affect the functionality of performing cleaning operations on nodes21:55
JoshNangcorrect21:55
Nishaas i think it impacts only ironic client?21:55
devanandaNisha: it is removing the REST API that allows you to query the steps that the driver believes it will take.21:55
Nishaok21:56
rloodevananda: 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
rloodevananda: well, it should be done now cuz of microversions, it is a bug.21:56
devanandarloo: i think we can defer all the clean_step API stuff21:56
devanandaerm21:56
rloodevananda: 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
devanandaugh21:57
rloodevananda: i don't know if that made sense but i hope you understood.21:57
devanandarloo: i think so. verifying right now21:58
rloodevananda: i added comments to that patch.21:58
Nishadevananda, 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/15590021:59
devanandarloo: so these fields are  not guarded by a microveresion: inspection_started_at inspection_finished_at21:59
rloodevananda: i thought the inspect* stuff was guarded already.21:59
Nishadevananda, they are for microversion 1.621:59
Nisharloo, correct22:00
devanandathey are not22:00
devanandahttp://paste.openstack.org/show/192738/22:00
rloodevananda: ugh. you're right. i thought they were, but those were missed too.22:01
Nishadevananda, guarded means?22:03
devanandaNisha: checking the microversion in the request and changing behavior as necessary to present that version of the API22:03
JoshNangdevananda: (meeting done) i can pul the guard bits out into their own tiny patch22:04
JoshNangpull, even22:04
Nishadevananda, so in the pasted output what was expected22:04
Nishaoh it should have shown for 1.122:05
devanandaNisha: the inspect_*_at fields should not be present when X-OpenStack-Ironic-API-Version: 1.122:05
Nishadevananda, got it22:05
NobodyCamNisha: 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
devanandawhile looking for the clean* fields, I noticed the inspect* fields were present (it's a separate issue)22:05
NishaNobodyCam, yes22:06
NobodyCam:)22:06
NishaNobodyCam, let me see22:06
*** absubram has joined #openstack-ironic22:06
*** kkoski has quit IRC22:07
NobodyCam_get_desired_macs_for_port_creation method's doc string, line 201 :)22:07
NobodyCams/string,/string and/22:08
NishaNobodyCam, but the function _get_desired_macs_for_port_creation() is not called when argument is "none"22:08
*** rloo is now known as rloo_afk22:09
NishaNobodyCam, please leave comments at exact locations22:09
Nishain the patch22:09
Nishaif it is just comments and all i shud be able to throw new version now22:09
*** harlowja_away is now known as harlowja22:11
*** ijw_ has quit IRC22:13
devanandaJoshNang: 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_32022:23
JoshNangyup, that's why we need a devstack patch22:23
JoshNangi'm testing locally right now22:23
jrollJoshNang: I had some ideas on how to test / figure that out, if you get stuck or whatever22:23
devanandaso that looks downright broken22:24
devanandawhich leads me to wonder how it landed22:24
jrollit didn't land?22:24
jrollthat's a gate job, how would it land22:24
devanandaoh blah22:24
devanandatoo many tabs again22:24
jroll:P22:24
devanandathat hasn' tlanded22:24
JoshNangyeah definitely not landed :)22:24
NobodyCam:-p22:25
jrolldevananda: do you know what's going on there? I can quickly explain22:25
jrollshort 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 devstack22:26
openstackgerritNisha Agarwal proposed openstack/ironic: iLO implementation for hardware inspection  https://review.openstack.org/15159622:26
devanandajroll: right22:27
devanandai 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 three22:27
jrollgot it22:28
openstackgerritNisha Agarwal proposed openstack/ironic: iLO driver updates node capabilities during inspection  https://review.openstack.org/16357222:28
* devananda deletes the already-merged things from etherpad22:30
*** ijw has joined #openstack-ironic22:34
adam_ganyone seen these unit tests failures locally? http://paste.openstack.org/show/192749/  my is up to date22:38
devanandaadam_g: not in a very long time22:38
adam_ghmm22:38
* adam_g recreates22:39
devanandaadam_g: see ironic/tests/__init__.py22:39
adam_goh, right22:40
adam_gprobably something to do with running via nose22:40
BadCubthanks devananda , I was just striking out the merged ones22:51
devanandaBadCub: yup, that's good too. but there were so many of them :)22:51
*** wanyen has quit IRC22:52
BadCubYeah, it was getting rather colorful22:52
BadCubdevananda: question on 159322.... Bumping that particular item does not kill the BP, correct?22:52
devanandaBadCub: we should indicate that something was completed on launchpad, yes22:53
devanandaBadCub: however the work isn't actually finished -- http://specs.openstack.org/openstack/ironic-specs/specs/kilo/implement-cleaning-states.html#rest-api-impact22:54
devanandathe spec clearly lists two things that we just decided to bump22:54
devanandaas 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 weeks22:55
devanandawe'll definitely need to track what parts of the spec weren't done, and probably go update it to indicate as much22:55
devanandaso that the historical record of what was done in Kilo is accurate22:56
BadCubokay22:56
BadCubI only noted one that was bumped. Probably missed the second22:56
devanandaeg, of the 5 items under REST API Impact, we've done 322:56
openstackgerritNisha Agarwal proposed openstack/ironic: iLO driver updates node capabilities during inspection  https://review.openstack.org/16357223:03
*** ChuckC has joined #openstack-ironic23:09
*** ramineni has joined #openstack-ironic23:11
*** Nisha has quit IRC23:14
*** stendulker has joined #openstack-ironic23:15
*** r-daneel has quit IRC23:19
*** andreykurilin_ has joined #openstack-ironic23:20
NobodyCamJoshNang: the topic on https://review.openstack.org/#/c/161453 seems strange. does it need a commit message *-bug tag?23:23
*** absubram has quit IRC23:23
JoshNangNobodyCam: i think that's left over from the execute clean steps patch. (which did have that bug)23:23
openstackgerritShivanand Tendulker proposed openstack/ironic: Common changes for secure boot support  https://review.openstack.org/15397423:23
NobodyCamJoshNang: ack23:24
*** yuanying has joined #openstack-ironic23:24
NobodyCamjust checking23:24
JoshNangupdated!23:24
NobodyCam:-p23:24
openstackgerritShivanand Tendulker proposed openstack/ironic: Secure boot support for pxe_ilo driver  https://review.openstack.org/15480823:25
JoshNangso 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 nothing23:25
openstackgerritAnusha Ramineni proposed openstack/ironic: Add Cleaning Operations for iLO drivers  https://review.openstack.org/15771523:26
JoshNanganyone have any ideas?23:26
ramineniJoshNang: 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
BadCubjroll: 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 IPA23:34
ramineniJoshNang: I mean in create_cleaning_ports23:34
JoshNangramineni: hmm lemme look23:35
JoshNangmight be the reason my nodes are hanging :)23:35
* BadCub needs a smoke23:37
devanandamrda: did you look into logical name support for /v1/ports API ?23:38
*** r-daneel has joined #openstack-ironic23:42
ramineniJoshNang: :) 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 back23:42
JoshNangramineni: the provision state api23:43
JoshNangwith the verb 'provide'23:44
ramineniironic node-set-provision-state 'provide'?23:45
JoshNanghrm no that won't work23:45
ramineniJoshNang: i dont see any option in CLI23:46
JoshNangthe verb is 'clean' in states.py23:46
*** r-daneel has quit IRC23:47
ramineniJoshNang: ya, got that, but for the end user to initiate?23:47
*** andreykurilin_ has quit IRC23:48
JoshNangramineni: yeah i think this got lost with all the patchsets :( i had thought the 'provide' verb would do it23:50
jrollBadCub: +3'd23:50
jrollJoshNang: doesn't it?23:50
BadCubjroll: thank ya kindly sir! :-)23:51
JoshNangjroll: apparently not. the verb from cleanfail->clean is 'clean', which isn't in the list of verbs or the api23:51
jrollJoshNang: oh, to restart cleaning. I see23:51
jrollfixfixfix23:51
jroll:P23:51
jrollwell23:51
jrollusing provide would put it in managed, right?23:52
JoshNangwell i've got a handful of minutes while waiting for devstack to restack >:(23:52
*** Haomeng has joined #openstack-ironic23:52
jrollerr23:52
jrollusing manage would put it in managed, then provide would clean and make it available, yes?23:52
JoshNangcorrect23:52
jrollramineni: ^ there's a workaround for nowe23:52
jrollnow23:52
JoshNangi think that was semi intended tbh23:53
openstackgerritDevananda van der Veen proposed openstack/python-ironicclient: Add support for logical names  https://review.openstack.org/15852023:53
jrollyeah, go to managed to fix23:53
raminenijroll: ok23:54
NobodyCamlol I was just about you about that one devananda23:54
NobodyCamgah23:54
devananda:)23:54
NobodyCamI was just about to ask about that one23:54
NobodyCam:-p23:54
BadCubdevananda: 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 IRC23:55
devanandaBadCub: cross project deps are a bit odd to track. but I think ^ is very likely to land23:56
raminenijroll: should this be triggered using API diretky?23:56
jrollramineni: you can use the client afaik23:56
BadCubI was going to move https://blueprints.launchpad.net/ironic/+spec/root-device-hints up to Approved/Waiting to Land23:57
*** alexpilotti has quit IRC23:57
jrollthough maybe our client doesn't support those yet, maybe run master version of client?23:57
devanandaBadCub: would it be good to have another section for "dependencies on other projects" ?23:58
jrollramineni: oh, this https://review.openstack.org/#/c/148804/23:58
*** takadayuiko has quit IRC23:58
BadCubdevananda: 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 standing23:59

Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!